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

[permissions] Renaming and organizing permissions phase 0 #7327

Merged
merged 7 commits into from
Mar 12, 2021

Conversation

ridz1208
Copy link
Collaborator

@ridz1208 ridz1208 commented Feb 5, 2021

Brief summary of changes

In an attempt to organise and standardise the existing permissions I have created a couple categories in the permissions table to leverage the modules architecture and add the action column and generate the lable based on the combination of columns as a first phase.

Changes in this PR:

  • addition of 2 new columns composing the permission label
  • deletion of 4 unsused permissions
  • renumbering of entries in permission patch
  • swap of the permission and modules patch (01 and 02)
  • RB permission changes
  • logic for label generation for permissions

new permissions......
image

]
];

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@driusan @kongtiaowang

I had to modify the test class to have the PR pass checks, the problem is, because I need toinstantiate a module to get its Long name, i resorted to use real modules (candidate_list and timepoint_list) which means I am king of breaking the rules of unit testing here because I am not only testing the getPermissionsVerbose() function but also the getActiveModulesIndexed() function of the module class. I know we should instead be using a mock module but I'm not sure how to do that. let me know if and how you want this PR changed

@laemtl
Copy link
Contributor

laemtl commented Feb 8, 2021

@ridz1208 #6770 is kind of related. Do you mind taking a look into it? It's open to discussion and was an attempt to solve #6687.

@kongtiaowang kongtiaowang added the Passed manual tests PR has been successfully tested by at least one peer label Feb 15, 2021
Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing changes to the filenames in the php/installer directory

@ridz1208
Copy link
Collaborator Author

@driusan changes made, tests running

@ridz1208
Copy link
Collaborator Author

@driusan anything still needed for this one ? you want someone else to review first ?

@driusan
Copy link
Collaborator

driusan commented Mar 12, 2021

I just re-skimmed it, but it looks good to me.

@driusan driusan merged commit cf56c0b into aces:main Mar 12, 2021
@ridz1208 ridz1208 mentioned this pull request Mar 31, 2021
5 tasks
driusan pushed a commit that referenced this pull request Apr 14, 2021
In PR #7327 the SQL patch for existing projects was missing the alter table statements that were required for the update statements to be run.
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
Add "actions" to and module to permissions table in order to harmonize and make the display of permissions in the frontend more consistent.
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
In PR aces#7327 the SQL patch for existing projects was missing the alter table statements that were required for the update statements to be run.
@ridz1208 ridz1208 added this to the 24.0.0 milestone Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed manual tests PR has been successfully tested by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permission System Revamp - Step 1: Rename current permissions to standardise format
4 participants