Skip to content

Conversation

@louwie17
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

This fixes a bug introduced in #34744 where we default the sort order to id. This change makes use of the orderby setting in attributes and defaults to name if it is not set.

Closes #35639 .

  • 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. Load this branch, create an attribute with several terms under Products > Attributes and set the Default sort order to name
  2. Create a new product (or edit an existing) and go to the attributes tab
  3. Add the newly created attribute and click on the attribute term search field, the dropdown should show the existing terms in the order set above.
  4. Now change the order of the attribute to Custom ordering or Term ID and see if the order within the product edit screen matches what is set.

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 the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Dec 16, 2022
@louwie17 louwie17 requested a review from a team December 16, 2022 11:04
@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2022

Test Results Summary

Commit SHA: 47ac958

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 54s
E2E Tests187006019314m 9s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@louwie17 louwie17 self-assigned this Dec 19, 2022
@joelclimbsthings joelclimbsthings requested review from joelclimbsthings and removed request for a team December 20, 2022 19:36
<select multiple="multiple"
data-minimum_input_length="0"
data-limit="50" data-return_id="id"
data-placeholder="<?php esc_attr_e( 'Select terms', 'woocommerce' ); ?>"
Copy link
Contributor

Choose a reason for hiding this comment

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

phew 😅

Thanks for straightening these out!

Copy link
Contributor

@joelclimbsthings joelclimbsthings left a comment

Choose a reason for hiding this comment

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

Working well for me! And definite improvement in the code 🚢

@louwie17 louwie17 merged commit 309ed63 into trunk Dec 21, 2022
@louwie17 louwie17 deleted the fix/35639_attribute_term_sort_order branch December 21, 2022 08:11
@github-actions github-actions bot added this to the 7.4.0 milestone Dec 21, 2022
joelclimbsthings pushed a commit that referenced this pull request Jan 6, 2023
* Make sure attribute term dropdown adheres to sort order setting of attribute

* Fix spacing

* Fix lint errors
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Since 7.1.0 attributes values dropdown display in Product Data > Attribute in order originally added in spite of Order By setting

3 participants