Skip to content

[OSDEV-1346] - Refactor index mapping and sorting for name and address of moderation event#439

Closed
VadimKovalenkoSNF wants to merge 3 commits intomainfrom
OSDEV-1346-fix-name-and-address-mapping
Closed

[OSDEV-1346] - Refactor index mapping and sorting for name and address of moderation event#439
VadimKovalenkoSNF wants to merge 3 commits intomainfrom
OSDEV-1346-fix-name-and-address-mapping

Conversation

@VadimKovalenkoSNF
Copy link
Contributor

@VadimKovalenkoSNF VadimKovalenkoSNF commented Dec 4, 2024

Follow-up PR for OSDEV-1346
Refactor index mapping and sorting for cleaned_data.name and cleaned_data.address of moderation event to prevent error:

'search_phase_execution_exception', 'Text fields are not optimised for operations that require per-document field data like aggregations and sorting, so these operations are disabled by default. Please use a keyword field instead.

@VadimKovalenkoSNF VadimKovalenkoSNF self-assigned this Dec 4, 2024
@barecheck
Copy link

barecheck bot commented Dec 4, 2024

React App | Jest test suite - Code coverage report

Total: 27.74%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Dec 4, 2024

Dedupe Hub App | Unittest test suite - Code coverage report

Total: 56.14%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Dec 4, 2024

Countries App | Unittest test suite - Code coverage report

Total: 100%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

@barecheck
Copy link

barecheck bot commented Dec 4, 2024

Contricleaner App | Unittest test suite - Code coverage report

Total: 98.91%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

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

🧹 Outside diff range and nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)

Line range hint 1-24: Ensure release instructions are complete

The release instructions for version 1.26.0 should explicitly mention checking OpenSearch functionality after deployment, given the significant changes to index mapping and null value handling.

Add these steps to the release instructions:

### Release instructions:
* Ensure that the following commands are included in the `post_deployment` command:
    * `migrate`
+* After deployment:
+    * Verify OpenSearch index mapping is correctly applied
+    * Confirm search functionality works with the updated null value handling
+    * Test search queries on fields with cleaned_data.name and cleaned_data.address
🧰 Tools
🪛 Markdownlint (0.35.0)

25-25: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 74934d2 and c2f9a23.

📒 Files selected for processing (1)
  • doc/release/RELEASE-NOTES.md (1 hunks)
🔇 Additional comments (1)
doc/release/RELEASE-NOTES.md (1)

22-22: Verify OpenSearch index mapping changes

The changes to disable null values and update index mapping for cleaned_data.name and cleaned_data.address fields could impact search functionality and data consistency.

✅ Verification successful

Based on the codebase analysis, I can now generate a final response regarding the OpenSearch changes mentioned in the release notes.

OpenSearch changes are properly implemented

The codebase shows comprehensive implementation of null value handling and index mapping changes:

  • The OpenSearch service has proper null value filtering in the response preparation (test_prepare_opensearch_response_with_varied_null_cases)
  • The changes affect os_id, claim_id, and source fields as mentioned in the release notes
  • The moderation events endpoint properly handles cleaned_data.name and cleaned_data.address fields
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the OpenSearch index mapping changes and null value handling

# Check the current index mapping configuration
curl -s "http://localhost:9200/production-locations/_mapping" | jq '.'

# Verify there are no documents with null values for these fields
curl -s "http://localhost:9200/production-locations/_search" -H 'Content-Type: application/json' -d '{
  "query": {
    "bool": {
      "should": [
        { "bool": { "must_not": { "exists": { "field": "cleaned_data.name" } } } },
        { "bool": { "must_not": { "exists": { "field": "cleaned_data.address" } } } }
      ]
    }
  }
}' | jq '.hits.total'

Length of output: 424


Script:

#!/bin/bash
# Let's verify the OpenSearch changes through the codebase instead
# Check for OpenSearch mapping configurations in the codebase
rg -l "opensearch|opensearchpy" --type yaml

# Look for OpenSearch related migrations or configuration changes
fd -e py -e yaml "opensearch|elastic" --exec cat {}

# Look for API endpoint changes related to moderation events
ast-grep --pattern 'def $_(self, moderation_id):
  $$$'

# Search for OpenSearch client configuration and mapping updates
rg -A 5 "cleaned_data\.(name|address)"

Length of output: 27412

### Code/API changes
* [OSDEV-1453](https://opensupplyhub.atlassian.net/browse/OSDEV-1453) - The `detail` keyword instead of `message` has been applied in error response objects for V1 endpoints.
* [OSDEV-1346](https://opensupplyhub.atlassian.net/browse/OSDEV-1346) - Disabled null values from the response of the OpenSearch. Disabled possible null `os_id`, `claim_id` and `source` from `PATCH api/v1/moderation-events/{moderation_id}` response.
* [OSDEV-1346](https://opensupplyhub.atlassian.net/browse/OSDEV-1346) - Disabled null values from the response of the OpenSearch. Disabled possible null `os_id`, `claim_id` and `source` from `PATCH api/v1/moderation-events/{moderation_id}` response. Updated index mapping for `cleaned_data.name` and `cleaned_data.address` for `GET api/v1/moderation-events`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please state the reason for the update? What value did your changes bring?
Please reflect this in the PR description as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated release notes.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 5, 2024

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

🧹 Outside diff range and nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)

22-22: LGTM with a minor formatting suggestion.

The change accurately documents the index mapping refactor and error fix. Consider breaking this long bullet point into sub-bullets for better readability:

-* [OSDEV-1346](https://opensupplyhub.atlassian.net/browse/OSDEV-1346) - Disabled null values from the response of the OpenSearch. Disabled possible null `os_id`, `claim_id` and `source` from `PATCH api/v1/moderation-events/{moderation_id}` response. Updated index mapping for `cleaned_data.name` and `cleaned_data.address` for `GET api/v1/moderation-events`. This change fixes the OpenSearch error while sorting by `cleaned_data.name` and `cleaned_data.address`: `Text fields are not optimised for operations that require per-document field data like aggregations and sorting, so these operations are disabled by default. Please use a keyword field instead.`.
+* [OSDEV-1346](https://opensupplyhub.atlassian.net/browse/OSDEV-1346)
+  * Disabled null values from the response of the OpenSearch
+  * Disabled possible null `os_id`, `claim_id` and `source` from `PATCH api/v1/moderation-events/{moderation_id}` response
+  * Updated index mapping for `cleaned_data.name` and `cleaned_data.address` for `GET api/v1/moderation-events`
+  * This change fixes the OpenSearch error while sorting by `cleaned_data.name` and `cleaned_data.address`: `Text fields are not optimised for operations that require per-document field data like aggregations and sorting, so these operations are disabled by default. Please use a keyword field instead.`
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between c2f9a23 and eb09553.

📒 Files selected for processing (1)
  • doc/release/RELEASE-NOTES.md (1 hunks)

Copy link
Contributor

@mazursasha1990 mazursasha1990 left a comment

Choose a reason for hiding this comment

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

LGTM

@VadimKovalenkoSNF
Copy link
Contributor Author

Abandon this PR in favor of PR: Fix OpenSearch index mapping updates

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.

4 participants