[OSDEV-2356] Add PartnerFieldGroup model and API endpoint

[protsack-stephan]

Introduce the PartnerFieldGroup model to organize partner fields.

This change adds a foreign key relationship from PartnerField to the new PartnerFieldGroup model and exposes these groups via a read-only API endpoint at GET /api/partner-field-groups/. It also registers the new model in the Django Admin, updates the PartnerField admin interface to include the group association, and configures CDN caching for the new endpoint in Terraform (also for additional endpoints, like contributors, contributors-sorted etc.).

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds PartnerFieldGroup model and nullable FK on PartnerField, registers admin/export updates, exposes a read-only paginated /api/partner-field-groups/ endpoint with tests, and adds CloudFront cache behaviors plus TTL variables for partner-related API paths.

Changes

Cohort / File(s)	Summary
Infrastructure & Caching Configuration deployment/terraform/cdn.tf, deployment/terraform/variables.tf	Added CloudFront cache behaviors for multiple partner-related API paths and ten new TTL variables (default 120/300s). Minor spacing formatting aligned in cdn.tf.
Model & Migration src/django/api/models/partner_field_group.py, src/django/api/migrations/0201_add_partnerfieldgroup_alter_partnerfield.py, src/django/api/models/partner_field.py	Introduced PartnerFieldGroup model and added nullable group FK to PartnerField (SET_NULL, related_name=partner_fields). Migration created to add model and field.
API: Serializers, Views, Routes src/django/api/serializers/partner_field_group/partner_field_group_serializer.py, src/django/api/views/partner_field_groups/partner_field_groups_view_set.py, src/django/oar/urls.py	Added PartnerFieldGroupSerializer, read-only PartnerFieldGroupsViewSet with cursor pagination and prefetching, and registered /api/partner-field-groups/ route.
Admin & Package Exports src/django/api/admin.py, src/django/api/models/__init__.py, src/django/api/views/__init__.py	Registered PartnerFieldGroup in admin; updated PartnerFieldAdmin to include/filter by group; exported PartnerField/PartnerFieldGroup/PartnerFieldGroupsViewSet.
Tests src/django/api/tests/test_partner_field_groups_view_set.py	Added tests for list access (anon/auth), pagination (limit), ordering, inclusion of related partner fields, and POST disallowed (405).
Documentation doc/release/RELEASE-NOTES.md	Added release note entry for OSDEV-2356 documenting new endpoints and pagination.

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant CDN as CDN (CloudFront)
  participant API as Django API
  participant DB as Database

  Client->>CDN: GET /api/partner-field-groups/?cursor...
  CDN->>API: Forward request (cache miss or origin request)
  API->>DB: Query PartnerFieldGroup with prefetch partner_fields (order)
  DB-->>API: Return groups + partner_fields
  API-->>CDN: Return paginated response (Cursor pagination headers)
  CDN-->>Client: Serve response (cached per TTL variables)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• [OSDEV-2357] Add API endpoint to list partner fields #873 — Adds PartnerField serializers and viewsets; overlaps PartnerField API surface and serialization patterns.
• [OSDEV-2328] Add CloudFront caching for facilities and production locations enpoints #851 — Modifies Terraform CDN/TTL variables for endpoint-specific cache behaviors; directly related infra changes.
• [Part-1] [OSDEV-2065] Update v1 production locations POST/PATCH endpoint to include partner field type validation #750 — Changes PartnerField model and admin configuration; touches same model/admin areas.

Suggested reviewers

• roman-stolar
• VadimKovalenkoSNF
• vlad-shapik

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: introducing a new PartnerFieldGroup model and its associated API endpoint, matching the changeset additions.
Description check	✅ Passed	The description is directly related to the changeset, covering the PartnerFieldGroup model, foreign key relationship, API endpoint, admin registration, and Terraform caching configuration.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch OSDEV-2356-add-partner-field-groups-endpoint

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (4)

src/django/api/serializers/partner_field_group/partner_field_group_serializer.py (1)

27-37: Prefer tuple for Meta.fields to avoid mutable class-attribute warnings.

Using a tuple here avoids RUF012-style mutable default concerns and is the common serializer pattern.

✅ Suggested fix

