[OSDEV-2329] Show MIT Living Wage and WageIndicator data in GET v1/production-locations

[VadimKovalenkoSNF]

Feature from OSDEV-2329

Pass MIT Living Wage and WageIndicator data (wage_indicator and mit_living_wage) to GET v1/production-locations/?os_id endpoint.

API spec PR

Example output:

"wage_indicator": {
        "living_wage_link_national": "https://paywizard.org/salary/living-wages",
        "living_wage_link_national_text": "Living Wage in national language",
        "minimum_wage_link_english": "https://wageindicator.org/salary/minimum-wage/united-states-of-america",
        "minimum_wage_link_english_text": "Minimum Wage in English",
        "minimum_wage_link_national": "https://paywizard.org/salary/minimum-wage",
        "minimum_wage_link_national_text": "Minimum Wage in national language"
},
"mit_living_wage": {
        "county_id": "12117"
}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/django/api/views/v1/production_locations.py (1)

298-339: Unused facility parameter creates dead code.

The facility parameter added to the method signature is never used. The method always fetches a new Facility instance at lines 335-339 using pk, ignoring any passed facility argument.

Either remove the unused parameter or use it to avoid redundant database queries when a facility is already available.

Option 1: Remove unused parameter

-    def __get_partner_fields(self, pk, facility: Facility = None):
+    def __get_partner_fields(self, pk):
         """
         Checks and returns partner extended fields for a
-        facility object by its ID or by the provided Facility instance.
+        facility object by its ID.

Option 2: Use the passed facility when available

-            facility = Facility.objects.filter(id=pk).only(
-                'id',
-                'country_code',
-                'location',
-            ).first()
+            if facility is None:
+                facility = Facility.objects.filter(id=pk).only(
+                    'id',
+                    'country_code',
+                    'location',
+                ).first()

🧹 Nitpick comments (1)

src/django/api/tests/test_production_locations_get.py (1)

147-148: Consider more targeted cache invalidation.

Using cache.clear() is aggressive and could interfere with other tests running in parallel. Since this test specifically deals with partner field caching, consider clearing only the relevant cache key.

♻️ Suggested targeted cache clear

+from api.constants import PARTNER_FIELD_NAMES_KEY
+
 def _create_facility_with_partner_data(self):
-    cache.clear()
+    cache.delete(PARTNER_FIELD_NAMES_KEY)

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 73feb8a and 34eccae.

📒 Files selected for processing (2)

• src/django/api/tests/test_production_locations_get.py
• src/django/api/views/v1/production_locations.py

🧰 Additional context used

🧠 Learnings (4)

📓 Common learnings

Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 757
File: src/django/api/extended_fields.py:186-207
Timestamp: 2025-09-25T10:52:27.455Z
Learning: In partner extended fields, OBJECT_FIELD_TYPE fields use 'raw_values' while other partner field types use 'raw_value'. This is the correct and intended behavior as confirmed by the implementation team.

📚 Learning: 2024-12-12T14:59:19.694Z

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 438
File: src/django/api/tests/test_moderation_events_add_production_location.py:21-65
Timestamp: 2024-12-12T14:59:19.694Z
Learning: In `src/django/api/tests/test_moderation_events_add_production_location.py`, the tests have been refactored, and the use of `POST` methods is intentional. Future suggestions to change HTTP methods in these tests may not be necessary.

Applied to files:

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

📚 Learning: 2024-11-28T11:29:28.139Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 431
File: src/django/api/serializers/v1/opensearch_common_validators/countries_validator.py:23-24
Timestamp: 2024-11-28T11:29:28.139Z
Learning: The files `src/django/api/tests/test_facility_claim_view_set.py`, `src/django/api/views/facility/facilities_view_set.py`, and `src/django/api/facility_actions/processing_facility_api.py` are not part of the V1 codebase in the Open Supply Hub project.

Applied to files:

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

📚 Learning: 2024-11-28T11:40:54.281Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 431
File: src/django/api/tests/test_production_locations_view_set.py:45-46
Timestamp: 2024-11-28T11:40:54.281Z
Learning: Files `src/django/api/views/facility/facilities_view_set.py`, `src/django/api/facility_actions/processing_facility_api.py`, and `src/django/api/tests/test_facility_claim_view_set.py` are not part of v1 and should not be considered in v1-related code reviews.

Applied to files:

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

🧬 Code graph analysis (2)

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

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

• PartnerField (14-143)

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

• User (71-213)

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

• USCountyTigerline (5-31)

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

• WageIndicatorCountryData (16-129)

src/django/api/views/v1/production_locations.py (3)

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

• APIV1LocationContributionErrorMessages (256-266)

src/django/api/partner_fields/registry.py (1)

• providers (16-18)

src/django/api/partner_fields/base_provider.py (1)

• fetch_data (20-50)

⏰ 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-contricleaner-code-quality
• GitHub Check: run-dd-code-quality
• GitHub Check: run-integration-test-code-quality
• GitHub Check: run-flake8-linter
• GitHub Check: run-django-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: run-countries-code-quality
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-fe-code-quality
• GitHub Check: get-base-branch-countries-cov
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: get-base-branch-dd-cov

🔇 Additional comments (5)

src/django/api/views/v1/production_locations.py (2)

70-84: LGTM! Helper correctly handles both raw_values and raw_value patterns.

The implementation aligns with the expected behavior where OBJECT_FIELD_TYPE fields use raw_values while other partner field types use raw_value. The guard clause at line 75 appropriately handles missing field names and non-dict values.

Based on learnings, the distinction between raw_values for object types and raw_value for other types is intentional.

---

341-352: LGTM!

The provider iteration correctly handles None returns and uses the helper method consistently for both ExtendedField data and provider-sourced data.

src/django/api/tests/test_production_locations_get.py (3)

2-16: LGTM!

The new imports are appropriate for creating the comprehensive test fixtures needed to test partner field functionality.

---

174-226: LGTM!

The test fixture creation is comprehensive and correctly sets up all the necessary related objects for testing both wage_indicator and mit_living_wage partner fields. The geometry transformation and buffering logic appropriately ensures the facility location falls within the county boundaries.

---

228-255: Good test coverage for partner field integration.

The test correctly validates that partner fields are included in the API response and checks specific values from both provider sources (wage_indicator and mit_living_wage).

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/django/api/tests/test_production_locations_get.py (1)

147-148: Consider moving cache.clear() to setUp or tearDown for consistent test isolation.

Calling cache.clear() inside a helper method can lead to inconsistent state if the helper is called multiple times or if other tests depend on cache state. Moving it to setUp or tearDown ensures predictable behavior across all tests in this class.

♻️ Suggested refactor

     def setUp(self):
         self.search_mock = unittest.mock.patch(OPEN_SEARCH_SERVICE).start()
         self.search_index_mock = self.search_mock.return_value.search_index
         self.os_id = "CN2021250D1DTN7"
+        cache.clear()
         self.ohs_response_mock = {
             ...
         }

+    def tearDown(self):
+        cache.clear()
+        super().tearDown()

     def _create_facility_with_partner_data(self):
-        cache.clear()
         user = User.objects.create(email="[email protected]")

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34eccae and 4fc446f.

📒 Files selected for processing (1)

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

🧰 Additional context used

🧠 Learnings (4)

📚 Learning: 2024-12-12T14:59:19.694Z

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 438
File: src/django/api/tests/test_moderation_events_add_production_location.py:21-65
Timestamp: 2024-12-12T14:59:19.694Z
Learning: In `src/django/api/tests/test_moderation_events_add_production_location.py`, the tests have been refactored, and the use of `POST` methods is intentional. Future suggestions to change HTTP methods in these tests may not be necessary.

Applied to files:

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

📚 Learning: 2024-11-28T11:29:28.139Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 431
File: src/django/api/serializers/v1/opensearch_common_validators/countries_validator.py:23-24
Timestamp: 2024-11-28T11:29:28.139Z
Learning: The files `src/django/api/tests/test_facility_claim_view_set.py`, `src/django/api/views/facility/facilities_view_set.py`, and `src/django/api/facility_actions/processing_facility_api.py` are not part of the V1 codebase in the Open Supply Hub project.

Applied to files:

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

📚 Learning: 2024-11-21T13:06:56.072Z

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 400
File: src/django/api/moderation_event_actions/approval/add_production_location_strategy.py:193-194
Timestamp: 2024-11-21T13:06:56.072Z
Learning: In `src/django/api/moderation_event_actions/approval/add_production_location_strategy.py`, within the `AddProductionLocationStrategy` class, when processing a moderation event for adding a production location, `item.geocoded_point` is guaranteed to be not `None` due to prior assignments in the `__apply_geocode_result` or `__apply_manual_location` methods.

Applied to files:

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

📚 Learning: 2024-11-28T11:40:54.281Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 431
File: src/django/api/tests/test_production_locations_view_set.py:45-46
Timestamp: 2024-11-28T11:40:54.281Z
Learning: Files `src/django/api/views/facility/facilities_view_set.py`, `src/django/api/facility_actions/processing_facility_api.py`, and `src/django/api/tests/test_facility_claim_view_set.py` are not part of v1 and should not be considered in v1-related code reviews.

Applied to files:

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

🧬 Code graph analysis (1)

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

src/django/api/views/v1/response_mappings/production_locations_response.py (1)

• ProductionLocationsResponseMapping (1-50)

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

• PartnerField (14-143)

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

• USCountyTigerline (5-31)

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

• WageIndicatorCountryData (16-129)

⏰ 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-dd-cov
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-eslint-linter-and-prettier-formatter
• GitHub Check: run-dd-code-quality
• GitHub Check: run-integration-test-code-quality
• GitHub Check: run-flake8-linter
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: run-countries-code-quality
• GitHub Check: run-django-code-quality
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: run-fe-code-quality
• GitHub Check: get-base-branch-countries-cov

🔇 Additional comments (3)

src/django/api/tests/test_production_locations_get.py (3)

2-16: LGTM!

The new imports correctly include all necessary GIS types, caching utilities, and model classes required for the partner field test fixtures.

---

230-239: LGTM on geometry setup.

The coordinate transformation from WGS84 (SRID 4326) to NAD83/Conus Albers (SRID 5070) and the MultiPolygon conversion are correctly handled to match the USCountyTigerline model's geometry field specification.

---

243-270: Good test coverage for the happy path.

The test correctly validates that partner fields are included in the single production location response. Consider adding coverage for edge cases in follow-up work:

• Missing county data (facility location not within any USCountyTigerline)
• Missing WageIndicatorCountryData for the country
• Verifying additional fields in the wage_indicator response (e.g., minimum_wage_link_english)

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/django/api/views/v1/production_locations.py (1)

298-310: Docstring is inconsistent with method signature.

The docstring states "by its ID or by the provided Facility instance," but the method only accepts pk (an ID). The Facility is fetched internally.

Suggested fix

     def __get_partner_fields(self, pk):
         """
         Checks and returns partner extended fields for a
-        facility object by its ID or by the provided Facility instance.
+        facility object by its ID.

         Caches the list of partner field names for one hour.

🤖 Fix all issues with AI agents

In @src/django/api/views/v1/production_locations.py:
- Around line 322-352: The provider iteration is currently nested inside the "if
partner_field_names:" block so providers are never invoked when
partner_field_names is empty; refactor by keeping the ExtendedField query and
__add_partner_field_value loop for partner_field_values inside the existing
conditional but move the facility fetch and the loop over
system_partner_field_registry.providers (calling provider.fetch_data and
__add_partner_field_value) outside and after that conditional so providers are
always queried; ensure you still fetch facility
(Facility.objects.filter(id=pk).only(...).first()) and check facility is not
None before iterating providers and calling provider.fetch_data.

🧹 Nitpick comments (1)

src/django/api/views/v1/production_locations.py (1)

341-352: Consider adding exception handling for provider failures.

If provider.fetch_data() raises an unexpected exception, the entire method fails and no partner fields are returned. Wrapping each provider call in a try/except would improve resilience:

Suggested improvement

         if facility:
             for provider in system_partner_field_registry.providers:
-                provider_data = provider.fetch_data(facility)
-
-                if provider_data is None:
-                    continue
-
-                self.__add_partner_field_value(
-                    partner_extended_fields,
-                    provider_data.get('field_name'),
-                    provider_data.get('value'),
-                )
+                try:
+                    provider_data = provider.fetch_data(facility)
+
+                    if provider_data is None:
+                        continue
+
+                    self.__add_partner_field_value(
+                        partner_extended_fields,
+                        provider_data.get('field_name'),
+                        provider_data.get('value'),
+                    )
+                except Exception:
+                    # Log and continue to next provider
+                    continue

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4fc446f and e3173b7.

📒 Files selected for processing (1)

• src/django/api/views/v1/production_locations.py

🧰 Additional context used

🧬 Code graph analysis (1)

src/django/api/views/v1/production_locations.py (2)

src/django/api/partner_fields/registry.py (1)

• providers (16-18)

src/django/api/partner_fields/base_provider.py (1)

• fetch_data (20-50)

⏰ 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-integration-test-code-quality
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: get-base-branch-countries-cov
• GitHub Check: run-dd-code-quality
• GitHub Check: get-base-branch-django-cov
• GitHub Check: get-base-branch-dd-cov
• GitHub Check: run-fe-code-quality
• GitHub Check: run-django-code-quality
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: run-countries-code-quality

🔇 Additional comments (3)

src/django/api/views/v1/production_locations.py (3)

40-53: LGTM!

The new imports for PARTNER_FIELD_NAMES_KEY and system_partner_field_registry are appropriate for the added partner field functionality.

---

70-84: LGTM!

The helper method cleanly encapsulates the partner field value extraction logic. The guard conditions properly handle edge cases where field_name is empty or value is not a dict.

---

335-339: Good use of .only() for query optimization.

Verified that all registered providers (WageIndicatorProvider and MITLivingWageProvider) only access the three fields included in the query: id, country_code, and location. No additional database queries will be triggered.

