From 5d86a25c83fca4bcef185fb10cfaa0ef7ebb9d2a Mon Sep 17 00:00:00 2001 From: Ed Olivares <34591886+eudoroolivares2016@users.noreply.github.com> Date: Wed, 6 Nov 2024 20:46:40 -0500 Subject: [PATCH] CMR-10132: Fix stac-browser Stac item and collection search unmapped CMR sort keys (#370) * CMR-10232: Fixing sort errors on radian-earth; passing searchType field and normalizing datetime param to cmr startDate field * CMR-10232: Fix prettier and add pull request template * CMR-10232: Update README --- .github/pull_request_template.md | 37 +++++++++++++++++++++++++++++ README.md | 5 +++- src/domains/__tests__/stac.spec.ts | 37 +++++++++++++++++++++-------- src/domains/stac.ts | 29 ++++++++++++++-------- src/middleware/index.ts | 2 +- src/routes/__tests__/browse.spec.ts | 2 +- src/routes/browse.ts | 2 +- src/routes/items.ts | 4 ++-- src/routes/search.ts | 1 + src/utils/__tests__/sort.spec.ts | 18 +++++++++++++- src/utils/sort.ts | 10 ++++++++ 11 files changed, 120 insertions(+), 27 deletions(-) create mode 100644 .github/pull_request_template.md diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 00000000..0fe37081 --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,37 @@ +# Overview + +### What is the feature? + +Please summarize the feature or fix. + +### What is the Solution? + +Summarize what you changed. + +### What areas of the application does this impact? + +List impacted areas. + +# Testing + +### Reproduction steps + +- **Environment for testing:** +- **Collection to test with:** + +1. Step 1 +2. Step 2... + +### Attachments + +Please include relevant screenshots or files that would be helpful in reviewing and verifying this change. + +# Checklist + +- [ ] I have added automated tests that prove my fix is effective or that my feature works +- [ ] New and existing unit tests pass locally with my changes +- [ ] I have performed a self-review of my own code +- [ ] I have commented my code, particularly in hard-to-understand areas +- [ ] I have made corresponding changes to the documentation +- [ ] My changes generate no new warnings + diff --git a/README.md b/README.md index 0112f857..9ba8a92c 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,10 @@ CMR-STAC follows the STAC API 1.0.0-beta.1 specification, see the ### Navigating CMR-STAC can be navigated manually using the endpoints provided above, or you can utilize available STAC software to browse and use the API. - + +A common STAC utility is Radiant Earth's `stac-browser` to use this tool against your development server navigate to + ```radiantearth.github.io/stac-browser/#/external/http:/localhost:3000/stac?.language=en``` + See the [Usage Documentation](docs/usage/usage.md) for examples of how to interact with the API and search for data. ### Limitations diff --git a/src/domains/__tests__/stac.spec.ts b/src/domains/__tests__/stac.spec.ts index 6ab5cf63..03f286f1 100644 --- a/src/domains/__tests__/stac.spec.ts +++ b/src/domains/__tests__/stac.spec.ts @@ -417,6 +417,8 @@ describe("sortByToSortKeys", () => { { input: "id", output: ["entryId"] }, { input: "-id", output: ["-entryId"] }, { input: "title", output: ["entryTitle"] }, + { input: "datetime", output: ["startDate"] }, + { input: "-datetime", output: ["-startDate"] }, { input: "-title", output: ["-entryTitle"] }, { input: "someOtherField", output: ["someOtherField"] }, { input: "-someOtherField", output: ["-someOtherField"] }, @@ -431,19 +433,34 @@ describe("sortByToSortKeys", () => { ].forEach(({ input, output }) => { describe(`given sortby=${input}`, () => { it("should return the corresponding sortKey", () => { - expect(sortByToSortKeys(input)).to.deep.equal(output); - }); - - it("should handle object-based sort specifications", () => { - const input: SortObject[] = [ - { field: "properties.eo:cloud_cover", direction: "desc" }, - { field: "id", direction: "asc" }, - { field: "title", direction: "desc" }, - ]; - expect(sortByToSortKeys(input)).to.deep.equal(["-cloudCover", "entryId", "-entryTitle"]); + expect(sortByToSortKeys(input, "collection")).to.deep.equal(output); }); }); }); + + it("should handle object-based sort specifications", () => { + const input: SortObject[] = [ + { field: "properties.eo:cloud_cover", direction: "desc" }, + { field: "id", direction: "asc" }, + { field: "title", direction: "desc" }, + ]; + expect(sortByToSortKeys(input, "collection")).to.deep.equal([ + "-cloudCover", + "entryId", + "-entryTitle", + ]); + }); + + it("should handle item searching differences", () => { + const input: SortObject[] = [ + { field: "id", direction: "asc" }, + { field: "id", direction: "desc" }, + ]; + expect(sortByToSortKeys(input, "item")).to.deep.equal([ + "readableGranuleName", + "-readableGranuleName", + ]); + }); }); describe("stringifyQuery", () => { diff --git a/src/domains/stac.ts b/src/domains/stac.ts index 426baeb7..d2e5823c 100644 --- a/src/domains/stac.ts +++ b/src/domains/stac.ts @@ -33,7 +33,7 @@ import { dateTimeToRange } from "../utils/datetime"; import { AssetLinks } from "../@types/StacCollection"; import { Collection, Granule, RelatedUrlType } from "../models/GraphQLModels"; -import { parseSortFields } from "../utils/sort"; +import { parseSortFields, mapIdSortKey } from "../utils/sort"; const CMR_ROOT = process.env.CMR_URL; @@ -340,7 +340,10 @@ const bboxQuery = (_req: Request, query: StacQuery) => ({ /** * Returns a list of sortKeys from the sortBy property */ -export const sortByToSortKeys = (sortBys?: string | SortObject[] | string[]): string[] => { +export const sortByToSortKeys = ( + sortBys?: string | SortObject[] | string[], + searchType = "" +): string[] => { const baseSortKeys: string[] = parseSortFields(sortBys); return baseSortKeys.reduce((sortKeys, sortBy) => { @@ -350,15 +353,16 @@ export const sortByToSortKeys = (sortBys?: string | SortObject[] | string[]): st const cleanSortBy = isDescending ? sortBy.slice(1) : sortBy; // Allow for `properties` prefix const fieldName = cleanSortBy.replace(/^properties\./, ""); - let mappedField; - if (fieldName.match(/^eo:cloud_cover$/i)) { mappedField = "cloudCover"; } else if (fieldName.match(/^id$/i)) { - mappedField = "entryId"; + mappedField = mapIdSortKey(searchType); } else if (fieldName.match(/^title$/i)) { mappedField = "entryTitle"; + } else if (fieldName.match(/^datetime$/i)) { + // If descending `-start_date` will sort by newest first + mappedField = "startDate"; } else { mappedField = fieldName; } @@ -367,10 +371,16 @@ export const sortByToSortKeys = (sortBys?: string | SortObject[] | string[]): st }, [] as string[]); }; -const sortKeyQuery = (_req: Request, query: StacQuery) => ({ - // Use the sortByToSortKeys function to convert STAC sortby to CMR sortKey - sortKey: sortByToSortKeys(query.sortby), -}); +const sortKeyQuery = (req: Request, query: StacQuery) => { + const { + params: { searchType }, + } = req; + + return { + // Use the sortByToSortKeys function to convert STAC sortby to CMR sortKey + sortKey: sortByToSortKeys(query.sortby, searchType), + }; +}; const idsQuery = (req: Request, query: StacQuery) => { const { @@ -494,7 +504,6 @@ export const buildQuery = async (req: Request) => { const { params: { providerId: provider }, } = req; - const query = mergeMaybe(req.query, req.body); const queryBuilders = [ diff --git a/src/middleware/index.ts b/src/middleware/index.ts index dc3f815f..760d9626 100644 --- a/src/middleware/index.ts +++ b/src/middleware/index.ts @@ -227,7 +227,7 @@ const validFreeText = (freeText: string) => { return false; }; -const VALID_SORT_FIELDS = ["startDate", "endDate", "id", "title", "eo:cloud_cover"]; +const VALID_SORT_FIELDS = ["startDate", "endDate", "id", "title", "eo:cloud_cover", "datetime"]; const validSortBy = (sortBy: string | string[] | SortObject[]) => { const fields: string[] = parseSortFields(sortBy); diff --git a/src/routes/__tests__/browse.spec.ts b/src/routes/__tests__/browse.spec.ts index e4958349..c0b22888 100644 --- a/src/routes/__tests__/browse.spec.ts +++ b/src/routes/__tests__/browse.spec.ts @@ -63,7 +63,7 @@ describe("addItemLinkIfPresent", () => { const numberoOfLinks = stacCollection.links.length; // Invoke method addItemLinkIfPresent(stacCollection, "https://foo.com/items"); - // Observe no addiitonal link in the STAC Collection and that the item link remains a CMR link + // Observe no additional link in the STAC Collection and that the item link remains a CMR link expect(stacCollection.links.length).to.equal(numberoOfLinks); expect(stacCollection).to.have.deep.property("links", [ { diff --git a/src/routes/browse.ts b/src/routes/browse.ts index 93acc72c..bf86c8a9 100644 --- a/src/routes/browse.ts +++ b/src/routes/browse.ts @@ -50,7 +50,7 @@ const collectionLinks = (req: Request, nextCursor?: string | null): Links => { export const collectionsHandler = async (req: Request, res: Response): Promise => { const { headers } = req; - + req.params.searchType = "collection"; const query = await buildQuery(req); const { cursor, items: collections } = await getCollections(query, { diff --git a/src/routes/items.ts b/src/routes/items.ts index e51beece..4c423fae 100644 --- a/src/routes/items.ts +++ b/src/routes/items.ts @@ -40,8 +40,8 @@ export const singleItemHandler = async (req: Request, res: Response) => { params: { collectionId, itemId }, } = req; + req.params.searchType = "item"; const itemQuery = await buildQuery(req); - const { items: [item], } = await getItems(itemQuery, { headers }); @@ -74,8 +74,8 @@ export const multiItemHandler = async (req: Request, res: Response) => { ); } + req.params.searchType = "item"; const itemQuery = await buildQuery(req); - const links = generateLinks(req); const { diff --git a/src/routes/search.ts b/src/routes/search.ts index bce211f3..aa80a8e4 100644 --- a/src/routes/search.ts +++ b/src/routes/search.ts @@ -63,6 +63,7 @@ const searchLinks = (req: Request, nextCursor: string | null): Link[] => { export const searchHandler = async (req: Request, res: Response): Promise => { const { headers } = req; + req.params.searchType = "item"; const gqlQuery = await buildQuery(req); const itemsResponse = await getItems(gqlQuery, { headers }); diff --git a/src/utils/__tests__/sort.spec.ts b/src/utils/__tests__/sort.spec.ts index 9aaf2945..e376da2b 100644 --- a/src/utils/__tests__/sort.spec.ts +++ b/src/utils/__tests__/sort.spec.ts @@ -1,5 +1,5 @@ import { expect } from "chai"; -import { parseSortFields } from "../sort"; +import { parseSortFields, mapIdSortKey } from "../sort"; import { SortObject } from "../../models/StacModels"; describe("parseSortFields", () => { @@ -46,3 +46,19 @@ describe("parseSortFields", () => { expect(parseSortFields(input)).to.deep.equal(["field1", "", "-field3"]); }); }); + +describe("mapIdSortKey", () => { + it("should return a valid cmr sort value based on searchType", () => { + const collectionMappedKey = mapIdSortKey("collection"); + expect(collectionMappedKey).to.equal("entryId"); + + const itemMappedKey = mapIdSortKey("item"); + expect(itemMappedKey).to.equal("readableGranuleName"); + + const unmappedKey = mapIdSortKey("anything"); + expect(unmappedKey).to.equal(""); + + const emptyKey = mapIdSortKey(); + expect(emptyKey).to.equal(""); + }); +}); diff --git a/src/utils/sort.ts b/src/utils/sort.ts index 1e2d7b51..ba075858 100644 --- a/src/utils/sort.ts +++ b/src/utils/sort.ts @@ -40,3 +40,13 @@ export const parseSortFields = (sortBys?: string | string[] | SortObject[]): str return []; }; + +export const mapIdSortKey = (searchType = ""): string => { + if (searchType === "collection") { + return "entryId"; + } else if (searchType === "item") { + return "readableGranuleName"; + } + + return ""; +};