Category Import fixes #121
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@avstudnitz, do you think we can merge this? |
|
@rbrown, would you mind updating your PR and sending it to the current |
Without this explodeEsacped can be called without glue.
The refactor changed this code to use Magento\Framework\Stdlib\DateTime, but it's constructor isn't the same as \DateTime, so I reverted it. \DateTime->new accepting null is deprecated, so I also fixed that warning
Import/Category->indexValueAttributes holds a list of attributes that use the value of a select, rather than the label to update the database. This doesn't match with the validation code, which uses Magento\ImportExport\Model\Import::getAttributeType to
|
Hi @sprankhub I'm not sure I've done this right now. There should be three commits, 1 to add a default category separator, with this I didn't need to reorder the method parameters, 2. Use the right DateTime class and 3. A fix for importing boolean attributes. |
sprankhub
requested changes
Oct 21, 2022
|
Thanks a lot, @rbrown! I hope my comments make sense to you :) |
This reverts commit db3d2ce.
sprankhub
approved these changes
Oct 24, 2022
|
Thanks, @rbrown! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This has the compatibility fixes I commented on, sets a default category_path_seperator "/", and reverses the parameters for explodeEscaped because having the first parameter optional is deprecated.