feat(web): settings feature — domain, application, infrastructure and presentation layers#78
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughA new settings feature is introduced with routing, domain models, abstract and HTTP-based repositories, application services using signal-based state, and a complete presentation layer including components for account, sessions, modules, widgets, and placeholder pages. ChangesSettings Feature Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoAdd complete settings feature with preferences, sessions, and account management
WalkthroughsDescription• Introduces complete settings feature with domain, application, infrastructure, and presentation layers following Clean Architecture • Implements preferences service with optimistic updates for feature and widget configuration management • Adds session management with browser/OS detection and device revocation capabilities • Creates settings layout with four-column navigation (Account, Workspace, Security, Data) and nine child routes • Provides read-only account profile card and stub tabs for future features (preferences, notifications, security, import-export, danger-zone) Diagramflowchart LR
A["Settings Routes<br/>settings.routes.ts"] --> B["Settings Layout<br/>Component"]
B --> C["Account Tab"]
B --> D["Modules Tab"]
B --> E["Widgets Tab"]
B --> F["Sessions Tab"]
B --> G["Stub Tabs"]
D --> H["PreferencesService"]
E --> H
H --> I["PreferencesRepository"]
I --> J["PreferencesHttpRepository"]
F --> K["SessionService"]
K --> L["SessionRepository"]
L --> M["SessionHttpRepository"]
J --> N["API: PATCH /preferences"]
M --> O["API: GET/DELETE /auth/sessions"]
File Changes1. apps/web/src/app/features/learning-resource/presentation/learning-resource.routes.ts
|
Code Review by Qodo
1. Session rollback race
|
|
✅ All CI checks passed! The code is ready for review. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
apps/web/src/app/features/settings/presentation/settings-layout/settings-layout.component.html (1)
15-15: ⚡ Quick winMigrate to Tailwind v4 postfix important modifier syntax — deprecated prefix form still works but should be updated
The code uses Tailwind v3 prefix syntax for the important modifier (
!text-violet-400), which is deprecated in v4 but remains functional through backwards compatibility. For consistency with v4 conventions and to future-proof the code, migrate these class names to use the postfix form (text-violet-400!).Update all 9 occurrences to the v4 postfix syntax
- routerLinkActive="bg-slate-800 !text-violet-400" + routerLinkActive="bg-slate-800 text-violet-400!"- routerLinkActive="bg-slate-800 !text-red-400" + routerLinkActive="bg-slate-800 text-red-400!"Applies to lines: 15, 26, 37, 55, 65, 85, 97, 114, 126
🤖 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 `@apps/web/src/app/features/settings/presentation/settings-layout/settings-layout.component.html` at line 15, The Tailwind important modifier is using the old prefix form (!text-violet-400); update all occurrences to the v4 postfix form (text-violet-400!) to future-proof styling. Search the template for class strings that contain "!text-violet-400" (examples include the routerLinkActive attribute on the settings layout links) and replace each "!text-violet-400" with "text-violet-400!" across the nine occurrences so the routerLinkActive and other link classes use the new postfix syntax.
🤖 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 `@apps/web/src/app/features/settings/application/preferences.service.ts`:
- Around line 12-14: The shared signals loading, saving, and error cause race
conditions across concurrent operations; change to per-operation private signals
(e.g., loadingFeature, loadingWidget, savingFeature, savingWidget, errorFeature,
errorWidget) and update loadFeatureConfig, loadWidgetConfig,
updateFeatureConfig, updateWidgetConfig to set only their corresponding private
signals and errors; then derive the existing public signals via computed
aggregates (loading = computed(() => loadingFeature() || loadingWidget()),
saving similarly, and error = computed(() => errorFeature() ?? errorWidget() ??
null) or another merge strategy) so the public API remains unchanged while
preventing cross-operation races.
In `@apps/web/src/app/features/settings/application/session.service.ts`:
- Around line 26-46: The issue: prior error messages remain visible after a
later successful revoke because revokeSession and revokeAllOtherSessions never
clear this.error before starting the optimistic update and async call; fix by
clearing the error at the start of each method (e.g., call this.error.set(null)
or clear value) before updating this.sessions and awaiting repository methods in
revokeSession and revokeAllOtherSessions, keeping the existing catch behavior
that restores prev and sets the error on failure.
In
`@apps/web/src/app/features/settings/presentation/account/account.component.ts`:
- Line 55: The template uses key.replace('-', ' ') which only replaces the first
hyphen (e.g., "resource-library-pro" → "resource library-pro"); update the usage
where the template binds {{ key.replace('-', ' ') }} to replace all hyphens
instead (for example use key.replaceAll('-', ' ') or key.split('-').join(' '))
so FeatureKey values render all hyphens as spaces; locate the interpolation
referencing key in account.component (the binding that calls replace) and swap
to a global replacement approach.
In
`@apps/web/src/app/features/settings/presentation/modules/modules.component.html`:
- Around line 46-59: The switch button with role="switch" (the element using
(click)="toggle(module)" and [class]="isOn(module.key) ? ..." ) has no
accessible name; link it to the module label by providing an accessible name
(either add aria-label using module.label or add aria-labelledby pointing to the
DOM element that renders module.label). Ensure the label element that displays
module.label has a unique id (e.g., based on module.key) and then set the
button's aria-labelledby to that id (or set aria-label="{{module.label}}") so
screen readers announce the switch; keep the existing disabled and state
bindings (saving(), isOn, module.locked) unchanged.
- Line 50: Replace the Tailwind focus utility on the toggle element that
currently contains the class string "relative inline-flex h-5 w-9 flex-shrink-0
rounded-full border-2 border-transparent transition-colors focus:outline-none
disabled:cursor-not-allowed" by changing focus:outline-none to
focus:outline-hidden; update the class attribute where this string appears in
modules.component.html (the toggle element in ModulesComponent template) so it
uses focus:outline-hidden instead of focus:outline-none.
In
`@apps/web/src/app/features/settings/presentation/sessions/sessions.component.html`:
- Around line 67-73: The "Revoke" buttons in sessions.component HTML are
announced generically; update the button element that calls revoke(session) to
include an aria-label that uniquely describes the session (e.g., use session.id,
session.device, or session.createdAt) so screen readers hear "Revoke session for
[identifier]". Locate the button with (click)="revoke(session)" and add an
aria-label binding like [attr.aria-label]="`Revoke session for ${session.device
|| session.id || session.createdAt}`" so each button is distinguishable.
In
`@apps/web/src/app/features/settings/presentation/sessions/sessions.component.ts`:
- Around line 31-39: In getBrowser, the Chrome check (/Chrome\// && !/Chromium/)
runs before the Opera check (/OPR\/|Opera\//), causing modern Opera UAs (which
contain "Chrome/") to be misidentified; fix by moving the Opera regex check so
it executes before the Chrome check (i.e., evaluate /OPR\/|Opera\// prior to
/Chrome\// && !/Chromium/) so Opera is detected correctly.
In
`@apps/web/src/app/features/settings/presentation/widgets/widgets.component.html`:
- Around line 39-52: The switch button in widgets.component.html (and the
identical toggle in modules.component.html) lacks an accessible name and a
visible focus indicator: update the button element (the one calling
toggle(widget) and using isOn(widget.key) and saving()) to include an accessible
label via [attr.aria-label]="'Toggle ' + widget.label" (so screen readers get a
name) and replace the inaccessible focus utility (focus:outline-none) with an
accessible combination such as focus:outline-hidden plus a visible focus-visible
style (e.g., focus-visible:ring-2 and a contrasting ring color and offset) so
keyboard and high-contrast users see focus. Ensure these changes are applied to
both the outer <button> and any identical toggle in modules.component.html.
---
Nitpick comments:
In
`@apps/web/src/app/features/settings/presentation/settings-layout/settings-layout.component.html`:
- Line 15: The Tailwind important modifier is using the old prefix form
(!text-violet-400); update all occurrences to the v4 postfix form
(text-violet-400!) to future-proof styling. Search the template for class
strings that contain "!text-violet-400" (examples include the routerLinkActive
attribute on the settings layout links) and replace each "!text-violet-400" with
"text-violet-400!" across the nine occurrences so the routerLinkActive and other
link classes use the new postfix syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 33c4dbbc-7fc5-48f6-89b3-88a07de1577e
📒 Files selected for processing (24)
apps/web/src/app/features/learning-resource/presentation/learning-resource.routes.tsapps/web/src/app/features/settings/application/preferences.service.tsapps/web/src/app/features/settings/application/session.service.tsapps/web/src/app/features/settings/domain/preferences.repository.tsapps/web/src/app/features/settings/domain/session.repository.tsapps/web/src/app/features/settings/domain/settings.model.tsapps/web/src/app/features/settings/infrastructure/preferences-http.repository.tsapps/web/src/app/features/settings/infrastructure/session-http.repository.tsapps/web/src/app/features/settings/infrastructure/settings.dto.tsapps/web/src/app/features/settings/presentation/account/account.component.tsapps/web/src/app/features/settings/presentation/danger-zone/danger-zone.component.tsapps/web/src/app/features/settings/presentation/import-export/import-export.component.tsapps/web/src/app/features/settings/presentation/modules/modules.component.htmlapps/web/src/app/features/settings/presentation/modules/modules.component.tsapps/web/src/app/features/settings/presentation/notifications/notifications.component.tsapps/web/src/app/features/settings/presentation/preferences/preferences.component.tsapps/web/src/app/features/settings/presentation/security/security.component.tsapps/web/src/app/features/settings/presentation/sessions/sessions.component.htmlapps/web/src/app/features/settings/presentation/sessions/sessions.component.tsapps/web/src/app/features/settings/presentation/settings-layout/settings-layout.component.htmlapps/web/src/app/features/settings/presentation/settings-layout/settings-layout.component.tsapps/web/src/app/features/settings/presentation/settings.routes.tsapps/web/src/app/features/settings/presentation/widgets/widgets.component.htmlapps/web/src/app/features/settings/presentation/widgets/widgets.component.ts
…load race in PreferencesService
…e in SessionService
… reader accessibility
…d v4 postfix syntax
|
✅ All CI checks passed! The code is ready for review. |
|
✅ All CI checks passed! The code is ready for review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(web): Settings UI — account, modules, widgets and session management
Introduces the full
settingsfeature following the same Clean Architecturestructure as
learning-resourceandauth(domain/,application/,infrastructure/,presentation/). Accessible at/settingsfrom the shellsidebar link added in the previous branch.
What's new
Domain
Two abstract repository classes —
PreferencesRepository(feature config +widget config CRUD) and
SessionRepository(list, revoke one, revoke allothers) — plus
settings.model.tsdefining theWidgetKeyconst object andthe
Sessioninterface used across layers.Application —
PreferencesServiceandSessionServiceInjectable services scoped to the settings layout (not
providedIn: 'root').Both expose signals for state (
featureConfig,widgetConfig,sessions,loading,saving,error) and implement optimistic updates with automaticrollback — the signal is updated immediately on user action, the HTTP call is
fired, and on failure the previous value is restored.
Infrastructure
PreferencesHttpRepositoryhitsPATCH /api/v1/preferences/featuresandPATCH /api/v1/preferences/widgets.SessionHttpRepositoryhitsGET /auth/sessions,DELETE /auth/sessions/:id, andDELETE /auth/sessions. Both extend their domain abstract classes and areprovided via Angular's class token DI in
SettingsLayoutComponent.Presentation —
SettingsLayoutComponentTwo-column layout: a fixed left sidebar with four nav groups (Account,
Workspace, Security, Data) and a
<router-outlet>main area. Declaresprovidersfor both services and both repository implementations — servicesare created once per settings visit and destroyed on leave.
Modules tab (
/settings/modules)Toggle list for all five
FeatureKeyvalues. Resource Library is locked(
Always onbadge, toggle disabled). Each toggle callspreferencesService.updateFeatureConfig()with the new set, which hits theAPI optimistically.
enabledKeysis acomputed()signal derived from theservice state.
Dashboard Widgets tab (
/settings/widgets)Same toggle pattern for the five
WidgetKeyvalues, backed byupdateWidgetConfig().Active Sessions tab (
/settings/sessions)Loads all sessions on init, displays browser name and OS extracted from
userAgent(Edge, Chrome, Firefox, Safari, Opera, with Windows/macOS/Linux/iOS/Android detection). Current session highlighted with a violet border and
This devicebadge. Individual Revoke buttons on all non-current rows.Sign out all other devicesbutton appears only when other sessions exist.All revocation is optimistic — list updates instantly, rolls back on error.
Account tab (
/settings/account)Read-only profile card using
AuthStore.userInitials()andAuthStore.displayName()signals. Shows first/last name, email, enabledmodules as violet badges, and account status.
Stub tabs (
preferences,notifications,security,import-export,danger-zone) — render a "Coming soon" placeholder, present in the sidebar toestablish the full navigation structure.
Routing
settingsRouteslazy-loadsSettingsLayoutComponentas parent with ninechild routes. Mounted under
learningResourceRoutesatsettingsso itrenders inside
ShellLayoutComponentand inherits theauthGuard.Summary by CodeRabbit