[barecheck]

Django App | Unittest test suite - Code coverage report

Total: 81.93%

Your code coverage diff: 0.06% ▴

✅ All code changes are covered

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/django/api/views/v1/production_locations.py (1)

299-301: Docstring is inconsistent with method signature.

The docstring says "by its ID or by the provided Facility instance", but the method only accepts pk (ID). The "Facility instance" part should be removed since the method fetches the Facility internally.

📝 Suggested fix

     def __get_partner_fields(self, pk):
         """
         Checks and returns partner extended fields for a
-        facility object by its ID or by the provided Facility instance.
+        facility object by its ID.

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

148-148: Consider using targeted cache key deletion instead of cache.clear().

Using cache.clear() clears the entire cache, which could cause issues if tests run in parallel or if other tests rely on cached data. Consider deleting only the specific cache key.

📝 Suggested fix

+from api.constants import PARTNER_FIELD_NAMES_KEY
+
     def _create_facility_with_partner_data(self):
-        cache.clear()
+        cache.delete(PARTNER_FIELD_NAMES_KEY)

---

24-26: Consider adding explicit mock cleanup in tearDown.

The mock is started in setUp but there's no explicit stop() call. While Django's test runner typically handles cleanup, adding an explicit tearDown improves clarity and ensures proper cleanup.

📝 Suggested fix

     def setUp(self):
-        self.search_mock = unittest.mock.patch(OPEN_SEARCH_SERVICE).start()
+        self.patcher = unittest.mock.patch(OPEN_SEARCH_SERVICE)
+        self.search_mock = self.patcher.start()
         self.search_index_mock = self.search_mock.return_value.search_index
         # ...
+
+    def tearDown(self):
+        self.patcher.stop()

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e3173b7 and c57aea1.

📒 Files selected for processing (2)

• src/django/api/tests/test_production_locations_get.py
• src/django/api/views/v1/production_locations.py

🧰 Additional context used

🧠 Learnings (4)

📚 Learning: 2024-12-12T14:59:19.694Z

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 438
File: src/django/api/tests/test_moderation_events_add_production_location.py:21-65
Timestamp: 2024-12-12T14:59:19.694Z
Learning: In `src/django/api/tests/test_moderation_events_add_production_location.py`, the tests have been refactored, and the use of `POST` methods is intentional. Future suggestions to change HTTP methods in these tests may not be necessary.

Applied to files:

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

📚 Learning: 2024-11-28T11:29:28.139Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 431
File: src/django/api/serializers/v1/opensearch_common_validators/countries_validator.py:23-24
Timestamp: 2024-11-28T11:29:28.139Z
Learning: The files `src/django/api/tests/test_facility_claim_view_set.py`, `src/django/api/views/facility/facilities_view_set.py`, and `src/django/api/facility_actions/processing_facility_api.py` are not part of the V1 codebase in the Open Supply Hub project.

Applied to files:

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

📚 Learning: 2024-11-21T13:06:56.072Z

Learnt from: mazursasha1990
Repo: opensupplyhub/open-supply-hub PR: 400
File: src/django/api/moderation_event_actions/approval/add_production_location_strategy.py:193-194
Timestamp: 2024-11-21T13:06:56.072Z
Learning: In `src/django/api/moderation_event_actions/approval/add_production_location_strategy.py`, within the `AddProductionLocationStrategy` class, when processing a moderation event for adding a production location, `item.geocoded_point` is guaranteed to be not `None` due to prior assignments in the `__apply_geocode_result` or `__apply_manual_location` methods.

Applied to files:

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

📚 Learning: 2024-11-28T11:40:54.281Z

Learnt from: Innavin369
Repo: opensupplyhub/open-supply-hub PR: 431
File: src/django/api/tests/test_production_locations_view_set.py:45-46
Timestamp: 2024-11-28T11:40:54.281Z
Learning: Files `src/django/api/views/facility/facilities_view_set.py`, `src/django/api/facility_actions/processing_facility_api.py`, and `src/django/api/tests/test_facility_claim_view_set.py` are not part of v1 and should not be considered in v1-related code reviews.

Applied to files:

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

🧬 Code graph analysis (2)

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

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

• PartnerField (14-143)

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

• User (71-213)

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

• USCountyTigerline (5-31)

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

• WageIndicatorCountryData (16-129)

src/django/api/views/v1/production_locations.py (2)

src/django/api/partner_fields/registry.py (1)

• providers (16-18)

src/django/api/partner_fields/base_provider.py (1)

• fetch_data (20-50)

⏰ 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: get-base-branch-django-cov
• GitHub Check: run-integration-test-code-quality
• GitHub Check: run-contricleaner-code-quality
• GitHub Check: run-flake8-linter
• GitHub Check: run-dd-code-quality
• GitHub Check: get-base-branch-contricleaner-cov
• GitHub Check: get-base-branch-fe-cov
• GitHub Check: get-base-branch-dd-cov
• GitHub Check: run-countries-code-quality
• GitHub Check: run-fe-code-quality
• GitHub Check: get-base-branch-countries-cov
• GitHub Check: run-django-code-quality

🔇 Additional comments (7)

src/django/api/views/v1/production_locations.py (4)

40-45: LGTM on import additions.

The new imports for PARTNER_FIELD_NAMES_KEY and system_partner_field_registry are correctly placed and necessary for the new functionality.

Also applies to: 53-53

---

70-84: LGTM on the helper method.

The helper correctly guards against missing field names and non-dict values, with appropriate precedence given to raw_values over raw_value.

---

335-353: LGTM on provider integration.

The provider loop correctly fetches minimal Facility data using .only(), handles None returns gracefully, and wraps provider data in a list format consistent with the test expectations.

---

328-333: System fields may return inconsistent structures depending on data availability.

The two code paths produce different response structures for the same field names:

• ExtendedField path (lines 328-333): Extracts raw_values or raw_value directly → unwrapped value
• Provider path (lines 353): Wraps provider data in a list → [{ field_name, value, ... }]

For system fields (wage_indicator, mit_living_wage), both paths can apply since line 316 retrieves ALL active partner fields (not filtered by system_field), and the provider path may return None. If a provider has no data, the unwrapped ExtendedField value is returned instead of the expected list structure, creating API inconsistency.

Consider explicitly handling system fields separately from non-system fields to ensure consistent response structures, or filter line 316 to exclude system fields from the ExtendedField path.

⛔ Skipped due to learnings

Learnt from: roman-stolar
Repo: opensupplyhub/open-supply-hub PR: 757
File: src/django/api/extended_fields.py:186-207
Timestamp: 2025-09-25T10:52:27.455Z
Learning: In partner extended fields, OBJECT_FIELD_TYPE fields use 'raw_values' while other partner field types use 'raw_value'. This is the correct and intended behavior as confirmed by the implementation team.

src/django/api/tests/test_production_locations_get.py (3)

2-16: LGTM on import additions.

All necessary imports for the GIS-enabled test are properly added.

---

230-239: LGTM on geometry setup.

The geometry creation correctly transforms the point to SRID 5070, applies a buffer, and defensively converts to MultiPolygon if needed. The small buffer (1 meter) is sufficient for test purposes.

---

243-293: LGTM on the test implementation.

The test comprehensively validates:

• Both partner fields are present in the response
• Response structure matches expected list format
• Nested values (URLs and county_id) are correctly populated

This provides good coverage for the new partner field functionality.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@doc/release/RELEASE-NOTES.md`:
- Around line 17-18: The nested list entries (`migrate` and `reindex_database`)
in RELEASE-NOTES.md use 4-space indentation; change their leading whitespace to
2 spaces to match the project's markdown style and other release entries so the
list indentation is consistent.

🧹 Nitpick comments (1)

doc/release/RELEASE-NOTES.md (1)

13-13: Consider clarifying the API endpoint notation.

The endpoint notation GET api/v1/production-locations/?os_id could be more precise. Based on the PR objectives and standard REST patterns, this appears to reference the endpoint for retrieving a single production location by its OS ID.

Consider one of these formats for clarity:

• GET /api/v1/production-locations/{os_id} (if os_id is a path parameter)
• GET /api/v1/production-locations?os_id={value} (if os_id is a query parameter)

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c57aea1 and 7f19bf6.

📒 Files selected for processing (1)

• doc/release/RELEASE-NOTES.md

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-06-02T13:24:57.659Z

Learnt from: VadimKovalenkoSNF
Repo: opensupplyhub/open-supply-hub PR: 641
File: doc/release/RELEASE-NOTES.md:6-35
Timestamp: 2025-06-02T13:24:57.659Z
Learning: The Open Supply Hub team keeps placeholders in release notes until code freeze, then fills in the actual content once all changes are finalized for the release.

Applied to files:

• doc/release/RELEASE-NOTES.md

🪛 markdownlint-cli2 (0.18.1)

doc/release/RELEASE-NOTES.md

17-17: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

---

18-18: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/django/api/views/v1/production_locations.py (1)

282-295: Docstring is inconsistent with method signature.

The docstring states the method works "by its ID or by the provided Facility instance," but the method signature only accepts pk (an ID), not a Facility instance.

Proposed fix

     def __get_partner_fields(self, pk):
         """
         Checks and returns partner extended fields for a
-        production location object by its ID or
-        by the provided Facility instance.
+        production location object by its ID.

         Caches the list of partner field names for one hour.

🧹 Nitpick comments (1)

src/django/api/views/v1/production_locations.py (1)

320-343: Consider avoiding the Facility query when no providers are registered.

The Facility query at lines 320-324 executes unconditionally, even when system_partner_field_registry.providers is empty. This results in an unnecessary database hit.

Proposed optimization

-        facility = Facility.objects.filter(id=pk).only(
-            'id',
-            'country_code',
-            'location',
-        ).first()
-
-        if facility:
+        if system_partner_field_registry.providers:
+            facility = Facility.objects.filter(id=pk).only(
+                'id',
+                'country_code',
+                'location',
+            ).first()
+        else:
+            facility = None
+
+        if facility:
             for provider in system_partner_field_registry.providers:

[protsack-stephan]

Good work! One small comment from me.

[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
@VadimKovalenkoSNF
Could you please update the PR title and description to clarify that this PR is about returning MIT Living Wage and WageIndicator data, not only MIT Living Wage?