-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
Task/multiple roles groups #4512
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
b10ef43
to
e0921c7
Compare
cc1559f
to
fa2ed10
Compare
b54295b
to
fdf6f96
Compare
cy.get(`[data-testid='${PA_ASSIGN_CREATE_ID}']`).click(); | ||
cy.wait('@editAccess'); | ||
cy.get("td span:contains('Owner')").should('have.length', 2); | ||
cy.get("td span:contains('2 roles')").should('have.length', 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 , actual readable tests.
<MultipleRoleSelect | ||
data-testid={PA_ROLE_ID} | ||
roles={roles} | ||
value={selectedRoles} | ||
setValue={setRoles} | ||
/> | ||
)} | ||
elseShow={() => ( | ||
<RoleSelect | ||
data-testid={PA_ROLE_ID} | ||
roles={roles} | ||
value={selectedRoles[0]} | ||
setValue={role => | ||
setRoles(role ? [role] : []) | ||
} | ||
/> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice way of using the flag to turn it on/off :)
try { | ||
const res = await makeRequest(req.caller, req.id); | ||
|
||
return res; | ||
} catch (e) { | ||
throw e; | ||
} | ||
return await makeRequest(req.caller, req.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good boyscouting here, why try-catch when we're just rethrowing :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 from @chriswk
Does what it says on the tin, should help with cleaning up #4512 and respective schema changes. --------- Co-authored-by: Gastón Fournier <gaston@getunleash.io>
https://linear.app/unleash/issue/2-1128/change-the-api-to-support-adding-multiple-roles-to-a-usergroup-on-a
https://linear.app/unleash/issue/2-1125/be-able-to-fetch-all-roles-for-a-user-in-a-project
https://linear.app/unleash/issue/2-1127/adapt-the-ui-to-be-able-to-do-a-multi-select-on-role-permissions-for
useProjectApi
to new methods that use new endpoints that support multiple rolesmultipleRoles
feature flag that controls the possibility of selecting multiple roles on the UIProjectAccessAssign
to support multiple role, using the new methodsMultipleRoleSelect
component that allows you to select multiple roles based on theRoleSelect
componentRoleCell
component to support either a single role or multiple rolesaccess.spec.ts
Cypress e2e test to reflect our new logicaccess-service.e2e.test.ts
with tests covering the multiple roles logic and covering some corner casesproject-service.e2e.test.ts
to adapt to the new logic, adding a test that covers adding access with[roles], [groups], [users]