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

IA-3123: Review the look and feel of permissions list (with Read/Write) #1664

Merged
merged 27 commits into from
Oct 4, 2024

Conversation

hakifran
Copy link
Contributor

@hakifran hakifran commented Oct 1, 2024

Explain what problem this PR is resolving

  • Review the look and feel of permissions list (with Read/Write)
    Related JIRA tickets : IA-3123

Self proofreading checklist

  • Did I use eslint and black formatters
  • Is my code clear enough and well documented
  • Are my typescript files well typed
  • New translations have been added or updated if new strings have been introduced in the frontend
  • My migrations file are included
  • Are there enough tests
  • Documentation has been included (for new feature)

Doc

  • iaso/docs/pages/dev/how_to/add_new_permission/add_new_permission.md
  • 4. If the added permission must be coupled with another permission like read and edit

Changes

  • Added a READ_EDIT_PERMISSIONS constant in hat/menupermissions/constants.py
  • Format Permissions to include the matching read-edit
  • Display the matching read-edit permission
  • Replace Switch component by checkbox
  • Add translations

How to test

  • Go Admin ---> Users ---> Management and Go to Admin ---> User Roles
  • Create or edit a user and a user role
  • Add and remove permissions and specifically those with read and edi permission

Print screen / video

Screencast from 2024-10-02 12-36-51.webm
Screencast from 2024-10-02 12-37-22.webm

@hakifran hakifran requested a review from beygorghor October 1, 2024 11:05
Copy link
Collaborator

@beygorghor beygorghor left a comment

Choose a reason for hiding this comment

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

Good job, We will improve lay-out a bit after. I'm just wondering why you have read twice in the checkboxes? it should be read and write no ?

Screenshot 2024-10-02 at 10 46 39

const hasPermission =
role.permissions.includes(value);
return (
<Stack
Copy link
Collaborator

Choose a reason for hiding this comment

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

never used this, well done ❤️

@hakifran
Copy link
Contributor Author

hakifran commented Oct 2, 2024

Good job, We will improve lay-out a bit after. I'm just wondering why you have read twice in the checkboxes? it should be read and write no ?
Screenshot 2024-10-02 at 10 46 39

Yeah it's a translation issue: "iaso.permissions.readEdit.edit": "Read",. I Will fix that

@hakifran hakifran requested a review from beygorghor October 2, 2024 13:12
@beygorghor beygorghor added the release Should be released in production at next deploy label Oct 4, 2024
Copy link
Collaborator

@beygorghor beygorghor left a comment

Choose a reason for hiding this comment

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

I have updated your branch with main and resolve conflicts. I have tested it after and works like a charm

@hakifran hakifran merged commit c97d964 into main Oct 4, 2024
3 checks passed
@hakifran hakifran deleted the IA-3123-review-look-and-feel-permission-list branch October 4, 2024 11:24
@kemar kemar added Released and removed release Should be released in production at next deploy labels Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants