-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Don't hardcode the Magento_Backend::admin index #4396
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paliarush
changed the title
don't hardcode the Magento_Backend::admin index
Don't hardcode the Magento_Backend::admin index
May 5, 2016
@annybs Thank you for contribution. Please fix failing builds and then we can accept your PR. Internal ticket number MAGETWO-52555. |
paliarush
added
Issue: Ready for Work
Gate 4. Acknowledged. Issue is added to backlog and ready for development
Progress: needs update
and removed
in progress
labels
May 5, 2016
vkorotun
added
Admin Panel
improvement
and removed
PS
Issue: Ready for Work
Gate 4. Acknowledged. Issue is added to backlog and ready for development
labels
Aug 3, 2016
mmansoor-magento
pushed a commit
that referenced
this pull request
Aug 21, 2016
… index #4396 - Merge branch 'develop' of https://github.com/annybs/magento2 into annybs-develop
mmansoor-magento
pushed a commit
that referenced
this pull request
Aug 21, 2016
mmansoor-magento
pushed a commit
that referenced
this pull request
Aug 21, 2016
vkorotun
added
Issue: Ready for Work
Gate 4. Acknowledged. Issue is added to backlog and ready for development
and removed
linked
labels
Aug 22, 2016
Will this fix work for 2.1 as well? If not, when will a 2.1 port occur? |
@mmansoorebay I see this merge wasn't included in 2.1.2. Is it safe to incorporate into a 2.1.2 production environment? |
magento-engcom-team
pushed a commit
that referenced
this pull request
Jun 24, 2019
[TSG-CSL3] For 2.2 (pr33)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
improvement
Issue: Ready for Work
Gate 4. Acknowledged. Issue is added to backlog and ready for development
Progress: accept
Progress: needs update
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I encountered a problem with a third party module that specified
Magento_Adminhtml::admin
(incorrectly, I realise) as its root resource inacl.xml
which, lacking a sortOrder, got shuffled to$resources[0]
pushing theMagento_Backend::admin
resource back to$resources[2]
. This resulted in jstree rendering no ACL resources in adminhtml, as the$resource[1]
being selected wasMagento_Backend::all
which has no children.This could be argued to be an issue with the third party module, but selecting a hard-coded index out of a non-immutable array is problematic regardless, and it's not good that such a small error completely breaks ACL management. I submit this PR to specify the correct resource explicitly in relevant locations, rather than relying on an assumption about the resource array's indices.