-
Notifications
You must be signed in to change notification settings - Fork 429
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: make OrganisationSubscriptionInformationCache.allowed_projects
nullable
#2716
fix: make OrganisationSubscriptionInformationCache.allowed_projects
nullable
#2716
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Uffizzi Preview |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2716 +/- ##
==========================================
- Coverage 95.47% 95.47% -0.01%
==========================================
Files 986 987 +1
Lines 27729 27755 +26
==========================================
+ Hits 26474 26498 +24
- Misses 1255 1257 +2
☔ View full report in Codecov by Sentry. |
@@ -582,6 +583,45 @@ def setUp(self) -> None: | |||
) | |||
self.subscription = Subscription.objects.get(organisation=self.organisation) | |||
|
|||
@mock.patch("organisations.views.extract_subscription_metadata") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not happy about having to continue to use these class based tests but ripping these up into functional pytest tests would be a fairly large effort that I don't think this PR warrants right now.
d6a9d07
to
cedad39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Changes
Fixes #2715
Note that null is assumed to mean no project limit (which should be the default for paid SaaS subscriptions).
How did you test this code?
Added unit test