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

Fixes the issue which causes custom menu items to be lost on component update #13857

Merged
merged 1 commit into from Feb 2, 2017

Conversation

izharaazmi
Copy link
Contributor

@izharaazmi izharaazmi commented Feb 2, 2017

Pull Request for Issue #13642.

Summary of Changes

  1. Changed \JInstallerAdapterComponent::_removeAdminMenus to remove only the protected menu items, i.e. - menutype == 'main' and leave alone other menutypes.
  2. Replicated \JInstallerAdapterComponent::_updateSiteMenus as \JInstallerAdapterComponent::_updateMenus to update the admin menus as well.

Testing Instructions

  1. Create a custom admin menu.
  2. Install any extension.
  3. Create a menu link for the component in your custom menu.
  4. Reinstall/update your component.

Extra: Uninstall your component. The menu item should still exist in your custom admin menu, though orphaned.

Expected result

The custom menu item in step 3 above, should still point to the same component.

Actual result

Without this patch The custom menu item in step 3 above is removed. After this patch, we get the expected result.

Documentation Changes Required

None

Pinging @rdeutz @infograf768 @Bakual

@ghost
Copy link

ghost commented Feb 2, 2017

Tested successfully (Item not found at Issue Tracker).

Test using Sermonspeaker installing, uninstalling, reinstalling (not tested on Update). All works like described in Instructions.

@rdeutz
Copy link
Contributor

rdeutz commented Feb 2, 2017

It seems there is a problem with the issue tracker, I added the PR-Staging label manuell.

Could you @mbabker have a lock when you have a minute

@rdeutz
Copy link
Contributor

rdeutz commented Feb 2, 2017

I also tested it successfully

@mbabker
Copy link
Contributor

mbabker commented Feb 2, 2017

There's already a tracking issue... joomla/jissues#939

@mbabker
Copy link
Contributor

mbabker commented Feb 2, 2017

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 2, 2017
@Kubik-Rubik
Copy link
Member

Thank you @izharaazmi and testers!

@Kubik-Rubik Kubik-Rubik merged commit e6f73bc into joomla:staging Feb 2, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 2, 2017
@izharaazmi izharaazmi deleted the am-component-update branch February 2, 2017 19:06
@izharaazmi
Copy link
Contributor Author

@rdeutz I am going to propose a little improvement tomorrow. After beta release it might not be possible to get it in 3.7. Could you please wait to atleast see it before you mark the beta release.

@rdeutz
Copy link
Contributor

rdeutz commented Feb 2, 2017

@izharaazmi I am preparing the beta release just now, we have a beta2 next week, if it makes sense we will add it.

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.

None yet

5 participants