Skip to content

Conversation

jamesrkiger
Copy link
Contributor

@jamesrkiger jamesrkiger commented Sep 11, 2025

🗒️ Checklist

  1. run linter locally
  2. update developer docs (API, README, inline, etc.), if any
  3. for user-facing doc changes create a Zulip thread at #Kobo support docs, if any
  4. draft PR with a title <type>(<scope>)<!>: <title> DEV-1234
  5. assign yourself, tag PR: at least Front end and/or Back end or workflow
  6. fill in the template below and delete template comments
  7. review thyself: read the diff and repro the preview as written
  8. open PR & confirm that CI passes & request reviewers, if needed
  9. delete this section before merging

📣 Summary

Removes custom asset usage page-size pagination, thereby using KPI's default limit-offset pagination, and adjusts drf_spectacular code for the endpoint to improve API docs/frontend helpers.

💭 Notes

I am marking this as a draft for the time being, but it is ready for review. We will merge it as part of the corresponding frontend changes (See DEV-1013).

👀 Preview steps

No need for testing on this. Testing will come with frontend PR.

Copy link
Contributor

@rgraber rgraber left a comment

Choose a reason for hiding this comment

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

LGTM

from kpi.utils.usage_calculator import ServiceUsageCalculator


class NlpUsageSerializer(serializers.Serializer):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the main difference from how I've seen the drf_spectacular code used in other parts of the app. I thought it would be preferable to use the actual AssetUsageSerializer in views.py, rather than creating an inline serializer. To me, it makes the code more readable. But I can also see that there might be some confusion of concerns, since the NlpUsageSerializer here is being passed into @extend_schema_field rather than used directly in the AssetUsageSerializer. Hoping to get your input on this, @noliveleger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants