Skip to content

Conversation

@shvlv
Copy link
Contributor

@shvlv shvlv commented Nov 21, 2022

All Submissions:

Changes proposed in this Pull Request:

The PR adds Denmark postcode validation according to https://en.wikipedia.org/wiki/Postal_codes_in_Denmark. Essentially it's a minor modification of https://rgxdb.com/r/335G4K1S with an optional DB prefix.

As the Denmark postcode can contain DK- prefix I restored it in the wc_format_postcode function (as I can see LV has already).

Regarding the unit tests, I was a bit confused because the current \WC_Validation::is_postcode tests are included just in \WC_Tests_Validation::data_provider_test_is_postcode method. It should not be touched according to README.md. Should I add a new testing class to plugins/woocommerce/tests/php/includes?

  • [] 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. Go to the checkout page to finish your order.
  2. Select Denmark as the billing (or shipping) country.
  3. Paste the postcode in the wrong format (like DK-3900).
  4. Try to complete the purchase.
  5. You get a notification about the wrong Postal code.
  6. Change the postal code to the valid one (like 1448 or DK-1448).
  7. You are able to complete the purchase.

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 Nov 21, 2022
@woocommercebot woocommercebot requested review from a team and Konamiman and removed request for a team November 21, 2022 13:09
@Konamiman
Copy link
Contributor

Hi @shvlv, thanks for your contribution.

Indeed, the general rule for adding new tests for WooCommerce code is to add them to classes in the tests/php directory. However in this case you would be doing a small extension to an existing data source for an existing class, so it's fine to just add them to \WC_Tests_Validation::data_provider_test_is_postcode. Feel free to add them there.

Also could you please add some testing instructions to the pull request description? Thank you!

@shvlv
Copy link
Contributor Author

shvlv commented Nov 25, 2022

Hi, @Konamiman thanks for the feedback. I've applied your suggestions.

@shvlv
Copy link
Contributor Author

shvlv commented Nov 25, 2022

Sorry, forgot to run PHPCS on tests :( Fixed now.

@Konamiman Konamiman merged commit 8cdacf3 into woocommerce:trunk Nov 25, 2022
@Konamiman
Copy link
Contributor

Approved and merged, thank you

@github-actions github-actions bot added this to the 7.3.0 milestone Nov 25, 2022
@shvlv shvlv deleted the denmark-post-code-validation branch November 30, 2022 08:45
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