Skip to content

I18n/ permissions rework #9535

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 53 commits into from
Mar 25, 2021
Merged

I18n/ permissions rework #9535

merged 53 commits into from
Mar 25, 2021

Conversation

@Convly
Copy link
Member

@Convly Convly commented Feb 26, 2021 •
edited
Loading

This PR aims to rework some components of the admin permissions system (rbac) in order to fit better with the use of plugins and/or future modifications.

  • Replaces the permission.fields attribute by permission.properties + dedicated migration
  • Adds fields as a possible attribute of permission.properties
  • Rework the GET /permissions layout. Make it more generic.
  • Adapt the permissions engine to the new permission format, allow registering permissions handlers to modify the current payload (add a condition to the permission, update its properties, etc...)
  • Adds the capability to register callbacks whenever a new action is added to the action provider
  • Adds a domain object for the permission (rbac) to handle business-related operations
  • Register i18n rbac handlers
  • Rework the action (rbac) shape (removes fieldsRestriction, adds applyToProperties)
  • Update the validators for the admin APIs
  • Update the tests(integration + unit)

Convly
Convly commented Feb 26, 2021
View reviewed changes
Convly
Convly commented Feb 26, 2021
View reviewed changes
Convly
Convly commented Feb 26, 2021
View reviewed changes
@Convly Convly force-pushed the i18n/permissions-engine-v2 branch from 7d34089 to 3029f0c Compare March 1, 2021 14:34
@soupette soupette mentioned this pull request Mar 2, 2021
Merged
@Convly Convly marked this pull request as ready for review March 4, 2021 16:13
@Convly Convly added the flag: don't merge This PR should not be merged at the moment label Mar 5, 2021
@petersg83 petersg83 self-requested a review March 8, 2021 08:50
@Convly Convly force-pushed the i18n/permissions-engine-v2 branch from ef92a94 to e0aef45 Compare March 8, 2021 10:44
Convly added 17 commits March 8, 2021 18:20
@Convly
Add a domain layer for the permission, rework the engine handling of …
5a219f1 
…the permissions

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Add permissions-fields-to-properties migration for the admin
113c7a3 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Removes useless console.log
86b0403 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Remove debug logLevel from provider-login.test.e2e.js
014b3d4 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Adds the new layout for the GET permissions, allow to subscribe to ac…
6ea73ff 
…tionRegistered events, adds i18n handlers

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Fix typo
940c79b 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Update permissions validators
5aac7b5 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Update unit tests
e692758 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Update integrations test + fix some validation issues
333f152 
Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>
@Convly
Change plugins & settings section format for the permissions layout
c228eb4
@Convly
only return locales property to localized subjects for the permission…
820b3bf 
…'s layout
@Convly
Do not send the locales property to the permission's layout when ther…
e97c083 
…e is no locales created
@Convly
Add the 'locales' property to publish & delete routes
6021bcf
@Convly
Fix unwanted mutation of the sections builder states on multiple builds
93f4ca4
@Convly
Fix units tests with (new engine)
5857b46
@Convly
Fix admin-role e2e test - Add locales property to the update payload
b75be40
@Convly
fix e2e testsé
3995c47
@Convly Convly requested a review from alexandrebodin March 22, 2021 08:51
alexandrebodin
alexandrebodin requested changes Mar 22, 2021
View reviewed changes
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

Overwall way cleaner than what we had before 💯 . Really hard to ready all of this thouroughly though I will still need to review the tests

alexandrebodin
alexandrebodin reviewed Mar 23, 2021
View reviewed changes
Convly and others added 17 commits March 23, 2021 14:24
@Convly
pr comments (without the migrations)
2379408
@Convly
fix fields-to-properties migration
2ff9339
@Convly
Add jsdoc for the sections-builder
393f3b2
@Convly
Moves createBoundAbstractDomain from permission domain to the engine …
4011198 
…service
@Convly
Merge branch 'features/i18n' into i18n/permissions-engine-v2
b7157f0
@Convly
Remove debug logLevel for admin role test (e2e)
f188254
@Convly
Fix core-store
074dd40
@Convly
Fix hooks & move business logic from i18n bootstrap to dedicated serv…
97dcf1c 
…ices
@petersg83
add route get-non-localized-fields
f52494b
@petersg83
use write and read permission
a8f487f
@petersg83
refacto
315eb43
@petersg83
add input validator
f78d7ce
@petersg83
add route doc
3cade52
@Convly
Merge branch 'features/i18n' of github.com:strapi/strapi into i18n/pe…
6fec5b7 
…rmissions-engine-v2
@petersg83
handle ST
31d8cb7
@alexandrebodin
Merge pull request #9800 from strapi/i18n/nonLocalizedRouteWithRBAC
b3b36ad 
[I18n] non localized route with rbac
@Convly
Merge branch 'features/i18n' of github.com:strapi/strapi into i18n/pe…
c6ae731 
…rmissions-engine-v2
alexandrebodin
alexandrebodin approved these changes Mar 25, 2021
View reviewed changes
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

Will add one more round of testing on the feature branch

@alexandrebodin alexandrebodin merged commit 1e5b1c9 into features/i18n Mar 25, 2021
@Convly Convly removed the flag: don't merge This PR should not be merged at the moment label Mar 25, 2021
@alexandrebodin alexandrebodin deleted the i18n/permissions-engine-v2 branch March 25, 2021 13:59
@tanmoyopenroot tanmoyopenroot mentioned this pull request Jun 5, 2021
Closed
@derrickmehaffy
Copy link
Member

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/problems-with-upgrade-to-3-6-3/5408/6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexandrebodin alexandrebodin alexandrebodin approved these changes

@petersg83 petersg83 Awaiting requested review from petersg83

+1 more reviewer

@soupette soupette soupette left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Convly @derrickmehaffy @alexandrebodin @soupette @petersg83