[OSDEV-1336] SLC. Implement PATCH api/v1/production-locations/{os_id}/

[vladsha-dev]

[OSDEV-1336]

• Introduced a new PATCH /api/v1/production-locations/{os_id}/ endpoint based on the API v1 specification. This endpoint allows the creation of a new moderation event for updating the production location with the given details. Basically, the endpoint can be used to contribute to an existing location.
• Dynamic mapping for the new fields in the moderation-events index has been disabled for those that don't have an explicit mapping defined. This change helps avoid indexing conflicts, such as when a field is initially indexed with one data type (e.g., long), but later an entry with a different data type for the same field is indexed, causing the entire entry to fail indexing. After this change, fields with an explicit mapping will be indexed, while other fields will not be indexed or searchable, but will still be displayed in the document.

[barecheck]

React App | Jest test suite - Code coverage report

Total: 27.88%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Dedupe Hub App | Unittest test suite - Code coverage report

Total: 56.14%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Contricleaner App | Unittest test suite - Code coverage report

Total: 98.91%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Countries App | Unittest test suite - Code coverage report

Total: 100%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[roman-stolar]

LGTM

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

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

18-18: Consider enhancing the API documentation.

While the purpose of the new PATCH endpoint is clear, consider adding:

• Request/response payload formats
• Example usage
• Authentication requirements
• Possible response codes

---

19-19: Consider adding deployment impact details.

While the script changes are clear, consider adding:

• Expected script execution time
• Whether this affects other indexes
• Any required pre/post deployment steps

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0a89e6 and 635a5e3.

📒 Files selected for processing (1)

• doc/release/RELEASE-NOTES.md (3 hunks)

🔇 Additional comments (2)

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

19-19: LGTM! Clear explanation of the dynamic mapping changes.

The description effectively explains:

• What changed (disabled dynamic mapping)
• Why it changed (prevent indexing conflicts)
• Impact (unindexed fields remain visible)

---

36-36: Enhance deployment instructions.

The release instructions should be more detailed to ensure a smooth deployment.

[VadimKovalenkoSNF]

Have a few questions.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

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

Line range hint 1-17: Update the release date

The release date is set to December 28, 2024, which is a future date. Consider updating it to the actual release date to avoid confusion.

---

18-19: Improve formatting of code/API changes

Consider structuring the changes as bullet points for better readability:

* [OSDEV-1336](https://opensupplyhub.atlassian.net/browse/OSDEV-1336):
  - Introduced a new PATCH `/api/v1/production-locations/{os_id}/` endpoint based on the API v1 specification. This endpoint allows the creation of a new moderation event for updating the production location with the given details.
  - Dynamic mapping for the new fields in the `moderation-events` index has been disabled for those that don't have an explicit mapping defined. This prevents indexing conflicts when:
    * A field is initially indexed with one data type (e.g., long)
    * Later, an entry with a different data type for the same field is indexed
    * This would cause the entire entry to fail indexing
    * After this change:
      - Fields with explicit mapping will be indexed
      - Other fields will not be indexed or searchable
      - All fields will still be displayed in the document

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 635a5e3 and b523b70.

📒 Files selected for processing (2)

• deployment/clear_opensearch/clear_opensearch.sh.tpl (0 hunks)
• doc/release/RELEASE-NOTES.md (3 hunks)

💤 Files with no reviewable changes (1)

• deployment/clear_opensearch/clear_opensearch.sh.tpl

🔇 Additional comments (1)

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

36-37: 🛠️ Refactor suggestion

Enhance deployment instructions

The release instructions should be more detailed to ensure a smooth deployment.

Consider adding:

* Run `[Release] Deploy` pipeline for the target environment with the flag `Clear the custom OpenSearch indexes and templates` set to true - to refresh the index mappings for the `moderation-events` index after disabling dynamic mapping for the new fields that don't have an explicit mapping defined. Note that the `clear_opensearch.sh.tpl` script has been edited to clear only resources related to the `moderation-events` index, so there will be no need to wait for the `production-locations` index to be refilled, as it will not be deleted.
+ * Pre-deployment checklist:
+   * Backup existing moderation events data
+   * Document current index mappings
+ * Post-deployment verification:
+   * Verify moderation events indexing
+   * Test search functionality
+   * Monitor for any indexing errors
+ * Rollback plan:
+   * Steps to restore previous index mappings if issues occur
+   * Steps to re-enable dynamic mapping if needed

Likely invalid or redundant comment.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
2 Accepted issues

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

See analysis details on SonarQube Cloud

[coderabbitai]

Actionable comments posted: 1

🔭 Outside diff range comments (1)

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

Line range hint 1-24: Standardize release instructions format across releases.

The release instructions format varies across different releases. Some releases have detailed instructions while others are minimal. A consistent format would help prevent deployment issues.

Consider adding a template section at the top of the file:

+## Release Instructions Template
+
+Each release should include clear instructions covering:
+
+1. Pre-deployment steps:
+   * Required environment variables
+   * Database backups
+   * Service dependencies
+
+2. Deployment steps:
+   * Code updates
+   * Database migrations
+   * Index rebuilds
+   * Service restarts
+
+3. Post-deployment verification:
+   * Health checks
+   * Data validation
+   * Service status
+
+4. Rollback plan:
+   * Rollback triggers
+   * Rollback steps
+   * Data recovery

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b523b70 and d2eeb88.

📒 Files selected for processing (1)

• doc/release/RELEASE-NOTES.md (3 hunks)

🔇 Additional comments (1)

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

18-19: Verify dynamic mapping changes in OpenSearch.

The changes to dynamic mapping in the moderation-events index could have unintended consequences if fields are not properly mapped.

Run the following script to verify the index mappings:

✅ Verification successful

Dynamic mapping configuration is properly implemented and secure

The moderation-events index configuration in src/logstash/indexes/moderation_events.json shows that:

• Dynamic mapping is explicitly disabled with "dynamic": false
• All necessary fields have explicit mappings defined, including nested objects
• The configuration follows the changes described in the release notes

The implementation ensures that:

1. Only explicitly mapped fields will be indexed
2. Unknown fields will be stored but not indexed, preventing indexing conflicts
3. Critical fields like moderation_id, os_id, dates, and nested cleaned_data have proper type definitions

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify OpenSearch index mappings and dynamic settings

# Check current index mappings
curl -s "http://localhost:9200/moderation-events/_mapping" | jq '.'

# Verify dynamic mapping settings
curl -s "http://localhost:9200/moderation-events/_settings" | jq '."moderation-events".settings.index.mapping.dynamic'

# List all fields that have explicit mappings
curl -s "http://localhost:9200/moderation-events/_mapping" | jq '."moderation-events".mappings.properties | keys'

Length of output: 298

---

Script:

#!/bin/bash
# Let's search for OpenSearch/Elasticsearch index configuration files and mapping definitions
fd -e json -e yaml -e yml . | xargs rg -l "moderation-events"

# Search for any code that configures or manages the index mappings
rg -l "moderation-events.*mapping" 

# Look for specific files that might contain index configurations
fd "(opensearch|elasticsearch).*\.(json|ya?ml|py)$"

# Search for Python code that handles index creation or mapping
ast-grep --pattern 'create_index($$$)' 

Length of output: 2652

---

Script:

#!/bin/bash
# Let's examine the moderation events index configuration
cat src/logstash/indexes/moderation_events.json

# Look for dynamic mapping configuration in OpenSearch service
rg -A 10 "moderation-events" src/django/api/services/opensearch/opensearch.py

# Search for any code that handles index creation or mapping updates
rg -l "create_index|put_mapping" src/django/api/services/opensearch/

# Check for any migration files related to moderation events index
fd -e py "migrations" | xargs rg -l "moderation.events"

Length of output: 2917

[VadimKovalenkoSNF]

Approved, good work!