Use the typescript client for Core API#273
Merged
devksingh4 merged 2 commits intomainfrom Feb 4, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request refactors the codebase to migrate from direct axios/fetch API calls to generated API clients from the Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
Deploying with Cloudflare Pages
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utils/api.ts (1)
26-39:⚠️ Potential issue | 🟠 MajorBest‑effort sync can still throw before the try/catch.
checkIfSyncNeededruns outside thetryblock, so any failure there will throw and stop the flow—contrary to the “best effort” comment. Consider guarding it.🔧 Suggested fix
- if (!force) { - const syncRequired = await checkIfSyncNeeded(accessToken); - if (!syncRequired) { - return; - } - } + if (!force) { + try { + const syncRequired = await checkIfSyncNeeded(accessToken); + if (!syncRequired) { + return; + } + } catch (e) { + // best-effort: ignore pre-check failures + } + }src/app/(membership)/store/transform.ts (1)
24-44:⚠️ Potential issue | 🟠 MajorFix price math and guard empty variant lists.
formatPriceRangedivides by 100 twice: theformatPricehelper divides by 100, but callers on lines 31 and 34 divide again before passing values to it. This produces prices 100x too small (e.g., $0.10 instead of $10.00). Additionally,Math.min(...[])on empty variant lists yieldsInfinity, which then cascades toitem_pricevalues. Guard the Math.min calls to handle empty arrays.🔧 Suggested fix
function formatPriceRange(prices: number[]): string { if (prices.length === 0) return ""; const uniquePrices = Array.from(new Set(prices)).sort((a, b) => a - b); const formatPrice = (cents: number) => `$${(cents / 100).toFixed(2)}`; if (uniquePrices.length === 1) { - return formatPrice(uniquePrices[0] / 100); + return formatPrice(uniquePrices[0]); } - return `${formatPrice(uniquePrices[0] / 100)} - ${formatPrice(uniquePrices[uniquePrices.length - 1] / 100)}`; + return `${formatPrice(uniquePrices[0])} - ${formatPrice(uniquePrices[uniquePrices.length - 1])}`; } export function transformApiProduct(product: ApiProduct): LegacyItem { const memberPrices = product.variants.map((v) => v.memberPriceCents); const nonmemberPrices = product.variants.map((v) => v.nonmemberPriceCents); // For item_price, use the minimum prices (or you could use average, etc.) - const minMemberPrice = Math.min(...memberPrices); - const minNonmemberPrice = Math.min(...nonmemberPrices); + const minMemberPrice = memberPrices.length ? Math.min(...memberPrices) : 0; + const minNonmemberPrice = nonmemberPrices.length ? Math.min(...nonmemberPrices) : 0;
🤖 Fix all issues with AI agents
In `@src/app/`(main)/calendar/page.tsx:
- Around line 34-38: The async IIFE inside useEffect calling
eventsApiClient.apiV1EventsGet can reject and currently isn't caught; wrap the
await call in a try/catch (inside the IIFE) to handle failures, log or report
the error (via console.error or your logger), and set a safe state (e.g.,
setAllEvents([]) or an error state) so transformApiDates and setAllEvents are
only called on success and the page doesn't suffer an unhandled rejection.
In `@src/app/`(membership)/check-membership/page.tsx:
- Around line 109-133: The catch block for the
membershipApiClient.apiV1MembershipGet call fails to clear loading state,
leaving the spinner stuck; update the catch handling in the function around
membershipApiClient.apiV1MembershipGet so that setIsLoading(false) is called
before opening the error modal (modalErrorMessage.onOpen), and ensure this
happens in both the ResponseError branch and the generic error branch (i.e.,
call setIsLoading(false) once at the start or end of the catch before
setErrorMessage/modalErrorMessage).
In `@src/app/`(membership)/membership/page.tsx:
- Around line 83-109: The catch block for the membership checkout flow fails to
clear loading state on the non-ResponseError path; ensure setIsLoading(false) is
called for all error paths by either calling setIsLoading(false) at the start of
the catch body (before checking instanceof ResponseError) or adding
setIsLoading(false) in the else branch; update the code around the try/catch
handling in page.tsx (references: setIsLoading, ResponseError, responseJson,
modalErrorMessage.onOpen, modalAlreadyMember.onOpen) so that loading is always
reset regardless of error type.
In `@src/app/`(membership)/store/item/page.tsx:
- Line 28: Remove the unused import transformApiResponse from the import
statement in page.tsx; keep only transformApiProduct (i.e., change the import
that currently reads import { transformApiProduct, transformApiResponse } from
'../transform' to import { transformApiProduct } from '../transform') so the
file no longer imports the unused symbol transformApiResponse.
- Around line 274-296: handleApiError currently calls await
error.response.json() which can throw on invalid/empty JSON; wrap that call in a
try/catch inside the ResponseError branch (function handleApiError, symbol
ResponseError and error.response.json()) and on parse failure fall back to using
error.response.status and a generic message when setting setErrorMessage; ensure
setIsLoading(false) and modalErrorMessage.onOpen() still run in all paths (both
network/no-response branch and ResponseError parse-failure branch) so the UI
always unblocks and shows the modal.
- Line 258: The type alias Product (defined as
ApiV1StoreProductsGet200Response['products'][number]) is declared inside the
component; move this type definition to the module-level near the other type
definitions (around where other types are declared, ~lines 45-56) and remove the
in-component declaration at line 258 so the component uses the module-level
Product type instead.
- Around line 260-271: The catch in metaLoader currently redirects to '/store'
for any error; instead, log the caught error (e.g., console.error or the app
logger) including context like the productId and the error object, then only
perform the redirect for a not-found/404 condition (inspect the error returned
by storeApiClient.apiV1StoreProductsProductIdGet), otherwise surface the error
(set an error state or leave loading false) so real failures are visible; update
references in the metaLoader flow that call transformApiProduct,
setForceIllinoisLogin, setMerchList, and setIsLoading accordingly.
In `@src/app/`(membership)/store/page.tsx:
- Line 13: The variable baseUrl declared as "const baseUrl =
process.env.NEXT_PUBLIC_CORE_API_BASE_URL;" is unused and should be removed;
delete that declaration from src/app/(membership)/store/page.tsx (search for the
identifier baseUrl) to clean up dead code and ensure no further references
remain.
Comment on lines
34
to
+38
| useEffect(() => { | ||
| const baseurl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL; | ||
| if (!baseurl) { | ||
| return setAllEvents([]); | ||
| } | ||
| async function fetcher() { | ||
| const urls = [ | ||
| `${baseurl}/api/v1/events`, | ||
| ]; | ||
|
|
||
| try { | ||
| const [eventsResponse] = | ||
| await Promise.allSettled(urls.map((url) => fetch(url))); | ||
| if (eventsResponse.status === 'fulfilled') { | ||
| const eventsData = await eventsResponse.value.json(); | ||
| setAllEvents(transformApiDates(eventsData as IEvent[])); | ||
| } else { | ||
| setAllEvents([]); // Handle error for events fetch | ||
| } | ||
| setValidOrganizations(OrganizationList); | ||
| } catch (err) { | ||
| console.error('Error in processing fetch results:', err); | ||
| setAllEvents([]); // Fallback error handling for critical failure | ||
| setValidOrganizations(OrganizationList); | ||
| } | ||
| } | ||
| fetcher(); | ||
| (async () => { | ||
| const allEvents = await eventsApiClient.apiV1EventsGet() | ||
| setAllEvents(transformApiDates(allEvents)); | ||
| })(); |
Contributor
There was a problem hiding this comment.
Add error handling for the events fetch to avoid unhandled rejections.
The async IIFE does not guard failures; a rejected promise will bubble and can leave the page in a broken state.
🔧 Suggested fix
useEffect(() => {
(async () => {
- const allEvents = await eventsApiClient.apiV1EventsGet()
- setAllEvents(transformApiDates(allEvents));
+ try {
+ const allEvents = await eventsApiClient.apiV1EventsGet();
+ setAllEvents(transformApiDates(allEvents));
+ } catch (e) {
+ console.error('Failed to load events', e);
+ setAllEvents([]);
+ }
})();
}, []);📝 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.
Suggested change
| useEffect(() => { | |
| const baseurl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL; | |
| if (!baseurl) { | |
| return setAllEvents([]); | |
| } | |
| async function fetcher() { | |
| const urls = [ | |
| `${baseurl}/api/v1/events`, | |
| ]; | |
| try { | |
| const [eventsResponse] = | |
| await Promise.allSettled(urls.map((url) => fetch(url))); | |
| if (eventsResponse.status === 'fulfilled') { | |
| const eventsData = await eventsResponse.value.json(); | |
| setAllEvents(transformApiDates(eventsData as IEvent[])); | |
| } else { | |
| setAllEvents([]); // Handle error for events fetch | |
| } | |
| setValidOrganizations(OrganizationList); | |
| } catch (err) { | |
| console.error('Error in processing fetch results:', err); | |
| setAllEvents([]); // Fallback error handling for critical failure | |
| setValidOrganizations(OrganizationList); | |
| } | |
| } | |
| fetcher(); | |
| (async () => { | |
| const allEvents = await eventsApiClient.apiV1EventsGet() | |
| setAllEvents(transformApiDates(allEvents)); | |
| })(); | |
| useEffect(() => { | |
| (async () => { | |
| try { | |
| const allEvents = await eventsApiClient.apiV1EventsGet(); | |
| setAllEvents(transformApiDates(allEvents)); | |
| } catch (e) { | |
| console.error('Failed to load events', e); | |
| setAllEvents([]); | |
| } | |
| })(); | |
| }, []); |
🤖 Prompt for AI Agents
In `@src/app/`(main)/calendar/page.tsx around lines 34 - 38, The async IIFE inside
useEffect calling eventsApiClient.apiV1EventsGet can reject and currently isn't
caught; wrap the await call in a try/catch (inside the IIFE) to handle failures,
log or report the error (via console.error or your logger), and set a safe state
(e.g., setAllEvents([]) or an error state) so transformApiDates and setAllEvents
are only called on success and the page doesn't suffer an unhandled rejection.
Comment on lines
+109
to
+133
| try { | ||
| const membershipResponse = await membershipApiClient.apiV1MembershipGet({ xUiucToken: accessToken }); | ||
| if (membershipResponse.givenName && membershipResponse.surname) { | ||
| setFullName(`${membershipResponse.givenName} ${membershipResponse.surname}`) | ||
| } | ||
| setIsPaidMember(membershipResponse.isPaidMember); | ||
| setIsLoading(false); | ||
| modalMembershipStatus.onOpen(); | ||
| } catch (e) { | ||
| console.error(e) | ||
| if (e instanceof ResponseError) { | ||
| setErrorMessage({ | ||
| code: error?.response?.status || 500, | ||
| message: error?.message || 'An unknown error occurred', | ||
| code: e?.response?.status || 500, | ||
| message: (await e.response.json()).message || 'An unknown error occurred', | ||
| }); | ||
| modalErrorMessage.onOpen(); | ||
| }); | ||
| }; | ||
| } else { | ||
| setErrorMessage({ | ||
| code: 400, | ||
| message: 'An unknown error occurred', | ||
| }); | ||
| } | ||
| modalErrorMessage.onOpen(); | ||
|
|
||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Reset isLoading on error to avoid a stuck spinner.
If the membership API call fails, the catch block opens an error modal but never clears the loading state.
🔧 Suggested fix
try {
const membershipResponse = await membershipApiClient.apiV1MembershipGet({ xUiucToken: accessToken });
if (membershipResponse.givenName && membershipResponse.surname) {
setFullName(`${membershipResponse.givenName} ${membershipResponse.surname}`)
}
setIsPaidMember(membershipResponse.isPaidMember);
- setIsLoading(false);
modalMembershipStatus.onOpen();
} catch (e) {
console.error(e)
if (e instanceof ResponseError) {
setErrorMessage({
code: e?.response?.status || 500,
message: (await e.response.json()).message || 'An unknown error occurred',
});
} else {
setErrorMessage({
code: 400,
message: 'An unknown error occurred',
});
}
modalErrorMessage.onOpen();
+ } finally {
+ setIsLoading(false);
}📝 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.
Suggested change
| try { | |
| const membershipResponse = await membershipApiClient.apiV1MembershipGet({ xUiucToken: accessToken }); | |
| if (membershipResponse.givenName && membershipResponse.surname) { | |
| setFullName(`${membershipResponse.givenName} ${membershipResponse.surname}`) | |
| } | |
| setIsPaidMember(membershipResponse.isPaidMember); | |
| setIsLoading(false); | |
| modalMembershipStatus.onOpen(); | |
| } catch (e) { | |
| console.error(e) | |
| if (e instanceof ResponseError) { | |
| setErrorMessage({ | |
| code: error?.response?.status || 500, | |
| message: error?.message || 'An unknown error occurred', | |
| code: e?.response?.status || 500, | |
| message: (await e.response.json()).message || 'An unknown error occurred', | |
| }); | |
| modalErrorMessage.onOpen(); | |
| }); | |
| }; | |
| } else { | |
| setErrorMessage({ | |
| code: 400, | |
| message: 'An unknown error occurred', | |
| }); | |
| } | |
| modalErrorMessage.onOpen(); | |
| } | |
| } | |
| try { | |
| const membershipResponse = await membershipApiClient.apiV1MembershipGet({ xUiucToken: accessToken }); | |
| if (membershipResponse.givenName && membershipResponse.surname) { | |
| setFullName(`${membershipResponse.givenName} ${membershipResponse.surname}`) | |
| } | |
| setIsPaidMember(membershipResponse.isPaidMember); | |
| modalMembershipStatus.onOpen(); | |
| } catch (e) { | |
| console.error(e) | |
| if (e instanceof ResponseError) { | |
| setErrorMessage({ | |
| code: e?.response?.status || 500, | |
| message: (await e.response.json()).message || 'An unknown error occurred', | |
| }); | |
| } else { | |
| setErrorMessage({ | |
| code: 400, | |
| message: 'An unknown error occurred', | |
| }); | |
| } | |
| modalErrorMessage.onOpen(); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/check-membership/page.tsx around lines 109 - 133, The
catch block for the membershipApiClient.apiV1MembershipGet call fails to clear
loading state, leaving the spinner stuck; update the catch handling in the
function around membershipApiClient.apiV1MembershipGet so that
setIsLoading(false) is called before opening the error modal
(modalErrorMessage.onOpen), and ensure this happens in both the ResponseError
branch and the generic error branch (i.e., call setIsLoading(false) once at the
start or end of the catch before setErrorMessage/modalErrorMessage).
Comment on lines
+83
to
+109
| try { | ||
| const response = await membershipApiClient.apiV2MembershipCheckoutGet({ | ||
| xUiucToken: accessToken, | ||
| }); | ||
| window.location.replace(response) | ||
| } catch (e) { | ||
| console.error("Error purchasing membership", e) | ||
| if (e instanceof ResponseError) { | ||
| setIsLoading(false); | ||
| if (error.response) { | ||
| if (error.response.status === 422) { | ||
| const errorObj = error.response.data; | ||
| setErrorMessage({ | ||
| code: errorObj.details[0].issue, | ||
| message: errorObj.details[0].description, | ||
| }); | ||
| modalErrorMessage.onOpen(); | ||
| } | ||
| else if (error.response.status === 403) { | ||
| setErrorMessage({ | ||
| code: 409, | ||
| message: 'Could not verify NetID.', | ||
| }); | ||
| modalAlreadyMember.onOpen(); | ||
| } else if (error.response.status === 400) { | ||
| const errorObj = error.response.data; | ||
| if ((errorObj.message as string).includes("is already a paid member")) { | ||
| setErrorMessage({ | ||
| code: 409, | ||
| message: 'The specified user is already a paid member.', | ||
| }); | ||
| modalAlreadyMember.onOpen(); | ||
| } else { | ||
| setErrorMessage({ | ||
| code: errorObj.id, | ||
| message: errorObj.message, | ||
| }); | ||
| modalErrorMessage.onOpen(); | ||
| } | ||
| } else if (error.response.status === 409) { | ||
| setErrorMessage({ | ||
| code: 500, | ||
| message: | ||
| 'Internal server error: ' + | ||
| (error.response.data.message || 'could not process request'), | ||
| }); | ||
| modalErrorMessage.onOpen(); | ||
| } | ||
| const responseJson = await e.response.json() as ValidationError; | ||
| if (responseJson.message.includes("is already a paid member")) { | ||
| modalErrorMessage.onClose(); | ||
| modalAlreadyMember.onOpen(); | ||
| return; | ||
| } | ||
| }); | ||
| setErrorMessage({ | ||
| code: responseJson.id ?? 400, | ||
| message: responseJson.message ?? "An error occurred creating your checkout session." | ||
| }) | ||
| } else { | ||
| setErrorMessage({ | ||
| code: 400, | ||
| message: "An error occurred creating your checkout session." | ||
| }) | ||
| } | ||
| modalErrorMessage.onOpen(); | ||
| } |
Contributor
There was a problem hiding this comment.
Ensure isLoading is cleared for all error paths.
In the current catch block, the non‑ResponseError path never resets isLoading, leaving the UI stuck in a loading state.
🔧 Suggested fix
- try {
+ try {
const response = await membershipApiClient.apiV2MembershipCheckoutGet({
xUiucToken: accessToken,
});
window.location.replace(response)
} catch (e) {
console.error("Error purchasing membership", e)
if (e instanceof ResponseError) {
- setIsLoading(false);
const responseJson = await e.response.json() as ValidationError;
if (responseJson.message.includes("is already a paid member")) {
modalErrorMessage.onClose();
modalAlreadyMember.onOpen();
return;
}
setErrorMessage({
code: responseJson.id ?? 400,
message: responseJson.message ?? "An error occurred creating your checkout session."
})
} else {
setErrorMessage({
code: 400,
message: "An error occurred creating your checkout session."
})
}
modalErrorMessage.onOpen();
+ } finally {
+ setIsLoading(false);
}📝 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.
Suggested change
| try { | |
| const response = await membershipApiClient.apiV2MembershipCheckoutGet({ | |
| xUiucToken: accessToken, | |
| }); | |
| window.location.replace(response) | |
| } catch (e) { | |
| console.error("Error purchasing membership", e) | |
| if (e instanceof ResponseError) { | |
| setIsLoading(false); | |
| if (error.response) { | |
| if (error.response.status === 422) { | |
| const errorObj = error.response.data; | |
| setErrorMessage({ | |
| code: errorObj.details[0].issue, | |
| message: errorObj.details[0].description, | |
| }); | |
| modalErrorMessage.onOpen(); | |
| } | |
| else if (error.response.status === 403) { | |
| setErrorMessage({ | |
| code: 409, | |
| message: 'Could not verify NetID.', | |
| }); | |
| modalAlreadyMember.onOpen(); | |
| } else if (error.response.status === 400) { | |
| const errorObj = error.response.data; | |
| if ((errorObj.message as string).includes("is already a paid member")) { | |
| setErrorMessage({ | |
| code: 409, | |
| message: 'The specified user is already a paid member.', | |
| }); | |
| modalAlreadyMember.onOpen(); | |
| } else { | |
| setErrorMessage({ | |
| code: errorObj.id, | |
| message: errorObj.message, | |
| }); | |
| modalErrorMessage.onOpen(); | |
| } | |
| } else if (error.response.status === 409) { | |
| setErrorMessage({ | |
| code: 500, | |
| message: | |
| 'Internal server error: ' + | |
| (error.response.data.message || 'could not process request'), | |
| }); | |
| modalErrorMessage.onOpen(); | |
| } | |
| const responseJson = await e.response.json() as ValidationError; | |
| if (responseJson.message.includes("is already a paid member")) { | |
| modalErrorMessage.onClose(); | |
| modalAlreadyMember.onOpen(); | |
| return; | |
| } | |
| }); | |
| setErrorMessage({ | |
| code: responseJson.id ?? 400, | |
| message: responseJson.message ?? "An error occurred creating your checkout session." | |
| }) | |
| } else { | |
| setErrorMessage({ | |
| code: 400, | |
| message: "An error occurred creating your checkout session." | |
| }) | |
| } | |
| modalErrorMessage.onOpen(); | |
| } | |
| try { | |
| const response = await membershipApiClient.apiV2MembershipCheckoutGet({ | |
| xUiucToken: accessToken, | |
| }); | |
| window.location.replace(response) | |
| } catch (e) { | |
| console.error("Error purchasing membership", e) | |
| if (e instanceof ResponseError) { | |
| const responseJson = await e.response.json() as ValidationError; | |
| if (responseJson.message.includes("is already a paid member")) { | |
| modalErrorMessage.onClose(); | |
| modalAlreadyMember.onOpen(); | |
| return; | |
| } | |
| setErrorMessage({ | |
| code: responseJson.id ?? 400, | |
| message: responseJson.message ?? "An error occurred creating your checkout session." | |
| }) | |
| } else { | |
| setErrorMessage({ | |
| code: 400, | |
| message: "An error occurred creating your checkout session." | |
| }) | |
| } | |
| modalErrorMessage.onOpen(); | |
| } finally { | |
| setIsLoading(false); | |
| } |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/membership/page.tsx around lines 83 - 109, The catch
block for the membership checkout flow fails to clear loading state on the
non-ResponseError path; ensure setIsLoading(false) is called for all error paths
by either calling setIsLoading(false) at the start of the catch body (before
checking instanceof ResponseError) or adding setIsLoading(false) in the else
branch; update the code around the try/catch handling in page.tsx (references:
setIsLoading, ResponseError, responseJson, modalErrorMessage.onOpen,
modalAlreadyMember.onOpen) so that loading is always reset regardless of error
type.
| import { membershipApiClient, storeApiClient, syncIdentity } from '@/utils/api'; | ||
| import { Turnstile } from '@marsidev/react-turnstile'; | ||
| import { transformApiResponse } from '../transform'; | ||
| import { transformApiProduct, transformApiResponse } from '../transform'; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused import transformApiResponse.
Only transformApiProduct is used in this file.
🧹 Proposed fix
-import { transformApiProduct, transformApiResponse } from '../transform';
+import { transformApiProduct } from '../transform';📝 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.
Suggested change
| import { transformApiProduct, transformApiResponse } from '../transform'; | |
| import { transformApiProduct } from '../transform'; |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/store/item/page.tsx at line 28, Remove the unused
import transformApiResponse from the import statement in page.tsx; keep only
transformApiProduct (i.e., change the import that currently reads import {
transformApiProduct, transformApiResponse } from '../transform' to import {
transformApiProduct } from '../transform') so the file no longer imports the
unused symbol transformApiResponse.
| } | ||
| return available; | ||
| }; | ||
| type Product = ApiV1StoreProductsGet200Response['products'][number]; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Move type alias outside the component.
Defining a type alias inside the component body is unconventional and may cause confusion. Move it to the module level near other type definitions (around line 45-56).
♻️ Proposed fix
Move the type alias outside the component:
enum InputStatus {
EMPTY,
INVALID,
VALID,
}
+
+type Product = ApiV1StoreProductsGet200Response['products'][number];
const coreBaseUrl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL;Then remove line 258.
📝 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.
Suggested change
| type Product = ApiV1StoreProductsGet200Response['products'][number]; | |
| enum InputStatus { | |
| EMPTY, | |
| INVALID, | |
| VALID, | |
| } | |
| type Product = ApiV1StoreProductsGet200Response['products'][number]; | |
| const coreBaseUrl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL; |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/store/item/page.tsx at line 258, The type alias Product
(defined as ApiV1StoreProductsGet200Response['products'][number]) is declared
inside the component; move this type definition to the module-level near the
other type definitions (around where other types are declared, ~lines 45-56) and
remove the in-component declaration at line 258 so the component uses the
module-level Product type instead.
Comment on lines
260
to
+271
| const metaLoader = async () => { | ||
| const url = `${coreBaseUrl}/api/v1/store/products/${itemid}`; | ||
| axios | ||
| .get(url) | ||
| .then((response) => { | ||
| const transformed = transformApiResponse({ products: [response.data] })[0]; | ||
| if (response.data['verifiedIdentityRequired']) { | ||
| setForceIllinoisLogin(true); | ||
| } | ||
| setMerchList(transformed); | ||
| setIsLoading(false); | ||
| }) | ||
| .catch((error) => { | ||
| if (error.response && error.response.status === 404) { | ||
| setTimeout(() => { | ||
| setErrorMessage({ | ||
| title: "Error retrieving product", | ||
| code: 404, | ||
| message: error.response.data.message, | ||
| }); | ||
| setMerchList({ failed: true }); | ||
| setIsLoading(false); | ||
| modalErrorMessage.onOpen(); | ||
| }, 1000); | ||
| } | ||
| }); | ||
| try { | ||
| const itemData = await storeApiClient.apiV1StoreProductsProductIdGet({ productId: itemid }) | ||
| const transformed = transformApiProduct(itemData); | ||
| if (itemData['verifiedIdentityRequired']) { | ||
| setForceIllinoisLogin(true); | ||
| } | ||
| setMerchList(transformed); | ||
| setIsLoading(false); | ||
| } catch (e) { | ||
| window.location.href = '/store'; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Silent redirect on all errors may hide issues.
The catch block redirects to /store for any error, not just 404s. Consider logging the error before redirecting to aid debugging, similar to the approach in page.tsx.
♻️ Proposed fix
} catch (e) {
+ console.error("Failed to load product", e);
window.location.href = '/store';
}📝 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.
Suggested change
| const metaLoader = async () => { | |
| const url = `${coreBaseUrl}/api/v1/store/products/${itemid}`; | |
| axios | |
| .get(url) | |
| .then((response) => { | |
| const transformed = transformApiResponse({ products: [response.data] })[0]; | |
| if (response.data['verifiedIdentityRequired']) { | |
| setForceIllinoisLogin(true); | |
| } | |
| setMerchList(transformed); | |
| setIsLoading(false); | |
| }) | |
| .catch((error) => { | |
| if (error.response && error.response.status === 404) { | |
| setTimeout(() => { | |
| setErrorMessage({ | |
| title: "Error retrieving product", | |
| code: 404, | |
| message: error.response.data.message, | |
| }); | |
| setMerchList({ failed: true }); | |
| setIsLoading(false); | |
| modalErrorMessage.onOpen(); | |
| }, 1000); | |
| } | |
| }); | |
| try { | |
| const itemData = await storeApiClient.apiV1StoreProductsProductIdGet({ productId: itemid }) | |
| const transformed = transformApiProduct(itemData); | |
| if (itemData['verifiedIdentityRequired']) { | |
| setForceIllinoisLogin(true); | |
| } | |
| setMerchList(transformed); | |
| setIsLoading(false); | |
| } catch (e) { | |
| window.location.href = '/store'; | |
| } | |
| const metaLoader = async () => { | |
| try { | |
| const itemData = await storeApiClient.apiV1StoreProductsProductIdGet({ productId: itemid }) | |
| const transformed = transformApiProduct(itemData); | |
| if (itemData['verifiedIdentityRequired']) { | |
| setForceIllinoisLogin(true); | |
| } | |
| setMerchList(transformed); | |
| setIsLoading(false); | |
| } catch (e) { | |
| console.error("Failed to load product", e); | |
| window.location.href = '/store'; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/store/item/page.tsx around lines 260 - 271, The catch
in metaLoader currently redirects to '/store' for any error; instead, log the
caught error (e.g., console.error or the app logger) including context like the
productId and the error object, then only perform the redirect for a
not-found/404 condition (inspect the error returned by
storeApiClient.apiV1StoreProductsProductIdGet), otherwise surface the error (set
an error state or leave loading false) so real failures are visible; update
references in the metaLoader flow that call transformApiProduct,
setForceIllinoisLogin, setMerchList, and setIsLoading accordingly.
Comment on lines
+274
to
296
| const handleApiError = async (error: any) => { | ||
| if (!error.response) { | ||
| setErrorMessage({ code: 500, message: 'Network error. Please try again.' }); | ||
| modalErrorMessage.onOpen(); | ||
| setIsLoading(false); | ||
| return; | ||
| } | ||
| const { status, data } = error.response; | ||
| setErrorMessage({ | ||
| code: data.id || status, | ||
| message: data.message || 'An error occurred and your request could not be processed.', | ||
| }); | ||
| if (error instanceof ResponseError) { | ||
| const response = await error.response.json() | ||
| setErrorMessage({ | ||
| code: response.id || error.response.status, | ||
| message: response.message || 'An error occurred and your request could not be processed.', | ||
| }); | ||
| } else { | ||
| setErrorMessage({ | ||
| code: 400, | ||
| message: 'An error occurred and your request could not be processed.', | ||
| }); | ||
| } | ||
|
|
||
| setIsLoading(false); | ||
| modalErrorMessage.onOpen(); | ||
| }; |
Contributor
There was a problem hiding this comment.
Unhandled exception if response body is not valid JSON.
error.response.json() can throw if the response body is malformed or empty. Wrap in try/catch to handle gracefully.
🛡️ Proposed fix
const handleApiError = async (error: any) => {
if (!error.response) {
setErrorMessage({ code: 500, message: 'Network error. Please try again.' });
modalErrorMessage.onOpen();
setIsLoading(false);
return;
}
if (error instanceof ResponseError) {
- const response = await error.response.json()
- setErrorMessage({
- code: response.id || error.response.status,
- message: response.message || 'An error occurred and your request could not be processed.',
- });
+ try {
+ const response = await error.response.json();
+ setErrorMessage({
+ code: response.id || error.response.status,
+ message: response.message || 'An error occurred and your request could not be processed.',
+ });
+ } catch {
+ setErrorMessage({
+ code: error.response.status,
+ message: 'An error occurred and your request could not be processed.',
+ });
+ }
} else {
setErrorMessage({
code: 400,
message: 'An error occurred and your request could not be processed.',
});
}
setIsLoading(false);
modalErrorMessage.onOpen();
};📝 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.
Suggested change
| const handleApiError = async (error: any) => { | |
| if (!error.response) { | |
| setErrorMessage({ code: 500, message: 'Network error. Please try again.' }); | |
| modalErrorMessage.onOpen(); | |
| setIsLoading(false); | |
| return; | |
| } | |
| const { status, data } = error.response; | |
| setErrorMessage({ | |
| code: data.id || status, | |
| message: data.message || 'An error occurred and your request could not be processed.', | |
| }); | |
| if (error instanceof ResponseError) { | |
| const response = await error.response.json() | |
| setErrorMessage({ | |
| code: response.id || error.response.status, | |
| message: response.message || 'An error occurred and your request could not be processed.', | |
| }); | |
| } else { | |
| setErrorMessage({ | |
| code: 400, | |
| message: 'An error occurred and your request could not be processed.', | |
| }); | |
| } | |
| setIsLoading(false); | |
| modalErrorMessage.onOpen(); | |
| }; | |
| const handleApiError = async (error: any) => { | |
| if (!error.response) { | |
| setErrorMessage({ code: 500, message: 'Network error. Please try again.' }); | |
| modalErrorMessage.onOpen(); | |
| setIsLoading(false); | |
| return; | |
| } | |
| if (error instanceof ResponseError) { | |
| try { | |
| const response = await error.response.json(); | |
| setErrorMessage({ | |
| code: response.id || error.response.status, | |
| message: response.message || 'An error occurred and your request could not be processed.', | |
| }); | |
| } catch { | |
| setErrorMessage({ | |
| code: error.response.status, | |
| message: 'An error occurred and your request could not be processed.', | |
| }); | |
| } | |
| } else { | |
| setErrorMessage({ | |
| code: 400, | |
| message: 'An error occurred and your request could not be processed.', | |
| }); | |
| } | |
| setIsLoading(false); | |
| modalErrorMessage.onOpen(); | |
| }; |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/store/item/page.tsx around lines 274 - 296,
handleApiError currently calls await error.response.json() which can throw on
invalid/empty JSON; wrap that call in a try/catch inside the ResponseError
branch (function handleApiError, symbol ResponseError and error.response.json())
and on parse failure fall back to using error.response.status and a generic
message when setting setErrorMessage; ensure setIsLoading(false) and
modalErrorMessage.onOpen() still run in all paths (both network/no-response
branch and ResponseError parse-failure branch) so the UI always unblocks and
shows the modal.
| type Product = ApiV1StoreProductsGet200Response['products'][number]; | ||
| const MerchStore = () => { | ||
| const [itemsList, setItemsList] = useState<Array<Record<string, any>>>([]); | ||
| const baseUrl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove unused variable baseUrl.
This variable is no longer used after migrating from axios to the API client.
🧹 Proposed fix
- const baseUrl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL;📝 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.
Suggested change
| const baseUrl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL; |
🤖 Prompt for AI Agents
In `@src/app/`(membership)/store/page.tsx at line 13, The variable baseUrl
declared as "const baseUrl = process.env.NEXT_PUBLIC_CORE_API_BASE_URL;" is
unused and should be removed; delete that declaration from
src/app/(membership)/store/page.tsx (search for the identifier baseUrl) to clean
up dead code and ensure no further references remain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Chores