Skip to content

Add support for char element to dto factory #22442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 12, 2020
Merged

Add support for char element to dto factory #22442

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
49ff38a
Add support for char element to dto factory
wardcapp Apr 21, 2019
c876e0d
Merge branch '2.4-develop' into magento-2.3/add-char-to-dto-elementfa…
engcom-Golf Feb 7, 2020
58dde94
Add support for db_schema.xml char type column
Nazar65 Feb 11, 2020
b1d4c72
cover changes with setup-integration test
Nazar65 Feb 27, 2020
6070770
add cgar column to fixtures
Nazar65 Feb 27, 2020
92ec594
added assertion
Nazar65 Feb 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/etc/di.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1435,6 +1435,7 @@
<item name="mediumtext" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\MediumText</item>
<item name="text" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\Text</item>
<item name="varchar" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\StringBinary</item>
<item name="char" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\StringBinary</item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not enough to add char in di.xml, you need to fix db_scrma definition too

Copy link
Contributor Author

@wardcapp wardcapp Jul 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, thank you for the follow-up! Not quite sure if I am understanding this correctly - did you mean the DB schema type declarations (within the very same di.xml)? If so, char has already been defined there as well within the current 2.3-develop branch (see app/etc/di.xml:1471).

Apart from that, it is possible to add support for the char type to Magento 2 setup scripts. Which would require adding of a char schema setup declaration (similar to lib/internal/Magento/Framework/Setup/Declaration/Schema/etc/schema.xsd:22) indeed, but does not relate to the intent of this pull request (and could be set up over a separate PR or feature request). But I might still be overlooking another required type of definition?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sidolov please respond :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @wardcapp This is a good idea extending compatible types, but unfortunately, this is not as easy as change one XSD. Here is an example of adding a new data type to a declarative schema. https://github.com/magento/magento2/pull/25479/files
In your case, it even could be more complicated because the CHAR type supports more options that JSON. Also, you can take a look at VARCHAR implementation as an example.

<item name="varbinary" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\StringBinary</item>
<item name="blob" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\Blob</item>
<item name="mediumblob" xsi:type="object">\Magento\Framework\Setup\Declaration\Schema\Dto\Factories\MediumBlob</item>
Expand Down