-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Issue 2448] use v1 search endpoint and frontend fetch pattern refactors #2518
Changes from all commits
2beea48
8f47112
fcf4c3d
f132180
12c291d
e7d65f9
642e75d
7a817f5
985f054
9b0f0dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,32 +15,26 @@ import { | |
ValidationError, | ||
} from "src/errors"; | ||
import { QueryParamData } from "src/services/search/searchfetcher/SearchFetcher"; | ||
// TODO (#1682): replace search specific references (since this is a generic API file that any | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe I've accomplished this now, though the ticket mentioned here was already closed. |
||
// future page or different namespace could use) | ||
import { APIResponse } from "src/types/apiResponseTypes"; | ||
|
||
export type ApiMethod = "DELETE" | "GET" | "PATCH" | "POST" | "PUT"; | ||
export interface JSONRequestBody { | ||
[key: string]: unknown; | ||
} | ||
|
||
interface APIResponseError { | ||
field: string; | ||
message: string; | ||
type: string; | ||
} | ||
|
||
export interface HeadersDict { | ||
[header: string]: string; | ||
} | ||
|
||
export default abstract class BaseApi { | ||
// Root path of API resource without leading slash. | ||
abstract get basePath(): string; | ||
// Root path of API resource without leading slash, can be overridden by implementing API classes as necessary | ||
get basePath() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since we are using the same basePath everywhere, I figured we may as well define it at a higher level |
||
return environment.API_URL; | ||
} | ||
|
||
// API version | ||
// API version, can be overridden by implementing API classes as necessary | ||
get version() { | ||
return "v0.1"; | ||
return "v1"; | ||
} | ||
|
||
// Namespace representing the API resource | ||
|
@@ -54,29 +48,28 @@ export default abstract class BaseApi { | |
if (environment.API_AUTH_TOKEN) { | ||
headers["X-AUTH"] = environment.API_AUTH_TOKEN; | ||
} | ||
headers["Content-Type"] = "application/json"; | ||
return headers; | ||
} | ||
|
||
/** | ||
* Send an API request. | ||
*/ | ||
async request( | ||
async request<ResponseType extends APIResponse>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use a generic to get around having to hardcode search specific typing |
||
method: ApiMethod, | ||
basePath: string, | ||
namespace: string, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can reference these as class variables, no need to pass in |
||
subPath: string, | ||
queryParamData?: QueryParamData, | ||
body?: JSONRequestBody, | ||
options: { | ||
additionalHeaders?: HeadersDict; | ||
} = {}, | ||
) { | ||
): Promise<ResponseType> { | ||
const { additionalHeaders = {} } = options; | ||
const url = createRequestUrl( | ||
method, | ||
basePath, | ||
this.basePath, | ||
this.version, | ||
namespace, | ||
this.namespace, | ||
subPath, | ||
body, | ||
); | ||
|
@@ -85,8 +78,7 @@ export default abstract class BaseApi { | |
...this.headers, | ||
}; | ||
|
||
headers["Content-Type"] = "application/json"; | ||
const response = await this.sendRequest( | ||
const response = await this.sendRequest<ResponseType>( | ||
url, | ||
{ | ||
body: method === "GET" || !body ? null : createRequestBody(body), | ||
|
@@ -102,16 +94,16 @@ export default abstract class BaseApi { | |
/** | ||
* Send a request and handle the response | ||
*/ | ||
private async sendRequest( | ||
private async sendRequest<ResponseType extends APIResponse>( | ||
url: string, | ||
fetchOptions: RequestInit, | ||
queryParamData?: QueryParamData, | ||
) { | ||
let response: Response; | ||
let responseBody: APIResponse; | ||
): Promise<ResponseType> { | ||
let response; | ||
let responseBody; | ||
try { | ||
response = await fetch(url, fetchOptions); | ||
responseBody = (await response.json()) as APIResponse; | ||
responseBody = (await response.json()) as ResponseType; | ||
} catch (error) { | ||
// API most likely down, but also possibly an error setting up or sending a request | ||
// or parsing the response. | ||
|
@@ -121,16 +113,7 @@ export default abstract class BaseApi { | |
handleNotOkResponse(responseBody, url, queryParamData); | ||
} | ||
|
||
const { data, message, pagination_info, status_code, warnings } = | ||
responseBody; | ||
|
||
return { | ||
data, | ||
message, | ||
pagination_info, | ||
status_code, | ||
warnings, | ||
}; | ||
return responseBody; | ||
} | ||
} | ||
|
||
|
@@ -149,14 +132,14 @@ export function createRequestUrl( | |
let url = [...cleanedPaths].join("/"); | ||
if (method === "GET" && body && !(body instanceof FormData)) { | ||
// Append query string to URL | ||
const body: { [key: string]: string } = {}; | ||
const newBody: { [key: string]: string } = {}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was completely broken before, but was unused so nobody noticed |
||
Object.entries(body).forEach(([key, value]) => { | ||
const stringValue = | ||
typeof value === "string" ? value : JSON.stringify(value); | ||
body[key] = stringValue; | ||
newBody[key] = stringValue; | ||
}); | ||
|
||
const params = new URLSearchParams(body).toString(); | ||
const params = new URLSearchParams(newBody).toString(); | ||
url = `${url}?${params}`; | ||
} | ||
return url; | ||
|
@@ -206,7 +189,7 @@ function handleNotOkResponse( | |
throwError(response, url, searchInputs); | ||
} else { | ||
if (errors) { | ||
const firstError = errors[0] as APIResponseError; | ||
const firstError = errors[0]; | ||
throwError(response, url, searchInputs, firstError); | ||
} | ||
} | ||
|
@@ -216,9 +199,9 @@ const throwError = ( | |
response: APIResponse, | ||
url: string, | ||
searchInputs?: QueryParamData, | ||
firstError?: APIResponseError, | ||
firstError?: unknown, | ||
) => { | ||
const { status_code, message } = response; | ||
const { status_code = 0, message = "" } = response; | ||
console.error( | ||
`API request error at ${url} (${status_code}): ${message}`, | ||
searchInputs, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,33 +1,21 @@ | ||
import "server-only"; | ||
|
||
import { environment } from "src/constants/environments"; | ||
import { OpportunityApiResponse } from "src/types/opportunity/opportunityResponseTypes"; | ||
|
||
import BaseApi from "./BaseApi"; | ||
|
||
export default class OpportunityListingAPI extends BaseApi { | ||
get version(): string { | ||
return "v1"; | ||
} | ||
|
||
get basePath(): string { | ||
return environment.API_URL; | ||
} | ||
|
||
get namespace(): string { | ||
return "opportunities"; | ||
} | ||
|
||
async getOpportunityById( | ||
opportunityId: number, | ||
): Promise<OpportunityApiResponse> { | ||
const subPath = `${opportunityId}`; | ||
const response = (await this.request( | ||
const response = await this.request<OpportunityApiResponse>( | ||
"GET", | ||
this.basePath, | ||
this.namespace, | ||
subPath, | ||
)) as OpportunityApiResponse; | ||
`${opportunityId}`, | ||
); | ||
return response; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not able to get this to work to use the chrome node debugger, and ended up using the VS code debugger, which did work to debug node based / server side next functions. If anyone else can get this to work, we should probably keep this, or we can just get rid of it.
see vercel/next.js#48767 and https://nextjs.org/docs/pages/building-your-application/configuring/debugging