Updating usage endpoints for organizations and subscriptions #4506
Conversation
|
The tests.py file won't actually be read by default by pytest, therefore it doesn't run in CI. |
kpi/serializers/v2/service_usage.py
Outdated
| id=organization_id, | ||
| ) | ||
| # If the user is in an organization, get all org users so we can query their total org usage | ||
| organization_users = OrganizationUser.objects.filter(organization=organization).select_related('user') |
There was a problem hiding this comment.
I think you could reduce the code here by something like (pseudo code)
User.objects.filter(orgnaization__id=organization_id)
Often it's best to base the FROM part of the query on the table you actually want to return a list of, when possible at least. Then join that table to where you need to apply a filter.
The 404 may not be right here. I kind of don't care and this is pedantic, but a filter should simply filter the existing data. If that filter returns nothing. But if it's easier to raise either a 404 or 400 - I'm fine with it.
kpi/serializers/v2/service_usage.py
Outdated
| raise Http404 | ||
| # Get the organization's subscription, if they have one | ||
| subscription = Subscription.objects.filter( | ||
| status__in=['active', 'past_due', 'trialing'], |
There was a problem hiding this comment.
We should probably make this list a constant. Even though it's used just once, it's more readable to understand why these three strings are special. Just the variable name itself would be good documentation.
jsapp/js/account/plan.component.tsx
Outdated
| @@ -27,7 +27,8 @@ import Button from 'js/components/common/button'; | |||
| import classnames from 'classnames'; | |||
| import LoadingSpinner from 'js/components/common/loadingSpinner'; | |||
| import {notify} from 'js/utils'; | |||
| import {BaseProduct} from "js/account/subscriptionStore"; | |||
| import {BaseProduct} from 'js/account/subscriptionStore'; | |||
There was a problem hiding this comment.
I know you're just running prettier, but this import isn't being used. We should remove it.
kpi/serializers/v2/service_usage.py
Outdated
| from django.db.models import Sum, Q | ||
| from django.db.models.functions import Coalesce | ||
| from django.utils import timezone | ||
| from djstripe.models import Subscription |
There was a problem hiding this comment.
This import appears to generate a RuntimeError at startup when stripe isn't configured. We decided that stripe should be optional. I suspect you could do a condition check and run organizations.something to fetch check if there is a subscription without needing to import.
Checklist
Description
Updated api/v2/service_usage to account for organizations and subscriptions.
Notes
Included changes to
service_usageorganization_idparameter, endpoint now tracks usage based on the organization's subscription date (if they have a subscription)current_monthandcurrent_yeartotalsGeneral changes:
service_usageandasset_usageendpoints now return yearly counts for submissions and NLP usage.