Skip to content

fix: Only enforce min seats for orgs#10572

Merged
joeauyeung merged 4 commits intomainfrom
fix/update-team-subscriptions
Aug 3, 2023
Merged

fix: Only enforce min seats for orgs#10572
joeauyeung merged 4 commits intomainfrom
fix/update-team-subscriptions

Conversation

@joeauyeung
Copy link
Copy Markdown
Contributor

What does this PR do?

This PR fixes the logic to only change team subscription amounts for orgs and not normal teams

Fixes # (issue)

Requirement/Documentation

  • If there is a requirement document, please, share it here.
  • If there is ab UI/UX design document, please, share it here.

Type of change

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

How should this be tested?

  • Enable team billing
  • Create a team and invite members
  • Delete some members
  • On Stripe the subscription count should be updated

Mandatory Tasks

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

Checklist

  • I haven't added tests that prove my fix is effective or that my feature works
  • I haven't checked if new and existing unit tests pass locally with my changes

@joeauyeung joeauyeung added High priority Created by Linear-GitHub Sync Urgent Created by Linear-GitHub Sync labels Aug 3, 2023
@joeauyeung joeauyeung self-assigned this Aug 3, 2023
@joeauyeung joeauyeung requested a review from a team August 3, 2023 16:27
@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 3, 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 Aug 3, 2023 6:24pm
cal-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2023 6:24pm
dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2023 6:24pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Aug 3, 2023 6:24pm
qa ⬜️ Ignored (Inspect) Visit Preview Aug 3, 2023 6:24pm
ui ⬜️ Ignored (Inspect) Visit Preview Aug 3, 2023 6:24pm

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 3, 2023

Thank you for following the naming conventions! 🙏

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.

We have done types failing

@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented Aug 3, 2023

🤖 Meticulous spotted visual differences in 30 of 119 screens tested: view and approve differences detected.

Last updated for commit d533e86. This comment will update as new commits are pushed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 3, 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 Aug 3, 2023

Current Playwright Test Results Summary

✅ 115 Passing - ⚠️ 1 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 08/03/2023 06:26:25pm UTC)

Run Details

Running Workflow PR Update on Github Actions

Commit: d533e86

Started: 08/03/2023 06:25:13pm UTC

⚠️ Flakes

📄   packages/embeds/embed-core/playwright/tests/action-based.e2e.ts • 1 Flake

Test Case Results

Test Case Last 7 days Failures Last 7 days Flakes
Popup Tests should be able to reschedule
Retry 1Initial Attempt
2.29% (7) 7 / 306 runs
failed over last 7 days
95.75% (293) 293 / 306 runs
flaked over last 7 days

View Detailed Build Results


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.

Some Qs, I'm trying to understand the why of this.

  • Where does the ORGANIZATION_MIN_SEATS number coming from?
  • Will this affect non orgs teams sub update logic? How can we verify this?
  • Is this related to the previous discussion where repeated members of different teams of the same org should not be charged?

Comment thread packages/lib/constants.ts
export const ALLOWED_HOSTNAMES = JSON.parse(`[${process.env.ALLOWED_HOSTNAMES || ""}]`) as string[];
export const RESERVED_SUBDOMAINS = JSON.parse(`[${process.env.RESERVED_SUBDOMAINS || ""}]`) as string[];

export const ORGANIZATION_MIN_SEATS = 30;
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.

I think we can move this to it's own file? Unless it's being used multiple times on several files.

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.

Do you have a suggestion on where to move it? I figured the constants file would be an easy place to find this variable.

Copy link
Copy Markdown
Contributor

@alannnc alannnc 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, left a NIT comment.

@joeauyeung
Copy link
Copy Markdown
Contributor Author

Some Qs, I'm trying to understand the why of this.

* Where does the ORGANIZATION_MIN_SEATS number coming from?

* Will this affect non orgs teams sub update logic? How can we verify this?

* Is this related to the previous discussion where repeated members of different teams of the same org should not be charged?
  • The minimum number is coming from @shirazdole
  • The problem was this logic before was affecting non-org teams. I tested this locally and checked the test Stripe dashboard, and the number of seats the customer was being charged is being updated with this fix. Regular teams will have normal members and not orgMembers
  • That logic is handled somewhere else. This is only for removing team members

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.

Thanks fro clarifying. LGTM

LGTM

@joeauyeung joeauyeung merged commit c7d8d43 into main Aug 3, 2023
@joeauyeung joeauyeung deleted the fix/update-team-subscriptions branch August 3, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core area: core, team members only High priority Created by Linear-GitHub Sync Urgent Created by Linear-GitHub Sync

Projects

No open projects
Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants