CMR-10686: Index a collection's collection progress value so that it can be searched. #2361
Conversation
📝 WalkthroughWalkthroughAdds collection-progress end-to-end: indexes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant SearchService as Search Service
participant ParamConv as Param Conversion
participant ES as Elasticsearch
note over Client,SearchService: User issues collection search with collection_progress parameter
Client->>SearchService: HTTP GET /collections?collection_progress=...
SearchService->>ParamConv: parameter->condition(:collection-progress, value, options)
ParamConv-->>SearchService: ES condition (string-condition or multi-value AND/OR)
SearchService->>ES: Query (uses collection-progress / collection-progress-lowercase fields)
ES-->>SearchService: Results
SearchService-->>Client: Response (collection refs)
par Indexing path
participant Ingest as Ingest Pipeline
participant Indexer as Indexer
Ingest->>Indexer: Ingest collection (CollectionProgress present)
Indexer->>ES: Index doc including :collection-progress and :collection-progress-lowercase
ES-->>Indexer: Index ack
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj (1)
26-51: LGTM! Test cases provide solid coverage.The parameterized test cases cover the essential scenarios:
- Single value and multi-value queries
- Case sensitivity handling
- Pattern matching with wildcards
- AND/OR operations
- Invalid value handling
The test assertions are correct and align with expected behavior.
Consider adding edge case tests
While current coverage is good, you could optionally add tests for:
- Empty string handling:
"" → []- Multiple values with patterns:
["ACT*" "PLAN*"]with pattern option- Mixed case in multi-value OR:
["active" "PLANNED"](should match both with default case-insensitive)These are nice-to-have additions for comprehensive edge case coverage but not essential for the current implementation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
indexer-app/src/cmr/indexer/data/concepts/collection.clj(1 hunks)indexer-app/src/cmr/indexer/data/index_set.clj(1 hunks)search-app/src/cmr/search/services/parameters/conversion.clj(2 hunks)search-app/src/cmr/search/services/parameters/parameter_validation.clj(2 hunks)system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj(1 hunks)
🔇 Additional comments (8)
indexer-app/src/cmr/indexer/data/concepts/collection.clj (1)
400-401: LGTM! Collection progress indexing implemented correctly.The addition of both
:collection-progressand:collection-progress-lowercasefields follows the established pattern for similar fields in the codebase. Usingutil/safe-lowercaseensures nil values are handled gracefully during indexing.indexer-app/src/cmr/indexer/data/index_set.clj (1)
420-421: LGTM! Elasticsearch mapping correctly defined.The collection-progress field mappings are properly defined using
m/string-field-mapping, consistent with similar string fields like:collection-data-type. The dual raw/lowercase field approach enables flexible search options.search-app/src/cmr/search/services/parameters/parameter_validation.clj (2)
42-42: LGTM! Parameter configuration correctly updated.Adding
:collection-progressto the:multiple-valueset appropriately enables multi-value search queries for this parameter.
155-155: LGTM! Parameter options correctly configured.The use of
cpv/string-plus-and-optionsenables AND/OR operations, case sensitivity control, and pattern matching for collection-progress searches, consistent with similar string-based parameters.search-app/src/cmr/search/services/parameters/conversion.clj (2)
33-33: LGTM! Parameter mapping correctly defined.The
:collection-progress :stringmapping is appropriately configured, following the pattern of similar parameters.
473-487: LGTM! Query condition conversion implemented correctly.The implementation properly handles both multi-value (AND/OR) and single-value queries with appropriate support for case sensitivity and pattern matching. The logic follows the established pattern used by similar parameters like
:collection-data-type.system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj (2)
1-12: LGTM! Test namespace and fixture setup are correct.The namespace declaration and fixture configuration follow established patterns for integration tests in this codebase.
14-24: LGTM! Test data setup is comprehensive.The test ingests collections with a diverse set of CollectionProgress values, providing good coverage for various search scenarios. The
index/wait-until-indexedcall ensures data is available before running search assertions.
daniel-zamora
force-pushed
the
CMR-10686
branch
from
539e255 to
0bffba9
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2361 +/- ##
==========================================
- Coverage 58.01% 58.01% -0.01%
==========================================
Files 1063 1063
Lines 72750 72765 +15
Branches 2111 2111
==========================================
+ Hits 42209 42215 +6
- Misses 28571 28580 +9
Partials 1970 1970 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
daniel-zamora
force-pushed
the
CMR-10686
branch
from
0bffba9 to
8ee4fa0
Compare
daniel-zamora
force-pushed
the
CMR-10686
branch
from
8ee4fa0 to
539e255
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj (1)
26-48: Add test case for AND semantics to improve coverage.The test suite covers OR semantics (line 41-42) but is missing a test case for AND logic. The Codecov report indicates 9 missing lines in
conversion.clj, likely corresponding to the AND branch (lines 479-481).🔎 Suggested AND test case
Add this test case to the
are3block:+ "AND search with multiple values - both must match" + [] ["ACTIVE" "PLANNED"] {"options[collection-progress][and]" "true"} + + "AND search with same value repeated" + [coll1] ["ACTIVE" "ACTIVE"] {"options[collection-progress][and]" "true"}Note: The first case should return empty results since no collection can have both ACTIVE and PLANNED simultaneously. The second case demonstrates AND logic with a value that does exist.
search-app/docs/api.md (2)
62-62: Align TOC indentation with surrounding bulletsThe new
Collection progressentry appears indented differently than neighboring TOC items. Please match its leading spaces to the other collection parameter bullets so markdownlint (MD007) passes and the rendered list remains consistent.
1938-1952: Collection progress docs look correct; update code block style to fencedThe new
collection_progressparameter description and examples are consistent with nearby sections (single value, multi-value “any of”, plusignore_case/patternsupport). To satisfy markdownlint (MD046) for these new lines, consider rewriting the threecurlexamples as fenced code blocks instead of indented ones, e.g.:Proposed fenced code blocks for curl examples
- Find collections matching 'collection_progress' param value - - curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE" + Find collections matching `collection_progress` param value + +```bash +curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE" +``` @@ - Find collections matching any of the 'collection_progress' param values - - curl "%CMR-ENDPOINT%/collections?collection_progress[]=ACTIVE&collection_progress[]=PLANNED" + Find collections matching any of the `collection_progress` param values + +```bash +curl "%CMR-ENDPOINT%/collections?collection_progress[]=ACTIVE&collection_progress[]=PLANNED" +``` @@ - Find collections using pattern matching - - curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true" + Find collections using pattern matching + +```bash +curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options[collection_progress][pattern]=true" +```
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
indexer-app/src/cmr/indexer/data/concepts/collection.cljindexer-app/src/cmr/indexer/data/index_set.cljsearch-app/docs/api.mdsearch-app/src/cmr/search/services/parameters/conversion.cljsearch-app/src/cmr/search/services/parameters/parameter_validation.cljsystem-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
🚧 Files skipped from review as they are similar to previous changes (2)
- search-app/src/cmr/search/services/parameters/parameter_validation.clj
- indexer-app/src/cmr/indexer/data/index_set.clj
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
search-app/docs/api.md
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
1944-1944: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1948-1948: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1952-1952: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (2)
indexer-app/src/cmr/indexer/data/concepts/collection.clj (1)
400-401: LGTM! Implementation follows established patterns.The addition of
:collection-progressand:collection-progress-lowercasefields follows the same pattern used for other searchable fields throughout this function (e.g.,:processing-level-idat lines 391-392,:latencyat lines 393-394). The use ofutil/safe-lowercaseis consistent and handles nil values appropriately.search-app/src/cmr/search/services/parameters/conversion.clj (1)
33-33: LGTM! Parameter mapping correctly added.The
:collection-progress :stringmapping is properly placed in the collection param-mappings and follows the same pattern as other string-based search parameters.
daniel-zamora
force-pushed
the
CMR-10686
branch
from
03d0337 to
1683089
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
indexer-app/src/cmr/indexer/data/concepts/collection.clj (1)
400-401: IndexingCollectionProgresslooks correct and consistentThe new
:collection-progressand:collection-progress-lowercasefields are wired in consistently with other indexed string fields, and usingutil/safe-lowercaseavoids nil issues. No functional problems spotted here.If you ever introduce a “not provided”/sentinel value for
CollectionProgress, consider reusingremove-index-irrelevant-defaultsto strip that from indexing, similar to processing level and related fields.search-app/docs/api.md (1)
1938-1952: Convert newcollection_progresscurl examples to fenced code blocks (MD046)The three new curl examples are using indented code blocks, which
markdownlint(MD046) now flags as a style violation. Switching them to fenced blocks keeps the content the same while satisfying the linter.Proposed fenced code blocks for the new examples
#### <a name="c-collection-progress"></a> Find collections by collection\_progress Supports ignore_case and pattern matching. Find collections matching 'collection\_progress' param value - curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE" +```bash +curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE" +``` Find collections matching any of the 'collection\_progress' param values - curl "%CMR-ENDPOINT%/collections?collection_progress\[\]=ACTIVE&collection_progress\[\]=PLANNED" +```bash +curl "%CMR-ENDPOINT%/collections?collection_progress\[\]=ACTIVE&collection_progress\[\]=PLANNED" +``` Find collections using pattern matching - curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true" +```bash +curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true" +```
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
indexer-app/src/cmr/indexer/data/concepts/collection.cljindexer-app/src/cmr/indexer/data/index_set.cljsearch-app/docs/api.mdsearch-app/src/cmr/search/services/parameters/conversion.cljsearch-app/src/cmr/search/services/parameters/parameter_validation.cljsystem-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
🚧 Files skipped from review as they are similar to previous changes (2)
- indexer-app/src/cmr/indexer/data/index_set.clj
- system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:02.702Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.
📚 Learning: 2025-12-30T13:06:02.702Z
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:02.702Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters do not make logical sense. In code reviews, ensure parameter handlers or validation logic do not treat collection-progress as multi-valued and avoid composing multiple progress values with AND. Treat collection-progress as a single-valued enum and validate inputs accordingly, documenting this constraint where relevant.
Applied to files:
search-app/src/cmr/search/services/parameters/conversion.cljsearch-app/src/cmr/search/services/parameters/parameter_validation.clj
📚 Learning: 2025-12-30T13:06:02.702Z
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:02.702Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.
Applied to files:
indexer-app/src/cmr/indexer/data/concepts/collection.cljsearch-app/docs/api.md
🪛 markdownlint-cli2 (0.18.1)
search-app/docs/api.md
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
1944-1944: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1948-1948: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1952-1952: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (4)
search-app/docs/api.md (1)
62-62: The review comment is based on an incorrect premise. Line 62 (* [Collection progress](#c-collection-progress)) is indented identically to all other ToC entries in the list—there is no indentation inconsistency unique to this line. Markdownlint's MD007 violation is a file-wide issue affecting the entire ToC section (lines 7–26+), not specific to line 62. All entries consistently use 4 spaces, while the local markdownlint config expects 2 spaces at that nesting level.Converting only line 62 to different indentation would break consistency with the surrounding ToC structure. The MD046 violations on lines 1944, 1948, and 1952 (indented code blocks) are real but reflect a pervasive file-wide pattern—the entire file contains 240+ indented code blocks rather than fenced blocks.
If linting violations need to be addressed, they should be tackled file-wide rather than in isolation.
Likely an incorrect or invalid review comment.
search-app/src/cmr/search/services/parameters/parameter_validation.clj (1)
42-42: LGTM! Collection progress parameter validation properly configured.The validation configuration correctly enables the new
collection-progresssearch parameter with multi-value support (OR/AND operations), case-sensitivity control, and pattern matching. This aligns with similar parameters like:collection-data-typeand follows established patterns.Based on learnings, while
collection-progressis a single-valued enum field on collections, the multi-value support here refers to search query capabilities (e.g., searching for "ACTIVE OR PLANNED"), which is the expected behavior.Also applies to: 155-155
search-app/src/cmr/search/services/parameters/conversion.clj (2)
33-33: LGTM! Parameter mapping correctly added.The
:collection-progressparameter is properly mapped to:stringtype in the collection param-mappings, consistent with other string-based parameters.
473-487: LGTM! Collection progress condition conversion correctly implemented.The implementation properly handles both multi-value (OR/AND) and single-value cases with case-sensitivity and pattern matching support. The structure appropriately mirrors the
:collection-data-typehandler for consistency.Based on learnings,
collection-progressis a single-valued enum field, so OR operations (the default for multi-value queries) are the primary use case, while AND operations are supported for consistency with other parameter handlers.
| [context concept-type param value options] | ||
| (if (sequential? value) | ||
| ;; Handle multiple values with AND or OR logic | ||
| (if (= "true" (get-in options [param :and])) |
There was a problem hiding this comment.
I can't remember if the value is normalized to lower case for you at this point. If so, this looks great.
daniel-zamora
force-pushed
the
CMR-10686
branch
from
1683089 to
9c702c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
indexer-app/src/cmr/indexer/data/concepts/collection.cljindexer-app/src/cmr/indexer/data/index_set.cljsearch-app/docs/api.mdsearch-app/src/cmr/search/services/parameters/conversion.cljsearch-app/src/cmr/search/services/parameters/parameter_validation.cljsystem-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
🚧 Files skipped from review as they are similar to previous changes (5)
- indexer-app/src/cmr/indexer/data/index_set.clj
- system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
- indexer-app/src/cmr/indexer/data/concepts/collection.clj
- search-app/src/cmr/search/services/parameters/conversion.clj
- search-app/src/cmr/search/services/parameters/parameter_validation.clj
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:02.702Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.
📚 Learning: 2025-12-30T13:06:02.702Z
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:02.702Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.
Applied to files:
search-app/docs/api.md
🪛 markdownlint-cli2 (0.18.1)
search-app/docs/api.md
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
1944-1944: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1948-1948: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1952-1952: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
| * [Science keywords](#c-science-keywords) | ||
| * [TwoD coordinate system](#c-twod-coordinate-system) | ||
| * [Collection data type](#c-collection-data-type) | ||
| * [Collection progress](#c-collection-progress) |
There was a problem hiding this comment.
Fix markdown list indentation to satisfy markdownlint (MD007).
markdownlint flags this new “Collection progress” TOC item for ul-indent (Expected: 2; Actual: 4). To keep CI green, adjust its indentation to match the configured nesting level (2 spaces here), even though older siblings still use 4-space indents.
Proposed indentation tweak
- * [Collection progress](#c-collection-progress)
+ * [Collection progress](#c-collection-progress)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [Collection progress](#c-collection-progress) | |
| * [Collection progress](#c-collection-progress) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In search-app/docs/api.md around line 62, the TOC entry "* [Collection
progress](#c-collection-progress)" is indented with 4 spaces causing
markdownlint MD007 (ul-indent); change its leading indentation to 2 spaces to
match the configured nesting level so the unordered list uses 2-space indents
for this item (leave the text unchanged).
| #### <a name="c-collection-progress"></a> Find collections by collection\_progress | ||
|
|
||
| Supports ignore_case and pattern matching. | ||
|
|
||
| Find collections matching 'collection\_progress' param value | ||
|
|
||
| curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE" | ||
|
|
||
| Find collections matching any of the 'collection\_progress' param values | ||
|
|
||
| curl "%CMR-ENDPOINT%/collections?collection_progress\[\]=ACTIVE&collection_progress\[\]=PLANNED" | ||
|
|
||
| Find collections using pattern matching | ||
|
|
||
| curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true" |
There was a problem hiding this comment.
Tighten collection_progress docs and switch new examples to fenced code blocks (MD046).
Content-wise this section looks correct and aligned with the implementation; it describes ignore_case, pattern, single and multi-value usage, and shows realistic values. Two small improvements:
- Clarify that each collection has a single
collection_progressvalue, and that multiple parameter values are effectively OR’ed (a collection can match at most one of them). This matches the backend behavior. Based on learnings, this helps set user expectations. markdownlint(MD046) wants fenced blocks for the new curl examples. Convert the three indented curl lines to fenced code blocks.
Proposed doc refinements and fenced code blocks
#### <a name="c-collection-progress"></a> Find collections by collection\_progress
-Supports ignore_case and pattern matching.
+Each collection has a single collection progress value, and this parameter supports `ignore_case` and pattern matching. When multiple
+`collection_progress` values are provided, collections matching any of the values are returned.
Find collections matching 'collection\_progress' param value
- curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE"
+```bash
+curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE"
+```
Find collections matching any of the 'collection\_progress' param values
- curl "%CMR-ENDPOINT%/collections?collection_progress\[\]=ACTIVE&collection_progress\[\]=PLANNED"
+```bash
+curl "%CMR-ENDPOINT%/collections?collection_progress[]=ACTIVE&collection_progress[]=PLANNED"
+```
Find collections using pattern matching
- curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true"
+```bash
+curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options[collection_progress][pattern]=true"
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1944-1944: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1948-1948: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1952-1952: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🤖 Prompt for AI Agents
In search-app/docs/api.md around lines 1938 to 1952, tighten the
collection_progress section by (1) adding a short clarification that each
collection has a single collection_progress value and that supplying multiple
collection_progress parameters is treated as an OR across values (a collection
can match at most one of them), and (2) replace the three indented curl example
lines with proper fenced code blocks (```bash ... ```) and un-escape square
brackets in query params (use collection_progress[]=... and
options[collection_progress][pattern]=true) so they satisfy MD046.
daniel-zamora
force-pushed
the
CMR-10686
branch
from
9c702c9 to
f95ad0d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @search-app/docs/api.md:
- Around line 1938-1953: Update the three curl examples under the "Find
collections by collection_progress" (anchor c-collection-progress) to use fenced
bash code blocks instead of indented blocks and normalize the query parameter
shape to the bracketed array form (collection_progress[]=...) even for single
values; also unescape square brackets in the options parameter so it reads
options[collection_progress][pattern]=true. Ensure each example is wrapped in a
```bash fenced block and the multi-value example uses
collection_progress[]=ACTIVE&collection_progress[]=PLANNED and the pattern
example uses
collection_progress[]=*RE*&options[collection_progress][pattern]=true.
- Around line 62-63: Fix the inconsistent list indentation in the table of
contents: align the "* [Collection progress](#c-collection-progress)" entry with
the other top-level TOC items so it uses the same left margin (remove the extra
leading space), ensuring all siblings like "* [Granule data
format](#c-granule-data-format)" share identical indentation to satisfy
markdownlint MD007.
🧹 Nitpick comments (1)
system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj (1)
26-47: Test cases provide solid coverage of the search parameter functionality.The
are3parameterized tests cover key scenarios:
- Single value exact match
- Case-insensitive matching (default behavior)
- Pattern/wildcard matching (
*RE*correctly matches DEPRECATED, PREPRINT, INREVIEW)- OR search with multiple values
- Case-sensitive matching via
ignore-case=false- Invalid value handling
Consider adding a test for values containing spaces (e.g., "NOT PROVIDED") to verify proper handling.
💡 Optional: Add test for space-containing value
"invalid value returns empty" - [] "INVALID" nil)))) + [] "INVALID" nil + + "value with space" + [coll5] "NOT PROVIDED" nil))))
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
indexer-app/src/cmr/indexer/data/concepts/collection.cljindexer-app/src/cmr/indexer/data/index_set.cljsearch-app/docs/api.mdsearch-app/src/cmr/search/services/parameters/conversion.cljsearch-app/src/cmr/search/services/parameters/parameter_validation.cljsystem-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj
🚧 Files skipped from review as they are similar to previous changes (2)
- indexer-app/src/cmr/indexer/data/index_set.clj
- indexer-app/src/cmr/indexer/data/concepts/collection.clj
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:11.311Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.
📚 Learning: 2025-12-30T13:06:02.702Z
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:02.702Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters do not make logical sense. In code reviews, ensure parameter handlers or validation logic do not treat collection-progress as multi-valued and avoid composing multiple progress values with AND. Treat collection-progress as a single-valued enum and validate inputs accordingly, documenting this constraint where relevant.
Applied to files:
search-app/src/cmr/search/services/parameters/parameter_validation.cljsearch-app/src/cmr/search/services/parameters/conversion.clj
📚 Learning: 2025-12-30T13:06:11.311Z
Learnt from: daniel-zamora
Repo: nasa/Common-Metadata-Repository PR: 2361
File: search-app/src/cmr/search/services/parameters/conversion.clj:473-487
Timestamp: 2025-12-30T13:06:11.311Z
Learning: The collection-progress field in CMR collections is a single-valued enum. A collection can only have one progress value at a time, so AND operations on collection-progress parameters don't make logical sense even though the implementation supports them for consistency with other parameter handlers.
Applied to files:
system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.cljsearch-app/docs/api.md
🪛 markdownlint-cli2 (0.18.1)
search-app/docs/api.md
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
1944-1944: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1948-1948: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1952-1952: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🔇 Additional comments (6)
search-app/src/cmr/search/services/parameters/parameter_validation.clj (2)
41-47: LGTM! Parameter configuration for collection-progress is correctly added.Adding
:collection-progressto the:multiple-valueset appropriately enables OR-ing multiple progress values in search queries (e.g., find collections with ACTIVE or PLANNED status).
154-156: Parameter options configured correctly.Using
cpv/string-plus-and-optionsprovides ignore-case, pattern, and AND options consistent with similar parameters. Based on learnings, note that AND operations on collection-progress don't make logical sense since it's a single-valued enum field (a collection has exactly one progress value), but supporting it maintains consistency with other parameter handlers.system-int-test/test/cmr/system_int_test/search/collection/collection_progress_search_test.clj (2)
1-10: LGTM! Proper namespace and imports.The test namespace follows CMR conventions with appropriate imports for integration testing.
14-24: Good test setup covering all CollectionProgress enum values.Creating collections with all 8 valid CollectionProgress values (ACTIVE, PLANNED, COMPLETE, DEPRECATED, NOT PROVIDED, PREPRINT, INREVIEW, SUPERSEDED) provides comprehensive fixture data for search testing.
search-app/src/cmr/search/services/parameters/conversion.clj (2)
33-33: LGTM! Parameter mapping added correctly.Mapping
:collection-progressto:stringtype is appropriate for an enum-valued field and consistent with similar parameters.
473-488: Verify the:collection-progressfield name exists in the Elasticsearch index mapping.The
parameter->conditionimplementation for:collection-progresscorrectly handles sequential and single values with configurable AND/OR logic. However, per the collection-progress field semantics (single-valued enum), AND operations don't make logical sense even though the implementation supports them for consistency with other parameter handlers.Add a brief code comment noting this constraint:
;; Note: AND operations don't apply logically to collection-progress (single-valued enum field) ;; but are supported here for implementation consistency across parameter handlers.Field mapping verification could not be completed due to repository access limitations.
| * [Collection progress](#c-collection-progress) | ||
| * [Granule data format](#c-granule-data-format) |
There was a problem hiding this comment.
Fix TOC list indentation to satisfy markdownlint (MD007).
Line 62 is indented more than peer entries and is flagged by markdownlint.
Proposed fix
- * [Collection progress](#c-collection-progress)
+ * [Collection progress](#c-collection-progress)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [Collection progress](#c-collection-progress) | |
| * [Granule data format](#c-granule-data-format) | |
| * [Collection progress](#c-collection-progress) | |
| * [Granule data format](#c-granule-data-format) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
62-62: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
63-63: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In @search-app/docs/api.md around lines 62 - 63, Fix the inconsistent list
indentation in the table of contents: align the "* [Collection
progress](#c-collection-progress)" entry with the other top-level TOC items so
it uses the same left margin (remove the extra leading space), ensuring all
siblings like "* [Granule data format](#c-granule-data-format)" share identical
indentation to satisfy markdownlint MD007.
| #### <a name="c-collection-progress"></a> Find collections by collection\_progress | ||
|
|
||
| Supports ignore_case and pattern matching. | ||
|
|
||
| Find collections matching 'collection\_progress' param value | ||
|
|
||
| curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE" | ||
|
|
||
| Find collections matching any of the 'collection\_progress' param values | ||
|
|
||
| curl "%CMR-ENDPOINT%/collections?collection_progress\[\]=ACTIVE&collection_progress\[\]=PLANNED" | ||
|
|
||
| Find collections using pattern matching | ||
|
|
||
| curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true" | ||
|
|
There was a problem hiding this comment.
Use fenced code blocks (MD046) and align example parameter shape with existing param[]=... style.
The new curl examples are indented code blocks (flagged) and the single-value example differs from surrounding sections that typically use []=... even for one value.
Proposed fix
#### <a name="c-collection-progress"></a> Find collections by collection\_progress
Supports ignore_case and pattern matching.
Find collections matching 'collection\_progress' param value
- curl "%CMR-ENDPOINT%/collections?collection_progress=ACTIVE"
+```bash
+curl "%CMR-ENDPOINT%/collections?collection_progress[]=ACTIVE"
+```
Find collections matching any of the 'collection\_progress' param values
- curl "%CMR-ENDPOINT%/collections?collection_progress\[\]=ACTIVE&collection_progress\[\]=PLANNED"
+```bash
+curl "%CMR-ENDPOINT%/collections?collection_progress[]=ACTIVE&collection_progress[]=PLANNED"
+```
Find collections using pattern matching
- curl "%CMR-ENDPOINT%/collections?collection_progress=*RE*&options\[collection_progress\]\[pattern\]=true"
+```bash
+curl "%CMR-ENDPOINT%/collections?collection_progress[]=*RE*&options[collection_progress][pattern]=true"
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1944-1944: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1948-1948: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
1952-1952: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
🤖 Prompt for AI Agents
In @search-app/docs/api.md around lines 1938 - 1953, Update the three curl
examples under the "Find collections by collection_progress" (anchor
c-collection-progress) to use fenced bash code blocks instead of indented blocks
and normalize the query parameter shape to the bracketed array form
(collection_progress[]=...) even for single values; also unescape square
brackets in the options parameter so it reads
options[collection_progress][pattern]=true. Ensure each example is wrapped in a
```bash fenced block and the multi-value example uses
collection_progress[]=ACTIVE&collection_progress[]=PLANNED and the pattern
example uses
collection_progress[]=*RE*&options[collection_progress][pattern]=true.
Overview
What is the objective?
This PR implements the functionality to index and search collections by their collection progress value (ticket CMR-10686). The objective is to allow users to filter collection searches based on collection progress status, with support for OR-ing multiple values in a single search request.
What are the changes?
What areas of the application does this impact?
Required Checklist
Additional Checklist
Summary by CodeRabbit
New Features
Indexing
Validation
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.