Add Account Settings page #8672
Merged
Conversation
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
- Changed the "Change Password" link to point to "/account/password". - Added new links for "Auth Token" and "Account Settings" in the navbar. - Introduced secured routes for account settings in the router.
📝 Walkthrough""" WalkthroughThis change introduces a new user account settings page consolidating profile, password, auth token, and theme preference management into a unified interface. It removes the old password and auth token views, updates navigation and routing to reflect the new structure, and moves relevant logic from the navbar to the dedicated account settings components. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…ount-settings-page
- Updated the navbar link for managing organizations to point to the overview page. - Replaced `AccountSettingsTitle` with `SettingsTitle` in multiple views for consistency. - Introduced `SettingsCard` component for better organization of settings-related UI. - Added new views for organization notifications and overview, enhancing the organization management experience. - Cleaned up unused code and improved the structure of organization-related components.
- Updated the CustomPlanUpgradeCard to include a MailOutlined icon and improved layout with responsive columns. - Modified OrganizationNotificationsView to dispatch the updated organization state after saving notification settings. - Added organization name editing functionality in OrganizationOverviewView with validation and state management. - Refactored organization overview stats display for better readability and structure. - Improved imports in organization-related components for consistency.
- Updated OrganizationController to include the current user's identity in the organization JSON response. - Modified organization cards to export components for better reusability. - Improved the upgrade plan modal by wrapping components in a ConfigProvider for consistent theming and layout adjustments. - Refactored the modal body to use responsive columns for displaying upgrade options. These changes aim to improve the user experience and maintainability of the organization management interface.
…ount-settings-page
Co-authored-by: Philipp Otto <philippotto@users.noreply.github.com>
…sos into orga-settings-page
philippotto
reviewed
Jun 25, 2025
7 tasks
Co-authored-by: Philipp Otto <philippotto@users.noreply.github.com>
…ount-settings-page
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
unreleased_changes/8672.md (1)
1-2: Avoid duplication & minor grammar tweak in changelog entryBecause the header already says “Added”, repeating the word in the bullet is redundant. Also, insert the serial/Oxford comma after “passwords” for consistency with the project’s existing changelog style.
-### Added -- Added a new Account Settings page for managing user preferences, passwords and authentication methods. +### Added +- Account Settings page for managing user preferences, passwords, and authentication methods.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
unreleased_changes/8672.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
unreleased_changes/8672.md
[duplication] ~1-~1: Possible typo: you repeated a word.
Context: ### Added - Added a new Account Settings page for managin...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/javascripts/admin/account/account_password_view.tsx (2)
76-90: Consider enhancing password validation rules.The current validation only checks minimum length and matching passwords. Consider adding additional security rules for stronger passwords.
+const validatePasswordStrength = (value: string) => { + if (!value) return Promise.resolve(); + + const hasUpperCase = /[A-Z]/.test(value); + const hasLowerCase = /[a-z]/.test(value); + const hasNumbers = /\d/.test(value); + const hasSpecialChar = /[!@#$%^&*(),.?":{}|<>]/.test(value); + + if (!hasUpperCase || !hasLowerCase || !hasNumbers || !hasSpecialChar) { + return Promise.reject(new Error("Password must contain uppercase, lowercase, numbers, and special characters")); + } + + return Promise.resolve(); +}; { min: MIN_PASSWORD_LENGTH, message: messages["auth.registration_password_length"], }, +{ + validator: (_, value: string) => validatePasswordStrength(value), +},Also applies to: 105-118
150-150: Consider improving password masking display.The hardcoded asterisks could be more dynamic or semantic.
-"***********" +"••••••••••••"Or use a more semantic approach:
-"***********" +<span style={{ fontFamily: 'monospace', letterSpacing: '2px' }}>••••••••••••</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/javascripts/admin/account/account_auth_token_view.tsx(1 hunks)frontend/javascripts/admin/account/account_password_view.tsx(1 hunks)frontend/javascripts/admin/account/account_settings_view.tsx(1 hunks)frontend/javascripts/admin/organization/organization_view.tsx(1 hunks)frontend/javascripts/admin/rest_api.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/javascripts/admin/account/account_settings_view.tsx
- frontend/javascripts/admin/organization/organization_view.tsx
- frontend/javascripts/admin/account/account_auth_token_view.tsx
- frontend/javascripts/admin/rest_api.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: backend-tests
- GitHub Check: frontend-tests
- GitHub Check: build-smoketest-push
🔇 Additional comments (3)
frontend/javascripts/admin/account/account_password_view.tsx (3)
22-34: Good implementation of error handling.The password change logic now includes proper error handling with user feedback, addressing the previous review concern. The flow correctly handles success and failure scenarios.
36-49: Well-implemented password validation logic.The password matching validation function properly handles both password fields and revalidates when necessary. The logic correctly checks for matching passwords and clears errors on the other field when validation passes.
158-165: TypeScript issue resolved appropriately.The addition of
action: undefinedproperly addresses the previous TypeScript error while maintaining the placeholder structure for future passkey functionality.
Co-authored-by: Philipp Otto <philippotto@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
unreleased_changes/8679.md (1)
1-2: Clarify wording and fix possessive apostrophe in changelog entryThe current phrasing is slightly confusing (“organizations settings page”) and is missing the possessive apostrophe flagged by the linter. A clearer wording would be:
-### Changed -- Updated the UI for the organizations settings page. +### Changed +- Updated the UI for the organization’s settings page.This keeps the entry crisp while eliminating the grammatical issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
unreleased_changes/8679.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
unreleased_changes/8679.md
[uncategorized] ~2-~2: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ### Changed - Updated the UI for the organizations settings page.
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
…ount-settings-page
…nossos into account-settings-page
philippotto
reviewed
Jun 27, 2025
philippotto
approved these changes
Jun 27, 2025
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/javascripts/admin/account/account_auth_token_view.tsx(1 hunks)frontend/javascripts/viewer/view/components/command_palette.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/admin/account/account_auth_token_view.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/javascripts/viewer/view/components/command_palette.tsx (2)
frontend/javascripts/theme.tsx (1)
getSystemColorTheme(67-74)frontend/javascripts/admin/rest_api.ts (1)
updateSelectedThemeOfUser(223-231)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
🔇 Additional comments (2)
frontend/javascripts/viewer/view/components/command_palette.tsx (2)
1-1: LGTM - Necessary imports for theme functionalityThe new imports correctly bring in the required dependencies for theme switching functionality.
Also applies to: 10-10, 15-16
217-217: LGTM - Proper integration of theme commandsThe theme entries are correctly integrated into the command palette following the established pattern used by other command generators.
Comment on lines
+169
to
+194
| const getThemeEntries = () => { | ||
| if (activeUser == null) return []; | ||
| const commands: CommandWithoutId[] = []; | ||
|
|
||
| const themesWithNames = [ | ||
| ["auto", "System-default"], | ||
| ["light", "Light"], | ||
| ["dark", "Dark"], | ||
| ] as const; | ||
|
|
||
| for (let [theme, name] of themesWithNames) { | ||
| commands.push({ | ||
| name: `Switch to “${name}” color theme`, | ||
| command: async () => { | ||
| if (theme === "auto") theme = getSystemColorTheme(); | ||
|
|
||
| const newUser = await updateSelectedThemeOfUser(activeUser.id, theme); | ||
| Store.dispatch(setThemeAction(theme)); | ||
| Store.dispatch(setActiveUserAction(newUser)); | ||
| }, | ||
| color: commandEntryColor, | ||
| }); | ||
| } | ||
|
|
||
| return commands; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and improve code clarity
The theme switching functionality is well-implemented and follows the established patterns in the component. However, consider these improvements:
- Missing error handling: The async API call could fail, leaving users without feedback
- Variable reassignment: Reassigning the
themeparameter can be confusing - Hardcoded strings: Theme names could be extracted to constants
Apply this diff to improve error handling and code clarity:
const getThemeEntries = () => {
if (activeUser == null) return [];
const commands: CommandWithoutId[] = [];
const themesWithNames = [
["auto", "System-default"],
["light", "Light"],
["dark", "Dark"],
] as const;
for (let [theme, name] of themesWithNames) {
commands.push({
name: `Switch to "${name}" color theme`,
- command: async () => {
- if (theme === "auto") theme = getSystemColorTheme();
-
- const newUser = await updateSelectedThemeOfUser(activeUser.id, theme);
- Store.dispatch(setThemeAction(theme));
- Store.dispatch(setActiveUserAction(newUser));
- },
+ command: async () => {
+ try {
+ const actualTheme = theme === "auto" ? getSystemColorTheme() : theme;
+ const newUser = await updateSelectedThemeOfUser(activeUser.id, actualTheme);
+ Store.dispatch(setThemeAction(actualTheme));
+ Store.dispatch(setActiveUserAction(newUser));
+ } catch (error) {
+ console.error("Failed to update theme:", error);
+ // TODO: Show user-friendly error notification
+ }
+ },
color: commandEntryColor,
});
}
return commands;
};📝 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
| const getThemeEntries = () => { | |
| if (activeUser == null) return []; | |
| const commands: CommandWithoutId[] = []; | |
| const themesWithNames = [ | |
| ["auto", "System-default"], | |
| ["light", "Light"], | |
| ["dark", "Dark"], | |
| ] as const; | |
| for (let [theme, name] of themesWithNames) { | |
| commands.push({ | |
| name: `Switch to “${name}” color theme`, | |
| command: async () => { | |
| if (theme === "auto") theme = getSystemColorTheme(); | |
| const newUser = await updateSelectedThemeOfUser(activeUser.id, theme); | |
| Store.dispatch(setThemeAction(theme)); | |
| Store.dispatch(setActiveUserAction(newUser)); | |
| }, | |
| color: commandEntryColor, | |
| }); | |
| } | |
| return commands; | |
| }; | |
| const getThemeEntries = () => { | |
| if (activeUser == null) return []; | |
| const commands: CommandWithoutId[] = []; | |
| const themesWithNames = [ | |
| ["auto", "System-default"], | |
| ["light", "Light"], | |
| ["dark", "Dark"], | |
| ] as const; | |
| for (let [theme, name] of themesWithNames) { | |
| commands.push({ | |
| name: `Switch to "${name}" color theme`, | |
| command: async () => { | |
| try { | |
| const actualTheme = theme === "auto" ? getSystemColorTheme() : theme; | |
| const newUser = await updateSelectedThemeOfUser(activeUser.id, actualTheme); | |
| Store.dispatch(setThemeAction(actualTheme)); | |
| Store.dispatch(setActiveUserAction(newUser)); | |
| } catch (error) { | |
| console.error("Failed to update theme:", error); | |
| // TODO: Show user-friendly error notification | |
| } | |
| }, | |
| color: commandEntryColor, | |
| }); | |
| } | |
| return commands; | |
| }; |
🤖 Prompt for AI Agents
In frontend/javascripts/viewer/view/components/command_palette.tsx around lines
169 to 194, add try-catch blocks around the async call to
updateSelectedThemeOfUser to handle potential errors and provide user feedback.
Avoid reassigning the theme variable by introducing a new variable for the
resolved theme value. Extract the theme names from the hardcoded array into
separate constants to improve clarity and maintainability.
7 tasks
hotzenklotz
added a commit
that referenced
this pull request
Jun 30, 2025
This PR fixes a regression of PR #8672 that would always show a warning that your orga plan is using too much storage. For some reason, in a previous WK build most of this code was commented out and got commented in with PR #8672 causing the regression. <img width="1295" alt="Screenshot 2025-06-30 at 16 37 44" src="https://github.com/user-attachments/assets/d6516a5b-5798-4893-8871-aa9d86aea67e" /> ### Issues: - fixes https://scm.slack.com/archives/C5AKLAV0B/p1751293404276169 ------ (Please delete unneeded items, merge only when none are left open) - [ ] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`) - [ ] Added migration guide entry if applicable (edit the same file as for the changelog) - [ ] Updated [documentation](../blob/master/docs) if applicable - [ ] Adapted [wk-libs python client](https://github.com/scalableminds/webknossos-libs/tree/master/webknossos/webknossos/client) if relevant API parts change - [ ] Removed dev-only changes like prints and application.conf edits - [ ] Considered [common edge cases](../blob/master/.github/common_edge_cases.md) - [ ] Needs datastore update after deployment
11 tasks
This was referenced Jul 16, 2025
hotzenklotz
added a commit
that referenced
this pull request
Jul 22, 2025
In preparation of restyling this component in a similar fashion as the account and organization settings, I first refactored several datasets settings & tab components as React functional components to use modern hook APIs etc. I move the beforeUnload into it's own hook. This might be related to issue #5977 ### URL of deployed dev instance (used for testing): - https://functionaldatasetsettings.webknossos.xyz/ ### Issues: - Related to #8672 ------ (Please delete unneeded items, merge only when none are left open) - [x] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`) - [ ] Added migration guide entry if applicable (edit the same file as for the changelog) - [ ] Updated [documentation](../blob/master/docs) if applicable - [ ] Adapted [wk-libs python client](https://github.com/scalableminds/webknossos-libs/tree/master/webknossos/webknossos/client) if relevant API parts change - [ ] Removed dev-only changes like prints and application.conf edits - [ ] Considered [common edge cases](../blob/master/.github/common_edge_cases.md) - [ ] Needs datastore update after deployment --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
7 tasks
hotzenklotz
added a commit
that referenced
this pull request
Aug 11, 2025
This PR moves the standalone "Change Email" view into the new account settings akin to passwords. <img width="1007" height="586" alt="Screenshot 2025-08-07 at 10 14 56" src="https://github.com/user-attachments/assets/4563e87a-350a-4f01-88c8-62171c665ed9" /> ### Steps to test: - Go to Account Settings - Click on Edit icon next to "Email" - Change Email to confirm that the form is still working - https://changeemailview.webknossos.xyz/ ### Issues: - Follow up to #8672 and #2494 ------ (Please delete unneeded items, merge only when none are left open) - [x] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`) - [ ] Added migration guide entry if applicable (edit the same file as for the changelog) - [ ] Updated [documentation](../blob/master/docs) if applicable - [ ] Adapted [wk-libs python client](https://github.com/scalableminds/webknossos-libs/tree/master/webknossos/webknossos/client) if relevant API parts change - [ ] Removed dev-only changes like prints and application.conf edits - [x] Considered [common edge cases](../blob/master/.github/common_edge_cases.md) - [ ] Needs datastore update after deployment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds a new "Account Settings" page to host various other sub-pages for:
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)