Skip to content

Fixed import and visibility tax values to lowercase #33820

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

Jump to bottom
Merged
jorgeatorres merged 2 commits into from
May 26, 2023
Merged

Fixed import and visibility tax values to lowercase #33820

jorgeatorres merged 2 commits into from
May 26, 2023

Conversation

@ajayghaghretiya-multidots
Copy link
Contributor

@ajayghaghretiya-multidots ajayghaghretiya-multidots commented Jul 12, 2022 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

Closes #33794.

How to test the changes in this Pull Request:

  1. Add the code changes to your local site.
  2. Check the import tool again with keeping visibility and tax value as "Visible", and "Taxable".
  3. Product will be imported successfully as per the below screenshot.

CleanShot 2022-07-12 at 13 31 23

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 successfully run tests with your changes locally?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

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 the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jul 12, 2022
@ajayghaghretiya-multidots ajayghaghretiya-multidots mentioned this pull request Jul 12, 2022
Closed
5 tasks
jorgeatorres
jorgeatorres requested changes Dec 14, 2022
View reviewed changes
Copy link
Member

@jorgeatorres jorgeatorres left a comment

Choose a reason for hiding this comment

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

Hi @ajayghaghretiya-multidots!

First of all, let me apologize for the delay in getting this reviewed and thank you for your contribution! 💯
Overall, the changes look good. I left one tiny request, but once addressed we should be good to go.

I understand it's been a while since you initially submitted this PR, so please let us know if you have the time to continue working on it or if you'd rather have us take over.

Looking forward to your response!

@jorgeatorres jorgeatorres added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jan 19, 2023
@samueljseay
Copy link
Contributor

samueljseay commented May 3, 2023

We are closing this pull request, as the last activity has been quite some time ago. If you are still interested or you feel like this is a mistake, please open another PR based on a recent version of the trunk branch. Thanks!

@samueljseay samueljseay closed this May 3, 2023
@jorgeatorres jorgeatorres reopened this May 24, 2023
@jorgeatorres jorgeatorres requested review from a team and rrennick and removed request for a team May 24, 2023 21:12
@github-actions
Copy link
Contributor

github-actions bot commented May 24, 2023

Hi ,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@jorgeatorres
Copy link
Member

jorgeatorres commented May 24, 2023 •
edited
Loading

@rrennick: I wanted to rescue this PR (authored by a community contributor) from the "purge". IMO it only needed a tiny change (see #33820 (review)), so I went head and added it on their behalf. Please let me know what you think.

@jorgeatorres jorgeatorres force-pushed the fixed-wc-import-visibility-tax-values branch from 57f4bed to 7c41c21 Compare May 24, 2023 21:15
rrennick
rrennick approved these changes May 26, 2023
View reviewed changes
Copy link
Contributor

@rrennick rrennick left a comment

Choose a reason for hiding this comment

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

LGTM

@rrennick
Copy link
Contributor

rrennick commented May 26, 2023

@jorgeatorres I think you need to approve this before merging. It looks good so feel free to merge.

@jorgeatorres jorgeatorres self-requested a review May 26, 2023 18:23
@jorgeatorres jorgeatorres merged commit 0780386 into woocommerce:trunk May 26, 2023
@jorgeatorres
Copy link
Member

jorgeatorres commented May 26, 2023

Thank you @rrennick!

@github-actions github-actions bot added this to the 7.9.0 milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorgeatorres jorgeatorres jorgeatorres approved these changes

+1 more reviewer

@rrennick rrennick rrennick approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution

Projects

None yet

Milestone

7.9.0

Development

Successfully merging this pull request may close these issues.

5 participants

@ajayghaghretiya-multidots @samueljseay @jorgeatorres @rrennick @psealock