-        fields = [
+        fields = (
             "uuid",
             "name",
             "order",
             "icon_file",
             "description",
             "helper_text",
             "partner_fields",
             "created_at",
             "updated_at",
-        ]
+        )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/django/api/serializers/partner_field_group/partner_field_group_serializer.py`
around lines 27 - 37, In the PartnerFieldGroupSerializer's Meta class, change
the mutable list assigned to Meta.fields to an immutable tuple to avoid mutable
class-attribute warnings (RUF012); locate the Meta class inside
PartnerFieldGroupSerializer and replace the fields = [...] list with fields =
("uuid", "name", "order", "icon_file", "description", "helper_text",
"partner_fields", "created_at", "updated_at") so the serializer uses a tuple
instead of a list.

src/django/api/views/partner_field_groups/partner_field_groups_view_set.py (1)

24-34: Consider explicitly setting permission classes.

The docstring states "Available for all users," but no permission_classes is defined. DRF will use the default from settings, which may or may not be AllowAny. For clarity and to ensure the intended public access, consider explicitly setting permissions.

💡 Suggested explicit permission

+from rest_framework.permissions import AllowAny
+
 class PartnerFieldGroupsViewSet(ReadOnlyModelViewSet):
     """
     Allows listing of the partner field groups.
     Available for all users.
     """

     queryset = PartnerFieldGroup.objects.prefetch_related(
         "partner_fields"
     ).all()
     serializer_class = PartnerFieldGroupSerializer
     pagination_class = PartnerFieldGroupCursorPagination
+    permission_classes = [AllowAny]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/django/api/views/partner_field_groups/partner_field_groups_view_set.py`
around lines 24 - 34, The view set claims to be public but doesn't set explicit
permissions; modify PartnerFieldGroupsViewSet to declare permission_classes =
[AllowAny] and import AllowAny from rest_framework.permissions so intent is
explicit and not reliant on project defaults; update the class definition near
queryset/serializer_class to include this attribute.

src/django/api/admin.py (1)

456-456: Consider adding a custom ModelAdmin for PartnerFieldGroup.

PartnerFieldGroup is registered without a custom admin class. Consider adding one to improve the admin experience with features like list_display, search_fields, and ordering.

💡 Suggested custom admin class

class PartnerFieldGroupAdmin(admin.ModelAdmin):
    list_display = ("name", "order", "created_at", "updated_at")
    search_fields = ("name",)
    ordering = ("order",)
    readonly_fields = ("uuid", "created_at", "updated_at")

admin_site.register(PartnerFieldGroup, PartnerFieldGroupAdmin)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/django/api/admin.py` at line 456, PartnerFieldGroup is registered without
a custom admin class — create a PartnerFieldGroupAdmin (subclassing
admin.ModelAdmin) and register it with admin_site.register(PartnerFieldGroup,
PartnerFieldGroupAdmin); in the admin class add list_display = ("name", "order",
"created_at", "updated_at"), search_fields = ("name",), ordering = ("order",)
and readonly_fields = ("uuid", "created_at", "updated_at") to improve list view,
searchability and prevent edits to audit fields.

src/django/api/migrations/0201_add_partnerfieldgroup_alter_partnerfield.py (1)

36-42: Consider adding an index on the order field.

The model uses ordering = ["order"] in Meta, and the viewset paginates by this field. For larger datasets, an index would improve query performance.

💡 Suggested index addition

 (
     "order",
     models.IntegerField(
         default=0,
         help_text="Order for the partner field group in the UI.",
+        db_index=True,
     ),
 ),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/django/api/migrations/0201_add_partnerfieldgroup_alter_partnerfield.py`
