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

Account for null XForm instances in service_usage endpoint #4635

Merged
merged 7 commits into from Sep 15, 2023

Conversation

LMNTL
Copy link
Contributor

@LMNTL LMNTL commented Sep 13, 2023

Checklist

  1. If you've added code that should be tested, add tests
  2. If you've changed APIs, update (or create!) the documentation
  3. Ensure the tests pass
  4. Make sure that your code lints and that you've followed our coding style
  5. Write a title and, if necessary, a description of your work suitable for publishing in our release notes
  6. Mention any related issues in this repository (as #ISSUE) and in other repositories (as kobotoolbox/other#ISSUE)
  7. Open an issue in the docs if there are UI/UX changes

Description

Fix /api/v2/service_usage/ calculating NLP and submission counts incorrectly.

Notes

Deleting a form with submissions currently causes the submission counter to decrement. The endpoint previously did handle this correctly, but it was refactored as part of feature/usage-limits, which introduced this regression. See kobocat#895 for additional background.

Related issues

Depends on kobotoolbox/kobocat#895

See issue on Release 2.023.37 QA board.

@LMNTL LMNTL added API Changes related to API endpoints Back end labels Sep 13, 2023
@LMNTL LMNTL marked this pull request as ready for review September 13, 2023 21:28
@noliveleger noliveleger self-requested a review September 14, 2023 15:14
@noliveleger noliveleger self-assigned this Sep 14, 2023
Comment on lines 144 to 155
Asset.objects.only(
Asset.all_objects.only(
'pk',
'uid',
'_deployment_data',
'owner_id',
'name',
)
.select_related('owner')
.exclude(_deployment_data=None)
.filter(
owner__in=self._users,
asset_type=ASSET_TYPE_SURVEY,
Copy link
Contributor

Choose a reason for hiding this comment

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

user_assets seems to be used no where else than to be used to init asset_list
I would probably get rid of user_assets and assign directly asset_list with the results of queryset with values_list()

Moreover, the query could simplified with the new date_deployed field. Only assets of type 'survey' are deployed. We just need to check whether the value of the field is not null to get all deployed projects.

So what about this?

        asset_list = list(
            Asset.all_objects.values_list('uid', flat=True).filter(
                owner__in=self._users,
                date_deployed__isnull=False,
            )
        )
        
        xforms = KobocatXForm.objects.only('bytes_sum', 'id', 'kpi_asset_uid').filter(
            kpi_asset_uid__in=asset_list,
        )

kpi/serializers/v2/service_usage.py Outdated Show resolved Hide resolved
kpi/serializers/v2/service_usage.py Outdated Show resolved Hide resolved
@noliveleger noliveleger merged commit 913fc11 into release/2.023.37 Sep 15, 2023
4 checks passed
@noliveleger noliveleger deleted the service-usage-null-xform branch September 15, 2023 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes related to API endpoints Back end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants