[#37395] - [4.1] Save As Copy moves the original menu item to another menu fix
- Fixed in Code Base
- 2 Apr 2022
- Medium
- Build: 4.1-dev
- # 37395
- Diff
- anmode:save-as-copy
User tests: Successful: Unsuccessful:
Pull Request for Issue #37378
Summary of Changes
$pk is detected wrongly:
1.The controller sets $data['id'] explicitly to 0 if we're in a save2copy task.
2.Therefore $data['id'] is detected as empty in the model and $pk falls back to $this->getState('item.id') which is the id of the menu item to be copied.
Testing Instructions
- Create two menus, e.g. "menu 1" and "menu 2".
- Create a menu item in menu 1. Click Save.
- Edit the first menu item. Change the title and set the menu to the menu 2. Select "Save as Copy"
Actual result BEFORE applying this Pull Request
Both the first and its copy end up in menu 2.
Expected result AFTER applying this Pull Request
The first menu item should remain in menu 1 and the copy should be found in menu 2.
Documentation Changes Required
No Document.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_menus |
| Labels |
Added:
?
|
||
@anmode I think your testing instructions are not complete. They cover only the test of the issue fixed by this PR. But the fix is made in the save method of the component's model, and this is used by any other kind of "Save", "Save & New", "Save & Close" or whatever may be available in frontend editing and in backend. It needs to test also if these are not broken with the change from this PR.
So I think you should add a final test step "Check that Save & Close, Save & New and Save as Copy in backend still work" or something like that.
@anmode I think your testing instructions are not complete. They cover only the test of the issue fixed by this PR. But the fix is made in the save method of the component's model, and this is used by any other kind of "Save", "Save & New", "Save & Close" or whatever may be available in frontend editing and in backend. It needs to test also if these are not broken with the change from this PR.
So I think you should add a final test step "Check that Save & Close, Save & New and Save as Copy in backend still work" or something like that.
Okay I got the point what you are trying to convey I will look into Save & Close, Save & New and Save as Copy too so this fix valid in those.
If any advice related to code. Please tell.
If any advice related to code. Please tell.
@anmode See my answer in the above review comment #37395 (comment) :
I think you should do it like it is already done in the admin model: Just use
isset($data['id'])instead of(!empty($data['id']) || $data['id'] === 0)like it is done here https://github.com/joomla/joomla-cms/blob/4.1-dev/libraries/src/MVC/Model/AdminModel.php#L1353 .
As said, with your current code it will fail when $data['id'] is not set.
If any advice related to code. Please tell.
@anmode See my answer in the above review comment #37395 (comment) :
I think you should do it like it is already done in the admin model: Just use
isset($data['id'])instead of(!empty($data['id']) || $data['id'] === 0)like it is done here https://github.com/joomla/joomla-cms/blob/4.1-dev/libraries/src/MVC/Model/AdminModel.php#L1353 .As said, with your current code it will fail when
$data['id']is not set.
Okay! I'll look into it.
Okay! I'll look into it.
What does it need to look into? I've provided all necessary information already.
Okay! I'll look into it.
What does it need to look into? I've provided all necessary information already.
? just doing changes and pushing commit. Thanks :).
I have tested this change and working correctly on machine.
@vorayash Could you mark your test result in the issue tracker by going to this page https://issues.joomla.org/tracker/joomla-cms/37395 , using the blue "Test this" button at the top left corner, selecting the test result ("Tested successfully" in this case) and then submit? Thanks in advance.
Ok sir.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37395.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37395.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37395.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37395.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37395.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-04-02 04:41:16 |
| Closed_By | ⇒ | Quy |
Thanks
I have tested this item✅ successfully on d27e6fe
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37395.