Skip to content

chore: preserve non-org dashboard for platform users#16181

Merged
zomars merged 14 commits intomainfrom
fix-dashboard-for-platform
Aug 14, 2024
Merged

chore: preserve non-org dashboard for platform users#16181
zomars merged 14 commits intomainfrom
fix-dashboard-for-platform

Conversation

@ThyMinimalDev
Copy link
Copy Markdown
Contributor

@ThyMinimalDev ThyMinimalDev commented Aug 13, 2024 •
edited
Loading

What does this PR do?

Preserve regular dashboard for platform users since they don't have custom domains

Screen.Recording.2024-08-13.at.13.08.10.mov

In order to preserve the dashboard with minimal change, we try to exclude the platform org profiles when it's not necessary

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A - I have added a Docs issue here if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • N/A - I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • create a platform org
  • try to access public profile page
  • it should still point to /username
  • /platform/settings should be accessible

@graphite-app graphite-app Bot requested a review from a team August 13, 2024 10:09
@keithwillcode keithwillcode added core area: core, team members only platform Anything related to our platform plan labels Aug 13, 2024
@graphite-app graphite-app Bot requested a review from a team August 13, 2024 10:09
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Aug 13, 2024

Graphite Automations

"Add foundation team as reviewer" took an action on this PR • (08/13/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add platform team as reviewer" took an action on this PR • (08/13/24)

1 reviewer was added to this PR based on Keith Williams's automation.

supalarry
supalarry previously approved these changes Aug 13, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@supalarry supalarry left a comment

Choose a reason for hiding this comment

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

🔥

hariombalhara
hariombalhara previously approved these changes Aug 13, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks like a clean solution but it will need some more work to ensure all links are working fine. Because we use other endpoints too to identify the organization, we will have to handle there as well.

I am not sure of a clean approach to handle it but it would be messy to make things behave like not an org when it is an org technically.

EventType Booking page isn't working: https://www.loom.com/share/62183473398f4027a2d1d49b60490634

I am fine merging it and taking care of other things in a followup though.

@ThyMinimalDev
Copy link
Copy Markdown
Contributor Author

ThyMinimalDev commented Aug 13, 2024

Looks like a clean solution but it will need some more work to ensure all links are working fine. Because we use other endpoints too to identify the organization, we will have to handle there as well.

I am not sure of a clean approach to handle it but it would be messy to make things behave like not an org when it is an org technically.

EventType Booking page isn't working: https://www.loom.com/share/62183473398f4027a2d1d49b60490634

I am fine merging it and taking care of other things in a followup though.

Hey Hariom, I have a new commit that I will push soon hopinh to fix all the links at once

@ThyMinimalDev ThyMinimalDev marked this pull request as draft August 13, 2024 12:08
@ThyMinimalDev ThyMinimalDev dismissed stale reviews from hariombalhara and supalarry via 981b9d3 August 13, 2024 14:21
@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 13, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Aug 14, 2024 6:03pm
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Aug 14, 2024 6:03pm

@ThyMinimalDev ThyMinimalDev marked this pull request as ready for review August 13, 2024 14:21
@ThyMinimalDev ThyMinimalDev added High priority Created by Linear-GitHub Sync high-risk Requires approval by Foundation team labels Aug 14, 2024
@vercel vercel Bot temporarily deployed to Preview – dev August 14, 2024 08:39 Inactive
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 14, 2024 •
edited
Loading

E2E results are ready!

zomars
zomars approved these changes Aug 14, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

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

LGTM. left a non blocking change

Comment thread packages/ui/components/icon/icon-list.mjs
"x",
"zap",
"blocks",
]);
Copy link
Copy Markdown
Contributor

@zomars zomars Aug 14, 2024

Choose a reason for hiding this comment

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

@ThyMinimalDev lets sort this alphabetically as well 🙏

@zomars zomars merged commit 8460654 into main Aug 14, 2024
@zomars zomars deleted the fix-dashboard-for-platform branch August 14, 2024 22:11
hariombalhara
hariombalhara reviewed Aug 15, 2024
View reviewed changes
Comment thread packages/features/eventtypes/lib/getPublicEvent.ts
select: publicEventSelect,
});

// If no event was found, check for platform org user event
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara Aug 15, 2024

Choose a reason for hiding this comment

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

// If no event was found, check for platform org's unmanaged user 's events

hariombalhara
hariombalhara reviewed Aug 15, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara left a comment

Choose a reason for hiding this comment

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

Looks good

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

Reviewers

@hariombalhara hariombalhara hariombalhara left review comments

@Ryukemeister Ryukemeister Awaiting requested review from Ryukemeister

@supalarry supalarry Awaiting requested review from supalarry

+1 more reviewer

@zomars zomars zomars approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync high-risk Requires approval by Foundation team platform Anything related to our platform plan ready-for-e2e

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ThyMinimalDev @hariombalhara @zomars @supalarry @keithwillcode