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-3102: Permissions : Org unit “view only” permission #1558

Merged
merged 7 commits into from
Aug 22, 2024

Conversation

hakifran
Copy link
Contributor

@hakifran hakifran commented Aug 19, 2024

Explain what problem this PR is resolving

  • Permissions : Org unit “view only” permission
    Related JIRA tickets : IA-3102

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)

Changes

  • Add new Organisation units - Read permission
  • Check on create_org_unit and edit org unit the Organisation units management permission
  • Hide some button and action if the user has not Organisation units management permission on frontend

How to test

  • Create or update a user and remove the Organisation units management and add Organisation units - Read permission to it
  • Check all the actions for create and update org units are possible

Print screen / video

Screencast.from.2024-08-19.14-23-35.webm

Notes

Things that the reviewers should know: known bugs that are out of the scope of the PR, other trade-offs that were made.
If the PR depends on a PR in bluesquare-components, or merges into another PR (i.o. main), it should also be mentioned here

@hakifran hakifran requested a review from kemar August 19, 2024 12:26
Copy link
Member

@kemar kemar left a comment

Choose a reason for hiding this comment

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

I've tested it locally by adding just the read-only perm to a user but I can't see any org unit in the list and I've got a bunch of 403.

Is this normal?

Screen.Recording.2024-08-21.at.15.54.46.mov

iaso/tests/api/test_orgunits.py Outdated Show resolved Hide resolved
@hakifran
Copy link
Contributor Author

hakifran commented Aug 21, 2024

I've tested it locally by adding just the read-only perm to a user but I can't see any org unit in the list and I've got a bunch of 403.

Is this normal?
Screen.Recording.2024-08-21.at.15.54.46.mov

That's right, I omitted the read permission on the groups, data source. It is fixed with the commit . Thank you

@hakifran hakifran requested a review from kemar August 22, 2024 09:18
@kemar
Copy link
Member

kemar commented Aug 22, 2024

Almost good to me.

My last concern is a UI problem:

  • if you can just READ org units
  • only form buttons are removed
  • so you still see inputs elements
  • this can make a user believe an input can be changed

I guess it's OK because there are no spec on this in the Jira ticket…

What do you think?

ui

@hakifran
Copy link
Contributor Author

Almost good to me.

My last concern is a UI problem:

* if you can just READ org units

* only form buttons are removed

* so you still see inputs elements

* this can make a user believe an input can be changed

I guess it's OK because there are no spec on this in the Jira ticket…

What do you think?

ui

I think you are right we should disable the inputs too. Thank you, let me fix it!

@hakifran
Copy link
Contributor Author

Almost good to me.

My last concern is a UI problem:

* if you can just READ org units

* only form buttons are removed

* so you still see inputs elements

* this can make a user believe an input can be changed

I guess it's OK because there are no spec on this in the Jira ticket…

What do you think?

ui

I'v just pushed the commit about disabling inputs when the use has not org unit managament permission. But there is on input <InputComponent keyValue="aliases" onChange={onChangeInfo} value={orgUnitState.aliases.value} type="arrayInput" /> Which I made a fix but which I think the change(disabled should be added to bluesquare-component)

Copy link
Member

@kemar kemar left a comment

Choose a reason for hiding this comment

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

LGTM

@hakifran hakifran merged commit da65326 into main Aug 22, 2024
5 checks passed
@hakifran hakifran deleted the IA-3102-Org-unit-view-only-permission branch August 22, 2024 13:48
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