[OSDEV-2180] Update Production Location Profile page to display dynamic partner field

[roman-stolar]

OSDEV-2180 Update Production Location Profile page to display dynamic partner field

• Introduces logic to return partner_fields for both /facilities/os_id and /facilities/ endpoints.
• Also implements dynamic rendering of any partner fields returned by the API, ensuring that new or custom fields are displayed automatically without additional frontend or backend updates.

[barecheck]

React App | Jest test suite - Code coverage report

Total: 35.09%

Your code coverage diff: 0.12% ▴

✅ All code changes are covered

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (2)

src/django/api/serializers/facility/facility_index_serializer.py (2)

122-126: Use logger.exception for automatic traceback logging.

This was already flagged in a previous review but has not been addressed. Replace logger.error with logger.exception to automatically capture the stack trace, which aids debugging.

Apply this diff:

             except Exception as exc:
-                logger.error(
-                    f"Failed to serialize partner field '{field_name}': " 
-                    f"{exc}"
-                )
+                logger.exception(
+                    f"Failed to serialize partner field '{field_name}'"
+                )
                 grouped_data[field_name] = []

---

414-419: N+1 query issue persists - cache PartnerField names.

This was already flagged in a previous review but has not been addressed. The PartnerField.objects.values_list(...) query executes for every facility being serialized, causing an N+1 problem. For a paginated response with 50 facilities, this results in 50 identical database queries.

Apply this diff to cache the field names on the serializer instance:

+    def __init__(self, *args, **kwargs):
+        exclude_fields = kwargs.pop('exclude_fields', None)
+        super(FacilityIndexSerializer, self).__init__(*args, **kwargs)
+        
+        # Cache partner field names to avoid N+1 queries
+        self._partner_field_names = list(
+            PartnerField.objects.values_list('name', flat=True)
+        )
+
+        if exclude_fields:
+            for field_name in exclude_fields:
+                self.fields.pop(field_name, None)
+
     def get_partner_fields(self, facility):
         request = self.__get_request()
 
         use_main_created_at = is_created_at_main_date(self)
         date_field_to_sort = self.__date_field_to_sort(
             use_main_created_at
         )
 
         fields = self.__filter_contributor_extended_fields(
             facility,
             request
         )
         grouped_fields = self.__group_fields_by_name(
             fields
         )
 
         user_can_see_detail = can_user_see_detail(self)
         embed_mode_active = is_embed_mode_active(self)
 
-        partner_field_names = list(
-            PartnerField.objects.values_list(
-                "name",
-                flat=True
-            )
-        )
-
         return self.__serialize_and_sort_partner_fields(
             grouped_fields,
-            partner_field_names,
+            self._partner_field_names,
             user_can_see_detail,
             embed_mode_active,
             use_main_created_at,
             date_field_to_sort
         )

🧹 Nitpick comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

88-89: Consider simplifying for readability.

The getattr with a lambda default is functional but unnecessarily complex. A more straightforward implementation would improve code clarity:

Apply this diff:

 def __get_request(self):
-    return getattr(self.context, 'get', lambda *_: None)('request')
+    if self.context is None:
+        return None
+    return self.context.get('request')

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 712900c and 1cde289.

📒 Files selected for processing (3)

• doc/release/RELEASE-NOTES.md (1 hunks)
• src/django/api/serializers/facility/facility_index_serializer.py (11 hunks)
• src/react/src/__tests__/utils.tests.js (2 hunks)

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

• src/react/src/tests/utils.tests.js
• doc/release/RELEASE-NOTES.md

🧰 Additional context used

🧬 Code graph analysis (1)

src/django/api/serializers/facility/facility_index_serializer.py (4)

src/django/api/models/partner_field.py (1)

• PartnerField (5-54)

src/django/api/serializers/facility/facility_index_extended_field_list_serializer.py (1)

• FacilityIndexExtendedFieldListSerializer (10-98)

src/django/api/models/facility/facility.py (1)

• extended_fields (148-192)

src/django/api/serializers/facility/utils.py (3)

