Skip to content
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.

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

Position is not updated in wp menu item update command #463

Open
2 tasks done
ernilambar opened this issue Feb 13, 2024 · 6 comments
Open
2 tasks done

Position is not updated in wp menu item update command #463

ernilambar opened this issue Feb 13, 2024 · 6 comments

Comments

@ernilambar
Copy link
Member

Bug Report

Describe the current, buggy behavior

Position is not updated in wp menu item update command.

Describe how other contributors can replicate this bug

wp menu create "Sample Menu"
wp menu item add-custom sample-menu Alpha https://alpha.com
wp menu item add-custom sample-menu Beta https://beta.com

wp menu item list sample-menu --format=csv

db_id,type,title,link,position
1272,custom,Alpha,https://alpha.com,1
1273,custom,Beta,https://beta.com,2

wp menu item update 1273 --link=https://beta.net --position=1

db_id,type,title,link,position
1272,custom,Alpha,https://alpha.com,1
1273,custom,Beta,https://beta.net,2

Describe what you would expect as the correct outcome

Position also should have been updated.

Let us know what environment you are running this on

OS:     Darwin 22.6.0 Darwin Kernel Version 22.6.0: Tue Nov  7 21:48:06 PST 2023; root:xnu-8796.141.3.702.9~2/RELEASE_X86_64 x86_64
Shell:  /bin/zsh
PHP binary:     /usr/local/Cellar/[email protected]/8.2.15/bin/php
PHP version:    8.2.15
php.ini used:   /usr/local/etc/php/8.2/php.ini
MySQL binary:   /usr/local/bin/mysql
MySQL version:  mysql  Ver 8.3.0 for macos13.6 on x86_64 (Homebrew)
SQL modes:
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /Users/nilambarsharma/Sites/staging
WP-CLI packages dir:    /Users/nilambarsharma/.wp-cli/packages/
WP-CLI cache dir:       /Users/nilambarsharma/.wp-cli/cache
WP-CLI global config:   /Users/nilambarsharma/.wp-cli/config.yml
WP-CLI project config:  /Users/nilambarsharma/Sites/staging/wp-cli.local.yml
WP-CLI version: 2.10.0
@danielbachhuber
Copy link
Member

Thanks for the report, @ernilambar !

This definitely seems like a bug. I was able to reproduce with the steps you provided.

With this being said, I'm not sure it ever worked in the first place 🙃

Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

@ernilambar
Copy link
Member Author

I dug deeper into adding/updating menu item stuffs. Yes, this has not worked till now because when there is new position value, menu_order value of all menu items in that menu should shuffled which we have not done.

We have done only for add:

if ( ( 'add' === $method ) && $menu_item_args['menu-item-position'] ) {
  $this->reorder_menu_items( $menu->term_id, $menu_item_args['menu-item-position'], +1, $result );
}

This reorder_menu_items() only works for an item and makes sense in adding. We cannot reuse this method. We need to introduce separate method for reshuffling menu order for update.

@ernilambar
Copy link
Member Author

Related: #493

@nateinaction
Copy link

nateinaction commented May 16, 2024

Additional information about this issue.

Slightly simpler reproduction steps

First, I was able to reproduce this issue with a slightly simpler command set by leaving off the --link flag during the update call:

Create the menu and items:

wp menu create "Sample Menu"
wp menu item add-custom sample-menu Alpha https://alpha.com/
wp menu item add-custom sample-menu Beta https://beta.com/

Try to move Beta item to position 1:

wp menu item update 5 --position=1

Show that Beta did not move:

wp menu item list sample-menu --format=csv
db_id,type,title,link,position
4,custom,Alpha,https://alpha.com,1
5,custom,Beta,https://beta.com,2

New behavior discovered by moving first menu element

Oddly enough, even though Beta cannot be moved to position 1, Alpha can be moved to position 2

wp menu item update 4 --position=2
wp menu item list sample-menu --format=csv
db_id,type,title,link,position
5,custom,Beta,https://beta.com,1
4,custom,Alpha,https://alpha.com,2

With Alpha in position 2, both Alpha and Beta can be moved freely:

Move Beta to position 2:

wp menu item update 5 --position=2  
wp menu item list sample-menu --format=csv
db_id,type,title,link,position
4,custom,Alpha,https://alpha.com,1
5,custom,Beta,https://beta.com,2

Move Beta back to position 1

wp menu item update 5 --position=1
wp menu item list sample-menu --format=csv
db_id,type,title,link,position
5,custom,Beta,https://beta.com,1
4,custom,Alpha,https://alpha.com,2

However, once Alpha is moved back to position 1 Beta is again prevented from changing position

Move Alpha back to position 1:

wp menu item update 4 --position=1
wp menu item list sample-menu --format=csv
db_id,type,title,link,position
4,custom,Alpha,https://alpha.com,1
5,custom,Beta,https://beta.com,2
wp menu item update 5 --position=1

Show that Beta did not move:

wp menu item list sample-menu --format=csv
db_id,type,title,link,position
4,custom,Alpha,https://alpha.com,1
5,custom,Beta,https://beta.com,2

Edit: I see what's happening. The item update command is always updating the menu_order column for the posts in the wp_posts table. When Alpha and Beta share the same order position, they still display in alphabetical order.

@petruchek
Copy link
Contributor

@ernilambar, is this still not fixed? I can see that some work was done in #502 but apparently it's not fully working.

@ernilambar
Copy link
Member Author

@petruchek Yah, not fixed. That approach did not work without breaking other things. Now in block era, this issue does not get enough traction as it is not much used. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants