Skip to content

fix: Make sure we get slug to decide things#9727

Merged
zomars merged 4 commits into
mainfrom
fix/org-update-handler
Jun 27, 2023
Merged

fix: Make sure we get slug to decide things#9727
zomars merged 4 commits into
mainfrom
fix/org-update-handler

Conversation

@leog
Copy link
Copy Markdown
Contributor

@leog leog commented Jun 22, 2023

What does this PR do?

Organization update handler was removing slug from everywhere in case the input didn't include it. Wrapping the existing logic just to when slug is present.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

When creating an organization, going through the about step (3rd) was removing the slug as the requested one, resulting in not being able to create teams as it checks the existence of a valid slug.

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

@leog leog added High priority Created by Linear-GitHub Sync organizations area: organizations, orgs labels Jun 22, 2023
@leog leog requested a review from a team June 22, 2023 19:56
@leog leog self-assigned this Jun 22, 2023
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 22, 2023

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

Name Status Preview Comments Updated (UTC)
api ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2023 8:53pm
cal ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2023 8:53pm
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2023 8:53pm
web-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 27, 2023 8:53pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 22, 2023

Thank you for following the naming conventions! 🙏

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 22, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

@deploysentinel
Copy link
Copy Markdown

deploysentinel Bot commented Jun 22, 2023

No failed tests 🎉

Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

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

L87 needs some feedback, not sure about it; left some nits

Comment thread packages/trpc/server/routers/viewer/organizations/update.handler.ts
) {
// Save it on the metadata so we can use it later
data.metadata = {
...cleanMetadata,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't look right to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't understand, that line was never there to begin with 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh yeah it's line 77 - is that OK? because it is gone and loses all metadata except requestedSlug

@sean-brydon sean-brydon mentioned this pull request Jun 23, 2023
6 tasks
@leog leog requested a review from emrysal June 23, 2023 21:08
@zomars zomars force-pushed the fix/org-update-handler branch from e48c327 to 506ac31 Compare June 27, 2023 20:33
@zomars zomars enabled auto-merge (squash) June 27, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

High priority Created by Linear-GitHub Sync organizations area: organizations, orgs

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants