Skip to content

[Part-2] [OSDEV-2065] Update v1 production locations POST/PATCH endpoint to save partner field to extended field model#757

Merged
roman-stolar merged 38 commits intomainfrom
OSDEV-2065-update-v1-production-location-emission-metrics-part-2
Sep 29, 2025
Merged

[Part-2] [OSDEV-2065] Update v1 production locations POST/PATCH endpoint to save partner field to extended field model#757
roman-stolar merged 38 commits intomainfrom
OSDEV-2065-update-v1-production-location-emission-metrics-part-2

Conversation

@roman-stolar
Copy link
Contributor

@roman-stolar roman-stolar commented Sep 23, 2025

[Part-2] OSDEV-2065 Update v1 production locations POST/PATCH endpoint to save partner field to extended field model

  • Added create_partner_extendedfields_for_single_item() function for bulk partner field processing
  • Enhanced all_values_empty() function to handle dictionaries and improve list processing
  • Integrated partner field creation into the moderation event approval workflow
  • Added label field to PartnerField model
  • Added functionality to display estimated_emissions_activity & estimated_annual_energy_consumption in Production Location Profile page

@coderabbitai
Copy link

coderabbitai bot commented Sep 24, 2025

📝 Walkthrough

Walkthrough

Adds partner-specific extended-field creation into moderation approval flow, enhances extended-field utilities to handle lists/dicts, adds two ExtendedField types, adds label to PartnerField (model, migration, admin), tweaks a validation message, adds backend tests, and exposes two new extended fields in frontend constants.

Changes

Cohort / File(s) Summary
PartnerField model & migrations
src/django/api/models/partner_field.py, src/django/api/migrations/0181_add_label_to_partner_field.py, src/django/api/migrations/0180_add_unit_to_partner_field.py
Adds label CharField to PartnerField (migration 0181); removes unused uuid import in migration 0180.
Admin updates
src/django/api/admin.py
Adds label to PartnerFieldAdmin list_display and search_fields.
Extended fields logic
src/django/api/extended_fields.py, src/django/api/models/extended_field.py
Enhances all_values_empty() to handle lists and dicts; adds OBJECT_FIELD_TYPE, create_partner_extendedfield, create_partner_extendedfields_for_single_item; updates create_extendedfields_for_single_item(item, raw_data) signature; adds estimated_emissions_activity and estimated_annual_energy_consumption to ExtendedField.FIELD_CHOICES.
Moderation approval flow
src/django/api/moderation_event_actions/approval/event_approval_template.py
Calls create_partner_extendedfields_for_single_item after standard extended-field creation and logs the action.
Validation messaging
src/django/api/moderation_event_actions/creation/location_contribution/processors/permission_processor.py
Rewords type validation error detail to: "Field {name} must be {expected}, not {actual}."
Backend tests
src/django/api/tests/test_location_contribution_strategy.py, src/django/api/tests/test_moderation_events_add_production_location.py, src/django/api/tests/test_moderation_events_update_production_location.py
Adds tests for partner field type validation (string/int/float/object) and tests asserting partner ExtendedField creation and stored values on add/update flows.
Frontend constants
src/react/src/util/constants.jsx
Adds EXTENDED_FIELD_TYPES entries for estimated_emissions_activity and estimated_annual_energy_consumption with formatValue: value => value.raw_value.
Docs
doc/release/RELEASE-NOTES.md
Documents partner-field processing, added functions/changes, new label field, extended-field additions, and frontend display updates.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Moderator
  participant FE as Frontend
  participant API as Moderation API
  participant EF as ExtendedFieldsModule
  participant DB as Database

  Moderator->>FE: Click "Approve"
  FE->>API: POST /api/moderation/approve (item, raw_data)
  API->>EF: create_extendedfields_for_single_item(item, raw_data)
  EF->>DB: write standard ExtendedField(s)
  API->>EF: create_partner_extendedfields_for_single_item(item, raw_data.fields)
  EF->>DB: write partner ExtendedField(s) (raw_value / raw_values for objects)
  DB-->>API: write OK
  API-->>FE: 200/202 Approved
  FE-->>Moderator: show success
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • protsack-stephan
  • VadimKovalenkoSNF
  • vlad-shapik

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title accurately describes the primary change of updating the v1 production locations POST/PATCH endpoint to persist partner fields into the extended field model, and it aligns with the main objectives and implementation detailed in the summary. It is specific enough to convey the core purpose without extraneous information.
Description Check ✅ Passed The pull request description clearly lists the key enhancements, including the new bulk partner field processing function, improvements to all_values_empty(), integration into the moderation workflow, model changes, and UI updates, all of which correspond directly to the changes in the codebase.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch OSDEV-2065-update-v1-production-location-emission-metrics-part-2

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8bdd8ae and df3092d.

📒 Files selected for processing (1)
  • doc/release/RELEASE-NOTES.md (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
doc/release/RELEASE-NOTES.md

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

(MD007, ul-indent)


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

(MD007, ul-indent)


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

(MD007, ul-indent)


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

(MD007, ul-indent)


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

(MD007, ul-indent)

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

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

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (10)
src/django/api/models/partner_field.py (1)

45-48: Optional: show label in admin via str when available

For better readability in admin and logs, prefer label over name when label is set.

You can change str as follows (outside the shown range):

def __str__(self):
    return self.label or self.name
src/django/api/migrations/0181_add_label_to_partner_field.py (1)

4-7: Correct migration docstring

Docstring says “add unit” but this migration adds label. Update to avoid confusion.

Apply this diff:

-    """
-    Migration add unit field to PartnerField model.
-    """
+    """
+    Migration: add label field to PartnerField model.
+    """
doc/release/RELEASE-NOTES.md (1)

26-31: Fix Markdown list indentation (MD007)

Indent nested bullets by 2 spaces to satisfy markdownlint.

Apply this diff:

-    * Added `create_partner_extendedfields_for_single_item()` function for bulk partner field processing
-    * Enhanced `all_values_empty()` function to handle dictionaries and improve list processing
-    * Integrated partner field creation into the moderation event approval workflow
-    * Added `label` field to `PartnerField` model
-    * Added functionality to display `estimated_emissions_activity` & `estimated_annual_energy_consumption` in Production Location Profile page
+  * Added `create_partner_extendedfields_for_single_item()` function for bulk partner field processing
+  * Enhanced `all_values_empty()` function to handle dictionaries and improve list processing
+  * Integrated partner field creation into the moderation event approval workflow
+  * Added `label` field to `PartnerField` model
+  * Added functionality to display `estimated_emissions_activity` & `estimated_annual_energy_consumption` in Production Location Profile page
src/django/api/tests/test_moderation_events_update_production_location.py (1)

310-368: Prefer ExtendedField constants over raw strings for field_name assertions

Reduces typos and keeps tests aligned with model declarations.

-        partner_extended_fields = ExtendedField.objects.filter(
-            facility_list_item__source__contributor=self.contributor,
-            field_name__in=[
-                'estimated_emissions_activity',
-                'estimated_annual_energy_consumption'
-            ]
-        )
+        partner_extended_fields = ExtendedField.objects.filter(
+            facility_list_item__source__contributor=self.contributor,
+            field_name__in=[
+                ExtendedField.ESTIMATED_EMISSIONS_ACTIVITY,
+                ExtendedField.ESTIMATED_ANNUAL_ENERGY_CONSUMPTION,
+            ],
+        )

-        emissions_field = partner_extended_fields.get(
-            field_name='estimated_emissions_activity'
-        )
-        energy_field = partner_extended_fields.get(
-            field_name='estimated_annual_energy_consumption'
-        )
+        emissions_field = partner_extended_fields.get(
+            field_name=ExtendedField.ESTIMATED_EMISSIONS_ACTIVITY
+        )
+        energy_field = partner_extended_fields.get(
+            field_name=ExtendedField.ESTIMATED_ANNUAL_ENERGY_CONSUMPTION
+        )
src/django/api/tests/test_location_contribution_strategy.py (4)

1403-1403: Silence “hardcoded password” warnings in tests

These are test-only fixtures. Add a per-line ignore to keep CI quiet.

-existing_location_user_password = '4567test'
+existing_location_user_password = '4567test'  # noqa: S105

Also applies to: 1507-1507, 1612-1612, 1717-1717


1433-1440: Avoid shadowing built-in name “list”

Rename local var to prevent confusion and IDE lint warnings.

-        list = FacilityList.objects.create(
+        facility_list = FacilityList.objects.create(
             header='header', file_name='one', name='New List Test'
         )
         source = Source.objects.create(
             source_type=Source.LIST,
-            facility_list=list,
+            facility_list=facility_list,
             contributor=existing_location_contributor
         )

Apply the same change in each test block shown in the selected ranges.

Also applies to: 1538-1544, 1643-1649, 1748-1754


1431-1432: Drop redundant save() after ManyToMany add()

Calling save() on the model isn’t needed after partner_fields.add(...).

-        existing_location_contributor.partner_fields.add(string_field)
-        existing_location_contributor.save()
+        existing_location_contributor.partner_fields.add(string_field)

(Apply similarly in the other tests.)

Also applies to: 1535-1536, 1640-1641, 1745-1746


1401-1839: Reduce duplication with a small factory/helper

The four partner-field validation tests largely duplicate contributor/location setup. Extract a helper to create (user, contributor, facility) and to attach a PartnerField to cut boilerplate and speed up maintenance.

src/django/api/extended_fields.py (2)

137-164: Avoid duplicating the “object” type constant

Rely on PartnerField.OBJECT to keep a single source of truth and prevent drift.

-from api.models import Contributor, ExtendedField, ProductType
+from api.models import Contributor, ExtendedField, ProductType
+from api.models.partner_field import PartnerField
@@
-OBJECT_FIELD_TYPE = "object"
@@
-    if field_value is not None and field_value != "" \
-            and not all_values_empty(field_value):
-        if field_type == OBJECT_FIELD_TYPE:
+    if field_value is not None and field_value != "" \
+            and not all_values_empty(field_value):
+        if field_type == PartnerField.OBJECT:
             field_value = {
                 'raw_values': field_value,
             }

If this introduces an import cycle in your environment, keep the local constant but add a brief comment noting it must match PartnerField.OBJECT.


193-214: Consider bulk_create for partner extended fields (optional)

If contributors commonly have many partner fields, batching writes can reduce DB roundtrips. Non-blocking suggestion for later.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f248a42 and c36469b.

📒 Files selected for processing (13)
  • doc/release/RELEASE-NOTES.md (1 hunks)
  • src/django/api/admin.py (1 hunks)
  • src/django/api/extended_fields.py (4 hunks)
  • src/django/api/migrations/0180_add_unit_to_partner_field.py (0 hunks)
  • src/django/api/migrations/0181_add_label_to_partner_field.py (1 hunks)
  • src/django/api/models/extended_field.py (2 hunks)
  • src/django/api/models/partner_field.py (1 hunks)
  • src/django/api/moderation_event_actions/approval/event_approval_template.py (2 hunks)
  • src/django/api/moderation_event_actions/creation/location_contribution/processors/permission_processor.py (1 hunks)
  • src/django/api/tests/test_location_contribution_strategy.py (1 hunks)
  • src/django/api/tests/test_moderation_events_add_production_location.py (2 hunks)
  • src/django/api/tests/test_moderation_events_update_production_location.py (2 hunks)
  • src/react/src/util/constants.jsx (1 hunks)
💤 Files with no reviewable changes (1)
  • src/django/api/migrations/0180_add_unit_to_partner_field.py
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-12-12T14:59:19.694Z
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#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_moderation_events_update_production_location.py
  • src/django/api/tests/test_location_contribution_strategy.py
  • src/django/api/tests/test_moderation_events_add_production_location.py
📚 Learning: 2024-12-17T15:59:02.112Z
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#438
File: src/django/api/tests/test_moderation_events_add_production_location.py:27-30
Timestamp: 2024-12-17T15:59:02.112Z
Learning: In tests for the PATCH endpoint `/api/v1/moderation-events/{moderation_id}/production-locations/`, it's acceptable to use POST requests in the tests.

Applied to files:

  • src/django/api/tests/test_moderation_events_update_production_location.py
📚 Learning: 2024-11-13T07:11:20.994Z
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#391
File: src/tests/v1/test_moderation_events.py:16-17
Timestamp: 2024-11-13T07:11:20.994Z
Learning: In integration tests like `src/tests/v1/test_moderation_events.py` that run separately from the Django container, we can't import Django models like `ModerationEvent`.

Applied to files:

  • src/django/api/tests/test_moderation_events_update_production_location.py
  • src/django/api/tests/test_moderation_events_add_production_location.py
📚 Learning: 2024-11-28T12:54:16.114Z
Learnt from: Innavin369
PR: opensupplyhub/open-supply-hub#431
File: src/django/api/tests/test_facility_claim_view_set.py:0-0
Timestamp: 2024-11-28T12:54:16.114Z
Learning: In files not related to v1, such as `src/django/api/tests/test_facility_claim_view_set.py`, error responses use `'message'` instead of `'detail'`.

Applied to files:

  • src/django/api/moderation_event_actions/creation/location_contribution/processors/permission_processor.py
🧬 Code graph analysis (6)
src/django/api/migrations/0181_add_label_to_partner_field.py (1)
src/django/api/migrations/0180_add_unit_to_partner_field.py (1)
  • Migration (3-22)
src/django/api/tests/test_moderation_events_update_production_location.py (3)
src/django/api/models/partner_field.py (1)
  • PartnerField (5-54)
src/django/api/tests/base_moderation_events_production_location_test.py (2)
  • login_as_superuser (107-110)
  • assert_success_response (158-172)
src/django/api/tests/test_moderation_events_add_production_location.py (1)
  • get_url (24-27)
src/django/api/tests/test_location_contribution_strategy.py (6)
src/django/api/views/v1/production_locations.py (1)
  • create (155-205)
src/django/api/models/partner_field.py (1)
  • PartnerField (5-54)
src/django/api/models/contributor/contributor.py (1)
  • Contributor (14-159)
src/django/api/models/moderation_event.py (3)
  • Source (24-26)
  • ModerationEvent (10-166)
  • RequestType (14-17)
src/django/api/moderation_event_actions/creation/dtos/create_moderation_event_dto.py (1)
  • CreateModerationEventDTO (12-22)
src/django/api/moderation_event_actions/creation/moderation_event_creator.py (1)
  • perform_event_creation (13-32)
src/django/api/moderation_event_actions/approval/event_approval_template.py (1)
src/django/api/extended_fields.py (2)
  • create_partner_extendedfields_for_single_item (193-213)
  • create_extendedfields_for_single_item (180-190)
src/django/api/tests/test_moderation_events_add_production_location.py (2)
src/django/api/models/partner_field.py (1)
  • PartnerField (5-54)
src/django/api/tests/base_moderation_events_production_location_test.py (2)
  • login_as_superuser (107-110)
  • assert_success_response (158-172)
src/django/api/extended_fields.py (1)
src/django/api/models/extended_field.py (1)
  • ExtendedField (7-121)
🪛 Ruff (0.13.1)
src/django/api/migrations/0181_add_label_to_partner_field.py

8-10: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


12-22: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

src/django/api/tests/test_location_contribution_strategy.py

1403-1403: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1507-1507: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1612-1612: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1717-1717: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)

🪛 markdownlint-cli2 (0.18.1)
doc/release/RELEASE-NOTES.md

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

(MD007, ul-indent)


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

(MD007, ul-indent)


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

(MD007, ul-indent)


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

(MD007, ul-indent)


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

(MD007, ul-indent)

⏰ 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-dd-code-quality
  • GitHub Check: run-countries-code-quality
  • GitHub Check: get-base-branch-dd-cov
  • GitHub Check: run-contricleaner-code-quality
  • GitHub Check: run-fe-code-quality
  • GitHub Check: run-eslint-linter-and-prettier-formatter
  • GitHub Check: get-base-branch-contricleaner-cov
  • GitHub Check: run-django-code-quality
  • GitHub Check: get-base-branch-django-cov
  • GitHub Check: get-base-branch-countries-cov
  • GitHub Check: get-base-branch-fe-cov
  • GitHub Check: run-flake8-linter
🔇 Additional comments (10)
src/django/api/admin.py (1)

245-246: LGTM – add label to admin list/search

Display/search of PartnerField.label looks good.

src/django/api/moderation_event_actions/approval/event_approval_template.py (2)

87-94: LGTM – extended fields creation step

Keeps existing behavior intact and logs appropriately.


96-103: Partner extended fields integration – verify dedupe/update behavior

Good addition and logging. Please confirm:

  • update_extendedfields_for_list_item updates partner extended fields with facility_id as well.
  • The partner field names (e.g., estimated_emissions_activity) are not in RAW_DATA_FIELDS to prevent duplicate ExtendedField rows.
src/react/src/util/constants.jsx (1)

1126-1135: LGTM – new extended field types

The FE mapping matches expected raw_value usage.

Confirm BE returns these fields (names and payload shape with raw_value) in facility details; otherwise the UI will show empty values.

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

13-15: LGTM – test imports

PartnerField and ExtendedField imports are appropriate for the new test.


275-332: LGTM – partner fields E2E test

Solid end-to-end coverage for creation and persistence of partner extended fields with typed values.

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

14-15: LGTM: necessary imports added

PartnerField and ExtendedField are used below; imports look correct.

src/django/api/extended_fields.py (2)

82-96: Improved empty-value handling for lists and dicts: LGTM

Covers the new object/list cases and prevents writing empty partner extended fields.


180-191: Resolved — all call sites pass raw_data

Verified callers in src/django/api/moderation_event_actions/approval/event_approval_template.py, src/django/api/facility_actions/processing_facility_list.py, and src/django/api/facility_actions/processing_facility_api.py pass raw_data (data["fields"] / row.fields).

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

25-26: Add AlterField migration for ExtendedField.field_name

src/django/api/models/extended_field.py (lines 25–26) adds 'estimated_emissions_activity' and 'estimated_annual_energy_consumption' but no migration was found updating api.ExtendedField.field_name choices to include them (searched migrations/*). Run makemigrations and include the generated AlterField migration in this PR.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
src/django/api/tests/test_location_contribution_strategy.py (5)

1507-1511: Avoid relying on errors[0] ordering; select by field name.

Pick the error for the specific field to make tests resilient to ordering.

Example for each test:

-        detail = result.errors['errors'][0]['detail']
+        err = next(e for e in result.errors['errors'] if e['field'] == 'string_field')
+        detail = err['detail']
-        detail = result.errors['errors'][0]['detail']
+        err = next(e for e in result.errors['errors'] if e['field'] == 'int_field')
+        detail = err['detail']
-        detail = result.errors['errors'][0]['detail']
+        err = next(e for e in result.errors['errors'] if e['field'] == 'float_field')
+        detail = err['detail']
-        detail = result.errors['errors'][0]['detail']
+        err = next(e for e in result.errors['errors'] if e['field'] == 'object_field')
+        detail = err['detail']

Also applies to: 1620-1624, 1733-1737, 1870-1874


1433-1440: Don’t shadow built-in “list”; use a clearer variable name.

Rename local variable to avoid overshadowing Python’s list.

-        list = FacilityList.objects.create(
+        facility_list = FacilityList.objects.create(
             header='header', file_name='one', name='New List Test'
         )
         source = Source.objects.create(
             source_type=Source.LIST,
-            facility_list=list,
+            facility_list=facility_list,
             contributor=existing_location_contributor
         )

Repeat similarly in the other occurrences within these ranges.

Also applies to: 1545-1552, 1658-1665, 1771-1778


1430-1432: Drop redundant save() after M2M add.

ManyToManyField.add() persists immediately; the following save() is unnecessary.

         existing_location_contributor.partner_fields.add(string_field)
-        existing_location_contributor.save()

Apply similarly to the int/float/object tests.

Also applies to: 1543-1544, 1656-1657, 1769-1770


1403-1403: Silence password lint in tests (Ruff S105).

These are test-only dummy passwords; annotate to satisfy the linter.

-        existing_location_user_password = '4567test'
+        existing_location_user_password = '4567test'  # noqa: S105

Also applies to: 1515-1515, 1628-1628, 1741-1741


1418-1456: Reduce duplication with a small factory/helper.

Large blocks for user/contributor/partner-field/facility setup are duplicated across tests. Extract a helper (e.g., _mk_contrib_with_field_and_facility(field_name, field_type)) to improve readability and speed up edits.

Also applies to: 1530-1569, 1643-1682, 1756-1795

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d61a6c and 3a7e475.

📒 Files selected for processing (1)
  • src/django/api/tests/test_location_contribution_strategy.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T14:59:19.694Z
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#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_location_contribution_strategy.py
🧬 Code graph analysis (1)
src/django/api/tests/test_location_contribution_strategy.py (5)
src/django/api/views/v1/production_locations.py (1)
  • create (155-205)
src/django/api/models/partner_field.py (1)
  • PartnerField (5-54)
src/django/api/models/contributor/contributor.py (1)
  • Contributor (14-159)
src/django/api/moderation_event_actions/creation/dtos/create_moderation_event_dto.py (1)
  • CreateModerationEventDTO (12-22)
src/django/api/moderation_event_actions/creation/moderation_event_creator.py (1)
  • perform_event_creation (13-32)
🪛 Ruff (0.13.1)
src/django/api/tests/test_location_contribution_strategy.py

1403-1403: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1515-1515: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1628-1628: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1741-1741: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)

⏰ 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: run-dd-code-quality
  • GitHub Check: get-base-branch-countries-cov
  • GitHub Check: run-eslint-linter-and-prettier-formatter
  • GitHub Check: run-django-code-quality
  • GitHub Check: run-fe-code-quality
  • GitHub Check: get-base-branch-django-cov
  • GitHub Check: get-base-branch-fe-cov
🔇 Additional comments (1)
src/django/api/tests/test_location_contribution_strategy.py (1)

1507-1511: Fix grammar in expected validation messages (“an int/object”).

Tests currently expect “a int/object”; should be “an int/object”. Update the expected substrings, and ensure the backend message builder picks the correct article.

Apply these diffs in this file:

-        self.assertIn(
-            'Field string_field must be a string, not a int.',
-            detail
-        )
+        self.assertIn(
+            'Field string_field must be a string, not an int.',
+            detail
+        )
-        self.assertIn(
-            'Field int_field must be a int, not a str.',
-            detail
-        )
+        self.assertIn(
+            'Field int_field must be an int, not a str.',
+            detail
+        )
-        self.assertIn(
-            'Field object_field must be a object, not a str.',
-            detail
-        )
+        self.assertIn(
+            'Field object_field must be an object, not a str.',
+            detail
+        )

To locate and fix the backend source of this message, run:

#!/bin/bash
# Find where the message is constructed so the article can be corrected.
rg -nP -C2 --type=py "must be a \{expected\}, not a \{|must be a [a-z_]+, not a"

Also applies to: 1621-1624, 1871-1874

Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF left a comment

Choose a reason for hiding this comment

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

Left few comments.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/django/api/extended_fields.py (1)

173-183: Return value contract changed silently.

Callers previously expected create_extendedfields_for_single_item(item) to return None. Returning False from the item.id is None case changes the function’s contract. Consider keeping the old behavior (return None) or document the new boolean semantics so upstream code doesn’t misinterpret the return value.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a7e475 and 30aca95.

📒 Files selected for processing (1)
  • src/django/api/extended_fields.py (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/django/api/extended_fields.py (1)
src/django/api/models/extended_field.py (1)
  • ExtendedField (7-121)
⏰ 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-eslint-linter-and-prettier-formatter
  • GitHub Check: run-flake8-linter
  • GitHub Check: get-base-branch-contricleaner-cov
  • GitHub Check: get-base-branch-dd-cov
  • GitHub Check: run-dd-code-quality
  • GitHub Check: run-contricleaner-code-quality
  • GitHub Check: run-integration-test-code-quality
  • 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-django-cov

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64d2ba8 and 8bdd8ae.

📒 Files selected for processing (1)
  • src/django/api/tests/test_location_contribution_strategy.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-12T14:59:19.694Z
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#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_location_contribution_strategy.py
🧬 Code graph analysis (1)
src/django/api/tests/test_location_contribution_strategy.py (5)
src/django/api/views/v1/production_locations.py (1)
  • create (155-205)
src/django/api/models/partner_field.py (1)
  • PartnerField (5-54)
src/django/api/models/contributor/contributor.py (1)
  • Contributor (14-159)
src/django/api/moderation_event_actions/creation/dtos/create_moderation_event_dto.py (1)
  • CreateModerationEventDTO (12-22)
src/django/api/moderation_event_actions/creation/moderation_event_creator.py (1)
  • perform_event_creation (13-32)
🪛 Ruff (0.13.1)
src/django/api/tests/test_location_contribution_strategy.py

1403-1403: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1515-1515: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1628-1628: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)


1741-1741: Possible hardcoded password assigned to: "existing_location_user_password"

(S105)

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

@roman-stolar
Copy link
Contributor Author

@VadimKovalenkoSNF Ready for re-review. Thank you!

Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@vlad-shapik vlad-shapik left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants