magento-lts icon indicating copy to clipboard operation
magento-lts copied to clipboard

Published 20 hours ago • verified publisher icon OpenMage

Remove deprecated Admin user multirole functionality

Open elidrissidev opened this issue 2 years ago • 8 comments

Description (*)

This PR aims to get rid of all the remnant code of multi-role functionality for Admin user. This functionality was deprecated ages ago and it's just tech debt at this point. Currently, only one Role can be assigned to an Admin user.

This is not meant to introduce any major breaking changes, although I did a change in a template file in adminhtml, but that's because I doubt someone will have that file overridden.

Todo

  • [ ] Consolidate Mage_Admin_Model_Resource_User::saveRelations() and Mage_Admin_Model_Resource_User::add() methods, both used to add a role to a user.
  • [ ] Deprecate methods that operate on array of roles (e.g. Mage_Admin_Model_Resource_User::getRoles), and replace them with methods that work with a single role.

Manual testing scenarios (*)

  1. Assigning a Role to an Admin user from user edit page should work as usual.
  2. Assigning a Role to an Admin user from role edit page (Role Users tab) should work as usual.

Contribution checklist (*)

  • [x] Pull request has a meaningful description of its purpose
  • [ ] All commits are accompanied by meaningful commit messages
  • [ ] All automated tests passed successfully (all builds are green)
  • [x] Add yourself to contributors list

elidrissidev avatar Jan 01 '23 17:01 elidrissidev

does it need to be in the 19.x releases?

Flyingmana avatar Jan 01 '23 18:01 Flyingmana

does it need to be in the 19.x releases?

Not necessarily, I can change the target branch when I'm done.

elidrissidev avatar Jan 01 '23 21:01 elidrissidev

Created a bit of a mess, but this refactor will now target v20.

elidrissidev avatar Jan 03 '23 11:01 elidrissidev

does it need to be in the 19.x releases?

Why not remove dead code at all?

sreichel avatar Jan 04 '23 01:01 sreichel

@elidrissidev what do we need to finish this?

fballiano avatar May 10 '23 15:05 fballiano

@elidrissidev do you remember what's needed in order to finish this PR?

fballiano avatar Sep 18 '23 16:09 fballiano

The Admin code is kinda messy and all over the place, the current state of the PR should be working fine, but there's still some classes that seem useless and could be removed (e.g. Mage_Admin_Model_Roles could be merged with Mage_Admin_Model_Role model since they represent the same entity) but I don't remember if refactoring will break BC.

elidrissidev avatar Sep 18 '23 22:09 elidrissidev

I kinda think that this should be in next but, @elidrissidev do you think you'll restart your work on this or what should we do on this PR?

fballiano avatar Feb 21 '24 23:02 fballiano