• get_efs_associated_with_contributor (148-155)
• can_user_see_detail (118-126)
• is_created_at_main_date (44-52)

🪛 Ruff (0.13.3)

src/django/api/serializers/facility/facility_index_serializer.py

121-121: Do not catch blind exception: Exception

(BLE001)

---

122-122: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: get-base-branch-django-cov
• GitHub Check: get-base-branch-countries-cov
• GitHub Check: run-django-code-quality
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-countries-code-quality

🔇 Additional comments (7)

src/django/api/serializers/facility/facility_index_serializer.py (7)

1-2: LGTM!

The imports and logger setup are appropriate for the new functionality being added.

Also applies to: 5-5, 20-20, 42-43

---

57-57: LGTM!

The partner_fields field is properly declared and included in the serializer's Meta fields.

Also applies to: 76-76

---

131-152: LGTM!

The static helper methods are well-structured, focused, and improve code organization. The sort order logic is clear and maintainable.

---

154-175: LGTM!

The refactoring with early returns improves readability and follows the suggested optimization from the previous review.

---

177-186: LGTM!

Efficient O(n) grouping implementation using defaultdict. This is a clean solution for organizing fields by name.

---

318-393: LGTM!

The refactoring to use helper methods improves code organization and maintainability while preserving the existing logic.

---

476-476: LGTM!

Consistent use of the __get_request() helper improves code maintainability.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

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

87-96: Consider removing the blank line and expanding test coverage.

Minor style inconsistency: blank line after method definition (Line 87).

Additionally, consider adding assertions to verify:

• The dict is empty when no partner fields exist in extended_fields
• The structure matches the expected schema

Apply this diff to remove the blank line:

 def test_partner_fields_returns_dict(self):
-
     facility_index = FacilityIndex.objects.get(id=self.facility.id)

---

98-138: Refactor test to use the natural data flow instead of manual array manipulation.

Manually constructing and appending to facility_index.extended_fields (Lines 108-123) bypasses the normal ORM relationships and data flow. This creates a brittle test that doesn't reflect how extended_fields are actually populated in production (typically via database triggers or materialized views).

Consider refactoring to:

1. Create the ExtendedField via ORM (already done on Line 99)
2. Trigger the mechanism that populates facility_index.extended_fields naturally
3. Or mock/stub the data retrieval if direct testing isn't feasible

Additionally, expand test coverage to verify:

• Multiple partner fields are handled correctly
• Sorting behavior (by verified_count, is_from_claim, value_count, timestamps)
• Filtering by contributor in embed mode
• Non-partner extended fields are excluded from partner_fields
• All serialized field attributes (contributor details, timestamps, verification status)
• Error handling when serialization fails

Example structure for comprehensive testing:

def test_partner_fields_multiple_and_sorted(self):
    # Create multiple ExtendedField instances with different attributes
    ef1 = ExtendedField.objects.create(
        facility=self.facility,
        field_name='test_data_field',
        value={'raw_value': 'Value 1'},
        contributor=self.contrib_one,
        is_verified=True
    )
    ef2 = ExtendedField.objects.create(
        facility=self.facility,
        field_name='test_data_field',
        value={'raw_value': 'Value 2'},
        contributor=self.contrib_one,
        is_verified=False
    )
    
    # Trigger natural population of facility_index.extended_fields
    # (implementation depends on your data flow)
    
    facility_index = FacilityIndex.objects.get(id=self.facility.id)
    data = FacilityIndexSerializer(facility_index).data
    
    # Verify sorting (verified items should come first)
    test_data_field = data["properties"]["partner_fields"]['test_data_field']
    self.assertEqual(test_data_field[0]['is_verified'], True)
    self.assertEqual(test_data_field[1]['is_verified'], False)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8261b4f and 8e5c155.

📒 Files selected for processing (1)

• src/django/api/tests/test_facility_index_serializer.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/django/api/tests/test_facility_index_serializer.py (4)

src/django/api/models/partner_field.py (1)

• PartnerField (5-54)

src/django/api/models/facility/facility_index.py (1)

• FacilityIndex (13-225)

src/django/api/serializers/facility/facility_index_serializer.py (1)

• FacilityIndexSerializer (45-550)

src/django/api/models/facility/facility.py (1)

• extended_fields (148-192)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

• GitHub Check: get-base-branch-countries-cov
• GitHub Check: run-countries-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: get-base-branch-dd-cov
• GitHub Check: run-integration-test-code-quality
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-flake8-linter
• GitHub Check: run-eslint-linter-and-prettier-formatter
• GitHub Check: run-dd-code-quality
• GitHub Check: run-django-code-quality
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: run-fe-code-quality

🔇 Additional comments (2)

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

1-19: LGTM!

Imports are appropriate for testing the FacilityIndexSerializer's partner_fields functionality.

---

22-86: LGTM!

The setUp method creates comprehensive test fixtures including the partner field association with the contributor.

[VadimKovalenkoSNF]

LGTM

[VadimKovalenkoSNF]

Re-approved.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

123-129: Log exceptions with traceback; avoid blind except

Use logger.exception to capture stack traces and consider narrowing the exception type.

As per static analysis hints

-            except Exception as exc:
-                logger.error(
-                    f"Failed to serialize partner field '{field_name}': "
-                    f"{exc}"
-                )
+            except Exception:
+                logger.exception(
+                    f"Failed to serialize partner field '{field_name}'"
+                )
                 grouped_data[field_name] = []

🧹 Nitpick comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

189-204: Make partner-field name fetching deterministic and leaner with cache.get_or_set

Order by name for stable output and use cache.get_or_set (atomic, fewer branches). Consider a slightly longer TTL.

-    def __get_partner_field_names():
-        cache_key = 'partner_field_names_list'
-        cached_names = cache.get(cache_key)
-
-        if cached_names is not None:
-            return cached_names
-
-        names = list(
-            PartnerField.objects.values_list("name", flat=True)
-        )
-
-        cache.set(cache_key, names, 60)
-
-        return names
+    def __get_partner_field_names():
+        cache_key = 'partner_field_names_list'
+        return cache.get_or_set(
+            cache_key,
+            lambda: list(
+                PartnerField.objects.order_by("name")
+                .values_list("name", flat=True)
+            ),
+            timeout=300,
+        )

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e5c155 and 4f4fde0.

📒 Files selected for processing (1)

• src/django/api/serializers/facility/facility_index_serializer.py (11 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/django/api/serializers/facility/facility_index_serializer.py (4)

src/django/api/models/partner_field.py (1)

• PartnerField (5-54)

src/django/api/serializers/facility/facility_index_extended_field_list_serializer.py (1)

• FacilityIndexExtendedFieldListSerializer (10-98)

src/django/api/models/facility/facility.py (1)

• extended_fields (148-192)

src/django/api/serializers/facility/utils.py (3)

• get_efs_associated_with_contributor (148-155)
• is_created_at_main_date (44-52)
• can_user_see_detail (118-126)

🪛 Ruff (0.13.3)

src/django/api/serializers/facility/facility_index_serializer.py

123-123: Do not catch blind exception: Exception

(BLE001)

---

124-127: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

• GitHub Check: run-integration-test-code-quality
• GitHub Check: run-eslint-linter-and-prettier-formatter
• GitHub Check: run-dd-code-quality
• GitHub Check: run-flake8-linter
• GitHub Check: get-base-branch-countries-cov
• GitHub Check: run-django-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: run-fe-code-quality
• GitHub Check: run-countries-code-quality
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: get-base-branch-dd-cov

🔇 Additional comments (2)

src/django/api/serializers/facility/facility_index_serializer.py (2)

178-188: Good: O(n) grouping replaces repeated filtering

Grouping once by field_name avoids prior O(n²) scans.

---

412-440: Good: partner_fields flow is clean and efficient

Reuses contributor filter, groups once, caches field names, and sorts consistently. Addresses earlier N+1 concerns on name lookups.

[VadimKovalenkoSNF]

Re-approved.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

123-127: Apply the previously suggested fix for exception logging.

This issue was already identified in a previous review. Use logger.exception to automatically capture the stack trace for easier debugging.

As previously suggested, apply this diff:

             except Exception as exc:
-                logger.error(
-                    f"Failed to serialize partner field '{field_name}': "
-                    f"{exc}"
-                )
+                logger.exception(
+                    f"Failed to serialize partner field '{field_name}'"
+                )
                 grouped_data[field_name] = []

🧹 Nitpick comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

189-204: Caching implementation looks good, but consider cache invalidation strategy.

The caching implementation effectively addresses the N+1 query issue identified in previous reviews. However, the 600-second timeout means newly added partner fields won't appear for up to 10 minutes.

Consider either:

1. Adding a comment explaining the cache timeout trade-off
2. Implementing explicit cache invalidation in the PartnerField model's save() method

Example for explicit invalidation (if partner fields are frequently updated):

# In src/django/api/models/partner_field.py
def save(self, *args, **kwargs):
    super().save(*args, **kwargs)
    cache.delete('partner_field_names_list')

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f4fde0 and e8b8e27.

📒 Files selected for processing (1)

• src/django/api/serializers/facility/facility_index_serializer.py (11 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/django/api/serializers/facility/facility_index_serializer.py (4)

src/django/api/models/partner_field.py (1)

• PartnerField (5-54)

src/django/api/serializers/facility/facility_index_extended_field_list_serializer.py (1)

• FacilityIndexExtendedFieldListSerializer (10-98)

src/django/api/models/facility/facility.py (1)

• extended_fields (148-192)

src/django/api/serializers/facility/utils.py (3)

• get_efs_associated_with_contributor (148-155)
• is_created_at_main_date (44-52)
• can_user_see_detail (118-126)

🪛 Ruff (0.13.3)

src/django/api/serializers/facility/facility_index_serializer.py

123-123: Do not catch blind exception: Exception

(BLE001)

---

124-127: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

• GitHub Check: run-flake8-linter
• GitHub Check: run-integration-test-code-quality
• GitHub Check: run-eslint-linter-and-prettier-formatter
• GitHub Check: run-dd-code-quality
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-django-code-quality
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: run-fe-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: get-base-branch-dd-cov
• GitHub Check: run-countries-code-quality
• GitHub Check: get-base-branch-countries-cov

🔇 Additional comments (5)

src/django/api/serializers/facility/facility_index_serializer.py (5)

1-6: LGTM! Clean import additions.

The new imports for logging, typing, cache, and PartnerField are all properly utilized in the implementation. Logger initialization follows Python best practices.

Also applies to: 21-21, 43-44

---

89-91: Excellent refactoring with well-organized helper methods.

The helper methods significantly improve code organization:

• Centralized request access via __get_request
• Clear separation of sorting logic
• Efficient grouping with defaultdict
• Appropriate use of static methods
• Type hints improve maintainability

Also applies to: 132-188

---

412-441: Clean implementation of partner fields serialization.

The get_partner_fields method effectively leverages the helper methods to:

• Filter fields by contributor (when in embed mode)
• Group fields by name efficiently
• Serialize and sort with proper error handling
• Return a well-structured dictionary

The implementation follows the established patterns in the codebase and integrates seamlessly with existing functionality.

---

335-410: Consistent refactoring improves maintainability.

The modifications to get_extended_fields and get_contributors improve consistency by using the shared __get_request helper and centralized sorting logic. The behavior remains unchanged while reducing code duplication.

Also applies to: 488-488

---

58-58: Proper integration of partner_fields.

The partner_fields field is correctly added as a SerializerMethodField and included in the Meta.fields tuple, following Django REST Framework conventions.

Also applies to: 77-77

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

123-128: Use logger.exception to include traceback.

This issue was previously flagged but not yet addressed. Catching a bare Exception and using logger.error hides error details and makes debugging difficult.

Apply this diff:

             except Exception as exc:
-                logger.error(
-                    f"Failed to serialize partner field '{field_name}': "
-                    f"{exc}"
-                )
+                logger.exception(
+                    f"Failed to serialize partner field '{field_name}'"
+                )
                 grouped_data[field_name] = []

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e8b8e27 and 0eb1c44.

📒 Files selected for processing (1)

• src/django/api/serializers/facility/facility_index_serializer.py (11 hunks)

🧰 Additional context used

🪛 Ruff (0.13.3)

src/django/api/serializers/facility/facility_index_serializer.py

123-123: Do not catch blind exception: Exception

(BLE001)

---

124-127: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🔇 Additional comments (7)

src/django/api/serializers/facility/facility_index_serializer.py (7)

1-6: LGTM!

The new imports and logger initialization are appropriate for the partner fields functionality and follow Django conventions.

Also applies to: 21-21, 43-44

---

132-153: LGTM!

The extracted sorting helper methods centralize the logic and improve maintainability. The implementation is clean and follows good refactoring practices.

---

155-176: LGTM!

The implementation correctly filters extended fields by contributor in embed mode. The use of early returns makes the logic clear and easy to follow, addressing previous review feedback.

---

178-187: LGTM!

The field grouping implementation efficiently groups fields in O(n) time using defaultdict, addressing the previous concern about O(n²) behavior from repeated filter() calls.

---

335-410: LGTM!

The refactoring to use the new helper methods improves code organization and maintainability while preserving the existing functionality. The sorting logic is now centralized and reusable.

---

412-439: Implementation looks correct, subject to fixing __get_request().

The get_partner_fields() method correctly implements partner field retrieval, filtering, grouping, and sorting. The logic follows the same pattern as get_extended_fields() and properly uses the helper methods.

Note: This method will work correctly once the __get_request() issue (lines 89-91) is fixed.

---

487-487: Refactoring to use __get_request() is good, but fix the underlying issue.

The change to use the centralized __get_request() method improves consistency. However, this will work correctly only after fixing the __get_request() implementation (lines 89-91).

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/django/api/serializers/facility/facility_index_serializer.py (1)

126-131: Log serialization failures with full traceback

Switch to logger.exception so traceback is preserved and aligns with static-analysis guidance.

-                logger.error(
-                    f"Failed to serialize partner field '{field_name}': "
-                    f"{exc}"
-                )
+                logger.exception(
+                    "Failed to serialize partner field '%s'",
+                    field_name,
+                )

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb1c44 and 298a26a.

📒 Files selected for processing (3)

• src/django/api/constants.py (1 hunks)
• src/django/api/models/partner_field.py (2 hunks)
• src/django/api/serializers/facility/facility_index_serializer.py (11 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/django/api/serializers/facility/facility_index_serializer.py (5)

src/django/api/models/facility/facility_index.py (1)

• FacilityIndex (13-225)

src/django/api/models/partner_field.py (1)

• PartnerField (7-64)

src/django/api/serializers/facility/facility_index_extended_field_list_serializer.py (1)

• FacilityIndexExtendedFieldListSerializer (10-98)

src/django/api/models/facility/facility.py (1)

• extended_fields (148-192)

src/django/api/serializers/facility/utils.py (3)

• get_efs_associated_with_contributor (148-155)
• is_created_at_main_date (44-52)
• can_user_see_detail (118-126)

🪛 Ruff (0.13.3)

src/django/api/serializers/facility/facility_index_serializer.py

126-126: Do not catch blind exception: Exception

(BLE001)

---

127-130: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)

• GitHub Check: run-eslint-linter-and-prettier-formatter
• GitHub Check: run-flake8-linter
• GitHub Check: run-django-code-quality
• GitHub Check: get-base-branch-countries-cov
• GitHub Check: run-integration-test-code-quality
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: run-fe-code-quality
• GitHub Check: get-base-branch-dd-cov
• GitHub Check: run-dd-code-quality
• GitHub Check: run-countries-code-quality

[VadimKovalenkoSNF]

Re-approved.

[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

[vlad-shapik]

LGTM