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

[4.1][regression] Correctly handle save2copy for menus #37813

Merged
merged 2 commits into from
May 18, 2022

Conversation

zero-24
Copy link
Contributor

@zero-24 zero-24 commented May 16, 2022

Pull Request for Issue #37812

Summary of Changes

Restore the existing behavior before the regression caused by #37395 as well as fix the Issue from the original report

Testing Instructions

  • setup a 4.1.2 site
  • create 2 menus
  • create one menu items for the first menu
  • open that menu item in edit mode
  • change the menu setting to the seccond menu
  • Use the "save2copy" feature.
  • the change to the menu setting is ignored
  • update to 4.1.3
  • repeat the steps from above
  • it works
  • but try it without changing the assigned menu
  • it fails as the alias exists already
  • apply the patch
  • confirm that both cases still work as expected
  • try to only change the menu item title
  • save2copy
  • confirm that the alias number is not just increased but followed the change to the menu title
  • try to only change the alias but keep the title
  • save2copy
  • confirm that the alias has been updated but not the title.

Actual result BEFORE applying this Pull Request

Issues of "alias exists already" while with save2copy we have an "increment the ID" feature

Expected result AFTER applying this Pull Request

The alias will be updated as expected any by using save2copy you should not get alias dubliceted issues.

Documentation Changes Required

n.a

Mentions

@toivo
@bembelimen

Involved within the PR which caused the regression:
@anmode
@chmst
@richard67
@vorayash
@Quy

@toivo
Copy link
Contributor

toivo commented May 17, 2022

I have tested this item ✅ successfully on 096a742

Tested successfully in 4.1.2 upgraded to 4.1.3 and patched, using PHP 8.0.15.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37813.

@Webdongle
Copy link
Contributor

I have tested this item ✅ successfully on 096a742

Full success


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37813.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.1.4 milestone May 17, 2022
@richard67
Copy link
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37813.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 17, 2022
@richard67 richard67 added this to the Joomla 4.1.4 milestone May 17, 2022
@laoneo laoneo added the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 18, 2022
@bembelimen bembelimen merged commit 233e144 into joomla:4.1-dev May 18, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 18, 2022
@bembelimen
Copy link
Contributor

Thx

@bembelimen bembelimen removed the Updates Requested Indicates that this pull request needs an update from the author and should not be tested. label May 18, 2022
@zero-24 zero-24 deleted the fixSave2Copy414 branch May 18, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants