mobile: widgets page, AI panel, sidebar polish + quick-filter chips#1822
Open
karinakharchenko wants to merge 13 commits into
Open
mobile: widgets page, AI panel, sidebar polish + quick-filter chips#1822karinakharchenko wants to merge 13 commits into
karinakharchenko wants to merge 13 commits into
Conversation
…nator polish - Add a "+" / "Create fast filter" trigger after the Filter button on mobile, with a vertical divider in between - Horizontally scroll the saved-filter chips inside the search row (custom chip render, absolutely positioned strip so it can't push the page wide) - Each chip has a three-dots menu (Edit / Delete) wired to the saved-filters-panel handlers via @ViewChild - Hide the panel's own mobile dropdown/list — its conditions editor remains visible below the row when a fast filter is selected - When conditions show, the panel takes the full second row (:has) and Columns/Sort stay right-aligned on the row above - Tighten spacing between conditions and the Columns/Sort row; align the "where" label and chips vertically (min-height: 32px) - Make Sort by default black/white, accent only when sort is active; pull the swap_vert icon tight to the label - Inline preview divider only spans the text area (::after with 20px insets) instead of edge-to-edge - Paginator on mobile: items-per-page flush left, range/buttons right, single row, "Rows per page:" label, looser select for readability - Drop the trailing period from the "No records match…" empty-state strings - Open the filter dialog as a mobile bottom sheet when the active filter chip is tapped (was a centered modal) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Widgets page mobile layout: stacked widget cards (display:contents → block), header with arrow-back + "Widgets" / table-name stack, hide breadcrumbs and per-card delete button, drop the bottom "Back" button (arrow-back covers it), Save aligns right - Widgets empty state mobile: hide right-side widget cards grid, hide divider and the tune icon, symmetric padding, Docs link + "Create first widget" button right-aligned together, breadcrumbs hidden, extra bottom buffer so Create doesn't collide with the version footer - AI panel: full-size only on mobile (removed expand/collapse toggle and the +175px expanded padding), trim "AI insights for <table>" to plain "AI insights" so the title doesn't push out, FAB Add row hides while the chat is open, and on mobile the chat panel state is reset on each table view init (so navigating away doesn't reopen the chat on return) - Sidebar drawer mobile: sits below the 44px nav-bar (with backdrop offset), hide the dark mat-toolbar, "Connections" hidden when already inside a connection, "Hosted databases" added between Company and Secrets, account-section items (.account-section-item) get smaller text + icons, drop the connection-nav top border, Upgrade button nudged up - Connections list dropdown: divider + "Hosted databases" link at the bottom - Logo: short variant only on mobile when inside a connection (connections-list, account, hosted-databases pages keep the full wordmark) - Row edit form: drop the leftover .widget grid column reservation so inputs fill width - Mobile loading skeleton: dedicated card-shape placeholder for ≤600px, no left label column, no elevation around the wrapper - Row preview dividers: inset 20px each side via ::after so they don't reach the popup edges - Filter dialog when tapping an active filter chip: opens as the mobile bottom sheet (panelClass: 'mobile-bottom-sheet-dialog') Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMobile-focused frontend updates: navigation/account menu restructure, responsive table/widgets/audit views, quick/saved filters with mobile chips, AI panel default-open, auto-configure streaming + live status UI, and backend additions: createdAt columns, time-limited repo lookups, a Mongo aggregation read-only guard, and a migration. ChangesMobile-First Dashboard and Navigation Enhancements
Auto-Configure Streaming & UI
Backend: Safety, Timestamps, and Migration
Sequence Diagram(s)sequenceDiagram
participant UserUI
participant AutoConfigureComponent
participant ConfigurationStateService
participant SetupEndpoint
participant ProgressQueue
participant ProgressSubject
UserUI->>AutoConfigureComponent: trigger start configuring
AutoConfigureComponent->>ConfigurationStateService: startConfiguring(connectionId)
ConfigurationStateService->>SetupEndpoint: fetch setup stream (masterpwd header)
SetupEndpoint-->>ConfigurationStateService: SSE-style data chunks
ConfigurationStateService->>ProgressQueue: enqueue parsed progress events
ProgressQueue->>ProgressSubject: drain -> emit ConfigProgress
ProgressSubject-->>AutoConfigureComponent: progress$ updates
AutoConfigureComponent->>UserUI: update status log / summary chips
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 unit tests (beta)
⚔️ Resolve merge conflicts
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/app/components/db-table-row-edit/db-table-row-edit.component.css (1)
191-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep a layout context for
.widget-info.Changing
.widgettodisplay: blockdrops the grid/flex context that.widget-info { margin-left: auto; }and.widget-info_centered { align-self: center; }depend on, so those controls stop aligning as intended. If the goal is a stacked layout, make the container a column flexbox instead of a plain block.Suggested fix
.widget { - display: block; + display: flex; + flex-direction: column; }🤖 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/db-table-row-edit/db-table-row-edit.component.css` around lines 191 - 202, The change to .widget removed its flex/grid context so .widget-info and .widget-info_centered no longer align; restore a layout context by making .widget a flex container with column flow (e.g., set display: flex and flex-direction: column) so stacked layout is preserved while margin-left: auto and align-self: center on .widget-info and .widget-info_centered continue to work; update the .widget rule (referenced as .widget) accordingly and verify .widget-info and .widget-info_centered keep their existing properties.frontend/src/app/components/skeletons/placeholder-table-data/placeholder-table-data.component.ts (1)
4-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
standalone: truetoPlaceholderTableDataComponent(frontend/.../placeholder-table-data.component.ts)
This component uses decorator-scopedimports: [NgForOf], which requiresstandalone: true; without it Angular metadata is invalid.Suggested fix
`@Component`({ selector: 'app-placeholder-table-data', + standalone: true, templateUrl: './placeholder-table-data.component.html', styleUrls: ['./placeholder-table-data.component.css'], imports: [NgForOf], }) export class PlaceholderTableDataComponent {🤖 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/skeletons/placeholder-table-data/placeholder-table-data.component.ts` around lines 4 - 12, The component decorator for PlaceholderTableDataComponent is missing standalone: true while using decorator-scoped imports (NgForOf); update the `@Component` metadata to include standalone: true so the NgForOf import is valid and Angular metadata is correct, leaving the existing imports: [NgForOf] and the class members numberOfDivs and numberOfCards unchanged.
🧹 Nitpick comments (3)
frontend/src/app/app.component.html (1)
9-9: ⚡ Quick winUse Angular’s built-in control flow in the new template branches.
The new conditions are still using
*ngIf. Please switch these added branches to@ifto match the repository rule for new component templates.As per coding guidelines, "Use Angular 19's built-in control flow (
@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code".Also applies to: 53-53, 99-99
🤖 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/app.component.html` at line 9, Replace the legacy structural directives with Angular 19’s built-in control flow by changing the template branches that use *ngIf to the `@if` form: locate the anchor with *ngIf="!connectionID" (the link with routerLink="/connections-list") and the other two occurrences tied to the same component state (references to connectionID at the other branches), and convert each to use the `@if` control-flow syntax so the conditions use Angular’s new built-in control flow instead of *ngIf.frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts (1)
184-184: ⚡ Quick winReplace
anyfor the quick-filter menu state.
chipFilterForMenuis new state that flows into edit/delete actions, soanyremoves the only type check on that contract. Give it the saved-filter item type (or a narrow local interface) and keep the nullability explicit.As per coding guidelines, "Avoid any types - use specific types or generics instead".
🤖 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/dashboard/db-table-view/db-table-view.component.ts` at line 184, Replace the use of any for the quick-filter menu state by typing chipFilterForMenu as a specific saved-filter type (e.g., SavedFilterItem | null) or a narrow local interface that matches the saved filter contract used by the edit/delete handlers; update the declaration of chipFilterForMenu (in db-table-view.component.ts) from any to that explicit type and keep nullability, and adjust any call sites that assume any to the chosen interface so the compiler enforces the expected properties for the edit/delete actions.frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html (1)
101-150: ⚡ Quick winUse Angular’s built-in control flow for this new template block.
This whole quick-filter section is new code, but it introduces fresh
*ngIf/*ngForusage instead of@if/@for, which diverges from the repo’s Angular template rule.As per coding guidelines, "Use Angular 19's built-in control flow (
@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code".🤖 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/dashboard/db-table-view/db-table-view.component.html` around lines 101 - 150, Replace all structural directives in the new quick-filter block with Angular 19 control-flow syntax: convert *ngIf checks (e.g., those using canEditConnection(), tableData.structure, isMobileView, savedFiltersPanel?.savedFilterData?.length) into `@if` blocks and convert the *ngFor over savedFiltersPanel?.savedFilterData into an `@for` loop, preserving the same bindings and event handlers (handleOpenCreateQuickFilter, posthog.capture, savedFiltersPanel?.selectFiltersSet, chipFilterForMenu assignment, and the mat-menu quickChipMenu interactions including savedFiltersPanel?.handleEditFilter and savedFiltersPanel?.handleDeleteFilter); ensure class bindings ([class.search-row__quick-chip_active]) and (click) stopPropagation logic remain unchanged when migrating to the `@if/`@for form.
🤖 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/app.component.css`:
- Around line 44-46: Remove the deep selector from the component stylesheet:
delete the ::ng-deep token from the rule in .main-menu-container ::ng-deep
.mat-drawer-backdrop.mat-drawer-shown and move that selector (without ::ng-deep)
into your global stylesheet so it applies outside Angular view encapsulation;
keep the rule body (top: 44px !important;) and ensure the selector is exactly
.main-menu-container .mat-drawer-backdrop.mat-drawer-shown in the global styles
to avoid stylelint error (do not disable the lint rule).
In `@frontend/src/app/app.component.html`:
- Around line 39-46: The desktop account menu (`#accountMenu`) is missing the new
Hosted databases router entry and the anchor lacks the account-section-item
class; add an <a> element with routerLink="/hosted-databases" and
routerLinkActive="nav-bar__button_active" into the `#accountMenu` account rows
(placing it between the Company and Secrets items to match mobile sidenav order)
and ensure the anchor includes the account-section-item class so the compact
sizing in app.component.css applies; update the existing anchor (the one already
added to the mobile sidenav) or duplicate its attributes (including
mat-list-item, aria-label="Hosted databases", mat-icon and matListItemTitle)
into the `#accountMenu` markup.
- Line 59: The external Help center anchor element with class
"account-section-item" that uses target="_blank" should include rel="noopener
noreferrer" to prevent exposing window.opener; update the <a> element (the
anchor with href="https://docs.rocketadmin.com/" and class
"account-section-item") to add rel="noopener noreferrer" alongside the existing
target attribute.
- Around line 63-66: Replace the clickable plain mat-list-item with a real
button element to restore native button semantics and keyboard interaction:
locate the logout entry using the account-section-item class and data-testid
"logout-button-account-menu" and change it to a button with the mat-list-item
attribute, keep the existing (click)="logOut()" handler and the child mat-icon
and title, and ensure the button has type="button" and any existing testid/aria
attributes preserved so behavior and tests remain the same.
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.ts`:
- Around line 100-101: The subscription to _tableState.aiPanelExpandedCast
currently ignores the emitted boolean and always sets isExpanded to true; change
the callback to accept the emitted value (e.g., expansion: boolean) and assign
it to this.isExpanded so false emissions collapse the panel (you can keep the
initial default true elsewhere but let aiPanelExpandedCast drive the state via
the subscribe handler on _tableState.aiPanelExpandedCast).
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html`:
- Around line 125-136: Replace the non-focusable clickable div with a semantic
button so each quick-filter chip is keyboard and screen-reader operable: render
the per-filter element using a <button type="button"> (keeping classes:
search-row__quick-chip and conditional search-row__quick-chip_active using
savedFiltersPanel?.selectedFilterSetId === f.id), wire its (click) to
savedFiltersPanel?.selectFiltersSet(f.id), and keep the overflow menu as the
separate button that still stops propagation and sets chipFilterForMenu = f;
ensure the menu button retains type="button" and existing [matMenuTriggerFor]
and canEditConnection() visibility logic.
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.html`:
- Around line 6-14: The mobile header (<div class="mobile-header">) is rendered
even when widgets is an empty array because the template checks the truthiness
of widgets; change the visibility condition to explicitly check for non-empty
(e.g., use widgets?.length > 0 or equivalent) or tie it to the same emptyState
guard used at line 55 so the mobile header is only shown when widgets has items
and is not the empty-state; update the *ngIf on the mobile-header block
(referencing widgets and emptyState) accordingly.
- Line 7: The routerLink currently interpolates path segments as a single string
on the anchor element with class "mobile-header__back" (the <a mat-icon-button
... routerLink="...">), which bypasses per-segment encoding; change it to the
array form by binding routerLink to an array of segments (['/dashboard',
connectionID, tableName]) on that anchor, and apply the same change to any other
back-link anchors using the interpolated routerLink pattern to ensure proper
per-segment URL encoding.
---
Outside diff comments:
In
`@frontend/src/app/components/db-table-row-edit/db-table-row-edit.component.css`:
- Around line 191-202: The change to .widget removed its flex/grid context so
.widget-info and .widget-info_centered no longer align; restore a layout context
by making .widget a flex container with column flow (e.g., set display: flex and
flex-direction: column) so stacked layout is preserved while margin-left: auto
and align-self: center on .widget-info and .widget-info_centered continue to
work; update the .widget rule (referenced as .widget) accordingly and verify
.widget-info and .widget-info_centered keep their existing properties.
In
`@frontend/src/app/components/skeletons/placeholder-table-data/placeholder-table-data.component.ts`:
- Around line 4-12: The component decorator for PlaceholderTableDataComponent is
missing standalone: true while using decorator-scoped imports (NgForOf); update
the `@Component` metadata to include standalone: true so the NgForOf import is
valid and Angular metadata is correct, leaving the existing imports: [NgForOf]
and the class members numberOfDivs and numberOfCards unchanged.
---
Nitpick comments:
In `@frontend/src/app/app.component.html`:
- Line 9: Replace the legacy structural directives with Angular 19’s built-in
control flow by changing the template branches that use *ngIf to the `@if` form:
locate the anchor with *ngIf="!connectionID" (the link with
routerLink="/connections-list") and the other two occurrences tied to the same
component state (references to connectionID at the other branches), and convert
each to use the `@if` control-flow syntax so the conditions use Angular’s new
built-in control flow instead of *ngIf.
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-view.component.html`:
- Around line 101-150: Replace all structural directives in the new quick-filter
block with Angular 19 control-flow syntax: convert *ngIf checks (e.g., those
using canEditConnection(), tableData.structure, isMobileView,
savedFiltersPanel?.savedFilterData?.length) into `@if` blocks and convert the
*ngFor over savedFiltersPanel?.savedFilterData into an `@for` loop, preserving the
same bindings and event handlers (handleOpenCreateQuickFilter, posthog.capture,
savedFiltersPanel?.selectFiltersSet, chipFilterForMenu assignment, and the
mat-menu quickChipMenu interactions including
savedFiltersPanel?.handleEditFilter and savedFiltersPanel?.handleDeleteFilter);
ensure class bindings ([class.search-row__quick-chip_active]) and (click)
stopPropagation logic remain unchanged when migrating to the `@if/`@for form.
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts`:
- Line 184: Replace the use of any for the quick-filter menu state by typing
chipFilterForMenu as a specific saved-filter type (e.g., SavedFilterItem | null)
or a narrow local interface that matches the saved filter contract used by the
edit/delete handlers; update the declaration of chipFilterForMenu (in
db-table-view.component.ts) from any to that explicit type and keep nullability,
and adjust any call sites that assume any to the chosen interface so the
compiler enforces the expected properties for the edit/delete actions.
🪄 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: 22f68bf7-b2ad-43cc-920b-9a817c1adcde
📒 Files selected for processing (19)
frontend/src/app/app.component.cssfrontend/src/app/app.component.htmlfrontend/src/app/app.component.tsfrontend/src/app/components/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.cssfrontend/src/app/components/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.htmlfrontend/src/app/components/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.tsfrontend/src/app/components/dashboard/db-table-view/db-table-row-view/db-table-row-view.component.cssfrontend/src/app/components/dashboard/db-table-view/db-table-view.component.cssfrontend/src/app/components/dashboard/db-table-view/db-table-view.component.htmlfrontend/src/app/components/dashboard/db-table-view/db-table-view.component.tsfrontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.cssfrontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.htmlfrontend/src/app/components/dashboard/db-table-view/db-table-widgets/widgets-empty-state/widgets-empty-state.component.cssfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.cssfrontend/src/app/components/db-table-row-edit/db-table-row-edit.component.cssfrontend/src/app/components/skeletons/placeholder-table-data/placeholder-table-data.component.cssfrontend/src/app/components/skeletons/placeholder-table-data/placeholder-table-data.component.htmlfrontend/src/app/components/skeletons/placeholder-table-data/placeholder-table-data.component.tsfrontend/src/styles.scss
Comment on lines
+44
to
+46
| .main-menu-container ::ng-deep .mat-drawer-backdrop.mat-drawer-shown { | ||
| top: 44px !important; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the referenced snippet with line numbers
echo "=== app.component.css: lines around 44-46 ==="
nl -ba frontend/src/app/app.component.css | sed -n '35,60p'
# Confirm how many ::ng-deep occurrences exist in this file
echo "=== Count of ::ng-deep in app.component.css ==="
rg -n "::ng-deep" frontend/src/app/app.component.css || true
# Find all occurrences of ::ng-deep in the frontend (to see if there’s an established pattern)
echo "=== Count of ::ng-deep in frontend ==="
rg -n "::ng-deep" frontend | head -n 50 || true
# Locate Stylelint config files (common locations)
echo "=== Locate stylelint config ==="
ls -la | rg -n "stylelint|\.stylelintrc|stylelint\.json|\.stylelint" || true
find . -maxdepth 4 -type f \( -iname "*stylelint*" -o -iname ".stylelintrc*" -o -iname "*.stylelintrc*" \) | head -n 50 || true
# Print likely stylelint config contents if small; otherwise just grep relevant rules
echo "=== Search stylelint config for ng-deep / deep selectors / unknown pseudo-element ==="
CONFIGS=$(find . -maxdepth 6 -type f \( -iname "*stylelint*" -o -iname ".stylelintrc*" -o -iname "*.stylelintrc*" \) 2>/dev/null | tr '\n' ' ')
if [ -z "${CONFIGS// /}" ]; then
echo "No stylelint config files found within depth."
else
# Grep for deep selectors or pseudo-element related rules
rg -n "ng-deep|::ng-deep|deep\s+selector|pseudo-element|unknown" -S $CONFIGS || true
fiRepository: rocket-admin/rocketadmin
Length of output: 175
🏁 Script executed:
#!/bin/bash
set -u
FILE="frontend/src/app/app.component.css"
echo "=== app.component.css: lines around 44-46 ==="
python3 - <<'PY'
import itertools
path="frontend/src/app/app.component.css"
start=35
end=60
for i,line in enumerate(open(path,'r',encoding='utf-8',errors='ignore'), start=1):
if start <= i <= end:
print(f"{i:4d} {line.rstrip()}")
if i > end:
break
PY
echo "=== Count of ::ng-deep in app.component.css ==="
rg -n "::ng-deep" "$FILE" || true
echo "=== Search stylelint config files (shallow) ==="
find . -maxdepth 6 -type f \( -iname "*stylelint*" -o -iname ".stylelintrc*" -o -iname "*.stylelintrc*" \) 2>/dev/null | head -n 50 || true
echo "=== Search stylelint configs for ::ng-deep / deep selector rules ==="
CONFIGS="$(find . -maxdepth 6 -type f \( -iname "*stylelint*" -o -iname ".stylelintrc*" -o -iname "*.stylelintrc*" \) 2>/dev/null)"
if [ -z "${CONFIGS}" ]; then
echo "No stylelint config files found within depth."
else
# Grep (via rg) across configs only
echo "$CONFIGS" | tr ' ' '\n' | xargs -I{} sh -c 'rg -n "ng-deep|::ng-deep|deep selector|pseudo-element|selector-pseudo-element-no-unknown" "{}" || true' | head -n 80
fiRepository: rocket-admin/rocketadmin
Length of output: 1145
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/app/app.component.css"
echo "=== Git diff for app.component.css (if available) ==="
git diff --stat -- "$FILE" || true
git diff -- "$FILE" | sed -n '1,160p' || true
echo "=== Show .stylelintrc.json ==="
cat .stylelintrc.json
echo "=== Stylelint rules related to pseudo-elements/unknown selectors ==="
rg -n "selector-pseudo-element-no-unknown|selector-pseudo-element-colon-notation|pseudo-element|ng-deep|::ng-deep" .stylelintrc.json || true
echo "=== Search other stylelint configs referencing pseudo-element rules ==="
rg -n "selector-pseudo-element-no-unknown|selector-pseudo-element-colon-notation|pseudo-element" -S . || true
echo "=== Count occurrences of ::ng-deep in the repo and list files ==="
rg -n "::ng-deep" frontend | cut -d: -f1 | sort -uRepository: rocket-admin/rocketadmin
Length of output: 6033
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for Stylelint config overrides/suppressions
rg -n "selector-pseudo-element-no-unknown|selector-pseudo-element-colon-notation|stylelint.*ng-deep|ng-deep.*stylelint" -S . || true
# Check how Stylelint is invoked (CI scripts / package.json)
if [ -f package.json ]; then
cat package.json | rg -n "\"stylelint\"|lint|styles" -n || true
fi
# Find likely stylelint config usage (common paths)
find . -maxdepth 4 -type f \( -iname "*lint*" -o -iname "*stylelint*" \) | head -n 50 || true
# Show any additional stylelint configs if present
find . -type f \( -iname ".stylelintrc*" -o -iname "*stylelint*.json" -o -iname "*stylelint*.rc*" \) 2>/dev/null | head -n 50 || trueRepository: rocket-admin/rocketadmin
Length of output: 208
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Root package.json scripts (stylelint-related) ==="
if [ -f package.json ]; then
python3 - <<'PY'
import json
p="package.json"
with open(p,'r',encoding='utf-8') as f: d=json.load(f)
scripts=d.get("scripts",{})
for k,v in scripts.items():
if "stylelint" in k.lower() or "stylelint" in str(v).lower():
print(k,":",v)
PY
else
echo "No root package.json"
fi
echo "=== Frontend package.json scripts (stylelint-related) ==="
if [ -f frontend/package.json ]; then
python3 - <<'PY'
import json, pathlib
p="frontend/package.json"
with open(p,'r',encoding='utf-8') as f: d=json.load(f)
scripts=d.get("scripts",{})
for k,v in scripts.items():
if "stylelint" in k.lower() or "stylelint" in str(v).lower():
print(k,":",v)
PY
else
echo "No frontend/package.json"
fi
echo "=== Repo-wide: where stylelint is referenced ==="
rg -n "stylelint" -S package.json frontend/package.json . || trueRepository: rocket-admin/rocketadmin
Length of output: 229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Repo-wide search for 'stylelint' (all files) ==="
rg -n "stylelint" -S . || true
echo "=== Search CI/workflows for stylelint ==="
find . -maxdepth 4 -type f \( -path "./.github/*" -o -path "./.gitlab/*" \) -print | head -n 200 || true
rg -n "stylelint" -S .github .gitlab 2>/dev/null || true
echo "=== Frontend package.json contents (to confirm scripts) ==="
ls -la frontend/package.json || true
sed -n '1,120p' frontend/package.json 2>/dev/null || true
echo "=== Root package.json contents (to confirm scripts) ==="
ls -la package.json || true
sed -n '1,120p' package.json 2>/dev/null || true
echo "=== Any lint-staged / precommit configs referencing stylelint ==="
ls -la | rg -n "lintstaged|husky|precommit|commitlint|lefthook|prettier|stylelint" || true
find . -maxdepth 3 -type f \( -iname "*husky*" -o -iname "*lint-staged*" -o -iname "*lefthook*" -o -iname "*pre-commit*" -o -iname "*lintstaged*" \) -print 2>/dev/null || true
rg -n "stylelint" -S .husky* .lintstaged* .lintstagedrc* .pre-commit* .lefthook* 2>/dev/null || trueRepository: rocket-admin/rocketadmin
Length of output: 5357
🌐 Web query:
stylelint-config-standard-scss selector-pseudo-element-no-unknown default enabled
💡 Result:
The rule selector-pseudo-element-no-unknown is enabled by default in stylelint-config-standard-scss [1][2]. This is because stylelint-config-standard-scss extends the standard Stylelint configuration (stylelint-config-standard), which includes core Stylelint rules such as selector-pseudo-element-no-unknown to enforce standard CSS practices [1][3][2]. If this rule causes issues in your project (for example, by flagging valid SCSS-specific selectors or non-standard vendor-prefixed pseudo-elements), you can disable or customize it by adding it to the rules section of your Stylelint configuration file [1][4][3]. Example of how to disable the rule: { "extends": "stylelint-config-standard-scss", "rules": { "selector-pseudo-element-no-unknown": null } } Example of how to allow specific pseudo-elements: { "extends": "stylelint-config-standard-scss", "rules": { "selector-pseudo-element-no-unknown": [true, { "ignorePseudoElements": ["your-custom-pseudo-element"] }] } }
Citations:
- 1: https://registry.npmjs.org/stylelint-config-standard-scss
- 2: https://stylelint.io/user-guide/rules
- 3: bug: combination of selector-pseudo-element-colon-notation selector-pseudo-element-no-unknown stylelint/stylelint#2208
- 4: https://stylelint.io/user-guide/rules/selector-pseudo-element-no-unknown
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== .husky/pre-commit ==="
if [ -f .husky/pre-commit ]; then
cat .husky/pre-commit
else
echo "Missing .husky/pre-commit"
fi
echo "=== Find stylelint usage in GitHub workflows ==="
rg -n "stylelint|ng lint|lint:" .github/workflows -S || true
echo "=== Show likely front-end quality workflow files ==="
for f in .github/workflows/frontend.yml .github/workflows/code-quality.yml .github/workflows/build.yml .github/workflows/build-agent.yml; do
if [ -f "$f" ]; then
echo "---- $f ----"
sed -n '1,220p' "$f"
fi
doneRepository: rocket-admin/rocketadmin
Length of output: 12806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether stylelint is a declared dependency anywhere
echo "=== Search for stylelint in package manifests ==="
rg -n "\"stylelint\"|stylelint-config|stylelint-" package.json frontend/package.json .github/workflows -S || true
echo "=== Search for stylelint in pnpm/lockfiles ==="
ls -ლა | rg -i "pnpm-lock|yarn-lock|package-lock" || true
find . -maxdepth 3 -type f \( -iname "*pnpm-lock*.yaml" -o -iname "*yarn.lock" -o -iname "*package-lock*.json" \) | head -n 20 | while read -r f; do
echo "--- $f ---"
rg -n "stylelint" "$f" -S || true
done
# Inspect Angular lint config (if any) for stylelint usage
echo "=== Inspect Angular workspace config ==="
if [ -f frontend/angular.json ]; then
sed -n '1,200p' frontend/angular.json
rg -n "stylelint|lintOptions|style" frontend/angular.json -S || true
fi
if [ -f frontend/.angular-cli.json ]; then
cat frontend/.angular-cli.json
fiRepository: rocket-admin/rocketadmin
Length of output: 6299
Remove ::ng-deep from frontend/src/app/app.component.css (lines 44-46)
::ng-deep in the backdrop override (.main-menu-container ::ng-deep .mat-drawer-backdrop...) will be flagged by Stylelint because .stylelintrc.json extends stylelint-config-standard-scss without disabling selector-pseudo-element-no-unknown. Move this backdrop override to a global stylesheet (or otherwise apply it outside view encapsulation) to avoid deep selectors.
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 44-44: Unknown pseudo-element selector "::ng-deep" (selector-pseudo-element-no-unknown)
(selector-pseudo-element-no-unknown)
🤖 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/app.component.css` around lines 44 - 46, Remove the deep
selector from the component stylesheet: delete the ::ng-deep token from the rule
in .main-menu-container ::ng-deep .mat-drawer-backdrop.mat-drawer-shown and move
that selector (without ::ng-deep) into your global stylesheet so it applies
outside Angular view encapsulation; keep the rule body (top: 44px !important;)
and ensure the selector is exactly .main-menu-container
.mat-drawer-backdrop.mat-drawer-shown in the global styles to avoid stylelint
error (do not disable the lint rule).
Comment on lines
+39
to
+46
| <a mat-list-item routerLink="/hosted-databases" | ||
| routerLinkActive="nav-bar__button_active" | ||
| aria-label="Hosted databases"> | ||
| <mat-icon matListItemIcon fontSet="material-symbols-outlined" class="connection-navigation__icon"> | ||
| database | ||
| </mat-icon> | ||
| <div matListItemTitle>Hosted databases</div> | ||
| </a> |
There was a problem hiding this comment.
Finish wiring the new Hosted databases entry across both account menus.
This adds the route in the mobile sidenav, but the desktop #accountMenu in this same template still jumps from Company straight to Secrets, so desktop users never see the new destination. Also, this anchor is the only new account-row item here without account-section-item, which means the compact sizing added in app.component.css will not apply to it.
🤖 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/app.component.html` around lines 39 - 46, The desktop
account menu (`#accountMenu`) is missing the new Hosted databases router entry and
the anchor lacks the account-section-item class; add an <a> element with
routerLink="/hosted-databases" and routerLinkActive="nav-bar__button_active"
into the `#accountMenu` account rows (placing it between the Company and Secrets
items to match mobile sidenav order) and ensure the anchor includes the
account-section-item class so the compact sizing in app.component.css applies;
update the existing anchor (the one already added to the mobile sidenav) or
duplicate its attributes (including mat-list-item, aria-label="Hosted
databases", mat-icon and matListItemTitle) into the `#accountMenu` markup.
| <div matListItemTitle>Zapier</div> | ||
| </a> | ||
| <a mat-list-item href="https://docs.rocketadmin.com/" target="_blank"> | ||
| <a mat-list-item class="account-section-item" href="https://docs.rocketadmin.com/" target="_blank"> |
There was a problem hiding this comment.
Add rel="noopener noreferrer" to the external Help center link.
Opening a new tab with target="_blank" without rel leaves window.opener exposed unnecessarily.
Suggested fix
- <a mat-list-item class="account-section-item" href="https://docs.rocketadmin.com/" target="_blank">
+ <a
+ mat-list-item
+ class="account-section-item"
+ href="https://docs.rocketadmin.com/"
+ target="_blank"
+ rel="noopener noreferrer">📝 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
| <a mat-list-item class="account-section-item" href="https://docs.rocketadmin.com/" target="_blank"> | |
| <a | |
| mat-list-item | |
| class="account-section-item" | |
| href="https://docs.rocketadmin.com/" | |
| target="_blank" | |
| rel="noopener noreferrer"> |
🤖 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/app.component.html` at line 59, The external Help center
anchor element with class "account-section-item" that uses target="_blank"
should include rel="noopener noreferrer" to prevent exposing window.opener;
update the <a> element (the anchor with href="https://docs.rocketadmin.com/" and
class "account-section-item") to add rel="noopener noreferrer" alongside the
existing target attribute.
Comment on lines
+63
to
66
| <mat-list-item class="account-section-item" (click)="logOut()" data-testid="logout-button-account-menu"> | ||
| <mat-icon matListItemIcon class="connection-navigation__icon">logout</mat-icon> | ||
| <div matListItemTitle>Log out</div> | ||
| </mat-list-item> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show the referenced region with line numbers
sed -n '40,90p' frontend/src/app/app.component.html | nl -ba | sed -n '1,120p'
# 2) Search for the logout handler usage
rg -n "logOut\\(" frontend/src/app/app.component.html frontend/src/app -S || true
# 3) Find other mat-list-item + click patterns for keyboard/a11y precedents
rg -n "mat-list-item\\b[^\\n]*\\(click\\)" frontend/src/app -S || true
# 4) Check whether mat-list-item is used as a button anywhere else
rg -n "<button[^>]*\\bmat-list-item\\b|\\bmat-list-item\\b[^\\n]*routerLink|mat-list-item\\b[^\\n]*href" frontend/src/app -S || trueRepository: rocket-admin/rocketadmin
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Show referenced region with line numbers (avoid nl)
sed -n '55,80p' frontend/src/app/app.component.html | cat -n
# 2) Search for the logout handler usage
rg -n "logOut\\(" frontend/src/app/app.component.html frontend/src/app -S || true
# 3) Find mat-list-item + click patterns for accessibility precedent
rg -n "mat-list-item\\b[^\\n]*\\(click\\)" frontend/src/app -S || true
# 4) Check for any existing pattern of using a real button with mat-list-item in this repo
rg -n "<button[^>]*\\bmat-list-item\\b|mat-list-item\\b[^\\n]*\\brole=\\\"button\\\"|mat-list-item\\b[^\\n]*\\btabindex=" frontend/src/app -S || trueRepository: rocket-admin/rocketadmin
Length of output: 3280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# A) Show the logout list-item region precisely
sed -n '55,75p' frontend/src/app/app.component.html | cat -n
# B) Show the other logout button usage near line ~222
sed -n '205,235p' frontend/src/app/app.component.html | cat -n
# C) Find all mat-list-item elements with (click)=
rg -n "mat-list-item\\b[^>]*\\(click\\)=" frontend/src/app -S || true
# D) Find whether repo already uses <button ... mat-list-item ...>
rg -n "<button[^>]*\\bmat-list-item\\b" frontend/src/app -S || true
# E) Find other mat-list-item usages to see intended pattern (a vs button vs bare mat-list-item)
rg -n "mat-list-item\\b" frontend/src/app -S | head -n 80Repository: rocket-admin/rocketadmin
Length of output: 8002
Use a real button for the “Log out” row (account menu)
In frontend/src/app/app.component.html, the logout entry uses (click) on a bare <mat-list-item>, which doesn’t provide native button semantics or keyboard interaction. The repo already uses real buttons with mat-list-item elsewhere, and this page already uses a real <button> for the logout inside the mat-menu.
<mat-list-item class="account-section-item" (click)="logOut()" data-testid="logout-button-account-menu">
<mat-icon matListItemIcon class="connection-navigation__icon">logout</mat-icon>
<div matListItemTitle>Log out</div>
</mat-list-item>Suggested fix
- <mat-list-item class="account-section-item" (click)="logOut()" data-testid="logout-button-account-menu">
+ <button type="button" mat-list-item class="account-section-item" (click)="logOut()" data-testid="logout-button-account-menu">
<mat-icon matListItemIcon class="connection-navigation__icon">logout</mat-icon>
<div matListItemTitle>Log out</div>
- </mat-list-item>
+ </button>🤖 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/app.component.html` around lines 63 - 66, Replace the
clickable plain mat-list-item with a real button element to restore native
button semantics and keyboard interaction: locate the logout entry using the
account-section-item class and data-testid "logout-button-account-menu" and
change it to a button with the mat-list-item attribute, keep the existing
(click)="logOut()" handler and the child mat-icon and title, and ensure the
button has type="button" and any existing testid/aria attributes preserved so
behavior and tests remain the same.
Comment on lines
+100
to
+101
| this._tableState.aiPanelExpandedCast.subscribe(() => { | ||
| this.isExpanded = true; |
There was a problem hiding this comment.
Preserve the emitted expansion state.
This subscription now discards the boolean from aiPanelExpandedCast, so any false emission is lost and the panel can never leave the expanded layout again. Keeping the default as true is fine, but the stream still needs to drive the actual state.
Suggested fix
- this._tableState.aiPanelExpandedCast.subscribe(() => {
- this.isExpanded = true;
+ this._tableState.aiPanelExpandedCast.subscribe((isExpanded: boolean) => {
+ this.isExpanded = isExpanded;
});📝 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
| this._tableState.aiPanelExpandedCast.subscribe(() => { | |
| this.isExpanded = true; | |
| this._tableState.aiPanelExpandedCast.subscribe((isExpanded: boolean) => { | |
| this.isExpanded = isExpanded; | |
| }); |
🤖 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/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.ts`
around lines 100 - 101, The subscription to _tableState.aiPanelExpandedCast
currently ignores the emitted boolean and always sets isExpanded to true; change
the callback to accept the emitted value (e.g., expansion: boolean) and assign
it to this.isExpanded so false emissions collapse the panel (you can keep the
initial default true elsewhere but let aiPanelExpandedCast drive the state via
the subscribe handler on _tableState.aiPanelExpandedCast).
Comment on lines
+125
to
+136
| <div *ngFor="let f of savedFiltersPanel?.savedFilterData" | ||
| class="search-row__quick-chip" | ||
| [class.search-row__quick-chip_active]="savedFiltersPanel?.selectedFilterSetId === f.id" | ||
| (click)="savedFiltersPanel?.selectFiltersSet(f.id)"> | ||
| <span class="search-row__quick-chip-label">{{ f.name }}</span> | ||
| <button *ngIf="canEditConnection()" type="button" | ||
| class="search-row__quick-chip-menu" | ||
| [matMenuTriggerFor]="quickChipMenu" | ||
| (click)="$event.stopPropagation(); chipFilterForMenu = f"> | ||
| <mat-icon>more_vert</mat-icon> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
Make each quick-filter chip an actual button.
The primary chip action is on a clickable <div>, so it isn't focusable or keyboard-operable. That blocks the new quick-filter flow for keyboard and assistive-tech users. Use a button for the chip itself and keep the overflow menu button as a separate control.
🤖 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/dashboard/db-table-view/db-table-view.component.html`
around lines 125 - 136, Replace the non-focusable clickable div with a semantic
button so each quick-filter chip is keyboard and screen-reader operable: render
the per-filter element using a <button type="button"> (keeping classes:
search-row__quick-chip and conditional search-row__quick-chip_active using
savedFiltersPanel?.selectedFilterSetId === f.id), wire its (click) to
savedFiltersPanel?.selectFiltersSet(f.id), and keep the overflow menu as the
separate button that still stops propagation and sets chipFilterForMenu = f;
ensure the menu button retains type="button" and existing [matMenuTriggerFor]
and canEditConnection() visibility logic.
Comment on lines
+6
to
+14
| <div class="mobile-header"> | ||
| <a mat-icon-button class="mobile-header__back" routerLink="/dashboard/{{connectionID}}/{{tableName}}" aria-label="Back to table"> | ||
| <mat-icon>arrow_back</mat-icon> | ||
| </a> | ||
| <div class="mobile-header__titles"> | ||
| <h1 class="mobile-header__title">Widgets</h1> | ||
| <span class="mobile-header__subtitle">{{ tableName }}</span> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Hide the mobile header on the empty-state path.
widgets is truthy for [], so Lines 6-14 still render when Line 55 falls through to emptyState. That reintroduces the mobile back/header chrome the empty state is supposed to remove.
Suggested fix
- <div class="mobile-header">
- <a mat-icon-button class="mobile-header__back" routerLink="/dashboard/{{connectionID}}/{{tableName}}" aria-label="Back to table">
- <mat-icon>arrow_back</mat-icon>
- </a>
- <div class="mobile-header__titles">
- <h1 class="mobile-header__title">Widgets</h1>
- <span class="mobile-header__subtitle">{{ tableName }}</span>
- </div>
- </div>
+ `@if` (widgets.length) {
+ <div class="mobile-header">
+ <a mat-icon-button class="mobile-header__back" routerLink="/dashboard/{{connectionID}}/{{tableName}}" aria-label="Back to table">
+ <mat-icon>arrow_back</mat-icon>
+ </a>
+ <div class="mobile-header__titles">
+ <h1 class="mobile-header__title">Widgets</h1>
+ <span class="mobile-header__subtitle">{{ tableName }}</span>
+ </div>
+ </div>
+ }📝 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 class="mobile-header"> | |
| <a mat-icon-button class="mobile-header__back" routerLink="/dashboard/{{connectionID}}/{{tableName}}" aria-label="Back to table"> | |
| <mat-icon>arrow_back</mat-icon> | |
| </a> | |
| <div class="mobile-header__titles"> | |
| <h1 class="mobile-header__title">Widgets</h1> | |
| <span class="mobile-header__subtitle">{{ tableName }}</span> | |
| </div> | |
| </div> | |
| `@if` (widgets.length) { | |
| <div class="mobile-header"> | |
| <a mat-icon-button class="mobile-header__back" routerLink="/dashboard/{{connectionID}}/{{tableName}}" aria-label="Back to table"> | |
| <mat-icon>arrow_back</mat-icon> | |
| </a> | |
| <div class="mobile-header__titles"> | |
| <h1 class="mobile-header__title">Widgets</h1> | |
| <span class="mobile-header__subtitle">{{ tableName }}</span> | |
| </div> | |
| </div> | |
| } |
🤖 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/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.html`
around lines 6 - 14, The mobile header (<div class="mobile-header">) is rendered
even when widgets is an empty array because the template checks the truthiness
of widgets; change the visibility condition to explicitly check for non-empty
(e.g., use widgets?.length > 0 or equivalent) or tie it to the same emptyState
guard used at line 55 so the mobile header is only shown when widgets has items
and is not the empty-state; update the *ngIf on the mobile-header block
(referencing widgets and emptyState) accordingly.
| <div *ngIf="widgets" class="wrapper"> | ||
| <header class="header"> | ||
| <div class="mobile-header"> | ||
| <a mat-icon-button class="mobile-header__back" routerLink="/dashboard/{{connectionID}}/{{tableName}}" aria-label="Back to table"> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Angular 20 Router, is [routerLink]="['/dashboard', connectionID, tableName]"the recommended way to build dynamic path segments so each segment is URL-encoded correctly, compared with interpolating a single string likerouterLink="/dashboard/{{connectionID}}/{{tableName}}"?
💡 Result:
Yes, using the array syntax [routerLink]="['/dashboard', connectionID, tableName]" is the recommended approach in Angular for building dynamic URL segments [1][2][3]. This approach is superior to string interpolation (e.g., routerLink="/dashboard/{{connectionID}}/{{tableName}}") for several key reasons: 1. Automatic Encoding: The Angular router, via createUrlTree, automatically handles the URL encoding of individual path segments when provided as an array [1][3]. This ensures that special characters within connectionID or tableName are correctly escaped, preventing malformed URLs or routing errors [4]. 2. Robustness: String interpolation relies on manual construction of the URL path, which is prone to errors, such as missing slashes or improper handling of path delimiters [5][6]. By passing an array, you delegate the path construction logic to the router, which treats each element in the array as a distinct segment [1][2]. 3. Flexibility: The array syntax is a first-class citizen of the Angular router and supports more than just simple string segments. You can easily include additional configuration, such as matrix parameters or query parameters, in a declarative way [1][4][3]. While both methods may technically work in simple scenarios, using string interpolation is considered a poor practice for dynamic paths because it bypasses the framework's built-in safety mechanisms for URL construction [1][7]. Always prefer the array syntax to ensure your dynamic routes are built correctly and securely [1][3].
Citations:
- 1: https://angular.dev/api/router/RouterLink
- 2: https://github.com/angular/angular/blob/9d9e9c6ff1eb37d54a6e4af0a88f05a41f75d7a9/modules/%40angular/router/src/directives/router_link.ts
- 3: https://github.com/angular/angular/blob/5205db5564f21c214a96000e018a08f3de7d44d8/packages/router/src/directives/router_link.ts
- 4: routerLink directive doesn't support dynamically binding to an observable angular/angular#13190
- 5: https://stackoverflow.com/questions/45090939/routerlink-array
- 6: Pass an encoded (base) URL to
routerLinkangular/angular#50950 - 7: https://stackoverflow.com/questions/38734635/angular-2-should-it-be-routerlink-theroute-or-routerlink-the-route
Use array form for routerLink path segments
Interpolating connectionID/tableName into a single routerLink string bypasses Angular’s per-segment URL encoding; switch the back link to [routerLink]="['/dashboard', connectionID, tableName]". Apply the same change to the other back link if it uses the same interpolated routerLink pattern.
🤖 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/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.html`
at line 7, The routerLink currently interpolates path segments as a single
string on the anchor element with class "mobile-header__back" (the <a
mat-icon-button ... routerLink="...">), which bypasses per-segment encoding;
change it to the array form by binding routerLink to an array of segments
(['/dashboard', connectionID, tableName]) on that anchor, and apply the same
change to any other back-link anchors using the interpolated routerLink pattern
to ensure proper per-segment URL encoding.
- Audit mobile feed: replace mat-table with a card list (sticky date headers grouping entries per day, avatar with initials, top row user/time, middle row action+table, bottom row status+Details, last card fades for an infinite-scroll hint, tap anywhere on the card opens the details dialog) - Audit header on mobile: arrow-back next to the "Audit" title, Tables/Users selects side-by-side without breaking layout (force min-width: 0 chain, no horizontal overflow), select arrow pinned to the right edge of the field - Audit paginator: match the table paginator (compact "Per page:" select on the left, range + nav buttons on the right, single row, capitalized label) - AI panel: raise z-index of the expanded content back to 100 so close/send buttons aren't covered (they were being intercepted by other UI when the panel is force-expanded) - Sidebar drawer: close automatically on any nav-list click and on Upgrade click Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
frontend/src/app/app.component.html (1)
8-8: ⚡ Quick winUse Angular built-in control flow for the touched template blocks.
Line 8 and Line 68 still use
*ngIfin newly touched markup. Please convert these blocks to@ifso this file stays aligned with the Angular 19 template standard used in the repo.As per coding guidelines,
frontend/**/*.component.html: Use Angular 19's built-in control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code.Also applies to: 68-70
🤖 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/app.component.html` at line 8, Replace the template structural directives using *ngIf with Angular 19 built-in control flow: convert the mat-nav-list element that currently uses "*ngIf='userLoggedIn === true' (click)='drawer.close()'" to use an `@if` block that checks "userLoggedIn" and keeps the (click)="drawer.close()" inside the shown template; likewise find the other template block(s) that use "*ngIf" with the same "userLoggedIn" condition (the block around the navigation items) and convert them to matching `@if/`@else sections. Ensure you preserve the original conditional logic (use boolean truthiness of userLoggedIn, not a string comparison) and keep all inner markup and event bindings unchanged while removing the star-prefixed structural directives.frontend/src/app/components/audit/audit.component.ts (2)
87-91: ⚖️ Poor tradeoffNew component code should follow the signals/
inject()conventions.Per the frontend guidelines, new state like
mobileGroupedLogsshould be modeled withsignal()/computed()(andisMobileViewis a goodcomputedcandidate), and the newly addedpaginatorIntldependency should be obtained viainject()rather than a constructor parameter. The surrounding file predates these rules, so apply judgment for consistency, but the added lines are net-new.As per coding guidelines: "All new code must use Angular signals (
signal(),computed(),effect()) for state management" and "Inject dependencies usinginject()function ... instead of constructor parameters".🤖 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/audit/audit.component.ts` around lines 87 - 91, The new code adds state and a dependency via the constructor instead of using Angular signals and inject(); refactor by replacing the constructor injection of paginatorIntl with injector usage (call inject(MatPaginatorIntl) where used) and convert any new state (mobileGroupedLogs) into a signal() and derive isMobileView as a computed() from viewport/size state, updating any usages to read the signal/computed values; ensure you remove the paginatorIntl parameter from the constructor and call paginatorIntl.changes.next() off the injected instance, and replace direct property mutations with signal update handlers so the component follows signals/inject() conventions.
72-72: ⚡ Quick winAvoid
anyin the grouped-logs types.
mobileGroupedLogsandgroupByDateuseany[]for entries. A sharedLog/audit-row interface (the shape produced byAuditDataSource.fetchLogs) would give the template real type safety.As per coding guidelines: "Avoid
anytypes - use specific types or generics instead".Also applies to: 188-196
🤖 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/audit/audit.component.ts` at line 72, Define a concrete interface matching the shape returned by AuditDataSource.fetchLogs (e.g., export interface AuditLog or AuditRow with the fields used in the template), replace any usage of any[] with this type, and update mobileGroupedLogs to be { date: string; entries: AuditLog[] }[] and the groupByDate function signature/return types to use AuditLog[] (also update the occurrences around the groupByDate usage at the area referenced 188-196). Ensure the template bindings and any helper methods accept the new AuditLog type so type checking replaces the previous any usages.
🤖 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/audit/audit.component.css`:
- Around line 346-347: The CSS uses deprecated properties grid-column-gap and
grid-row-gap in audit.component.css; replace them with the standard column-gap
and row-gap properties respectively (preserving the same values, e.g.,
grid-column-gap: 12px -> column-gap: 12px and grid-row-gap: 4px -> row-gap: 4px)
so the grid container keeps identical spacing and satisfies stylelint's
property-no-deprecated rule.
In `@frontend/src/app/components/audit/audit.component.ts`:
- Around line 113-115: The RxJS subscriptions in AuditComponent are never torn
down and several `any` usages exist; to fix, inject a DestroyRef (e.g., private
_destroyRef = inject(DestroyRef)) and import takeUntilDestroyed from
`@angular/core/rxjs-interop`, then pipe(takeUntilDestroyed(this._destroyRef)) onto
the observable returned by dataSource.connect(...) (replace connect(null as any)
with a properly typed argument or overload) and onto the
merge(this.paginator.page, ...) stream in ngAfterViewInit to automatically
unsubscribe; also replace loose any types by adding proper interfaces/types for
mobileGroupedLogs, the groupByDate(rows: any[]) signature (e.g.,
groupByDate(rows: LogRow[]) and return Map<string, LogRow[]>), and update usages
to use the new LogRow type so there are no any typings left.
---
Nitpick comments:
In `@frontend/src/app/app.component.html`:
- Line 8: Replace the template structural directives using *ngIf with Angular 19
built-in control flow: convert the mat-nav-list element that currently uses
"*ngIf='userLoggedIn === true' (click)='drawer.close()'" to use an `@if` block
that checks "userLoggedIn" and keeps the (click)="drawer.close()" inside the
shown template; likewise find the other template block(s) that use "*ngIf" with
the same "userLoggedIn" condition (the block around the navigation items) and
convert them to matching `@if/`@else sections. Ensure you preserve the original
conditional logic (use boolean truthiness of userLoggedIn, not a string
comparison) and keep all inner markup and event bindings unchanged while
removing the star-prefixed structural directives.
In `@frontend/src/app/components/audit/audit.component.ts`:
- Around line 87-91: The new code adds state and a dependency via the
constructor instead of using Angular signals and inject(); refactor by replacing
the constructor injection of paginatorIntl with injector usage (call
inject(MatPaginatorIntl) where used) and convert any new state
(mobileGroupedLogs) into a signal() and derive isMobileView as a computed() from
viewport/size state, updating any usages to read the signal/computed values;
ensure you remove the paginatorIntl parameter from the constructor and call
paginatorIntl.changes.next() off the injected instance, and replace direct
property mutations with signal update handlers so the component follows
signals/inject() conventions.
- Line 72: Define a concrete interface matching the shape returned by
AuditDataSource.fetchLogs (e.g., export interface AuditLog or AuditRow with the
fields used in the template), replace any usage of any[] with this type, and
update mobileGroupedLogs to be { date: string; entries: AuditLog[] }[] and the
groupByDate function signature/return types to use AuditLog[] (also update the
occurrences around the groupByDate usage at the area referenced 188-196). Ensure
the template bindings and any helper methods accept the new AuditLog type so
type checking replaces the previous any usages.
🪄 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: 07ba481d-2a1f-4b5c-a28f-fd36b630ff58
📒 Files selected for processing (6)
frontend/src/app/app.component.htmlfrontend/src/app/components/audit/audit.component.cssfrontend/src/app/components/audit/audit.component.htmlfrontend/src/app/components/audit/audit.component.tsfrontend/src/app/components/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.cssfrontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/app/components/dashboard/db-table-view/db-table-view.component.ts
- frontend/src/app/components/dashboard/db-table-view/db-table-ai-panel/db-table-ai-panel.component.css
Comment on lines
+346
to
+347
| grid-column-gap: 12px; | ||
| grid-row-gap: 4px; |
There was a problem hiding this comment.
Replace deprecated grid-column-gap/grid-row-gap with column-gap/row-gap.
Stylelint flags these as deprecated (property-no-deprecated). The shorthand-less standard properties behave identically in a grid container.
Proposed fix
- grid-column-gap: 12px;
- grid-row-gap: 4px;
+ column-gap: 12px;
+ row-gap: 4px;📝 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
| grid-column-gap: 12px; | |
| grid-row-gap: 4px; | |
| column-gap: 12px; | |
| row-gap: 4px; |
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 346-346: Expected "grid-column-gap" to be "column-gap" (property-no-deprecated)
(property-no-deprecated)
[error] 347-347: Expected "grid-row-gap" to be "row-gap" (property-no-deprecated)
(property-no-deprecated)
🤖 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/audit/audit.component.css` around lines 346 -
347, The CSS uses deprecated properties grid-column-gap and grid-row-gap in
audit.component.css; replace them with the standard column-gap and row-gap
properties respectively (preserving the same values, e.g., grid-column-gap: 12px
-> column-gap: 12px and grid-row-gap: 4px -> row-gap: 4px) so the grid container
keeps identical spacing and satisfies stylelint's property-no-deprecated rule.
Comment on lines
+113
to
+115
| this.dataSource.connect(null as any).subscribe((rows) => { | ||
| this.mobileGroupedLogs = this.groupByDate(rows); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/app/components/audit/audit.component.ts"
echo "== File exists? =="
ls -la "$FILE"
echo
echo "== Lines 1-260 with numbers (for context) =="
nl -ba "$FILE" | sed -n '1,260p'
echo
echo "== Search within file for subscription/lifecycle management =="
rg -n "connect\\(|disconnect\\(|takeUntilDestroyed|takeUntil\\(|ngOnDestroy|OnDestroy|DestroyRef|_destroyRef" "$FILE" || true
echo
echo "== Show any `groupByDate` definition and where private methods are located =="
rg -n "groupByDate\\b" "$FILE" || trueRepository: rocket-admin/rocketadmin
Length of output: 297
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/app/components/audit/audit.component.ts"
echo "== Lines 1-260 (numbered via cat -n) =="
cat -n "$FILE" | sed -n '1,260p'
echo
echo "== Search within file for subscription/lifecycle management =="
rg -n "connect\\(|disconnect\\(|takeUntilDestroyed|takeUntil\\(|ngOnDestroy|OnDestroy|DestroyRef|_destroyRef" "$FILE" || true
echo
echo "== Search for null as any / any usage =="
rg -n "null as any|: any\\b|\\bas any\\b|\\bany\\b" "$FILE" || true
echo
echo "== Show groupByDate definition =="
rg -n "groupByDate\\b" "$FILE" || trueRepository: rocket-admin/rocketadmin
Length of output: 8353
Tear down RxJS subscriptions (and reduce any) in AuditComponent
this.dataSource.connect(null as any).subscribe(...)(around lines 113-115) is never unsubscribed (noOnDestroy/takeUntilDestroyed, and no disconnect path).merge(this.paginator.page)...subscribe()inngAfterViewInit()(around lines 93-99) is also never unsubscribed.- Remove/replace
anyusage:connect(null as any),mobileGroupedLogsentries typed asany[], andgroupByDate(rows: any[])/Map<string, any[]>.
Proposed fix using takeUntilDestroyed
- this.dataSource.connect(null as any).subscribe((rows) => {
- this.mobileGroupedLogs = this.groupByDate(rows);
- });
+ this.dataSource
+ .connect(null as any)
+ .pipe(takeUntilDestroyed(this._destroyRef))
+ .subscribe((rows: any[]) => {
+ this.mobileGroupedLogs = this.groupByDate(rows);
+ });Inject private _destroyRef = inject(DestroyRef) and import takeUntilDestroyed from @angular/core/rxjs-interop. Apply the same teardown approach to the paginator merge(...).subscribe() as well.
🤖 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/audit/audit.component.ts` around lines 113 - 115,
The RxJS subscriptions in AuditComponent are never torn down and several `any`
usages exist; to fix, inject a DestroyRef (e.g., private _destroyRef =
inject(DestroyRef)) and import takeUntilDestroyed from
`@angular/core/rxjs-interop`, then pipe(takeUntilDestroyed(this._destroyRef)) onto
the observable returned by dataSource.connect(...) (replace connect(null as any)
with a properly typed argument or overload) and onto the
merge(this.paginator.page, ...) stream in ngAfterViewInit to automatically
unsubscribe; also replace loose any types by adding proper interfaces/types for
mobileGroupedLogs, the groupByDate(rows: any[]) signature (e.g.,
groupByDate(rows: LogRow[]) and return Map<string, LogRow[]>), and update usages
to use the new LogRow type so there are no any typings left.
… polish - Permissions (Cedar policy) dialog opens as a mobile bottom sheet via panelClass - Empty state in the policy list gets an inline "Add policy" CTA (accent color), and the dialog actions now keep only one "Add policy" once there's at least one policy - Cancel pinned to the left of the dialog footer; Save stays on the right - User add dialog: swap order so the primary "Add" sits before "Cancel" - Users page on mobile: wrapper full-width with 16px padding, header flex-wraps the New group button if needed, smaller h1 - Connection settings on mobile: keep Primary/Accented color inputs on one row by forcing min-width: 0 through the Material form-field internals Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.html (1)
28-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winVerify dialog button order follows Material Design patterns.
The Cancel button now appears after the primary action. With
align="end", both buttons are right-aligned, placing Cancel as the rightmost button. Material Design conventions typically place the primary action (Add/Open Company page) on the right and Cancel on the left.Compare to
cedar-policy-editor-dialog.component.html(lines 51-62), which uses a spacer to achieve the conventional layout:[Cancel] <spacer> [Add policy] [Save].📐 Suggested fix to match Material Design button order
<mat-dialog-actions align="end"> + <button type="button" mat-flat-button mat-dialog-close>Cancel</button> + <span class="dialog-actions-spacer"></span> `@if` (data.availableMembers.length) { <button type="submit" mat-flat-button color="primary" [disabled]="submitting() || addUserForm.form.invalid"> Add </button> } `@else` { <a mat-flat-button mat-dialog-close color="primary" routerLink="/company"> Open Company page </a> } - <button type="button" mat-flat-button mat-dialog-close>Cancel</button> </mat-dialog-actions>You'll also need to add the spacer CSS (if not already present):
.dialog-actions-spacer { flex: 1 1 auto; }🤖 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/users/user-add-dialog/user-add-dialog.component.html` around lines 28 - 42, Reorder the dialog actions so the Cancel button appears to the left of the primary action by placing the Cancel button first inside the <mat-dialog-actions> block, then add a flex spacer element (use class "dialog-actions-spacer") and then render the primary action (the Add button or the Open Company page anchor that checks data.availableMembers.length and uses [disabled]="submitting() || addUserForm.form.invalid"); also ensure the CSS for .dialog-actions-spacer ("flex: 1 1 auto") is present in the component styles so the primary action stays right-aligned per Material Design conventions.
🤖 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/connection-settings/connection-settings.component.css`:
- Around line 182-210: Replace the deprecated grid-gap usage in the responsive
rules for .color-theme with the modern gap property; update the CSS block that
targets .color-theme (inside the `@media` (width <= 600px) query) to use gap: 8px
instead of grid-gap: 8px while leaving the rest of the rules for .color-item and
.color-input (and the ::ng-deep selectors) unchanged.
---
Outside diff comments:
In
`@frontend/src/app/components/users/user-add-dialog/user-add-dialog.component.html`:
- Around line 28-42: Reorder the dialog actions so the Cancel button appears to
the left of the primary action by placing the Cancel button first inside the
<mat-dialog-actions> block, then add a flex spacer element (use class
"dialog-actions-spacer") and then render the primary action (the Add button or
the Open Company page anchor that checks data.availableMembers.length and uses
[disabled]="submitting() || addUserForm.form.invalid"); also ensure the CSS for
.dialog-actions-spacer ("flex: 1 1 auto") is present in the component styles so
the primary action stays right-aligned per Material Design conventions.
🪄 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: 2bb418a4-199d-4cd7-afc8-3c1fc09b8047
📒 Files selected for processing (7)
frontend/src/app/components/connection-settings/connection-settings.component.cssfrontend/src/app/components/users/cedar-policy-editor-dialog/cedar-policy-editor-dialog.component.htmlfrontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.cssfrontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.htmlfrontend/src/app/components/users/user-add-dialog/user-add-dialog.component.htmlfrontend/src/app/components/users/users.component.cssfrontend/src/app/components/users/users.component.ts
- 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>
…ngoQueryIsSafe function
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
frontend/src/app/services/configuration-state.service.ts (1)
19-19: ⚖️ Poor tradeoff
progress$introduces a newBehaviorSubjectin a service.Per repo guidelines, new service state should prefer Angular signals. Streaming progress doesn't map cleanly onto
rxResource(), butprogress$could be asignal<ConfigProgress | null>(the consumer already uses signals), keeping the data layer consistent. Deferrable given the streaming nature, but flagging for awareness.As per coding guidelines: "All new code must use Angular signals (
signal(),computed(),effect()) for state management instead of BehaviorSubject".🤖 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` at line 19, Replace the BehaviorSubject-based progress$ with an Angular signal in ConfigurationStateService: remove the BehaviorSubject import and declaration, create a writable signal like progress = signal<ConfigProgress | null>(null) (or readonly getter if you want external immutability), and update any internal updates to call progress.set(...) (or assign) and consumers to read progress() instead of subscribing; keep the symbol name (progress or progress$ if you prefer) consistent with existing consumers and add a small setter method (e.g., setProgress) on the service if updates are done externally.
🤖 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 `@backend/src/entities/user/user-password/password-reset.entity.ts`:
- Around line 12-13: The createdAt column is currently defined with type
'timestamp' which is timezone-naive; update the `@Column` decorator for the
createdAt property in password-reset.entity.ts (and likewise in
email-verification.entity.ts) to use type: 'timestamptz' so DB stores UTC-aware
timestamps, ensuring comparisons against Constants.ONE_DAY_AGO() (and any
createdAt >= :valid_after queries) behave correctly across timezones; keep the
property name createdAt and only change the column type definition in the
`@Column`(...) metadata.
In `@backend/src/entities/visualizations/panel/utils/check-query-is-safe.util.ts`:
- Around line 179-185: The safety check calls
isReadOnlyMongoAggregationPipeline(query) but that symbol is not exported from
backend/src/ai-core/tools/query-validators.ts and/or may expect a parsed
pipeline rather than the raw query string; fix by either exporting
isReadOnlyMongoAggregationPipeline from query-validators (or re-importing the
correct module that defines it), or change the call in
check-query-is-safe.util.ts to the appropriate validator (e.g.,
isValidMongoDbCommand or a new isReadOnlyMongoQuery) that accepts raw strings;
ensure the validator's contract is respected (parse the string before calling if
the validator expects a parsed pipeline) and broaden the check to allow
legitimate read-only non-aggregation queries like find/distinct so visualization
panels aren't incorrectly rejected.
In `@frontend/angular.json`:
- Around line 78-79: The build budget anyComponentStyle in frontend/angular.json
(maximumWarning: 20kb / maximumError: 32kb) is being hit; locate the specific
component style files under frontend/src that exceed 20kb (and the one >32kb),
inspect them for repeated selector blocks such as .wrapper and .actions, and
determine if the excess is from duplicated shared layout rules versus unique
component styling; refactor by extracting common rules into a shared/global
stylesheet or a shared SCSS module and import it into those components (update
component styleUrls to remove duplicated rules), remove unused selectors and
compress verbose selectors, then re-run the build to confirm budgets are met
before considering raising anyComponentStyle values.
In `@frontend/src/app/components/auto-configure/auto-configure.component.html`:
- Around line 4-39: Replace the legacy structural directives in the template
with Angular 19 control-flow syntax: convert *ngIf="..." blocks to `@if`(...) /
`@else` where appropriate (e.g., the spinner/success blocks, summary and error
display using the existing errorText(), done(), hasSummary() predicates) and
convert the status list *ngFor="let m of messages(); let last = last; trackBy:
trackByIndex" to the new `@for` syntax (e.g., `@for`(let m of messages(); let last =
last; track $index) and remove the trackBy: trackByIndex usage), then delete the
now-unused trackByIndex() method from the component class; keep all current
bindings and class expressions ([class.status-log__item_current],
[class.status-log__item_done]) and calls to done(), hasSummary(), messages(),
tablesConfigured(), widgetsCount(), settingsCount(), and errorText() intact.
In `@frontend/src/app/components/auto-configure/auto-configure.component.ts`:
- Around line 64-67: startConfiguring's boolean result is ignored so navigation
always happens; change the flow in the async block that calls
this._configState.startConfiguring(connectionId) to capture its return value and
only perform the post-wait navigation this._router.navigate(['/dashboard',
connectionId]) if the result is truthy (or if this.errorText() is empty). If
startConfiguring returns false or errorText() is set, skip the navigation and
keep the component in the error state so the user can read the message (you can
still await the short delay for UX), and ensure any subsequent UI state updates
rely on the same boolean/result to avoid racing into the dashboard.
In `@frontend/src/app/services/configuration-state.service.ts`:
- Around line 44-48: In ConfigurationStateService's _runSetup method reset the
progress$ subject to null at the start of each run to avoid replaying stale
terminal state: update the _runSetup implementation (in
ConfigurationStateService) to call progress$.next(null) (or reinitialize the
BehaviorSubject) before enqueuing the "Starting configuration…" message so new
subscribers (e.g. AutoConfigureComponent) don't immediately get a previous
complete/error value.
- Around line 53-83: The streaming fetch in
ConfigurationStateService._runSetup() needs an AbortController with a bounded
timeout to avoid a stalled stream blocking startConfiguring() and preventing
finally cleanup; modify _runSetup() to create an AbortController, pass
controller.signal into fetch, set a timeout to call controller.abort(), and
ensure any reader loops handle abort errors (clean up reader and rethrow or
handle accordingly). Expose a way to cancel the controller (e.g., store
controller keyed by connectionId on the ConfigurationStateService and add a
cancelConnection(connectionId) method) and call that from
AutoConfigureComponent.ngOnDestroy() (or clear the timeout and abort the
controller there) so the stream is aborted when the component is destroyed.
Ensure startConfiguring() still resolves/rejects consistently and that
_configuring cleanup in finally runs when the abort triggers.
---
Nitpick comments:
In `@frontend/src/app/services/configuration-state.service.ts`:
- Line 19: Replace the BehaviorSubject-based progress$ with an Angular signal in
ConfigurationStateService: remove the BehaviorSubject import and declaration,
create a writable signal like progress = signal<ConfigProgress | null>(null) (or
readonly getter if you want external immutability), and update any internal
updates to call progress.set(...) (or assign) and consumers to read progress()
instead of subscribing; keep the symbol name (progress or progress$ if you
prefer) consistent with existing consumers and add a small setter method (e.g.,
setProgress) on the service if updates are done externally.
🪄 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: 5e489c83-b067-4a5b-9166-1457f726e292
📒 Files selected for processing (13)
backend/src/entities/company-info/company-info.controller.tsbackend/src/entities/email/email-verification.entity.tsbackend/src/entities/email/repository/email-verification-custom-repository-extension.tsbackend/src/entities/user/user-password/password-reset.entity.tsbackend/src/entities/user/user-password/repository/user-password-custom-repository-extension.tsbackend/src/entities/visualizations/panel/utils/check-query-is-safe.util.tsbackend/src/migrations/1780411916141-AddCreatedAtColumnsIntoPasswordResetAndEmailVerificationEntities.tsfrontend/angular.jsonfrontend/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/components/connection-settings/connection-settings.component.cssfrontend/src/app/services/configuration-state.service.ts
✅ Files skipped from review due to trivial changes (2)
- backend/src/entities/email/email-verification.entity.ts
- backend/src/migrations/1780411916141-AddCreatedAtColumnsIntoPasswordResetAndEmailVerificationEntities.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/components/connection-settings/connection-settings.component.css
Comment on lines
+12
to
+13
| @Column({ type: 'timestamp', default: () => 'CURRENT_TIMESTAMP' }) | ||
| createdAt: Date; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PostgreSQL TypeORM timestamp without time zone comparison with JS Date UTC behavior
💡 Result:
When using PostgreSQL's "timestamp without time zone" column type with TypeORM, the library often experiences behavior that results in unexpected time offsets because it treats these columns as "naive" (lacking timezone information) [1][2][3]. Key behaviors and technical details include: 1. Naive Interpretation: PostgreSQL "timestamp without time zone" stores date and time values without any timezone offset [4][5]. When TypeORM fetches this data, it frequently interprets the value relative to the local time of the Node.js process rather than UTC, or vice versa, leading to shifts based on the server's or database's timezone settings [1][2][3]. 2. Implicit Conversion: During read/write operations, TypeORM or the underlying PostgreSQL driver (pg) may apply timezone offsets when converting between the database's format and JavaScript Date objects [6][3]. Because "timestamp without time zone" lacks an explicit offset, the driver or ORM assumes a timezone, often resulting in "off-by-hours" issues if the local system timezone deviates from the intended storage timezone [2][3]. 3. Recommended Best Practice: The most authoritative and reliable way to avoid this ambiguity is to use "timestamp with time zone" (frequently aliased as "timestamptz" in PostgreSQL) for all timestamp columns [7][1][2][3]. This column type stores a point in time internally as UTC, which allows TypeORM to map the value to a JavaScript Date object consistently without implicit shifts [4][2][5]. If you must use "timestamp without time zone," common workarounds include: - Setting the TZ environment variable to UTC (e.g., TZ=UTC node main.ts) for your Node.js process to ensure consistency [1][3]. - Explicitly configuring the database driver to parse dates as UTC [1]. - Using @Column({ type: 'timestamptz' }) or @CreateDateColumn({ type: 'timestamptz' }) in your entity definitions to force correct timezone handling, even if the underlying column in legacy schemas is "timestamp" [2][3].
Citations:
- 1: [BUG]: Column with
timestamp without time zonetype is read as local date, not UTC (PostgreSQL) typeorm/typeorm#9627 - 2: https://stackoverflow.com/questions/79812429/typeorm-createdatecolumn-returns-wrong-time-compared-to-postgresql
- 3: UpdatedAt timestamp value is UTC but with GMT offset typeorm/typeorm#11598
- 4: https://dev.wanago.io/2021/03/15/postgresql-typeorm-date-time/
- 5: https://wanago.io/2021/03/15/postgresql-typeorm-date-time/
- 6: TypeORM force add/substruct the local time zone offset from Javascript Date for timestamp type of PostgreSQL typeorm/typeorm#9943
- 7: https://stackoverflow.com/questions/76702923/typeorm-is-always-mapping-timestamp-without-timezone-to-date-object
🏁 Script executed:
find backend -name "password-reset.entity.ts" -o -name "*email-verification*entity.ts" | head -20Repository: rocket-admin/rocketadmin
Length of output: 189
🏁 Script executed:
grep -r "Constants.ONE_DAY_AGO" backend --include="*.ts" -A 3 -B 3 | head -50Repository: rocket-admin/rocketadmin
Length of output: 2556
🏁 Script executed:
grep -r "createdAt" backend/src/entities/user --include="*.ts" -A 2 -B 2 | grep -E "(timestamp|createdAt)" | head -20Repository: rocket-admin/rocketadmin
Length of output: 2376
🏁 Script executed:
grep -r "ONE_DAY_AGO" backend --include="*.ts" -B 2 -A 2 | head -40Repository: rocket-admin/rocketadmin
Length of output: 2215
🏁 Script executed:
cat backend/src/entities/email/email-verification.entity.tsRepository: rocket-admin/rocketadmin
Length of output: 627
🏁 Script executed:
cat backend/src/entities/user/user-password/password-reset.entity.tsRepository: rocket-admin/rocketadmin
Length of output: 610
Use timestamptz to avoid timezone-dependent comparison issues.
createdAt is declared as timestamp (without time zone) but is compared downstream against Constants.ONE_DAY_AGO() (a JS Date/UTC instant). If the database session timezone is not UTC, the createdAt >= :valid_after comparison can be skewed by the timezone offset, causing password reset and email verification links to expire earlier or later than the intended 24-hour window. Using timestamptz ensures the timestamp is stored and compared as UTC, making the comparison unambiguous. This applies to both password-reset.entity.ts and email-verification.entity.ts.
🤖 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 `@backend/src/entities/user/user-password/password-reset.entity.ts` around
lines 12 - 13, The createdAt column is currently defined with type 'timestamp'
which is timezone-naive; update the `@Column` decorator for the createdAt property
in password-reset.entity.ts (and likewise in email-verification.entity.ts) to
use type: 'timestamptz' so DB stores UTC-aware timestamps, ensuring comparisons
against Constants.ONE_DAY_AGO() (and any createdAt >= :valid_after queries)
behave correctly across timezones; keep the property name createdAt and only
change the column type definition in the `@Column`(...) metadata.
Comment on lines
+179
to
+185
| if (!isReadOnlyMongoAggregationPipeline(query)) { | ||
| return { | ||
| isSafe: false, | ||
| reason: | ||
| 'Query must be a read-only aggregation pipeline (write stages or server-side JavaScript operators such as $out, $merge, $function, $accumulator, $where are not allowed)', | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the validator implementation and its expected input type.
fd -t f 'query-validators.ts' --exec cat {}
echo "----- callers of isReadOnlyMongoAggregationPipeline -----"
rg -nP '\bisReadOnlyMongoAggregationPipeline\s*\(' -C3
echo "----- existing tests / sample Mongo queries for visualization panels -----"
rg -nP 'checkMongoQueryIsSafe' -C3Repository: rocket-admin/rocketadmin
Length of output: 5485
Ensure isReadOnlyMongoAggregationPipeline is implemented/exported and accepts the raw Mongo query string (don’t accidentally reject valid non-aggregation reads).
backend/src/entities/visualizations/panel/utils/check-query-is-safe.util.tscallsisReadOnlyMongoAggregationPipeline(query)at lines 179-185, butbackend/src/ai-core/tools/query-validators.tsdoes not exportisReadOnlyMongoAggregationPipeline(onlyisValidSQLQuery,isValidMongoDbCommand,wrapQueryWithLimit,cleanAIJsonResponseare present). Confirm the imported.../query-validators.jsactually contains this function (or fix the import/export mismatch), otherwise the safety check may fail at build/runtime.- After locating the correct validator, confirm its input contract (does it parse the raw
stringitself vs expecting a parsed pipeline) and whether it incorrectly rejects legitimate read-only non-aggregation queries (e.g.,find/distinct) for visualization panels.
🤖 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 `@backend/src/entities/visualizations/panel/utils/check-query-is-safe.util.ts`
around lines 179 - 185, The safety check calls
isReadOnlyMongoAggregationPipeline(query) but that symbol is not exported from
backend/src/ai-core/tools/query-validators.ts and/or may expect a parsed
pipeline rather than the raw query string; fix by either exporting
isReadOnlyMongoAggregationPipeline from query-validators (or re-importing the
correct module that defines it), or change the call in
check-query-is-safe.util.ts to the appropriate validator (e.g.,
isValidMongoDbCommand or a new isReadOnlyMongoQuery) that accepts raw strings;
ensure the validator's contract is respected (parse the string before calling if
the validator expects a parsed pipeline) and broaden the check to allow
legitimate read-only non-aggregation queries like find/distinct so visualization
panels aren't incorrectly rejected.
Comment on lines
+78
to
+79
| "maximumWarning": "20kb", | ||
| "maximumError": "32kb" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the largest component style files and check for potential duplication
echo "=== Top 20 largest component style files ==="
fd -e scss -e css --type f -E 'node_modules' -E 'dist' | \
xargs wc -c | \
sort -rn | \
head -n 21
echo -e "\n=== Components with styles over 10kb ==="
fd -e scss -e css --type f -E 'node_modules' -E 'dist' | \
while read file; do
size=$(wc -c < "$file")
if [ "$size" -gt 10240 ]; then
echo "$size bytes - $file"
fi
done | sort -rn
echo -e "\n=== Check for duplicate CSS patterns (common selectors) ==="
rg -t scss -t css --no-filename -oP '^\.[a-z-]+\s*\{' frontend/src | \
sort | uniq -c | sort -rn | head -n 20Repository: rocket-admin/rocketadmin
Length of output: 2781
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== angular.json anyComponentStyle (lines 70-90) ==="
nl -ba frontend/angular.json | sed -n '70,90p'
echo "=== angular.json anyComponentStyle (lines 120-150) ==="
nl -ba frontend/angular.json | sed -n '120,150p'
echo "=== Component style sizes over thresholds (CSS/SCSS under frontend/src) ==="
# Count/list sizes; include both .css and .scss; exclude node_modules/dist
mapfile -t files < <(fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path)
python3 - <<'PY'
import os
files = os.environ.get("FILES","")
PY
python3 - <<'PY'
import os, sys, subprocess, json
# Re-run fd within python to avoid shell env passing
import subprocess
res = subprocess.check_output(["bash","-lc", "fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path"], text=True)
paths = [p for p in res.splitlines() if p.strip()]
items=[]
for p in paths:
try:
items.append((os.path.basename(p), p, os.path.getsize(p)))
except Exception:
pass
items.sort(key=lambda x: x[2], reverse=True)
def summarize(th):
over=[x for x in items if x[2]>th]
over.sort(key=lambda x:x[2], reverse=True)
print(f"\n=== count > {th} bytes ({th/1024:.1f}kb) ===")
print(len(over))
for name,p,size in over[:20]:
print(f"{size} bytes - {p}")
summarize(10240) # 10kb
summarize(20480) # 20kb
summarize(32768) # 32kb
print("\n=== Top 15 ===")
for name,p,size in items[:15]:
print(f"{size} bytes - {p}")
PY
echo "=== Duplicate selector-ish patterns check (no rg -t scss/cs filetype) ==="
# Look for repeated simple class selectors blocks; exclude node_modules/dist
# (This is a heuristic; it won't prove duplication, but it avoids the prior rg filetype failure.)
fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path | \
xargs rg -n --no-filename -o '^\.[A-Za-z_-][A-Za-z0-9_-]*\s*\{' | \
sort | uniq -c | sort -nr | head -n 20Repository: rocket-admin/rocketadmin
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== angular.json anyComponentStyle (production) around lines 70-95 ==="
awk 'NR>=70 && NR<=95 {printf "%d:%s\n", NR, $0}' frontend/angular.json
echo "=== angular.json anyComponentStyle (saas-production) around lines 120-155 ==="
awk 'NR>=120 && NR<=155 {printf "%d:%s\n", NR, $0}' frontend/angular.json
echo "=== Component style sizes (CSS/SCSS under frontend/src) > 10kb / 20kb / 32kb ==="
# Use fd under frontend/src so we don't scan node_modules/dist
mapfile -t paths < <(fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path)
python3 - <<'PY'
import os, sys
paths = os.environ.get("PATHS","").splitlines()
# (If PATHS isn't set, fall back to reading from stdin)
if not paths or paths==['']:
paths = sys.stdin.read().splitlines()
items=[]
for p in paths:
p=p.strip()
if not p:
continue
try:
items.append((os.path.getsize(p), p))
except OSError:
pass
items.sort(reverse=True)
def show(th):
over=[(s,p) for (s,p) in items if s>th]
print(f"\n=== count > {th} bytes ({th/1024:.1f}kb) ===")
for s,p in over[:25]:
print(f"{s} bytes - {p}")
show(10*1024)
show(20*1024)
show(32*1024)
print("\n=== Top 20 by size ===")
for s,p in items[:20]:
print(f"{s} bytes - {p}")
PY <<EOF
$(printf "%s\n" "${paths[@]}")
EOF
echo "=== Selector-block duplication heuristic (counts repeated simple class selectors) ==="
# Avoid rg -t (filetype) issues by using globs on extensions.
fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path | \
xargs rg -n --no-filename -o --glob '*.scss' --glob '*.css' '^\.[A-Za-z_-][A-Za-z0-9_-]*[[:space:]]*\{' 2>/dev/null | \
sort | uniq -c | sort -nr | head -n 30Repository: rocket-admin/rocketadmin
Length of output: 2133
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Git diff for anyComponentStyle budgets (frontend/angular.json) ==="
# Show only the relevant portion of the diff to capture the previous values
git diff -U3 -- frontend/angular.json | sed -n '/anyComponentStyle/,+30p'
echo "=== Clean size listing: CSS/SCSS under frontend/src, >10kb ==="
fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path | \
while IFS= read -r f; do
[ -f "$f" ] || continue
sz=$(wc -c < "$f" 2>/dev/null || echo 0)
if [ "$sz" -gt 10240 ]; then
printf "%s\t%s\n" "$sz" "$f"
fi
done | sort -nr | head -n 50
echo "=== Count of files >10kb / >20kb / >32kb ==="
fd -e css -e scss --type f -E 'node_modules' -E 'dist' frontend/src --full-path | \
while IFS= read -r f; do
[ -f "$f" ] || continue
sz=$(wc -c < "$f" 2>/dev/null || echo 0)
if [ "$sz" -gt 10240 ]; then echo "gt10:$f:$sz"; fi
if [ "$sz" -gt 20480 ]; then echo "gt20:$f:$sz"; fi
if [ "$sz" -gt 32768 ]; then echo "gt32:$f:$sz"; fi
done | awk -F: '{c[$1]++} END {for(k in c) print k,c[k]}' | sort -t' ' -k2 -nrRepository: rocket-admin/rocketadmin
Length of output: 1039
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for anyComponentStyle warning/error values in frontend/angular.json ==="
rg -n --no-heading 'type": "anyComponentStyle"|maximumWarning|maximumError' frontend/angular.json
echo "=== Search for '10kb' occurrences in frontend/angular.json ==="
rg -n --no-heading '10kb' frontend/angular.json || true
echo "=== Duplicate selector-ish heuristic: repeated simple class selector blocks ==="
# Heuristic: count top occurrences of ".class {"
fd --type f -e css -e scss --full-path --exclude node_modules --exclude dist frontend/src | \
xargs rg -n --no-filename -o '^\.[A-Za-z_-][A-Za-z0-9_-]*[[:space:]]*\{' 2>/dev/null | \
sed 's/.*\(\.[A-Za-z_-][A-Za-z0-9_-]*[[:space:]]*{\).*/\1/' | \
sort | uniq -c | sort -nr | head -n 20Repository: rocket-admin/rocketadmin
Length of output: 1131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for anyComponentStyle warning/error values in frontend/angular.json ==="
rg -n --no-heading 'type": "anyComponentStyle"|maximumWarning|maximumError' frontend/angular.json
echo "=== Search for '10kb' occurrences in frontend/angular.json ==="
rg -n --no-heading '10kb' frontend/angular.json || true
echo "=== Duplicate selector-ish heuristic: repeated simple class selector blocks ==="
# Heuristic: count top occurrences of ".class {" across frontend/src (*.css/*.scss), excluding node_modules/dist
fd --type f -e css -e scss --full-path --exclude node_modules --exclude dist frontend/src | \
xargs rg -n --no-filename -o '^\.[A-Za-z_-][A-Za-z0-9_-]*[[:space:]]*\{' 2>/dev/null | \
sed 's/.*\(\.[A-Za-z_-][A-Za-z0-9_-]*[[:space:]]*{\).*/\1/' | \
sort | uniq -c | sort -nr | head -n 20Repository: rocket-admin/rocketadmin
Length of output: 1131
Investigate component style bloat before further relaxing anyComponentStyle budgets (production/saas-production).
frontend/angular.json sets anyComponentStyle to maximumWarning: 20kb / maximumError: 32kb in both the production and saas-production configurations. In frontend/src, there are currently 2 component style files > 20kb and 1 > 32kb (CSS/SCSS). The selector repetition heuristic also shows common class blocks repeated across styles (e.g., .wrapper, .actions), which may indicate duplication.
Before relying on higher limits for mobile performance, identify the specific component styles crossing these thresholds and determine whether the growth is from duplication/overlapping selectors vs genuinely new UI; then extract shared styles / reduce duplication where possible.
🤖 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/angular.json` around lines 78 - 79, The build budget
anyComponentStyle in frontend/angular.json (maximumWarning: 20kb / maximumError:
32kb) is being hit; locate the specific component style files under frontend/src
that exceed 20kb (and the one >32kb), inspect them for repeated selector blocks
such as .wrapper and .actions, and determine if the excess is from duplicated
shared layout rules versus unique component styling; refactor by extracting
common rules into a shared/global stylesheet or a shared SCSS module and import
it into those components (update component styleUrls to remove duplicated
rules), remove unused selectors and compress verbose selectors, then re-run the
build to confirm budgets are met before considering raising anyComponentStyle
values.
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.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use Angular's built-in control flow instead of *ngIf/*ngFor.
This is new markup, so it should use @if/@for. @for requires a track expression (track $index), which also makes the trackByIndex method in the component unnecessary.
♻️ Proposed conversion (representative)
- <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">
- ...
- </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">
+ ...
+ </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">
- ...
- </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">
+ ...
+ </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, Replace the legacy structural directives in the template
with Angular 19 control-flow syntax: convert *ngIf="..." blocks to `@if`(...) /
`@else` where appropriate (e.g., the spinner/success blocks, summary and error
display using the existing errorText(), done(), hasSummary() predicates) and
convert the status list *ngFor="let m of messages(); let last = last; trackBy:
trackByIndex" to the new `@for` syntax (e.g., `@for`(let m of messages(); let last =
last; track $index) and remove the trackBy: trackByIndex usage), then delete the
now-unused trackByIndex() method from the component class; keep all current
bindings and class expressions ([class.status-log__item_current],
[class.status-log__item_done]) and calls to done(), hasSummary(), messages(),
tablesConfigured(), widgetsCount(), settingsCount(), and errorText() intact.
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.
Navigation proceeds to the dashboard even on the error path.
startConfiguring returns false on failure, but the result is ignored and the component still waits ~1.8s and navigates to /dashboard. The user only glimpses errorText() briefly before being redirected. Consider branching on the result (or errorText()) to keep the user on the error state.
🛠 Proposed fix
- 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 ok = await this._configState.startConfiguring(connectionId);
+ if (!ok) return; // leave the error state visible
+ // 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 ok = await this._configState.startConfiguring(connectionId); | |
| if (!ok) return; // leave the error state visible | |
| // 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, startConfiguring's boolean result is ignored so navigation
always happens; change the flow in the async block that calls
this._configState.startConfiguring(connectionId) to capture its return value and
only perform the post-wait navigation this._router.navigate(['/dashboard',
connectionId]) if the result is truthy (or if this.errorText() is empty). If
startConfiguring returns false or errorText() is set, skip the navigation and
keep the component in the error state so the user can read the message (you can
still await the short delay for UX), and ensure any subsequent UI state updates
rely on the same boolean/result to avoid racing into the dashboard.
Comment on lines
44
to
+48
| private async _runSetup(connectionId: string): Promise<boolean> { | ||
| this._queue = []; | ||
| this._stopDrain(); | ||
| this._currentDrainIntervalMs = this._drainIntervalMsNormal; | ||
| this._enqueue({ type: 'message', text: 'Starting configuration…' }); |
There was a problem hiding this comment.
Reset progress$ to null at the start of each run to avoid replaying stale state.
ConfigurationStateService is providedIn: 'root' and progress$ is a BehaviorSubject that retains its last value. On a second auto-configure (without a full reload), a freshly mounted AutoConfigureComponent subscribes and immediately receives the previous complete/error event. That makes done()/errorText() momentarily true and pushes a stale "All done…" line before any new progress arrives. Resetting here clears the prior state.
🐛 Proposed fix
private async _runSetup(connectionId: string): Promise<boolean> {
this._queue = [];
this._stopDrain();
+ this.progress$.next(null);
this._currentDrainIntervalMs = this._drainIntervalMsNormal;
this._enqueue({ type: 'message', text: 'Starting configuration…' });🤖 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 44 -
48, In ConfigurationStateService's _runSetup method reset the progress$ subject
to null at the start of each run to avoid replaying stale terminal state: update
the _runSetup implementation (in ConfigurationStateService) to call
progress$.next(null) (or reinitialize the BehaviorSubject) before enqueuing the
"Starting configuration…" message so new subscribers (e.g.
AutoConfigureComponent) don't immediately get a previous complete/error value.
Comment on lines
+53
to
+83
| const response = await fetch(url, { | ||
| method: 'GET', | ||
| credentials: 'include', | ||
| headers: { masterpwd }, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Setup failed with status ${response.status}`); | ||
| } | ||
|
|
||
| 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); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant code in the service
FILE="frontend/src/app/services/configuration-state.service.ts"
echo "== File: $FILE =="
stat -c '%s bytes' "$FILE" || true
echo
echo "== Matches in service =="
rg -n "getReader\(\)|while \(true\)|_configuring|_parseAndEnqueue|fetch\(|AbortController|timeout" "$FILE" || true
echo
echo "== Context around fetch/streaming reader loop =="
# Print a focused window around the provided line range (53-83) plus surrounding cleanup
sed -n '1,140p' "$FILE" | nl -ba | sed -n '40,120p'
echo
echo "== Where _configuring is set/cleared =="
rg -n "_configuring\s*=" "$FILE" || true
rg -n "finally\s*\{|catch\s*\(|throw|return" "$FILE" || true
echo
echo "== AbortController usage across frontend (quick scan) =="
rg -n "AbortController" frontend/src/app || trueRepository: rocket-admin/rocketadmin
Length of output: 949
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/app/services/configuration-state.service.ts"
echo "== File (first 220 lines) =="
python3 - <<'PY'
import itertools
path="frontend/src/app/services/configuration-state.service.ts"
with open(path,'r',encoding='utf-8') as f:
for i,line in zip(range(1,221), f):
print(f"{i:4d}: {line.rstrip()}")
PY
echo
echo "== AbortController / timeout usage in service =="
rg -n "AbortController|timeout|setTimeout|clearTimeout|signal:" "$FILE" || true
echo
echo "== _configuring set/clear context =="
python3 - <<'PY'
import re
path="frontend/src/app/services/configuration-state.service.ts"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,l in enumerate(lines, start=1):
if "_configuring" in l:
print(f"{i:4d}: {l.rstrip()}")
PY
echo
echo "== Call sites / method usage (search for configuration-state.service import) =="
rg -n "configuration-state\.service" frontend/src/app || true
echo
echo "== AbortController usage anywhere in frontend/src/app =="
rg -n "AbortController" frontend/src/app || trueRepository: rocket-admin/rocketadmin
Length of output: 8451
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect auto-configure consumer
FILE1="frontend/src/app/components/auto-configure/auto-configure.component.ts"
echo "== $FILE1 =="
python3 - <<'PY'
import itertools
path="frontend/src/app/components/auto-configure/auto-configure.component.ts"
with open(path,'r',encoding='utf-8') as f:
for i,line in zip(range(1,260), f):
print(f"{i:4d}: {line.rstrip()}")
PY
echo
echo "== Matches for startConfiguring / ngOnDestroy in auto-configure.component.ts =="
rg -n "startConfiguring\(|ngOnDestroy|AbortController" "$FILE1" || true
echo
# Inspect dashboard consumer(s)
FILE2="frontend/src/app/components/dashboard/dashboard.component.ts"
echo "== $FILE2 (startConfiguring references only) =="
rg -n "startConfiguring\(|ngOnDestroy|AbortController" "$FILE2" || trueRepository: rocket-admin/rocketadmin
Length of output: 8417
Add a timeout/AbortController for the streaming fetch so stalled streams can’t block config
ConfigurationStateService._runSetup() reads the response stream with while (true) and no AbortController/timeout, so if the server stalls without closing the stream, startConfiguring() never resolves and the caller (AutoConfigureComponent.ngOnInit() awaiting it) won’t navigate; the finally that clears _configuring also won’t run until the stream ends. Add an AbortController (with a bounded timeout) and cancel it in AutoConfigureComponent.ngOnDestroy() (or via a service cancel method keyed by 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/services/configuration-state.service.ts` around lines 53 -
83, The streaming fetch in ConfigurationStateService._runSetup() needs an
AbortController with a bounded timeout to avoid a stalled stream blocking
startConfiguring() and preventing finally cleanup; modify _runSetup() to create
an AbortController, pass controller.signal into fetch, set a timeout to call
controller.abort(), and ensure any reader loops handle abort errors (clean up
reader and rethrow or handle accordingly). Expose a way to cancel the controller
(e.g., store controller keyed by connectionId on the ConfigurationStateService
and add a cancelConnection(connectionId) method) and call that from
AutoConfigureComponent.ngOnDestroy() (or clear the timeout and abort the
controller there) so the stream is aborted when the component is destroyed.
Ensure startConfiguring() still resolves/rejects consistently and that
_configuring cleanup in finally runs when the abort triggers.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/app/components/users/users.component.css (1)
115-120: ⚡ Quick winAdd list-style removal for the parent
<ul>element.The
.userclass styles list items, but the parent<ul>element in the HTML (line 99) will display default bullet points unless explicitly removed.🎨 Suggested addition
+ul { + list-style: none; + padding: 0; + margin: 0; +} + .user { display: flex; align-items: center;🤖 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/users/users.component.css` around lines 115 - 120, The .user rule styles li elements but doesn't disable default bullets on the parent list; update the stylesheet to target the parent <ul> (e.g., the component's users list selector or a .users-list / ul selector used in the template) and add list-style: none; margin: 0; padding: 0; so the bullets are removed and spacing is controlled; modify the CSS near the .user declaration to include the parent list selector and those properties.
🤖 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/users/users.component.html`:
- Around line 112-120: The button inside users.component (the element that calls
openDeleteUserDialog(user, groupItem.group) and posthog.capture(...)) still has
the Angular Material directive matListItemMeta which is only valid on
mat-list-item; remove the matListItemMeta attribute from that button to
eliminate the invalid directive usage and any related lint/runtime warnings and
leave the other attributes (mat-icon-button, matTooltip, angulartics2On,
angularticsAction, (click) handler) unchanged.
---
Nitpick comments:
In `@frontend/src/app/components/users/users.component.css`:
- Around line 115-120: The .user rule styles li elements but doesn't disable
default bullets on the parent list; update the stylesheet to target the parent
<ul> (e.g., the component's users list selector or a .users-list / ul selector
used in the template) and add list-style: none; margin: 0; padding: 0; so the
bullets are removed and spacing is controlled; modify the CSS near the .user
declaration to include the parent list selector and those properties.
🪄 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: 79cf5f1f-370a-4678-9d8e-09f4dc5be014
📒 Files selected for processing (2)
frontend/src/app/components/users/users.component.cssfrontend/src/app/components/users/users.component.html
Comment on lines
+112
to
+120
| @if (currentUser()?.email !== user.email && canManage()) { | ||
| <button matListItemMeta type="button" mat-icon-button | ||
| angulartics2On="click" | ||
| angularticsAction="Users access: delete user is clicked" | ||
| matTooltip="Delete user" | ||
| (click)="openDeleteUserDialog(user, groupItem.group); posthog.capture('Users access: delete user is clicked')"> | ||
| <mat-icon>person_remove</mat-icon> | ||
| </button> | ||
| } |
There was a problem hiding this comment.
Remove invalid matListItemMeta directive.
The matListItemMeta directive on line 113 is specific to Angular Material's mat-list-item component. Since the markup was refactored to use plain <li> elements, this directive no longer applies and will have no effect.
🔧 Proposed fix
`@if` (currentUser()?.email !== user.email && canManage()) {
- <button matListItemMeta type="button" mat-icon-button
+ <button type="button" mat-icon-button
angulartics2On="click"
angularticsAction="Users access: delete user is clicked"📝 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
| @if (currentUser()?.email !== user.email && canManage()) { | |
| <button matListItemMeta type="button" mat-icon-button | |
| angulartics2On="click" | |
| angularticsAction="Users access: delete user is clicked" | |
| matTooltip="Delete user" | |
| (click)="openDeleteUserDialog(user, groupItem.group); posthog.capture('Users access: delete user is clicked')"> | |
| <mat-icon>person_remove</mat-icon> | |
| </button> | |
| } | |
| `@if` (currentUser()?.email !== user.email && canManage()) { | |
| <button type="button" mat-icon-button | |
| angulartics2On="click" | |
| angularticsAction="Users access: delete user is clicked" | |
| matTooltip="Delete user" | |
| (click)="openDeleteUserDialog(user, groupItem.group); posthog.capture('Users access: delete user is clicked')"> | |
| <mat-icon>person_remove</mat-icon> | |
| </button> | |
| } |
🤖 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/users/users.component.html` around lines 112 -
120, The button inside users.component (the element that calls
openDeleteUserDialog(user, groupItem.group) and posthog.capture(...)) still has
the Angular Material directive matListItemMeta which is only valid on
mat-list-item; remove the matListItemMeta attribute from that button to
eliminate the invalid directive usage and any related lint/runtime warnings and
leave the other attributes (mat-icon-button, matTooltip, angulartics2On,
angularticsAction, (click) handler) unchanged.
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
4 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
Two commits on top of
main(#1814 was already merged):8cedb7754quick-filter chips inline: horizontal-scroll saved-filter chips in the search row, "+" / "Create fast filter" trigger with vertical divider, three-dot menu on each chip wired to the panel, conditions editor on its own row via :has(), tightened spacing and Sort by text color, paginator with "Rows per page:" → "per page:" label, items-per-page flush left, range/buttons right0893e98a4widgets page + AI panel + sidebar polish: mobile widget cards layout with arrow-back header + table name, hide breadcrumbs and per-card delete on mobile, empty state cleanups (icon hidden, Docs+Create right-aligned together, symmetric padding, no Back button at the bottom), AI Insights forced full-size only and reset when navigating away, FAB Add row hides while chat is open, sidebar drawer sits below the nav header on mobile + hides dark toolbar, Hosted databases added between Company and Secrets, smaller account-section items, Upgrade nudged up, logo shortens only inside a connection, mobile loading skeleton cards, row preview dividers inset to text width, filter-edit dialog opens as bottom sheet from active-filter chip tapTest plan
[Filter] │ [+ chip chip chip →]and chips scroll horizontally inside their slot only (no page-wide horizontal scroll)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI & Style
Other