Skip to content
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

Provide proactive feedback when trying to change team type to incompatible type #4378

Merged
merged 7 commits into from
Sep 2, 2024

Conversation

knolleary
Copy link
Member

@knolleary knolleary commented Aug 14, 2024

Closes #4377

This provides better feedback if a team upgrade is blocked due to the current usage exceeding any limits on the target team.

My first iteration was to add the check on the backend for the path where a team is setting up billing - it was already handled for the case of changing type when billing is already setup. This ensures any issue is reported before they are sent to stripe.

However, the feedback isn't great - and would be better to be more proactive in the UI. So the second iteration (3rd commit) copies the limit checks into the front end and lets us show a more helpful message to the user:

image

The specific errors shows above are based on how I happen to have my Starter team type setup locally; so don't get distracted by the specific numbers.

Also need to highlight the checks are based purely on Members/Device/Instance count limits. It doesn't check if they are using features (eg Team Library) that are unavailable at the target tier. We don't maintain flags on feature usage within a team, so that's out of scope (and not proposing it comes into scope any time soon).


There is currently limit test coverage for this. We're missing base coverage in a couple different areas here for the existing code.

I have added basic verification the UX blocks changing to a team that is not compatible with current usage - but it isn't exhaustive.

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

Attention: Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.

Project coverage is 78.12%. Comparing base (ccd5b75) to head (7004aaf).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
forge/ee/routes/billing/index.js 0.00% 13 Missing ⚠️
forge/db/models/Team.js 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4378      +/-   ##
==========================================
- Coverage   78.17%   78.12%   -0.06%     
==========================================
  Files         295      295              
  Lines       13985    13994       +9     
  Branches     3159     3163       +4     
==========================================
  Hits        10933    10933              
- Misses       3052     3061       +9     
Flag Coverage Δ
backend 78.12% <0.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@knolleary
Copy link
Member Author

Have added basic coverage to verify the UI blocks changing to a 'lower' type.

@knolleary knolleary requested a review from Steve-Mcl August 15, 2024 11:05
Copy link
Contributor

@Steve-Mcl Steve-Mcl left a comment

Choose a reason for hiding this comment

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

Happy with the code.

Do we have a team on prod that we can verify this after it is deployed?

@hardillb
Copy link
Contributor

hardillb commented Sep 2, 2024

@Steve-Mcl do we want to merge this and then I can test it?

@Steve-Mcl
Copy link
Contributor

@hardillb - yes, fine by me Ben

@hardillb hardillb merged commit b207277 into main Sep 2, 2024
13 of 14 checks passed
@hardillb hardillb deleted the 4377-team-upgrade-feedback branch September 2, 2024 13:31
@hardillb
Copy link
Contributor

hardillb commented Sep 2, 2024

Verified on Staging where I had a Teams, team with more users and instances than allowed in starter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve feedback when changing team type blocked due to resource usage
3 participants