Conversation
…ce offerings and API integration
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| ResolvedServiceOffering | ||
| } from 'src/@types/VisionsTrust' | ||
|
|
||
| const VISIONS_TRUST_API_BASE = 'https://api.visionstrust.com/v1' |
There was a problem hiding this comment.
todo: This can be put into app.config.js like all other endpoint configs.
| search | ||
| }) | ||
|
|
||
| const response = await fetch( |
| export async function resolveServiceOfferings( | ||
| offerings: VisionsTrustServiceOffering[] | ||
| ): Promise<ResolvedServiceOffering[]> { | ||
| const providerCache = new Map<string, VisionsTrustProvider>() |
There was a problem hiding this comment.
thought: It's unfortunate that the provider id needs to be resolved to the name via extra calls. To reduce the refetching save the map higher up in the component or in a global state to prevent refetching during page changes.
| /** | ||
| * Resolve service offerings with provider details | ||
| */ | ||
| export async function resolveServiceOfferings( |
There was a problem hiding this comment.
suggestion: The name should reflect that this function is resolving the provider id and not the service offering. e.g. resolveProviderIdToName
| export async function fetchServiceOfferings( | ||
| params: FetchServiceOfferingsParams = {} | ||
| ): Promise<ServiceOfferingsResponse> { | ||
| const { page = 1, limit = 50, search = '' } = params |
| return { | ||
| offerings: resolvedOfferings, | ||
| totalCount: response.count, | ||
| totalPages: Math.ceil(response.count / (params.limit || 50)) |
There was a problem hiding this comment.
todo: reuse 50 from the constant, see above
| import Loader from '@shared/atoms/Loader' | ||
| import styles from '@shared/AssetList/index.module.css' | ||
|
|
||
| const ITEMS_PER_PAGE = 25 |
There was a problem hiding this comment.
question: why is it 25 here and 50 in the other functions? Reuse the constant.
| export interface FetchServiceOfferingsParams { | ||
| page?: number | ||
| limit?: number | ||
| search?: string |
There was a problem hiding this comment.
thought: It seems search is never used.
2 participants
Proposed Changes