Skip to content

Commit

Permalink
[Issue #3464] Better agency display/sort in search (#3738)
Browse files Browse the repository at this point in the history
## Summary
Fixes #3464 

### Time to review: __10 mins__

## Changes proposed
Added support for the API change that takes an array of sort criteria
instead of individual arguments for filed and direction

---------

Co-authored-by: nava-platform-bot <[email protected]>
  • Loading branch information
mdragon and nava-platform-bot authored Feb 11, 2025
1 parent dc3c24b commit 003c64c
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 39 deletions.
2 changes: 2 additions & 0 deletions api/openapi.generated.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1522,6 +1522,8 @@ components:
- post_date
- close_date
- agency_code
- agency_name
- top_level_agency_name
description: The field to sort the response by
sort_direction:
description: Whether to sort the response ascending or descending
Expand Down
2 changes: 2 additions & 0 deletions api/src/api/opportunities_v1/opportunity_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ class OpportunitySearchRequestV1Schema(Schema):
"post_date",
"close_date",
"agency_code",
"agency_name",
"top_level_agency_name",
],
default_sort_order=[{"order_by": "opportunity_id", "sort_direction": "descending"}],
),
Expand Down
2 changes: 1 addition & 1 deletion api/src/db/models/agency_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Agency(ApiSchemaTable, TimestampMixin):
ForeignKey(agency_id),
nullable=True,
)
top_level_agency: Mapped["Agency"] = relationship(
top_level_agency: Mapped["Agency | None"] = relationship(
lambda: Agency,
remote_side=[agency_id],
)
Expand Down
2 changes: 1 addition & 1 deletion api/src/db/models/opportunity_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def top_level_agency_name(self) -> str | None:
if self.agency_record is not None and self.agency_record.top_level_agency is not None:
return self.agency_record.top_level_agency.agency_name

return None
return self.agency_name

@property
def agency_name(self) -> str | None:
Expand Down
2 changes: 2 additions & 0 deletions api/src/services/opportunities_v1/search_opportunities.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
"close_date": "summary.close_date",
"agency_code": "agency_code.keyword",
"agency": "agency_code.keyword",
"agency_name": "agency_name.keyword",
"top_level_agency_name": "top_level_agency_name.keyword",
"opportunity_status": "opportunity_status.keyword",
"funding_instrument": "summary.funding_instruments.keyword",
"funding_category": "summary.funding_categories.keyword",
Expand Down
20 changes: 12 additions & 8 deletions frontend/src/components/search/SearchResultsListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,21 @@ export default function SearchResultsListItem({
: "--"}
</span>
</div>
<div className="grid-col tablet:order-3 overflow-hidden font-body-xs">
<div className="grid-col tablet:order-2 overflow-hidden font-body-xs">
<span className={metadataBorderClasses}>
<strong>{t("resultsListItem.summary.agency")}</strong>
{opportunity?.agency ||
(opportunity?.summary?.agency_name &&
opportunity?.summary?.agency_code &&
agencyNameLookup
? // Use same exact label we're using for the agency filter list
agencyNameLookup[opportunity?.summary?.agency_code]
: "--")}
{opportunity?.top_level_agency_name &&
opportunity?.agency_name &&
opportunity?.top_level_agency_name !== opportunity?.agency_name
? `${opportunity?.top_level_agency_name} - ${opportunity?.agency_name}`
: opportunity?.agency_name ||
(agencyNameLookup && opportunity?.summary?.agency_code
? // Use same exact label we're using for the agency filter list
agencyNameLookup[opportunity?.summary?.agency_code]
: "--")}
</span>
</div>
<div className="grid-col tablet:order-3 overflow-hidden font-body-xs">
<span className={metadataBorderClasses}>
<strong>{t("resultsListItem.opportunity_number")}</strong>
{opportunity?.opportunity_number}
Expand Down
45 changes: 30 additions & 15 deletions frontend/src/services/fetch/fetchers/searchFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { fetchOpportunitySearch } from "src/services/fetch/fetchers/fetchers";
import {
PaginationOrderBy,
PaginationRequestBody,
PaginationSortOrder,
QueryParamData,
SearchFetcherActionType,
SearchFilterRequestBody,
Expand All @@ -12,12 +13,12 @@ import {
import { SearchAPIResponse } from "src/types/search/searchResponseTypes";

const orderByFieldLookup = {
relevancy: "relevancy",
opportunityNumber: "opportunity_number",
opportunityTitle: "opportunity_title",
agency: "agency_code",
postedDate: "post_date",
closeDate: "close_date",
relevancy: ["relevancy"],
opportunityNumber: ["opportunity_number"],
opportunityTitle: ["opportunity_title"],
agency: ["top_level_agency_name", "agency_name"],
postedDate: ["post_date"],
closeDate: ["close_date"],
};

type FrontendFilterNames =
Expand Down Expand Up @@ -134,24 +135,38 @@ export const buildPagination = (
? 1
: page;

let order_by: PaginationOrderBy = "relevancy";
let sort_order: PaginationSortOrder = [
{ order_by: "relevancy", sort_direction: "descending" },
{ order_by: "post_date", sort_direction: "descending" },
];

if (sortby) {
sort_order = [];
// this will need to change in a future where we allow the user to set an ordered set of sort columns.
// for now we're just using the multiple internally behind a single column picker drop down so this is fine.
for (const [key, value] of Object.entries(orderByFieldLookup)) {
if (sortby.startsWith(key)) {
order_by = value as PaginationOrderBy;
break; // Stop searching after the first match is found
const sort_direction =
sortby && sortby.endsWith("Asc") ? "ascending" : "descending";
value.forEach((item) => {
sort_order.push({
order_by: <PaginationOrderBy>item,
sort_direction,
});
if (item === "relevancy") {
sort_order.push({
order_by: "post_date",
sort_direction,
});
}
});
}
}
}

// sort relevancy descending without suffix
const sort_direction =
sortby && sortby.endsWith("Asc") ? "ascending" : "descending";

return {
order_by,
page_offset,
page_size: 25,
sort_direction,
sort_order,
};
};
10 changes: 7 additions & 3 deletions frontend/src/types/search/searchRequestTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,19 @@ export type PaginationOrderBy =
| "opportunity_id"
| "opportunity_number"
| "opportunity_title"
| "agency_code"
| "agency_name"
| "top_level_agency_code"
| "post_date"
| "close_date";
export type PaginationSortDirection = "ascending" | "descending";
export interface PaginationRequestBody {
export type PaginationSortOrder = {
order_by: PaginationOrderBy;
sort_direction: PaginationSortDirection;
}[];
export interface PaginationRequestBody {
page_offset: number;
page_size: number;
sort_direction: PaginationSortDirection;
sort_order: PaginationSortOrder;
}

export type SearchRequestBody = {
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/types/search/searchResponseTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export interface Summary {

export interface Opportunity {
agency: string | null;
agency_name: string | null;
category: string | null;
category_explanation: string | null;
created_at: string;
Expand All @@ -52,6 +53,7 @@ export interface Opportunity {
opportunity_status: OpportunityStatus;
opportunity_title: string;
summary: Summary;
top_level_agency_name: string | null;
updated_at: string;
}

Expand Down
30 changes: 19 additions & 11 deletions frontend/tests/services/fetch/fetchers/SearchFetcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,14 @@ describe("searchForOpportunities", () => {
expect(mockfetchOpportunitySearch).toHaveBeenCalledWith({
body: {
pagination: {
order_by: "opportunity_number", // This should be the actual value being used in the API method
sort_order: [
{
order_by: "opportunity_number",
sort_direction: "ascending",
},
], // This should be the actual value being used in the API method
page_offset: 1,
page_size: 25,
sort_direction: "ascending", // or "descending" based on your sortby parameter
},
query: "research",
filters: {
Expand Down Expand Up @@ -86,10 +90,14 @@ describe("downloadOpportunities", () => {
expect(mockfetchOpportunitySearch).toHaveBeenCalledWith({
body: {
pagination: {
order_by: "opportunity_number", // This should be the actual value being used in the API method
sort_order: [
{
order_by: "opportunity_number", // This should be the actual value being used in the API method
sort_direction: "ascending", // or "descending" based on your sortby parameter
},
],
page_offset: 1,
page_size: 5000,
sort_direction: "ascending", // or "descending" based on your sortby parameter
},
query: "research",
filters: {
Expand Down Expand Up @@ -157,9 +165,9 @@ describe("buildPagination", () => {
...{ page: 5, sortby: null },
});

expect(pagination.order_by).toEqual("relevancy");
expect(pagination.sort_order[0].order_by).toEqual("relevancy");
expect(pagination.page_offset).toEqual(5);
expect(pagination.sort_direction).toEqual("descending");
expect(pagination.sort_order[0].sort_direction).toEqual("descending");
});

it("builds correct offset based on action type and field changed", () => {
Expand Down Expand Up @@ -206,14 +214,14 @@ describe("buildPagination", () => {
...{ sortby: "closeDateAsc" },
});

expect(pagination.order_by).toEqual("close_date");
expect(pagination.sort_order[0].order_by).toEqual("close_date");

const secondPagination = buildPagination({
...searchProps,
...{ sortby: "postedDateAsc" },
});

expect(secondPagination.order_by).toEqual("post_date");
expect(secondPagination.sort_order[0].order_by).toEqual("post_date");
});

it("builds correct sort_direction based on sortby", () => {
Expand All @@ -222,20 +230,20 @@ describe("buildPagination", () => {
...{ sortby: "opportunityNumberDesc" },
});

expect(pagination.sort_direction).toEqual("descending");
expect(pagination.sort_order[0].sort_direction).toEqual("descending");

const secondPagination = buildPagination({
...searchProps,
...{ sortby: "postedDateAsc" },
});

expect(secondPagination.sort_direction).toEqual("ascending");
expect(secondPagination.sort_order[0].sort_direction).toEqual("ascending");

const thirdPagination = buildPagination({
...searchProps,
...{ sortby: null },
});

expect(thirdPagination.sort_direction).toEqual("descending");
expect(thirdPagination.sort_order[0].sort_direction).toEqual("descending");
});
});

0 comments on commit 003c64c

Please sign in to comment.