Update GC Forms extension integration#34
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis pull request introduces stream-based extension configuration, enabling operators to configure extensions on specific streams through a new dynamically-routed page. It restructures GCForms extension metadata and seed configuration, adds database persistence for form submission tracking, and updates extension metadata resolution to support the new streamConfigPage component field. ChangesStream-Based Extension Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
omarmir
force-pushed
the
gcform-ext-changes
branch
from
6d5a84b to
911680e
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a dedicated configuration page for extensions by introducing the streamConfigPage property in the extension metadata. It includes the new page component, routing logic, and internationalization strings. Additionally, the database schema is updated to include a GC Forms submission UUID, and seed data is refined for better claim mapping. The review feedback suggests enhancing the robustness of the data fetching process on the new configuration page by ensuring that failures in retrieving non-essential breadcrumb data do not block the primary configuration functionality.
| try { | ||
| isLoading.value = true | ||
| loadError.value = null | ||
| const [extensionResponse, profileResponse, streamResponse] = await Promise.all([ | ||
| fetch(getClientRequestUrl(`/api/extensions/streams/${streamId.value}`)), | ||
| transferPaymentId.value ? fetch(getClientRequestUrl(`/api/transfer-payments/${transferPaymentId.value}`)) : Promise.resolve(null), | ||
| transferPaymentId.value ? fetch(getClientRequestUrl(`/api/transfer-payments/${transferPaymentId.value}/streams/${streamId.value}`)) : Promise.resolve(null) | ||
| ]) | ||
|
|
||
| if (!extensionResponse.ok) await throwFetchResponseError(extensionResponse) | ||
| if (profileResponse && !profileResponse.ok) await throwFetchResponseError(profileResponse) | ||
| if (streamResponse && !streamResponse.ok) await throwFetchResponseError(streamResponse) | ||
|
|
||
| const payload = await extensionResponse.json() as ExtensionStreamRegistryResponse | ||
| item.value = payload.items.find(row => row.extension.key === extensionKey) ?? null | ||
| profile.value = profileResponse ? await profileResponse.json() as TransferPaymentProfileItem : null | ||
| stream.value = streamResponse ? await streamResponse.json() as TransferPaymentStreamItem : null | ||
|
|
||
| if (!item.value) { | ||
| throw new Error(t('apiErrors.extensions.not_found')) | ||
| } | ||
|
|
||
| resetDraft(item.value.config) | ||
| } catch (error: unknown) { | ||
| loadError.value = error | ||
| showError(error) | ||
| } finally { | ||
| isLoading.value = false | ||
| } |
There was a problem hiding this comment.
The current implementation of the refresh function fetches both essential extension data and secondary breadcrumb data within a single Promise.all. This means that if the fetches for breadcrumb data (profile or stream) fail, the entire page will render an error state, preventing the user from configuring the extension.
To improve robustness, consider handling failures in fetching breadcrumb data gracefully (e.g., by logging it and showing a toast notification) without blocking the primary functionality of the page.
try {
isLoading.value = true
loadError.value = null
const [extensionResponse, profileResponse, streamResponse] = await Promise.all([
fetch(getClientRequestUrl(`/api/extensions/streams/${streamId.value}`)),
transferPaymentId.value ? fetch(getClientRequestUrl(`/api/transfer-payments/${transferPaymentId.value}`)) : Promise.resolve(null),
transferPaymentId.value ? fetch(getClientRequestUrl(`/api/transfer-payments/${transferPaymentId.value}/streams/${streamId.value}`)) : Promise.resolve(null)
])
if (!extensionResponse.ok) await throwFetchResponseError(extensionResponse)
// Gracefully handle breadcrumb fetch errors
if (profileResponse && !profileResponse.ok) {
showError(new Error('Failed to load profile for breadcrumbs'))
}
if (streamResponse && !streamResponse.ok) {
showError(new Error('Failed to load stream for breadcrumbs'))
}
const payload = await extensionResponse.json() as ExtensionStreamRegistryResponse
item.value = payload.items.find(row => row.extension.key === extensionKey) ?? null
profile.value = profileResponse?.ok ? await profileResponse.json() as TransferPaymentProfileItem : null
stream.value = streamResponse?.ok ? await streamResponse.json() as TransferPaymentStreamItem : null
if (!item.value) {
throw new Error(t('apiErrors.extensions.not_found'))
}
resetDraft(item.value.config)
} catch (error: unknown) {
loadError.value = error
showError(error)
} finally {
isLoading.value = false
}
There was a problem hiding this comment.
Addressed in the current commit. The required extension registry fetch now runs independently, and the optional profile/stream breadcrumb fetches are consumed through fetchOptionalJson, which catches failures, shows the existing error toast, and returns null without setting the page-level load error.
There was a problem hiding this comment.
Confirmed against current commit 1dab0a2: the extension registry fetch is awaited independently, and the optional transfer payment / stream breadcrumb fetches are wrapped in fetchOptionalJson, which catches failures and returns null. Optional breadcrumb failures no longer block the configuration page.
omarmir
force-pushed
the
gcform-ext-changes
branch
2 times, most recently
from
8f0161c to
fe10ed5
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
omarmir
force-pushed
the
gcform-ext-changes
branch
from
fe10ed5 to
25ab233
Compare
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
omarmir
force-pushed
the
gcform-ext-changes
branch
from
25ab233 to
1dab0a2
Compare
|
@coderabbitai review |
|
/gemini |
|
✅ Actions performedReview triggered.
|
omarmir
force-pushed
the
gcform-ext-changes
branch
from
1dab0a2 to
a55125b
Compare
|
@coderabbitai review |
|
/gemini |
|
✅ Actions performedReview triggered.
|
omarmir
force-pushed
the
gcform-ext-changes
branch
from
a55125b to
a9ae5e2
Compare
|
@coderabbitai review |
|
/gemini |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/Extension/StreamExtensionsTab.vue`:
- Around line 75-81: The click handler openConfiguration dereferences
item.extension without guarding for null which can throw; update
openConfiguration to first check that item.extension exists and that
item.extension.admin.streamConfigPage?.componentName is truthy before calling
navigateTo(localePath(appRouteLocations.extensionStreamConfig(...))); if
item.extension is null/undefined, call the existing fallback openConfigure
instead. Ensure you reference the same symbols (openConfiguration,
item.extension, openConfigure, navigateTo, localePath,
appRouteLocations.extensionStreamConfig) when adding the null-check and
branching logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33f2b184-2f64-4125-a582-8776696dc806
📒 Files selected for processing (13)
app/components/Extension/StreamExtensionsTab.vueapp/pages/extension/[id]/config.vueapp/utils/route-locations.tsextensions/gcs-gcforms-integrationi18n/locales/en.jsoni18n/locales/fr.jsonmodules/gcs-extensions.tspackages/gcs-ssc-extensionsserver/database/migrations/0009_funding_case_agreement.tsserver/database/migrations/9999_seed.tsshared/types/database.d.tstests/unit/extensions-core.test.tstests/unit/seed-advance-assessment.test.ts
omarmir
force-pushed
the
gcform-ext-changes
branch
from
a9ae5e2 to
1a222a0
Compare
|
/gemini |
|
@coderabbitai review |
|
✅ Actions performedReview triggered.
|
1 participant
Summary
Depends on
Validation
Ready for review.
Summary by CodeRabbit
New Features
Database
Updates
Localization