Migrate FacilityViewSet to serializer-derived pattern and split auth domain into viewsets/#14873
Open
rtibblesbot wants to merge 7 commits into
Open
Migrate FacilityViewSet to serializer-derived pattern and split auth domain into viewsets/#14873rtibblesbot wants to merge 7 commits into
rtibblesbot wants to merge 7 commits into
Conversation
github-actions
Bot
added
DEV: backend
Contributor
Build Artifacts
Smoke test screenshot |
rtibblesbot
force-pushed
the
issue-14297-rebased
branch
from
91e3a3f to
a935f5b
Compare
8 tasks
rtibblesbot
force-pushed
the
issue-14297-rebased
branch
from
a935f5b to
5f4ea40
Compare
Contributor
Author
|
The Browser smoke test failure is a transient CI infrastructure issue, not a code regression. The job was cancelled during No code changes are needed. Please re-run the failed job. |
rtibblesbot
force-pushed
the
issue-14297-rebased
branch
from
5f4ea40 to
d474215
Compare
Capture the current API contract before migrating FacilityViewSet to serializer-derived mode: dataset is a nested object with exactly the expected keys, num_classrooms and num_users are present and numeric. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract KolibriAuthPermissions, KolibriAuthPermissionsFilter, OptionalPageNumberPagination, and _ensure_raw_dict from kolibri.core.auth.api into kolibri.core.auth.permissions.__init__.py. Update all callers across auth, analytics, attendance, bookmarks, content, courses, device, exams, lessons, logger, and coach to import from the new location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erived pattern Create kolibri/core/auth/viewsets/facility.py containing FacilityViewSet, PublicFacilityViewSet, and all facility serializers. The migration replaces the explicit values tuple and field_map with a FacilityDatasetNestedSerializer that nests the dataset relationship directly, eliminating the _map_dataset helper. Move ExtraFieldsSerializer and validate_pin_code from serializers.py into facility_dataset.py where they are exclusively used. Update device/tasks.py to import FacilitySerializer from its new location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create kolibri/core/auth/viewsets/membership.py and viewsets/role.py, moving MembershipViewSet, RoleViewSet, their serializers (MembershipSerializer, RoleSerializer with bulk-create list serializers), domain-specific filters, and the _prepare_for_bulk_create/_get_batch_size helpers out of api.py and serializers.py. Update test_api.py to import _prepare_for_bulk_create from its new location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create kolibri/core/auth/viewsets/session.py and viewsets/signup.py, moving SessionViewSet (with CreateSessionSerializer) and the signup family (BaseSignUpViewSet, SignUpViewSet, PublicSignUpViewSet) out of api.py. Simplify the missing-password guard in CreateSessionSerializer to raise regardless of whether the username exists. Update test mock patch paths for valid_app_key_on_request and allow_other_browsers_to_connect to target their new module. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Create kolibri/core/auth/viewsets/auth_views.py containing IsPINValidView, UsernameAvailableView, DeleteImportedUserView, SetNonSpecifiedPasswordView, RemoteFacilityUserViewset, and RemoteFacilityUserAuthenticatedViewset with their serializers. Move IsPINValidPermissions inline (it is used only here). Simplify IsPINValidView.post and UsernameAvailableView with cleaner control flow. Update test mock patch path for NetworkClient to target the new module. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All domains have been relocated: FacilityViewSet to viewsets/facility.py, MembershipViewSet/RoleViewSet to viewsets/membership.py and viewsets/role.py, SessionViewSet/signup family to viewsets/session.py and viewsets/signup.py, and auth APIViews to viewsets/auth_views.py. Shared infrastructure was moved to kolibri/core/auth/permissions/ in an earlier commit. Update api_urls.py to import all viewsets and views from their new locations. Update public/api_urls.py to import PublicFacilityViewSet from viewsets/facility.py and PublicSignUpViewSet from viewsets/signup.py. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rtibblesbot
force-pushed
the
issue-14297-rebased
branch
from
ac77d1b to
cd8b1ec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plan: Migrate
FacilityViewSetfrom explicitvalues/field_mapto serializer-derived pattern, then relocate all auth domain code out ofapi.pyandserializers.py, ending with both files deleted.FacilityDatasetNestedSerializer; expandFacilitySerializervalues,field_map, and helpers fromFacilityViewSetviewsets/facility.pykolibri/core/auth/permissions.pyapi.pyandserializers.pycodeFacilityDatasetViewSetdomain toviewsets/facility_dataset.pyClassroomViewSetandLearnerGroupViewSetMembershipViewSetandRoleViewSetSessionViewSetand signup familyviewsets/auth_views.pyapi.pyandserializers.pySummary
Migrates
FacilityViewSetfrom explicitvalues/field_mapto serializer-derived pattern (#14036), then evacuates all remaining code fromkolibri/core/auth/api.pyandserializers.pyinto theviewsets/package, ending with both legacy files deleted.The Facility migration adds
FacilityDatasetNestedSerializerto replace the_map_datasethelper that collapsed flatdataset__*fields into a nested dict.annotate_queryset()is unchanged.Benchmark
api.FacilityViewSet)viewsets.facility.FacilityViewSet)f844c188…8a26b499…No timing or memory regression. The output hash differs; data values and
json_size_bytes(2,002) are identical. The most likely cause is JSON field ordering — serializerMeta.fieldsorder vs. the previousvalues()tuple order. Sinceapi.pyis now deleted, the baseline path cannot be re-run to verify; reviewers should confirm whether this is acceptable.References
Closes #14297. Part of #14036.
Reviewer guidance
5e00fa6): lock in the full response shape before migration; pass before and after unchanged.2ca58de): pure move — usegit show 2ca58deto confirm no logic changes.3654b85): combined migration (serializer-derived pattern) and move — reviewers should check both the structural change and that the response shape is preserved.0579225,33f0b44,ff65eeb): Membership/Role, Session/Signup, and auth APIViews — usegit show <sha>to confirm no logic changes.cd8b1ec): removesapi.pyandserializers.pyand updates URL routing.extra_fieldsorpicture_password_settings(JSONField) returning a subtly different Python type than_map_datasetdid.KolibriAuthPermissions,KolibriAuthPermissionsFilter,IsPINValidPermissions,OptionalPageNumberPaginationmoved tokolibri/core/auth/permissions/— all cross-app imports updated.AI usage
Implemented with Claude Code (claude-sonnet-4-6) following the step-by-step plan in the issue comment. TDD approach: pre-migration tests first, then migration, then pure-move commits. Tests run before each commit.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?