-
-
Notifications
You must be signed in to change notification settings - Fork 723
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
fix: group roles assumption, refactor group types #4576
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
cy.get("td span:contains('2 roles')").should('have.length', 1); | ||
}); | ||
// TODO: This works locally but not on GH actions, I assume because of the way we (don't) handle feature flagging for Cypress tests in GH actions. | ||
// This should be uncommented again once we remove the multipleRoles flag or if we find a better solution in the meantime. |
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.
FYI
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.
I can reproduce the error locally, but unsure why it's happening. I'm receiving users without roles
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
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.
When you're around let's check this test that you commented out
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.
LG!
c4c404b
to
6b46924
Compare
: access?.roles.find( | ||
({ id }) => id === row.entity.roleId | ||
)?.name | ||
: 'No Roles!', |
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.
@gastonfournier I think this changed slipped by me and I'm curious: What was the reason for it? Do we expect any reason why roles
can be falsy?
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.
Oh, I think this was accidental when trying to figure out why the tests were failing, I added this to try to see if this was the issue in cypress
Does what it says on the tin, should help with cleaning up #4512 and respective schema changes.