Skip to content

Conversation

@nicomollet
Copy link
Contributor

@nicomollet nicomollet commented Dec 7, 2022

Hello

I had problems with product import.
After some time, I managed to identify that CSV keys with line breaks cause the import to skip the column, while the mapping looks like it should have worked.
The PR remove line breaks in the keys of the CSV imported file, to avoid mismatch mapping of keys.

All Submissions:

Changes proposed in this Pull Request:

This PR fixes product import when the CSV imported file has a line break in a key name.
If a key has a line break, mapping fields will look OK, but the import will simply skip the values of this key.

Example of a CSV file with line break below:

ID,Published,"Regular 
price"
3011,1,168

With a line break, the mapping looks like it will work (image below):
image

The PR removes line breaks in all the CSV keys.
Regular\nprice becomes Regular price.

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

  1. Export the products using WC core tool
  2. In the CSV file, add a line break inside the key Regular price
  3. Change the price of some products
  4. Keep the columns: ID, Published, Regular price, and remove the others
  5. Use the product import tool and map the 3 fields
  6. After it is done, the price should be be updated thanks to the PR. Without the PR, prices won't be updated

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@github-actions github-actions bot added plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution labels Dec 7, 2022
@woocommercebot woocommercebot requested review from a team and barryhughes and removed request for a team December 7, 2022 16:08
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

Great catch, thank you.

barryhughes
barryhughes previously approved these changes Dec 8, 2022
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

Once checks pass, we can go ahead and merge 👍

@nicomollet
Copy link
Contributor Author

nicomollet commented Dec 9, 2022

Once checks pass, we can go ahead and merge 👍

Thanks @barryhughes for generating the changelog, I had trouble with pnpm on my Lando environment (nvm, and node version).

It looks like the checks ran into a connectivity error? If I am not mistaken.

curl: (22) The requested URL returned error: 403 Forbidden

1) WC_AJAX_Test::test_create_api_key_long_description_failure An unexpected error occurred. Something may be wrong with WordPress.org or this server&#8217;s configuration. If you continue to have problems, please try the <a href="https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.)

Any way to rerun the actions?
Thanks

@barryhughes
Copy link
Member

It looks like the checks ran into a connectivity error?

Yep, that happens periodically: I triggered a re-run and we don't need to worry about the 'Build Live Branch' check failing (that's a known problem). I have it on my list to check back in on this PR a little later: we'll get it merged one way or another :-)

nicomollet and others added 4 commits December 9, 2022 14:25
Remove line breaks in keys, to avoid mismatch mapping of keys.
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

Alright, checks are now passing (except for one which we know to be problematic). Merging: thanks again!

@barryhughes barryhughes merged commit f403265 into woocommerce:trunk Dec 13, 2022
@github-actions github-actions bot added this to the 7.3.0 milestone Dec 13, 2022
@nicomollet
Copy link
Contributor Author

Alright, checks are now passing (except for one which we know to be problematic). Merging: thanks again!

Thanks a lot @barryhughes ;)

samueljseay pushed a commit that referenced this pull request Dec 15, 2022
* Product import: Remove line breaks in keys

Remove line breaks in keys, to avoid mismatch mapping of keys.

* Fix syntax

* PHPCS

* Changelog.

Co-authored-by: barryhughes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants