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

fix: Excessive 404s on subscription metadata #2985

Merged
merged 7 commits into from
Nov 16, 2023

Conversation

zachaysan
Copy link
Contributor

@zachaysan zachaysan commented Nov 16, 2023

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

Full ticket context here, in short, subscriptions were added by default to organisations and the metadata endpoint was bypassing the free accounts because it relied on a method previously called has_subscription to 404 requests. Changes introduced:

  • Renamed the method to has_paid_subscription which is closer to the intent, but still a little confusing since it includes paid subscriptions that have been canceled. I added a comment on the method, but a better method name would be even better.
  • Removed the check and the 404 error since going forward we are to assume that every organisation has a related subscription model.
  • Re-introduced a migration we ran in the past to create subscriptions for organisations that are missing them. This is for the small number of organisations that lacked subscriptions due to a temporary bug earlier in the year.

How did you test this code?

I updated the one test that was designed for the 404 page and ran tests from around where the code has changed.

Copy link

vercel bot commented Nov 16, 2023

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

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Nov 16, 2023 4:22pm
flagsmith-frontend-preview ✅ Ready (Inspect) Visit Preview Nov 16, 2023 4:22pm
flagsmith-frontend-staging ✅ Ready (Inspect) Visit Preview Nov 16, 2023 4:22pm

@github-actions github-actions bot added the api Issue related to the REST API label Nov 16, 2023
Copy link
Contributor

github-actions bot commented Nov 16, 2023

Uffizzi Preview deployment-40904 was deleted.

@zachaysan zachaysan linked an issue Nov 16, 2023 that may be closed by this pull request
@zachaysan zachaysan added this pull request to the merge queue Nov 16, 2023
Merged via the queue into main with commit 627a6fa Nov 16, 2023
@zachaysan zachaysan deleted the fix/excessive_404s_on_subscription_metadata branch November 16, 2023 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issue related to the REST API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid excess calls to get-subscription-metadata
2 participants