feat: prerequisites v2#1757
feat: prerequisites v2#1757JustARatherRidiculouslyLongUsername wants to merge 3 commits intoqwc-regen-landing-pagefrom
Conversation
WalkthroughThis pull request refactors the QWC file regeneration flow by introducing a new prerequisites component, renaming the type system (NextStepOption to QwcRegenerationFlowType), restructuring routing with nested child routes, and updating landing component navigation logic to support the new flow. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts (1)
17-20: Subscription cleanup is safe but considertake(1)for clarity.Since route data is set statically in the routing config (not expected to change while this component lives), using
take(1)would make the intent explicit and auto-unsubscribe after the first emission.♻️ Suggested improvement
+import { take } from 'rxjs'; // ... ngOnInit(): void { - this.route.data.subscribe(data => { + this.route.data.pipe(take(1)).subscribe(data => { this.flowType = data.flowType; }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts` around lines 17 - 20, The subscription in ngOnInit uses this.route.data.subscribe(...) to set this.flowType but never explicitly limits emissions; change it to use RxJS take(1) so it auto-unsubscribes after the first emission and makes intent explicit: import { take } from 'rxjs/operators' (or 'rxjs') and replace this.route.data.subscribe(...) with this.route.data.pipe(take(1)).subscribe(...) to set flowType; reference ngOnInit, this.route.data.subscribe, and this.flowType when making the change.src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts (1)
76-85: Consider adding error handling for the connector settings call.
getQBDConnectorSettings()has no error handler — if the API call fails,isLoadingremainstrueindefinitely and the user is stuck on a loading screen. While this may be pre-existing, the form initialization changes here make it a good time to add a minimal error callback.♻️ Suggested improvement
this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe( - (qbdConnectorSettings: QbdConnectorGet) => { + { + next: (qbdConnectorSettings: QbdConnectorGet) => { this.connectorSettings = qbdConnectorSettings; this.isLoading = false; - } + }, + error: () => { + this.isLoading = false; + // Optionally show an error toast + } + } );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts` around lines 76 - 85, The getQBDConnectorSettings() subscription in ngOnInit lacks an error handler so isLoading can remain true if the API fails; update the subscription on qbdDirectConnectorService.getQBDConnectorSettings() to add an error callback that sets this.isLoading = false, logs or surfaces the error (e.g., via a logger or user-facing notification), and leaves/regenerates the form (regenerateQwcForm) usable; ensure you still assign this.connectorSettings on success and handle the error path gracefully so the UI doesn't stay stuck loading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-prerequisites-v2/qbd-direct-prerequisites-v2.component.spec.ts`:
- Around line 1-22: Remove the generated spec file or, if you must keep it,
update the test to call fixture.detectChanges() after creating the component so
ngOnInit runs; specifically modify the QbdDirectPrerequisitesV2Component spec
that uses TestBed.configureTestingModule and fixture =
TestBed.createComponent(QbdDirectPrerequisitesV2Component) to either delete the
spec per project convention (use --skip-tests when generating) or insert
fixture.detectChanges() (and await fixture.whenStable() if needed) before the
expect(component).toBeTruthy() assertion.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-prerequisites-v2/qbd-direct-prerequisites-v2.component.ts`:
- Around line 21-22: The parent template is not handling the
QbdDirectPrerequisitesV2Component's output event; bind the component's
(continue) EventEmitter so the parent can react and progress the flow by adding
(continue)="handleContinue()" to the <app-qbd-direct-prerequisites-v2> element
and implement a handleContinue() method in the parent component (the parent
should import/declare any state changes or navigation that should occur when
QbdDirectPrerequisitesV2Component.emit() fires); locate the emitter defined as
continue in class QbdDirectPrerequisitesV2Component and add the corresponding
handler in the parent component that owns the <app-qbd-direct-prerequisites-v2>
tag.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.spec.ts`:
- Around line 1-23: Either delete this spec (per project convention skip
generating *.spec.ts files) or fix it: in TestBed.configureTestingModule provide
a mock ActivatedRoute (use provider for ActivatedRoute with a stubbed value for
route.data.flowType) and declare QbdDirectRegenerateQwcFileComponent in
declarations (don’t pass the component in imports), then after creating the
fixture call fixture.detectChanges() (so ngOnInit runs) instead of awaiting only
whenStable; ensure the test imports/DI reference the
QbdDirectRegenerateQwcFileComponent, ActivatedRoute provider, and uses
fixture.detectChanges() before assertions.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`:
- Around line 12-21: Remove the unused flowType property and its assignment in
QbdDirectRegenerateQwcFileComponent: delete the flowType declaration on the
class and remove the route.data subscription inside ngOnInit (the
this.route.data.subscribe(...) block) so ngOnInit no longer assigns an unused
value; keep the constructor( private route: ActivatedRoute ) intact if route is
still needed elsewhere, otherwise remove the unused constructor parameter as
well.
In `@src/assets/i18n/en.json`:
- Around line 1689-1704: Reorder the child keys inside the
qbdDirectPrerequisitesV2 i18n object so they are alphabetically sorted;
specifically move "contentText" to be the first child, move "step1Tooltip" so it
follows "step1Point3Sub2" (i.e., before "step2Label"), and move "stepName" to
the end so the final key order is alphabetically correct (contentText, done,
markAsDone, step1Label, step1Point1, step1Point2, step1Point3, step1Point3Sub1,
step1Point3Sub2, step1Tooltip, step2Label, step2Point1, step2Point2, stepName).
- Around line 1689-1704: Add the new i18n key "qbdDirectPrerequisitesV2" (same
structure and properties as in en.json: stepName, contentText, done, markAsDone,
step1Label, step1Point1, step1Point2, step1Point3, step1Point3Sub1,
step1Point3Sub2, step2Label, step2Point1, step2Point2, step1Tooltip) to the
other supported language files missing it (the co and fyle language JSONs
referenced in the review), either with appropriate translations or the English
placeholders, making sure the JSON keys/nesting match exactly and the files
remain valid JSON (no trailing commas). Ensure the added key appears in the same
location/format as the existing i18n entries so automated checks pick it up.
---
Nitpick comments:
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts`:
- Around line 76-85: The getQBDConnectorSettings() subscription in ngOnInit
lacks an error handler so isLoading can remain true if the API fails; update the
subscription on qbdDirectConnectorService.getQBDConnectorSettings() to add an
error callback that sets this.isLoading = false, logs or surfaces the error
(e.g., via a logger or user-facing notification), and leaves/regenerates the
form (regenerateQwcForm) usable; ensure you still assign this.connectorSettings
on success and handle the error path gracefully so the UI doesn't stay stuck
loading.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`:
- Around line 17-20: The subscription in ngOnInit uses
this.route.data.subscribe(...) to set this.flowType but never explicitly limits
emissions; change it to use RxJS take(1) so it auto-unsubscribes after the first
emission and makes intent explicit: import { take } from 'rxjs/operators' (or
'rxjs') and replace this.route.data.subscribe(...) with
this.route.data.pipe(take(1)).subscribe(...) to set flowType; reference
ngOnInit, this.route.data.subscribe, and this.flowType when making the change.
Description
Clickup
https://app.clickup.com/t/86d1x6zz9
Summary by CodeRabbit