around lines 36 - 42, Update the migration so the "order" IntegerField is
indexed: modify the field definition in the migration (the
models.IntegerField(...) for "order") to include db_index=True (e.g.,
models.IntegerField(default=0, db_index=True, help_text="...")), or
alternatively add an AddIndex operation for the PartnerFieldGroup model
targeting the "order" column; ensure the change targets the same migration that
defines the field so the database will create the index when applied.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deployment/terraform/variables.tf`:
- Around line 70-128: Several variable descriptions (e.g.,
api_partner_field_groups_cache_default_ttl,
api_partner_field_groups_cache_max_ttl, api_partner_fields_cache_default_ttl,
api_partner_fields_cache_max_ttl, api_contributors_cache_default_ttl,
api_contributors_cache_max_ttl, api_contributor_lists_sorted_cache_default_ttl,
api_contributor_lists_sorted_cache_max_ttl,
api_parent_companies_cache_default_ttl, api_parent_companies_cache_max_ttl)
still reference "OS ID detail endpoint" due to copy/paste; update each
description to accurately describe the TTL as applying to the list/collection or
endpoint type they control (for example: "Default TTL (seconds) for partner
field groups list endpoint" or "Max TTL (seconds) for API contributors list
endpoints"), ensuring wording matches whether the variable is for a
single-resource detail vs. a list/collection endpoint.

In `@src/django/api/tests/test_partner_field_groups_view_set.py`:
- Around line 55-61: The test test_returns_200_for_user is making an
unauthenticated request — update it to authenticate the test client first (e.g.
call self.client.force_authenticate(user=self.user) or set credentials/token on
self.client) before calling self.client.get(self.url) so the request exercises
the authenticated path; ensure you de-authenticate after if needed or rely on
test isolation. Reference the test method name test_returns_200_for_user and the
test client usages (self.client.get) to locate where to insert the
authentication step.
- Around line 117-118: In the test loop comparing sorted_uuids and
response_uuids, change the zip() call to include the explicit strict=True
parameter to satisfy lint rule B905; update the loop that binds sorted_uuid and
response_uuid (where zip(sorted_uuids, response_uuids) is used) to
zip(sorted_uuids, response_uuids, strict=True) so the test will error if the
iterables differ in length.

---

Nitpick comments:
In `@src/django/api/admin.py`:
- Line 456: PartnerFieldGroup is registered without a custom admin class —
create a PartnerFieldGroupAdmin (subclassing admin.ModelAdmin) and register it
with admin_site.register(PartnerFieldGroup, PartnerFieldGroupAdmin); in the
admin class add list_display = ("name", "order", "created_at", "updated_at"),
search_fields = ("name",), ordering = ("order",) and readonly_fields = ("uuid",
"created_at", "updated_at") to improve list view, searchability and prevent
edits to audit fields.

In `@src/django/api/migrations/0201_add_partnerfieldgroup_alter_partnerfield.py`:
- Around line 36-42: Update the migration so the "order" IntegerField is
indexed: modify the field definition in the migration (the
models.IntegerField(...) for "order") to include db_index=True (e.g.,
models.IntegerField(default=0, db_index=True, help_text="...")), or
alternatively add an AddIndex operation for the PartnerFieldGroup model
targeting the "order" column; ensure the change targets the same migration that
defines the field so the database will create the index when applied.

In
`@src/django/api/serializers/partner_field_group/partner_field_group_serializer.py`:
- Around line 27-37: In the PartnerFieldGroupSerializer's Meta class, change the
mutable list assigned to Meta.fields to an immutable tuple to avoid mutable
class-attribute warnings (RUF012); locate the Meta class inside
PartnerFieldGroupSerializer and replace the fields = [...] list with fields =
("uuid", "name", "order", "icon_file", "description", "helper_text",
"partner_fields", "created_at", "updated_at") so the serializer uses a tuple
instead of a list.

In `@src/django/api/views/partner_field_groups/partner_field_groups_view_set.py`:
- Around line 24-34: The view set claims to be public but doesn't set explicit
permissions; modify PartnerFieldGroupsViewSet to declare permission_classes =
[AllowAny] and import AllowAny from rest_framework.permissions so intent is
explicit and not reliant on project defaults; update the class definition near
queryset/serializer_class to include this attribute.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0299fbe and 69fb543.

📒 Files selected for processing (13)

• deployment/terraform/cdn.tf
• deployment/terraform/variables.tf
• doc/release/RELEASE-NOTES.md
• src/django/api/admin.py
• src/django/api/migrations/0201_add_partnerfieldgroup_alter_partnerfield.py
• src/django/api/models/__init__.py
• src/django/api/models/partner_field.py
• src/django/api/models/partner_field_group.py
• src/django/api/serializers/partner_field_group/partner_field_group_serializer.py
• src/django/api/tests/test_partner_field_groups_view_set.py
• src/django/api/views/__init__.py
• src/django/api/views/partner_field_groups/partner_field_groups_view_set.py
• src/django/oar/urls.py

[coderabbitai]

♻️ Duplicate comments (2)

src/django/api/tests/test_partner_field_groups_view_set.py (2)

52-58: ⚠️ Potential issue | 🟡 Minor

Authenticated-path test still does not authenticate.

test_returns_200_for_user at Line 54 performs the same unauthenticated request as the anonymous test, so it doesn’t exercise authenticated behavior.

✅ Suggested fix

+from django.contrib.auth import get_user_model
 from rest_framework import status
 from rest_framework.test import APITestCase
@@
     def test_returns_200_for_user(self):
         """Verify endpoint returns 200 for authenticated user requests."""
+        user_model = get_user_model()
+        user = user_model.objects.create_user(
+            email="[email protected]",
+            password="strong-test-password",
+        )
+        self.client.force_authenticate(user=user)
         response = self.client.get(self.url)
         self.assertEqual(
             response.status_code,
             status.HTTP_200_OK,
         )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/django/api/tests/test_partner_field_groups_view_set.py` around lines 52 -
