feat: Add Organization Admin scope to API V1 #14860
Conversation
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 3 Ignored Deployments
|
alishaz-polymath
changed the title
Current Playwright Test Results Summary✅ 66 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 05/17/2024 06:45:28pm UTC) Run DetailsRunning Workflow PR Update on Github Actions Commit: 8bd1cfb Started: 05/17/2024 06:42:22pm UTC
|
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Stripe integration Can book a paid booking
Retry 1 • Initial Attempt |
4.24% (10)10 / 236 runsfailed over last 7 days |
15.25% (36)36 / 236 runsflaked over last 7 days |
📄 apps/web/playwright/hash-my-url.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
hash my url generate url hash
Retry 1 • Initial Attempt |
5.69% (14)14 / 246 runsfailed over last 7 days |
29.67% (73)73 / 246 runsflaked over last 7 days |
📄 apps/web/playwright/event-types.e2e.ts • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Event Types tests -- future user can add multiple organizer address
Retry 1 • Initial Attempt |
1.56% (4)4 / 256 runsfailed over last 7 days |
21.48% (55)55 / 256 runsflaked over last 7 days |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 Three Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
alishaz-polymath
added
api
|
@alishaz-polymath @zomars Update here: I've added a bunch of unit and integration tests here. All pass locally except 1 integration test that I think is due to a change in functionality that I'm not totally sure is correct. @alishaz-polymath is going to follow up on it. What I'm going to focus on now is making sure our integration test suite can properly run on CI. Right now, when I run it, the tests pass but then almost all downstream E2E jobs fail so seems like the integration test suite is corrupting something. |
|
Ok - CI has been updated and is now correctly running all tests. See https://github.com/calcom/cal.com/actions/runs/9132154000. The 1 failing integration as mentioned can be seen there. |
| @@ -36,7 +36,7 @@ jobs: | |||
| secrets: inherit | |||
|
|
|||
| test: | |||
| name: Unit tests | |||
| name: Tests | |||
There was a problem hiding this comment.
Naming all of these Tests results in the following grouping, which is nicer than having all of the tests separated.
So I had a look and basically the system-wide admin functionality hasn't been changed. As you can see here the system-admin check is only supposed to function when userId is passed along with it. Now I'm not entirely sure about the reasoning behind this, but I think with pagination, and continuing to allow this as well, we could very well do the same for systemwide admins as we do with org admins here as well. tldr; I didn't modify the functionality of systemwide admins in this PR, but I think we should, as that's intuitive IMO |
What does this PR do?
isAdmin→isSystemWideAdminfor added clarity by clarifying scope of access to be system wideisOrganizationOwnerOrAdminand the scope of access is the members of the organizationisOrganizationOwnerOrAdminon bookings endpoint as an example:This PR also adds Pagination to the bookings endpoint, accessible by passing
take&pageparameters, where take represents the number of items in the resulting array and page represents the page number when taking 'take' number of items per page.For example, if there are 20 results, we can use take=5 and page=3 to get results 11-15.
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist