-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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: Update visibility of team members based on user role #14629
fix: Update visibility of team members based on user role #14629
Conversation
… are not org admin
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Ignored Deployments
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @sean-brydon and the rest of your teammates on Graphite |
const hideMembers = | ||
(ctx.user.profile?.organization?.isPrivate && !ctx.user.organization?.isOrgAdmin) || | ||
(isOrgPrivate && !isOrgAdminOrOwner && input.teamId === ctx.user.organizationId) || |
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.
We need to check the teamId here to ensure we are not targeting a team other than the org ID
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
Current Playwright Test Results Summary✅ 314 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 04/22/2024 01:44:25pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 2b546f6 Started: 04/22/2024 01:40:47pm UTC
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Workflow Tab - Event Type Check the functionalities of the Workflow Tab Team Workflows Admin user
Retry 1 • Initial Attempt |
0.47% (1)1 / 214 runfailed over last 7 days |
0.47% (1)1 / 214 runflaked over last 7 days |
📄 apps/web/playwright/profile.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Update Profile Can update a users email (verification enabled)
Retry 1 • Initial Attempt |
6.02% (13)13 / 216 runsfailed over last 7 days |
28.24% (61)61 / 216 runsflaked over last 7 days |
📄 apps/web/playwright/payment-apps.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Payment app Should not display App is not setup already for non payment app
Retry 1 • Initial Attempt |
0% (0)0 / 217 runsfailed over last 7 days |
0.46% (1)1 / 217 runflaked over last 7 days |
📄 apps/web/playwright/event-types.e2e.ts • 2 Flakes
Top 1 Common Error Messages
|
2 Test Cases Affected |
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Event Types tests -- future user Different Locations Tests can add Attendee Phone Number location and book with it
Retry 1 • Initial Attempt |
0% (0)0 / 244 runsfailed over last 7 days |
6.97% (17)17 / 244 runsflaked over last 7 days |
Event Types tests -- future user Different Locations Tests Can add Cal video location and book with it
Retry 1 • Initial Attempt |
11.89% (29)29 / 244 runsfailed over last 7 days |
3.69% (9)9 / 244 runsflaked over last 7 days |
📄 apps/web/playwright/teams.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Teams - NonOrg -- future Can create a booking for Round Robin EventType
Retry 1 • Initial Attempt |
13.25% (33)33 / 249 runsfailed over last 7 days |
17.27% (43)43 / 249 runsflaked over last 7 days |
📄 apps/web/playwright/settings-admin.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Settings/admin tests -- future should render /settings/admin page
Retry 1 • Initial Attempt |
1.19% (3)3 / 252 runsfailed over last 7 days |
1.98% (5)5 / 252 runsflaked over last 7 days |
📄 packages/app-store/routing-forms/playwright/tests/basic.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Routing Forms Seeded Routing Form Test preview should return correct route
Retry 1 • Initial Attempt |
0% (0)0 / 241 runsfailed over last 7 days |
34.85% (84)84 / 241 runsflaked over last 7 days |
📄 packages/embeds/embed-core/playwright/tests/preview.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Preview Preview - embed-core should load
Retry 1 • Initial Attempt |
0% (0)0 / 243 runsfailed over last 7 days |
41.56% (101)101 / 243 runsflaked over last 7 days |
📄 apps/web/playwright/organization/across-org/across-org.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
user1NotMemberOfOrg1 is part of team1MemberOfOrg1 EventTypes listing should show correct link for user events and team1MemberOfOrg1's events
Retry 2 • Retry 1 • Initial Attempt |
1.59% (4)4 / 251 runsfailed over last 7 days |
7.17% (18)18 / 251 runsflaked over last 7 days |
📄 apps/web/playwright/app-store.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
App Store - Authed -- future Browse apple-calendar and try to install
Retry 1 • Initial Attempt |
0% (0)0 / 243 runsfailed over last 7 days |
0.82% (2)2 / 243 runsflaked over last 7 days |
📄 apps/web/playwright/reschedule.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Reschedule Tests Should display request reschedule send on bookings/cancelled
Retry 1 • Initial Attempt |
0% (0)0 / 220 runsfailed over last 7 days |
0.45% (1)1 / 220 runflaked over last 7 days |
📄 packages/embeds/embed-react/playwright/tests/basic.e2e.ts • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
React Embed Element Click Popup should verify that the iframe got created with correct URL - namespaced
Retry 1 • Initial Attempt |
21.78% (22)22 / 101 runsfailed over last 7 days |
35.64% (36)36 / 101 runsflaked over last 7 days |
Graphite Automations"Add consumer team as reviewer" took an action on this PR • (04/17/24)1 reviewer was added to this PR based on Keith Williams's automation. |
const hideMembers = | ||
(ctx.user.profile?.organization?.isPrivate && !ctx.user.organization?.isOrgAdmin) || | ||
(isOrgPrivate && !isOrgAdminOrOwner && input.teamId === ctx.user.organizationId) || |
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 think it could make more sense to convert it into a function that does it something like this
function shouldHideMembers() {
if (!team.isPrivate) {
return false;
}
return isTeamAnOrg ? !isOrgAdminOrOwner : !isOrgAdminOrOwner && !isTeamAdminOrOwner
}
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.
@sean-brydon can you also add automated tests for the fix?
Yep working on it now <3 |
}) { | ||
return await prisma.team.create({ | ||
data: { | ||
name: name, | ||
slug: slug, | ||
isOrganization: true, | ||
isPrivate: isPrivate, |
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.
Allow us to create private orgs
@@ -788,7 +788,7 @@ const createUser = ( | |||
}, | |||
}, | |||
accepted: true, | |||
role: MembershipRole.ADMIN, | |||
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.
Actualy pass in the role to the figure - previously all were admin
const tableLocator = await page.getByTestId("team-member-list-container"); | ||
await expect(tableLocator).toBeVisible(); | ||
|
||
const memberUser = await prisma.membership.findFirst({ |
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.
Easier than selecting from the UI
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.
LGTM now !! Thanks for the tests @sean-brydon
…n_when_org_is_private_and_they_are_not_org_admin
…n_when_org_is_private_and_they_are_not_org_admin
What does this PR do?
Fixes CAL-3484
Requirement/Documentation
Type of change
How should this be tested?
Mandatory Tasks
Checklist