58, test_returns_200_for_user is currently making an unauthenticated GET;
authenticate the test client before calling self.client.get so the authenticated
path is exercised (e.g., call self.client.force_authenticate(user=self.user) or
self.client.login(...) at the start of test_returns_200_for_user, then perform
the GET and assert status.HTTP_200_OK; if your test suite requires cleanup, call
self.client.force_authenticate(None) or logout after the request).

---

114-115: ⚠️ Potential issue | 🟡 Minor

Add explicit strict= to zip() (Ruff B905).

At Line 114, zip() should be explicit to avoid silent truncation and satisfy lint gates.

✅ Suggested fix

-        for sorted_uuid, response_uuid in zip(sorted_uuids, response_uuids):
+        for sorted_uuid, response_uuid in zip(
+            sorted_uuids, response_uuids, strict=True
+        ):
             self.assertEqual(sorted_uuid, response_uuid)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/django/api/tests/test_partner_field_groups_view_set.py` around lines 114
- 115, The zip over sorted_uuids and response_uuids should be explicit about
truncation; update the zip(...) call in the test (where sorted_uuids and
response_uuids are iterated) to zip(sorted_uuids, response_uuids, strict=True)
so the test will raise if lengths differ and satisfy the Ruff B905 lint rule.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/django/api/tests/test_partner_field_groups_view_set.py`:
- Around line 52-58: test_returns_200_for_user is currently making an
unauthenticated GET; authenticate the test client before calling self.client.get
so the authenticated path is exercised (e.g., call
self.client.force_authenticate(user=self.user) or self.client.login(...) at the
start of test_returns_200_for_user, then perform the GET and assert
status.HTTP_200_OK; if your test suite requires cleanup, call
self.client.force_authenticate(None) or logout after the request).
- Around line 114-115: The zip over sorted_uuids and response_uuids should be
explicit about truncation; update the zip(...) call in the test (where
sorted_uuids and response_uuids are iterated) to zip(sorted_uuids,
response_uuids, strict=True) so the test will raise if lengths differ and
satisfy the Ruff B905 lint rule.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69fb543 and 91fb8fc.

📒 Files selected for processing (1)

• src/django/api/tests/test_partner_field_groups_view_set.py

[VadimKovalenkoSNF]

Looks clear, left few minor comments.

[vlad-shapik]

LGTM

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[VadimKovalenkoSNF]

Approved.

[vlad-shapik]

LGTM

[coderabbitai]

🧹 Nitpick comments (1)

src/django/api/views/partner_field_groups/partner_field_groups_view_set.py (1)

23-23: Stabilize cursor pagination ordering with a deterministic tie-breaker.

Line 23 orders cursors by order only, which is a non-unique integer field with default=0. This can lead to missing or duplicated items at page boundaries under concurrent inserts or updates. Add a stable secondary key to ensure deterministic ordering.

♻️ Proposed change

-    ordering = "order"
+    ordering = ("order", "uuid")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/django/api/views/partner_field_groups/partner_field_groups_view_set.py`
at line 23, The view's cursor pagination ordering is non-deterministic because
ordering = "order" uses a non-unique field; update the PartnerFieldGroupsViewSet
(where ordering is set) to use a deterministic compound ordering such as
("order", "pk") or ("order", "id") so the cursor has a stable tie-breaker;
change the ordering attribute to a sequence with the secondary key and ensure
any related ordering_fields/ordering_filters accept the secondary key if
present.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/django/api/views/partner_field_groups/partner_field_groups_view_set.py`:
- Line 23: The view's cursor pagination ordering is non-deterministic because
ordering = "order" uses a non-unique field; update the PartnerFieldGroupsViewSet
(where ordering is set) to use a deterministic compound ordering such as
("order", "pk") or ("order", "id") so the cursor has a stable tie-breaker;
change the ordering attribute to a sequence with the secondary key and ensure
any related ordering_fields/ordering_filters accept the secondary key if
present.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 91fb8fc and d58a9ef.

📒 Files selected for processing (4)

• deployment/terraform/variables.tf
• src/django/api/serializers/partner_field_group/partner_field_group_serializer.py
• src/django/api/tests/test_partner_field_groups_view_set.py
• src/django/api/views/partner_field_groups/partner_field_groups_view_set.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/django/api/tests/test_partner_field_groups_view_set.py