10up / simple-page-ordering

Order your pages and other hierarchical post types with simple drag and drop right from the standard page list.
https://wordpress.org/plugins/simple-page-ordering/
GNU General Public License v2.0
143 stars 22 forks source link

Add functionality to reset page order #129

Closed ruscoe closed 1 year ago

ruscoe commented 1 year ago

Description of the Change

In the page admin section, the Simple Page Ordering plugin help section has been modified to include a "Reset page order" link. Clicking this link first displays a confirmation dialog then, if the user confirms, resets the "menu_order" field of all pages to 0, resetting any changes made with this plugin.

I think this is the most minimally invasive solution, adding little code to achieve the purpose. I chose to place the reset link in the help menu to minimize chances of accidental order resets. Plus, if someone needs to reset the order of their posts, they have probably made a mistake and will be looking for help.

Closes #24

How to test the Change

Changelog Entry

Added - Feature to reset page order

Credits

Props @pattonwebz, @ruscoe, @Sidsector9, @dkotter

Checklist:

ruscoe commented 1 year ago

@Sidsector9 Very good point! Thank you for reviewing and catching that. I'll take your suggestion and submit an updated PR shortly.

ruscoe commented 1 year ago

@Sidsector9 Changes made! It's now using the admin AJAX functionality rather than a REST endpoint. Is there something better I could do to avoid the code duplication between ajax_simple_page_ordering and the new ajax_reset_simple_page_ordering or is that as efficient as I can get it?

Sidsector9 commented 1 year ago

@ruscoe Thanks for working on those changes, tested well for me 👍

No pressure at all, but would you be comfortable at writing end-to-end tests for the same? We use Cypress, you can see the tests for existing features here - https://github.com/10up/simple-page-ordering/tree/develop/tests/cypress/integration

Else let us know if you'd like someone from our team to pick up writing the tests.

Answering your question:

Is there something better I could do to avoid the code duplication between ajax_simple_page_ordering and the new ajax_reset_simple_page_ordering or is that as efficient as I can get it?

Looking at the code, I think we can let it pass for now as the duplication is less. In the future when the number of AJAX handlers increase using the same set of checks, then we can look at refactoring it under a common handler.

ruscoe commented 1 year ago

@Sidsector9 I'd like to write the tests. It'll be a great opportunity for me to learn how they work. Will update soon!

ruscoe commented 1 year ago

@Sidsector9 Thank you for all your assistance with this! I've just committed my first attempt at a Cypress test. I took a lot of guidance from the other tests.

The tests pass for me and look correct when I run them through the command line and UI / browser aspect of Cypress. Let me know what you think!

Sidsector9 commented 1 year ago

@ruscoe Thanks so much for the tests! I have reviewed and requested some minor changes. You can use npm run cypress:open for visual reference.

ruscoe commented 1 year ago

@Sidsector9 Thank you! I replaced my test code with your modified code and it works perfectly. Do you mind if I just commit your modified test in place of mine?