feat(auto-configure): live progress loader with streaming + summary#1817
Merged
Conversation
- ConfigurationStateService: replace the blocking GET on /ai/v2/setup/:id with a fetch + ReadableStream consumer that parses newline-delimited JSON or SSE-style "data:" lines and pushes typed ConfigProgress events to a progress$ BehaviorSubject. Falls back cleanly when the body is empty (current backend behavior). - AutoConfigureComponent: subscribes to progress$ and exposes a signal + computed percent; renders the current step / table name and a determinate progress bar fill when total is known, otherwise keeps the indeterminate look. - Template shows "Configuring <table>" with a "X of N" counter once the backend starts streaming events. Backend endpoint update pending. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Consume streaming /ai/v2/setup with masterpwd header (bypasses HttpClient for ReadableStream access) - Throttle log display at 400ms; fast-drain (80ms) once backend signals complete, capped at 1.5s - Translate backend messages to user-facing phrases (renamed X, search by Y, widget for table.column, etc.) - Replace spinner with animated stroke-draw checkmark on complete - Title swap to "Your database is ready" + green chips with tables/widgets/settings counts - Stable popup size via min-height on the status log and the subtitle/chips area so rows don't push the card - 1.8s settle delay before navigating so the summary is readable - Lighter, smaller done-item text in light theme for clearer current-vs-done hierarchy Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Reduce card body gap (16→10) and trim top/bottom padding so title hugs the loader and the log sits closer to the subtitle - Drop the spinner/check margin-bottom (gap handles it now) - Shrink the log to 78px fixed height with a top fade mask so older lines dissolve gracefully instead of clipping abruptly Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
📝 WalkthroughWalkthroughThe PR refactors the auto-configure flow to display real-time setup progress. The backend now streams setup events via fetch; the service parses and queues them, draining through a paced BehaviorSubject. The component subscribes, maintains state (progress, messages, counters), parses messages to extract metrics, and renders dynamic UI (spinner→success, summary chips, status log) with animations. ChangesAuto-Configure Streaming Progress UI
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
frontend/src/app/components/auto-configure/auto-configure.component.html (1)
25-39: ⚡ Quick winConsider announcing live progress to assistive tech.
The status log updates dynamically as messages stream in, but there's no
aria-liveregion, so screen-reader users won't hear progress. Adding a polite live region (androle="status") makes the streaming updates perceivable.♿ Suggested attributes
- <ul class="status-log" *ngIf="!errorText(); else errorBlock"> + <ul class="status-log" role="status" aria-live="polite" *ngIf="!errorText(); else errorBlock">🤖 Prompt for 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. In `@frontend/src/app/components/auto-configure/auto-configure.component.html` around lines 25 - 39, The status list currently rendered by the *ngIf branch (the <ul> that iterates messages() with trackByIndex) is not announced to assistive tech; add ARIA attributes to make updates perceivable — add aria-live="polite" role="status" (and optionally aria-atomic="false") to that <ul> so dynamic messages() updates are announced, and mark the error output (the <p> that renders errorText()) with role="alert" to ensure immediate announcement of failures; keep the same structural bindings (messages(), errorText(), trackByIndex) when adding these attributes.
🤖 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 `@frontend/src/app/components/auto-configure/auto-configure.component.html`:
- Around line 4-39: The template uses Angular's legacy structural directives
(*ngIf, *ngFor and "else errorBlock" plus trackBy: trackByIndex); replace them
with the project's built-in control flow: use `@if`(done()) / `@if`(!done()) blocks
instead of *ngIf(done()) and inline the error branch with `@else` rather than a
separate <ng-template>, and convert the status list to `@for`="let m of
messages(); let last = last" (or the project's `@for` syntax) instead of *ngFor
and remove the "trackBy: trackByIndex" usage (drop trackBy or implement
equivalent in the `@for` if supported). Update all occurrences referencing done(),
hasSummary(), tablesConfigured(), widgetsCount(), settingsCount(), messages(),
and errorText() to the new `@if/`@for constructs and ensure class bindings
([class.xxx]) remain applied to the items.
In `@frontend/src/app/components/auto-configure/auto-configure.component.ts`:
- Around line 64-67: The code currently always waits 1.8s and navigates after
calling this._configState.startConfiguring(connectionId); change ngOnInit (or
wherever startConfiguring is called) to capture its return value (e.g., const
success = await this._configState.startConfiguring(connectionId);) and only
perform the await new Promise(... 1800) and this._router.navigate(['/dashboard',
connectionId]) when success is truthy; if success is false, do not navigate and
let the component remain to show the error state.
- Around line 9-14: The `@Component` decorator for AutoConfigureComponent
currently lists imports (CommonModule, MatButtonModule, RouterModule) which
requires the component to be standalone; update the component decorator for
AutoConfigureComponent to include standalone: true so the imports array is valid
and the component follows the repo's standalone-component convention (i.e., add
standalone: true inside the `@Component` metadata for AutoConfigureComponent).
In `@frontend/src/app/services/configuration-state.service.ts`:
- Around line 19-27: The service currently uses shared fields (_queue,
_drainTimer, _currentDrainIntervalMs, progress$) so concurrent
startConfiguring() calls for different connectionId collide; change the
implementation to maintain per-connection state keyed by connectionId (e.g., a
Map<connectionId, { queue, drainTimer, currentDrainIntervalMs, progressSubject
}>) so startConfiguring(), enqueue progress and the drain logic operate on that
connection's state only, create a fresh BehaviorSubject/Subject for each
connection instead of a single progress$, and ensure cleanup (clearInterval and
delete map entry) when a run completes so terminal events don’t leak to new
subscribers.
- Around line 63-85: The stream-handling in _runSetup() currently treats any
successful fetch as success even if no 'complete' frame was received; update
_runSetup() to track whether a completion event was actually parsed (e.g., add a
local seenComplete boolean) and only consider the setup successful when
seenComplete is true. To do this, modify _parseAndEnqueue(...) to signal when it
parsed a 'complete' frame (either by returning a boolean/enum or by calling a
new method like this._markSetupComplete()), set seenComplete when that signal is
received while reading lines from response.body, and after the reader finishes
(and before resolving success / showing the success snackbar / redirect) fail
cleanly (return false or throw) if seenComplete is still false. Ensure the same
change is applied to the other stream-reading block referenced (lines 102-116)
so EOF without a 'complete' frame is handled consistently.
- Around line 16-19: Replace the shared BehaviorSubject-based state in
ConfigurationStateService (_configuring and progress$) with Angular signals or
rxResource per frontend/service conventions so consumers read reactive state
without subscribing to Subjects; change startConfiguring() and _runSetup() to
allocate per-connection run state (separate _queue, _drainTimer,
_currentDrainIntervalMs) rather than mutating singletons so concurrent
connectionIds do not clobber each other; ensure _runSetup() and _waitForDrain()
maintain a per-connection "complete" flag and only return success when a `{
type: 'complete' }` frame is actually received for that connection (do not treat
stream end alone as success); update callers to use the new signals/rxResource
instead of subscribing to progress$/_configuring.
---
Nitpick comments:
In `@frontend/src/app/components/auto-configure/auto-configure.component.html`:
- Around line 25-39: The status list currently rendered by the *ngIf branch (the
<ul> that iterates messages() with trackByIndex) is not announced to assistive
tech; add ARIA attributes to make updates perceivable — add aria-live="polite"
role="status" (and optionally aria-atomic="false") to that <ul> so dynamic
messages() updates are announced, and mark the error output (the <p> that
renders errorText()) with role="alert" to ensure immediate announcement of
failures; keep the same structural bindings (messages(), errorText(),
trackByIndex) when adding these attributes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 253854db-55bd-4362-a9ec-ab3910ecdb34
📒 Files selected for processing (4)
frontend/src/app/components/auto-configure/auto-configure.component.cssfrontend/src/app/components/auto-configure/auto-configure.component.htmlfrontend/src/app/components/auto-configure/auto-configure.component.tsfrontend/src/app/services/configuration-state.service.ts
Comment on lines
+4
to
+39
| <div *ngIf="!done()" class="spinner"> | ||
| <div class="spinner__ring spinner__ring--outer"></div> | ||
| <div class="spinner__ring spinner__ring--inner"></div> | ||
| </div> | ||
| <h2 class="card__title">Configuring your database</h2> | ||
| <p class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p> | ||
| <div class="progress-bar"> | ||
| <div class="progress-bar__fill"></div> | ||
| <div *ngIf="done()" class="success-mark" aria-label="Configuration complete"> | ||
| <svg viewBox="0 0 52 52" xmlns="http://www.w3.org/2000/svg"> | ||
| <circle class="success-mark__circle" cx="26" cy="26" r="25"></circle> | ||
| <path class="success-mark__check" d="M14 27 l8 8 l16 -18"></path> | ||
| </svg> | ||
| </div> | ||
| <h2 class="card__title">{{ done() ? 'Your database is ready' : 'Configuring your database' }}</h2> | ||
| <div class="card__head-detail"> | ||
| <p *ngIf="!done()" class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p> | ||
| <div *ngIf="hasSummary()" class="summary-chips"> | ||
| <span class="summary-chip"><strong>{{ tablesConfigured().size }}</strong> {{ tablesConfigured().size === 1 ? 'table' : 'tables' }}</span> | ||
| <span *ngIf="widgetsCount() > 0" class="summary-chip"><strong>{{ widgetsCount() }}</strong> {{ widgetsCount() === 1 ? 'widget' : 'widgets' }}</span> | ||
| <span *ngIf="settingsCount() > 0" class="summary-chip"><strong>{{ settingsCount() }}</strong> {{ settingsCount() === 1 ? 'setting' : 'settings' }}</span> | ||
| </div> | ||
| <p *ngIf="done() && !hasSummary()" class="card__subtitle">All set — opening your dashboard…</p> | ||
| </div> | ||
|
|
||
| <ul class="status-log" *ngIf="!errorText(); else errorBlock"> | ||
| <li *ngFor="let m of messages(); let last = last; trackBy: trackByIndex" | ||
| class="status-log__item" | ||
| [class.status-log__item_current]="last" | ||
| [class.status-log__item_done]="!last"> | ||
| <span class="status-log__icon" aria-hidden="true"> | ||
| <span *ngIf="!last" class="status-log__check">✓</span> | ||
| <span *ngIf="last" class="status-log__spinner"></span> | ||
| </span> | ||
| <span class="status-log__text">{{ m }}</span> | ||
| </li> | ||
| </ul> | ||
| <ng-template #errorBlock> | ||
| <p class="card__status card__status_error">{{ errorText() }}</p> | ||
| </ng-template> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C2 '\btrackByIndex\s*[=(:]' frontend/src/app/components/auto-configure/Repository: rocket-admin/rocketadmin
Length of output: 500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba frontend/src/app/components/auto-configure/auto-configure.component.html | sed -n '1,140p'Repository: rocket-admin/rocketadmin
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n frontend/src/app/components/auto-configure/auto-configure.component.html | sed -n '1,120p'Repository: rocket-admin/rocketadmin
Length of output: 2840
Replace structural directives with Angular built-in control flow in auto-configure template
frontend/src/app/components/auto-configure/auto-configure.component.html still uses *ngIf/*ngFor (including else errorBlock) and trackBy: trackByIndex in the template region; update to @if/@for to match the project guideline.
♻️ Proposed conversion to built-in control flow
- <div *ngIf="!done()" class="spinner">
- <div class="spinner__ring spinner__ring--outer"></div>
- <div class="spinner__ring spinner__ring--inner"></div>
- </div>
- <div *ngIf="done()" class="success-mark" aria-label="Configuration complete">
- <svg viewBox="0 0 52 52" xmlns="http://www.w3.org/2000/svg">
- <circle class="success-mark__circle" cx="26" cy="26" r="25"></circle>
- <path class="success-mark__check" d="M14 27 l8 8 l16 -18"></path>
- </svg>
- </div>
+ `@if` (!done()) {
+ <div class="spinner">
+ <div class="spinner__ring spinner__ring--outer"></div>
+ <div class="spinner__ring spinner__ring--inner"></div>
+ </div>
+ } `@else` {
+ <div class="success-mark" aria-label="Configuration complete">
+ <svg viewBox="0 0 52 52" xmlns="http://www.w3.org/2000/svg">
+ <circle class="success-mark__circle" cx="26" cy="26" r="25"></circle>
+ <path class="success-mark__check" d="M14 27 l8 8 l16 -18"></path>
+ </svg>
+ </div>
+ }
<h2 class="card__title">{{ done() ? 'Your database is ready' : 'Configuring your database' }}</h2>
<div class="card__head-detail">
- <p *ngIf="!done()" class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p>
- <div *ngIf="hasSummary()" class="summary-chips">
- <span class="summary-chip"><strong>{{ tablesConfigured().size }}</strong> {{ tablesConfigured().size === 1 ? 'table' : 'tables' }}</span>
- <span *ngIf="widgetsCount() > 0" class="summary-chip"><strong>{{ widgetsCount() }}</strong> {{ widgetsCount() === 1 ? 'widget' : 'widgets' }}</span>
- <span *ngIf="settingsCount() > 0" class="summary-chip"><strong>{{ settingsCount() }}</strong> {{ settingsCount() === 1 ? 'setting' : 'settings' }}</span>
- </div>
- <p *ngIf="done() && !hasSummary()" class="card__subtitle">All set — opening your dashboard…</p>
+ `@if` (!done()) {
+ <p class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p>
+ }
+ `@if` (hasSummary()) {
+ <div class="summary-chips">
+ <span class="summary-chip"><strong>{{ tablesConfigured().size }}</strong> {{ tablesConfigured().size === 1 ? 'table' : 'tables' }}</span>
+ `@if` (widgetsCount() > 0) {
+ <span class="summary-chip"><strong>{{ widgetsCount() }}</strong> {{ widgetsCount() === 1 ? 'widget' : 'widgets' }}</span>
+ }
+ `@if` (settingsCount() > 0) {
+ <span class="summary-chip"><strong>{{ settingsCount() }}</strong> {{ settingsCount() === 1 ? 'setting' : 'settings' }}</span>
+ }
+ </div>
+ }
+ `@if` (done() && !hasSummary()) {
+ <p class="card__subtitle">All set — opening your dashboard…</p>
+ }
</div>
- <ul class="status-log" *ngIf="!errorText(); else errorBlock">
- <li *ngFor="let m of messages(); let last = last; trackBy: trackByIndex"
- class="status-log__item"
- [class.status-log__item_current]="last"
- [class.status-log__item_done]="!last">
- <span class="status-log__icon" aria-hidden="true">
- <span *ngIf="!last" class="status-log__check">&`#10003`;</span>
- <span *ngIf="last" class="status-log__spinner"></span>
- </span>
- <span class="status-log__text">{{ m }}</span>
- </li>
- </ul>
- <ng-template `#errorBlock`>
- <p class="card__status card__status_error">{{ errorText() }}</p>
- </ng-template>
+ `@if` (!errorText()) {
+ <ul class="status-log">
+ `@for` (m of messages(); track $index; let last = $last) {
+ <li class="status-log__item"
+ [class.status-log__item_current]="last"
+ [class.status-log__item_done]="!last">
+ <span class="status-log__icon" aria-hidden="true">
+ `@if` (!last) {
+ <span class="status-log__check">&`#10003`;</span>
+ } `@else` {
+ <span class="status-log__spinner"></span>
+ }
+ </span>
+ <span class="status-log__text">{{ m }}</span>
+ </li>
+ }
+ </ul>
+ } `@else` {
+ <p class="card__status card__status_error">{{ errorText() }}</p>
+ }📝 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
| <div *ngIf="!done()" class="spinner"> | |
| <div class="spinner__ring spinner__ring--outer"></div> | |
| <div class="spinner__ring spinner__ring--inner"></div> | |
| </div> | |
| <h2 class="card__title">Configuring your database</h2> | |
| <p class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p> | |
| <div class="progress-bar"> | |
| <div class="progress-bar__fill"></div> | |
| <div *ngIf="done()" class="success-mark" aria-label="Configuration complete"> | |
| <svg viewBox="0 0 52 52" xmlns="http://www.w3.org/2000/svg"> | |
| <circle class="success-mark__circle" cx="26" cy="26" r="25"></circle> | |
| <path class="success-mark__check" d="M14 27 l8 8 l16 -18"></path> | |
| </svg> | |
| </div> | |
| <h2 class="card__title">{{ done() ? 'Your database is ready' : 'Configuring your database' }}</h2> | |
| <div class="card__head-detail"> | |
| <p *ngIf="!done()" class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p> | |
| <div *ngIf="hasSummary()" class="summary-chips"> | |
| <span class="summary-chip"><strong>{{ tablesConfigured().size }}</strong> {{ tablesConfigured().size === 1 ? 'table' : 'tables' }}</span> | |
| <span *ngIf="widgetsCount() > 0" class="summary-chip"><strong>{{ widgetsCount() }}</strong> {{ widgetsCount() === 1 ? 'widget' : 'widgets' }}</span> | |
| <span *ngIf="settingsCount() > 0" class="summary-chip"><strong>{{ settingsCount() }}</strong> {{ settingsCount() === 1 ? 'setting' : 'settings' }}</span> | |
| </div> | |
| <p *ngIf="done() && !hasSummary()" class="card__subtitle">All set — opening your dashboard…</p> | |
| </div> | |
| <ul class="status-log" *ngIf="!errorText(); else errorBlock"> | |
| <li *ngFor="let m of messages(); let last = last; trackBy: trackByIndex" | |
| class="status-log__item" | |
| [class.status-log__item_current]="last" | |
| [class.status-log__item_done]="!last"> | |
| <span class="status-log__icon" aria-hidden="true"> | |
| <span *ngIf="!last" class="status-log__check">✓</span> | |
| <span *ngIf="last" class="status-log__spinner"></span> | |
| </span> | |
| <span class="status-log__text">{{ m }}</span> | |
| </li> | |
| </ul> | |
| <ng-template #errorBlock> | |
| <p class="card__status card__status_error">{{ errorText() }}</p> | |
| </ng-template> | |
| `@if` (!done()) { | |
| <div class="spinner"> | |
| <div class="spinner__ring spinner__ring--outer"></div> | |
| <div class="spinner__ring spinner__ring--inner"></div> | |
| </div> | |
| } `@else` { | |
| <div class="success-mark" aria-label="Configuration complete"> | |
| <svg viewBox="0 0 52 52" xmlns="http://www.w3.org/2000/svg"> | |
| <circle class="success-mark__circle" cx="26" cy="26" r="25"></circle> | |
| <path class="success-mark__check" d="M14 27 l8 8 l16 -18"></path> | |
| </svg> | |
| </div> | |
| } | |
| <h2 class="card__title">{{ done() ? 'Your database is ready' : 'Configuring your database' }}</h2> | |
| <div class="card__head-detail"> | |
| `@if` (!done()) { | |
| <p class="card__subtitle">We're analyzing your structure and applying the best settings. This is running in the background.</p> | |
| } | |
| `@if` (hasSummary()) { | |
| <div class="summary-chips"> | |
| <span class="summary-chip"><strong>{{ tablesConfigured().size }}</strong> {{ tablesConfigured().size === 1 ? 'table' : 'tables' }}</span> | |
| `@if` (widgetsCount() > 0) { | |
| <span class="summary-chip"><strong>{{ widgetsCount() }}</strong> {{ widgetsCount() === 1 ? 'widget' : 'widgets' }}</span> | |
| } | |
| `@if` (settingsCount() > 0) { | |
| <span class="summary-chip"><strong>{{ settingsCount() }}</strong> {{ settingsCount() === 1 ? 'setting' : 'settings' }}</span> | |
| } | |
| </div> | |
| } | |
| `@if` (done() && !hasSummary()) { | |
| <p class="card__subtitle">All set — opening your dashboard…</p> | |
| } | |
| </div> | |
| `@if` (!errorText()) { | |
| <ul class="status-log"> | |
| `@for` (m of messages(); track $index; let last = $last) { | |
| <li class="status-log__item" | |
| [class.status-log__item_current]="last" | |
| [class.status-log__item_done]="!last"> | |
| <span class="status-log__icon" aria-hidden="true"> | |
| `@if` (!last) { | |
| <span class="status-log__check">&`#10003`;</span> | |
| } `@else` { | |
| <span class="status-log__spinner"></span> | |
| } | |
| </span> | |
| <span class="status-log__text">{{ m }}</span> | |
| </li> | |
| } | |
| </ul> | |
| } `@else` { | |
| <p class="card__status card__status_error">{{ errorText() }}</p> | |
| } |
🤖 Prompt for 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.
In `@frontend/src/app/components/auto-configure/auto-configure.component.html`
around lines 4 - 39, The template uses Angular's legacy structural directives
(*ngIf, *ngFor and "else errorBlock" plus trackBy: trackByIndex); replace them
with the project's built-in control flow: use `@if`(done()) / `@if`(!done()) blocks
instead of *ngIf(done()) and inline the error branch with `@else` rather than a
separate <ng-template>, and convert the status list to `@for`="let m of
messages(); let last = last" (or the project's `@for` syntax) instead of *ngFor
and remove the "trackBy: trackByIndex" usage (drop trackBy or implement
equivalent in the `@for` if supported). Update all occurrences referencing done(),
hasSummary(), tablesConfigured(), widgetsCount(), settingsCount(), messages(),
and errorText() to the new `@if/`@for constructs and ensure class bindings
([class.xxx]) remain applied to the items.
Comment on lines
9
to
14
| @Component({ | ||
| selector: 'app-auto-configure', | ||
| templateUrl: './auto-configure.component.html', | ||
| styleUrls: ['./auto-configure.component.css'], | ||
| imports: [MatButtonModule, RouterModule], | ||
| imports: [CommonModule, MatButtonModule, RouterModule], | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Angular 20, is the importsfield on@componentonly valid whenstandalone: true is set?
💡 Result:
In Angular 20, the imports field on the @Component decorator is only valid for standalone components [1]. Attempting to use the imports property on a component that is not marked as standalone (i.e., when standalone is false or omitted) will result in a compilation error [1]. While recent versions of Angular have moved toward making standalone the default for components—meaning the standalone: true flag is often implicit—the imports field remains strictly associated with the standalone architecture [1][2]. If a component is configured as non-standalone, it must instead be declared in an NgModule, and its dependencies must be managed through that module's imports array [2].
Citations:
Add standalone: true to AutoConfigureComponent
@Component({ imports: [...] }) is only valid for standalone components; without standalone: true, Angular will error at compile time and the component violates the repo’s standalone-component convention.
Proposed fix
`@Component`({
+ standalone: true,
selector: 'app-auto-configure',
templateUrl: './auto-configure.component.html',
styleUrls: ['./auto-configure.component.css'],
imports: [CommonModule, MatButtonModule, RouterModule],
})📝 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
| @Component({ | |
| selector: 'app-auto-configure', | |
| templateUrl: './auto-configure.component.html', | |
| styleUrls: ['./auto-configure.component.css'], | |
| imports: [MatButtonModule, RouterModule], | |
| imports: [CommonModule, MatButtonModule, RouterModule], | |
| }) | |
| `@Component`({ | |
| standalone: true, | |
| selector: 'app-auto-configure', | |
| templateUrl: './auto-configure.component.html', | |
| styleUrls: ['./auto-configure.component.css'], | |
| imports: [CommonModule, MatButtonModule, RouterModule], | |
| }) |
🤖 Prompt for 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.
In `@frontend/src/app/components/auto-configure/auto-configure.component.ts`
around lines 9 - 14, The `@Component` decorator for AutoConfigureComponent
currently lists imports (CommonModule, MatButtonModule, RouterModule) which
requires the component to be standalone; update the component decorator for
AutoConfigureComponent to include standalone: true so the imports array is valid
and the component follows the repo's standalone-component convention (i.e., add
standalone: true inside the `@Component` metadata for AutoConfigureComponent).
Comment on lines
64
to
67
| await this._configState.startConfiguring(connectionId); | ||
| // Let the user read the summary chips before navigating away. | ||
| await new Promise((resolve) => setTimeout(resolve, 1800)); | ||
| this._router.navigate(['/dashboard', connectionId]); |
There was a problem hiding this comment.
Don't redirect after a failed configuration run.
startConfiguring() already resolves false on stream/fetch failure, but ngOnInit() ignores that result and still navigates away 1.8 seconds later. That hides the error state almost immediately.
Proposed fix
- await this._configState.startConfiguring(connectionId);
+ const configured = await this._configState.startConfiguring(connectionId);
+ if (!configured) return;
// Let the user read the summary chips before navigating away.
await new Promise((resolve) => setTimeout(resolve, 1800));
this._router.navigate(['/dashboard', connectionId]);📝 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
| await this._configState.startConfiguring(connectionId); | |
| // Let the user read the summary chips before navigating away. | |
| await new Promise((resolve) => setTimeout(resolve, 1800)); | |
| this._router.navigate(['/dashboard', connectionId]); | |
| const configured = await this._configState.startConfiguring(connectionId); | |
| if (!configured) return; | |
| // Let the user read the summary chips before navigating away. | |
| await new Promise((resolve) => setTimeout(resolve, 1800)); | |
| this._router.navigate(['/dashboard', connectionId]); |
🤖 Prompt for 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.
In `@frontend/src/app/components/auto-configure/auto-configure.component.ts`
around lines 64 - 67, The code currently always waits 1.8s and navigates after
calling this._configState.startConfiguring(connectionId); change ngOnInit (or
wherever startConfiguring is called) to capture its return value (e.g., const
success = await this._configState.startConfiguring(connectionId);) and only
perform the await new Promise(... 1800) and this._router.navigate(['/dashboard',
connectionId]) when success is truthy; if success is false, do not navigate and
let the component remain to show the error state.
Comment on lines
16
to
+19
| private _configuring = new BehaviorSubject<Set<string>>(new Set()); | ||
|
|
||
| /** Start configuring — fires the API and manages state. Returns a promise that resolves when setup completes. */ | ||
| /** Latest progress event from the streaming setup endpoint (null when idle). */ | ||
| public progress$ = new BehaviorSubject<ConfigProgress | null>(null); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Angular 20, what is the recommended way to expose HTTP-backed reactive state from a service with signals or rxResource(), and when is a BehaviorSubject still appropriate?
💡 Result:
In Angular 20, the recommended approach for exposing HTTP-backed reactive state is to use the Resource API, specifically resource() or rxResource() [1][2][3]. These primitives are designed to integrate asynchronous server state directly into Angular’s signal-based reactivity model, eliminating the need for manual subscription management, loading state handling, or boilerplate RxJS pipelines [1][3]. Recommended Approaches: - Use resource() when your data-fetching logic is Promise-based (e.g., standard fetch or HttpClient with firstValueFrom) [2][3]. - Use rxResource() when you are working with observable streams, such as WebSockets, Server-Sent Events, or complex RxJS pipelines [4][3]. - Both APIs provide reactive wrappers that expose value, loading, and error signals, allowing your UI to react automatically to changes in parameters without requiring the async pipe or manual cleanup [1][3]. When is BehaviorSubject still appropriate? While the trend is to use signals for UI and local state [5][6], BehaviorSubject remains a valid and powerful tool in Angular 20 for specific scenarios: - Complex Asynchronous Orchestration: If your logic involves heavy event composition, sophisticated retry strategies, time-based operators (e.g., debounceTime, switchMap, zip), or managing multiple coordinated async streams, RxJS is often more ergonomic than signals [1][5][6]. - Legacy Integration: If you are working within a codebase heavily invested in RxJS patterns or interacting with legacy services that already rely on observable streams, BehaviorSubject is the appropriate choice [5][6][7]. - Multicast Events: If you have a requirement for a stream that multiple independent components need to subscribe to, where late subscribers must receive the "current" state upon initialization, BehaviorSubject provides a robust, established pattern [7][8][9]. Architectural Boundary: The modern Angular best practice is to treat RxJS as an integration layer for complex async flows and external data, while using Signals for presentation and local reactive state [5][6]. You can bridge these worlds using toSignal() (to convert RxJS to Signals) or toObservable() (to use Signal values in an RxJS pipe) when necessary [4][6]. Stop using BehaviorSubject as a default for simple component-level state, as signal() is more performant and requires significantly less boilerplate [6][7].
Citations:
- 1: https://medium.com/@marcomatto/angular-22-embracing-the-signal-first-era-b1c8803acea6
- 2: https://stackoverflow.com/questions/79432612/upto-angular-18-what-were-the-different-methods-of-handling-asynchronous-api-cal
- 3: https://javascript.plainenglish.io/resource-and-rxresource-in-angular-19-use-cases-comparison-with-angular-query-and-72a01eea172b
- 4: https://angular.dev/ecosystem/rxjs-interop
- 5: https://dev.to/aleksandr_gusev_it/should-angular-apps-still-rely-on-rxjs-in-2025-92p
- 6: https://www.c-sharpcorner.com/article/behaviorsubject-vs-signals-in-angular/
- 7: https://medium.com/@lielshani/stop-using-behaviorsubject-in-every-angular-service-theres-a-better-way-27a93c8b0cd4
- 8: https://javascript.plainenglish.io/the-difference-between-subject-and-behaviorsubject-in-rxjs-with-angular-a-comprehensive-guide-9e362cab45d9
- 9: https://dev.to/mariazayed/subjects-and-behaviorsubjects-in-angular-a-deep-dive-4kf2
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/app/services/configuration-state.service.ts"
echo "=== File: $FILE ==="
test -f "$FILE" && echo "FOUND" || { echo "NOT FOUND: $FILE"; exit 1; }
echo "=== BehaviorSubject usages ==="
rg -n "BehaviorSubject" "$FILE" || true
echo "=== Search for complete/protocol flags ==="
rg -n "complete|Completed|COMPLETE" "$FILE" || true
echo "=== Service shape + relevant sections (lines 1-240) ==="
nl -ba "$FILE" | sed -n '1,240p'Repository: rocket-admin/rocketadmin
Length of output: 971
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/app/services/configuration-state.service.ts"
echo "=== Show key ranges (no nl) ==="
echo "--- lines 1-120 ---"
sed -n '1,120p' "$FILE" | cat
echo "--- lines 120-220 ---"
sed -n '120,220p' "$FILE" | cat
echo "--- lines 220-320 ---"
sed -n '220,320p' "$FILE" | cat
echo "=== Where progress$ is mutated ==="
rg -n "progress\$|_configuring" "$FILE"
echo "=== Where shared queue/timer fields are defined/used ==="
rg -n "_queue|_drainTimer|_currentDrainIntervalMs|_runSetup|startConfiguring" "$FILE"Repository: rocket-admin/rocketadmin
Length of output: 7484
Refactor ConfigurationStateService to remove BehaviorSubject and fix concurrent/protocol-completion semantics
The service currently models HTTP-streamed progress with shared BehaviorSubjects:
private _configuring = new BehaviorSubject<Set<string>>(new Set());
/** Latest progress event from the streaming setup endpoint (null when idle). */
public progress$ = new BehaviorSubject<ConfigProgress | null>(null);- Replace
BehaviorSubjectstate with Angular signals /rxResource()per the frontend/service conventions; consumers shouldn’t need to manually subscribe to subjects for service state. startConfiguring()allows differentconnectionIds concurrently, but_runSetup()mutates shared singletons (_queue,_drainTimer,_currentDrainIntervalMs, andprogress$). Starting a second run will clear/stop the first run’s queue/drain and interleave or clobber progress.- “complete” is only used to change drain pacing:
_runSetup()returnstrueafter the stream ends and_waitForDrain()completes even if no{ type: 'complete' }frame was ever received. Track per-connection completion and require it for success.
🤖 Prompt for 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.
In `@frontend/src/app/services/configuration-state.service.ts` around lines 16 -
19, Replace the shared BehaviorSubject-based state in ConfigurationStateService
(_configuring and progress$) with Angular signals or rxResource per
frontend/service conventions so consumers read reactive state without
subscribing to Subjects; change startConfiguring() and _runSetup() to allocate
per-connection run state (separate _queue, _drainTimer, _currentDrainIntervalMs)
rather than mutating singletons so concurrent connectionIds do not clobber each
other; ensure _runSetup() and _waitForDrain() maintain a per-connection
"complete" flag and only return success when a `{ type: 'complete' }` frame is
actually received for that connection (do not treat stream end alone as
success); update callers to use the new signals/rxResource instead of
subscribing to progress$/_configuring.
Comment on lines
+19
to
+27
| public progress$ = new BehaviorSubject<ConfigProgress | null>(null); | ||
|
|
||
| private _queue: ConfigProgress[] = []; | ||
| private _drainTimer: ReturnType<typeof setInterval> | null = null; | ||
| private _currentDrainIntervalMs = 400; | ||
| private readonly _drainIntervalMsNormal = 400; | ||
| private readonly _drainIntervalMsFast = 80; | ||
| private readonly _drainCapAfterCompleteMs = 1500; | ||
|
|
There was a problem hiding this comment.
This singleton progress state can corrupt concurrent setups.
startConfiguring() only blocks duplicate starts for the same connectionId, so different IDs can run in parallel. But _queue, _drainTimer, _currentDrainIntervalMs, and progress$ are shared service-wide, so starting a second setup can reset the first run's drain and mix both pages onto the same event stream. Because progress$ is sticky, a fresh subscriber can also see the previous run's terminal event before the next stream emits anything.
Also applies to: 29-36
🤖 Prompt for 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.
In `@frontend/src/app/services/configuration-state.service.ts` around lines 19 -
27, The service currently uses shared fields (_queue, _drainTimer,
_currentDrainIntervalMs, progress$) so concurrent startConfiguring() calls for
different connectionId collide; change the implementation to maintain
per-connection state keyed by connectionId (e.g., a Map<connectionId, { queue,
drainTimer, currentDrainIntervalMs, progressSubject }>) so startConfiguring(),
enqueue progress and the drain logic operate on that connection's state only,
create a fresh BehaviorSubject/Subject for each connection instead of a single
progress$, and ensure cleanup (clearInterval and delete map entry) when a run
completes so terminal events don’t leak to new subscribers.
Comment on lines
+63
to
+85
| if (response.body) { | ||
| const reader = response.body.getReader(); | ||
| const decoder = new TextDecoder(); | ||
| let buffer = ''; | ||
| while (true) { | ||
| const { done, value } = await reader.read(); | ||
| if (done) { | ||
| const tail = buffer.trim(); | ||
| if (tail) this._parseAndEnqueue(tail); | ||
| break; | ||
| } | ||
| buffer += decoder.decode(value, { stream: true }); | ||
| let newlineIdx: number; | ||
| while ((newlineIdx = buffer.indexOf('\n')) >= 0) { | ||
| const line = buffer.slice(0, newlineIdx).trim(); | ||
| buffer = buffer.slice(newlineIdx + 1); | ||
| if (!line) continue; | ||
| this._parseAndEnqueue(line); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| await this._waitForDrain(); |
There was a problem hiding this comment.
Don't report success unless the stream actually completes.
_runSetup() returns true whenever the fetch finishes without throwing, even if response.body is missing or the stream closes before a complete frame arrives. That turns protocol failures into a success snackbar and redirect. Track whether a complete event was parsed and fail cleanly on EOF without it.
Also applies to: 102-116
🤖 Prompt for 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.
In `@frontend/src/app/services/configuration-state.service.ts` around lines 63 -
85, The stream-handling in _runSetup() currently treats any successful fetch as
success even if no 'complete' frame was received; update _runSetup() to track
whether a completion event was actually parsed (e.g., add a local seenComplete
boolean) and only consider the setup successful when seenComplete is true. To do
this, modify _parseAndEnqueue(...) to signal when it parsed a 'complete' frame
(either by returning a boolean/enum or by calling a new method like
this._markSetupComplete()), set seenComplete when that signal is received while
reading lines from response.body, and after the reader finishes (and before
resolving success / showing the success snackbar / redirect) fail cleanly
(return false or throw) if seenComplete is still false. Ensure the same change
is applied to the other stream-reading block referenced (lines 102-116) so EOF
without a 'complete' frame is handled consistently.
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
3 participants
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
Reworks the Configure all loader (
/auto-configure/:connection-id) so the user actually sees what the AI setup is doing, instead of staring at a generic spinner for ~20 seconds.ConfigurationStateServicenow readsGET /ai/v2/setup/:idas aReadableStream, parses newline-delimited{ type: 'message' | 'complete' | 'error', text?, message? }chunks. BypassesHttpClient(needed for stream access) and injects themasterpwdheader thatTokenInterceptorwould normally add.complete, the drain switches to 80 ms with a 1.5 s cap so the final widget/setting messages flash through quickly instead of dragging out.Set up settings for table "X", display_name parameter set to "Y"→Renamed X → Y;Added Phone widget for table "X" on column "Y"→X.Y formatted as phone numbers, etc.). A generic fallback turns unknown settings parameters intoX: <param> configured.N tables · M widgets · K settings. Counts are derived from the same backend messages.Test plan
complete: spinner → checkmark, title → "Your database is ready", green chips appear with non-zero counts/dashboard/:idmasterpwdin localStorage, fetch succeeds (no 400 due to missing header)🤖 Generated with Claude Code
Summary by CodeRabbit