Fix multiple things related to adding service accunts #1598
Conversation
VilppeRiskidev
commented
Nov 3, 2025
VilppeRiskidev
commented
- Form validation (for empty value and max. length), which is relflected on the UI
- Removed success/error toasts
- Display errors on the form itself
- Refresh the service account list after one is created
- Hide "Add service account" button if the user isn't the owner of the team
- Fix mobile device responsivity
- Submit form via enter key press
WalkthroughAdds a CSS class for the ServiceAccounts form layout and a responsive breakpoint that stacks settings items at ≤48rem. Updates ServiceAccounts.tsx to derive currentUserTeam and isOwner and render AddServiceAccountForm only for owners; converts nickname to a controlled input with maxLength=32 and inline client-side validation; replaces toast feedback with inline NewAlert error handling; triggers revalidation after create/remove; and resets form, token, nickname, and error state when the modal opens or closes. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
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: 1
🧹 Nitpick comments (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
326-359: Form submission and validation work correctly, but missing input label.The form properly handles Enter key submission and enforces validation. However, the
NewTextInputat line 340 lacks an associated label, which is an accessibility issue.Consider adding a label (check if
NewTextInputsupports alabelprop or wrap it appropriately):<div className="service-accounts__nickname-input"> <NewTextInput + label="Service account nickname" onChange={(e) => { updateFormFieldState({ field: "nickname", value: e.target.value, }); }} placeholder={"ExampleName"} maxLength={32} /> <div className="service-accounts__nickname-input-max-length"> Max. 32 characters </div> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(7 hunks)apps/cyberstorm-remix/app/styles/settingsItem.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
packages/cyberstorm/src/newComponents/Modal/Modal.tsx (1)
Modal(177-276)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Generate visual diffs
🔇 Additional comments (8)
apps/cyberstorm-remix/app/styles/settingsItem.css (1)
62-74: LGTM! Clean responsive implementation.The media query properly handles narrow viewports by switching to a vertical layout and expanding content to full width.
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css (1)
41-59: LGTM! CSS classes properly structured.The new form styling classes follow the project's conventions and support the updated form layout effectively.
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (6)
79-82: LGTM! Ownership check is straightforward.
89-101: Verify if toast for remove errors should remain.The PR objective mentions removing toasts, but the remove service account action still displays an error toast (lines 95-99). Since the removal modal lacks inline error display, this might be intentional. Confirm whether this toast should also be replaced with inline error handling.
183-189: LGTM! Access control properly implemented.The form is correctly hidden from non-owners.
270-281: LGTM! Inline error handling and list refresh implemented correctly.The form now displays errors inline and refreshes the service account list after successful creation, addressing the PR objectives.
284-293: LGTM! Modal reset logic properly clears state.
364-366: LGTM! Button state properly reflects form validity.
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1598 +/- ##
=========================================
- Coverage 9.86% 9.86% -0.01%
=========================================
Files 309 309
Lines 22542 22555 +13
Branches 405 405
=========================================
Hits 2224 2224
- Misses 20318 20331 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
VilppeRiskidev
force-pushed
the
create-service-account-qa-fixes
branch
from
a738a5b to
1b68b0a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
364-366: Consider adding loading state during submission.The button is disabled when the form is invalid, but there's no disabled or loading state during submission. This could allow duplicate submissions if a user clicks multiple times before the request completes.
If
strongFormdoesn't handle submission state internally, consider tracking it:const [isSubmitting, setIsSubmitting] = useState(false); async function submitor(data: typeof formInputs): Promise<SubmitorOutput> { setIsSubmitting(true); try { return await teamAddServiceAccount({ config: props.config, params: { team_name: props.teamName }, queryParams: {}, data: { nickname: data.nickname.trim() }, }); } finally { setIsSubmitting(false); } }Then update the button:
<NewButton onClick={strongForm.submit} disabled={!isValid || isSubmitting}> {isSubmitting ? "Adding..." : "Add Service Account"} </NewButton>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(7 hunks)apps/cyberstorm-remix/app/styles/settingsItem.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css
- apps/cyberstorm-remix/app/styles/settingsItem.css
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
packages/thunderstore-api/src/post/teamAddServiceAccount.ts (1)
teamAddServiceAccount(21-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Generate visual diffs
🔇 Additional comments (6)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (6)
79-82: LGTM! Clean ownership gating.The ownership check safely handles undefined user state and correctly identifies the team owner.
183-189: LGTM! Ownership-based rendering implemented correctly.The form is appropriately gated for team owners only, meeting the PR objective.
247-253: LGTM! Previous review comment addressed.The nickname is now properly trimmed before submission, ensuring consistency between validation and the submitted payload.
270-277: LGTM! List revalidation on success implemented.The form now refreshes the service account list after creation, meeting the PR objective. The TODO note about appending instead of refreshing is a reasonable future optimization.
278-281: LGTM! Inline error handling implemented.Errors are now displayed on the form itself instead of toasts, meeting the PR objective.
326-333: LGTM! Enter key submission enabled.The form's
onSubmithandler correctly prevents default behavior and validates before submission, enabling users to submit via Enter key as per the PR objective.
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx
Outdated
Show resolved
Hide resolved
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx
Show resolved
Hide resolved
VilppeRiskidev
force-pushed
the
create-service-account-qa-fixes
branch
2 times, most recently
from
c1ae783 to
74d8f28
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
242-244: Cache the trimmed nickname onceYou already trim in multiple places; stashing the trimmed value in a local keeps the validation readable and guarantees you use the exact same value everywhere.
- const isValid = - formInputs.nickname.trim().length > 0 && - formInputs.nickname.trim().length <= 32; + const trimmedNickname = formInputs.nickname.trim(); + const isValid = + trimmedNickname.length > 0 && trimmedNickname.length <= 32;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(7 hunks)apps/cyberstorm-remix/app/styles/settingsItem.css(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (2)
packages/thunderstore-api/src/post/teamAddServiceAccount.ts (1)
teamAddServiceAccount(21-45)packages/cyberstorm/src/newComponents/Modal/Modal.tsx (1)
Modal(177-276)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test
- GitHub Check: Generate visual diffs
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx
Outdated
Show resolved
Hide resolved
VilppeRiskidev
force-pushed
the
create-service-account-qa-fixes
branch
from
74d8f28 to
097cdfd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (1)
341-346: Consider clearing error on input change.Currently, API errors persist until the next submission or modal close. For better UX, you might clear the error when the user modifies the input.
onChange={(e) => { + if (error) setError(null); updateFormFieldState({ field: "nickname", value: e.target.value, }); }}Also applies to: 354-354
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css(1 hunks)apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx(7 hunks)apps/cyberstorm-remix/app/styles/settingsItem.css(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cyberstorm-remix/app/styles/settingsItem.css
- apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.css
🧰 Additional context used
🧬 Code graph analysis (1)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (2)
packages/thunderstore-api/src/post/teamAddServiceAccount.ts (1)
teamAddServiceAccount(21-45)packages/cyberstorm/src/newComponents/Modal/Modal.tsx (1)
Modal(178-285)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build
- GitHub Check: Generate visual diffs
🔇 Additional comments (6)
apps/cyberstorm-remix/app/settings/teams/team/tabs/ServiceAccounts/ServiceAccounts.tsx (6)
79-82: LGTM! Owner check is clean and safe.The optional chaining and role check correctly determine ownership. Non-owners won't see the Add Service Account form.
183-189: Access control implemented correctly.Only team owners can access the Add Service Account form, as intended.
242-243: Validation and trimming logic is solid.Non-empty check on trimmed input, with trimming on submission, ensures clean data reaches the API. The
maxLengthprop on the input handles the upper bound.Also applies to: 251-251
271-275: Error handling updated as intended.Inline errors replace toasts, list refreshes on success, and state resets on modal close. All aligned with PR objectives.
Also applies to: 278-279, 289-289
325-355: Form structure enables Enter key submission correctly.The form element with
onSubmithandler allows Enter key submission while the button inModal.Footer(outside the form) handles click events separately. Controlled input, character limit indicator, and inline error display are all implemented well.
360-362: Button state management is correct.The button correctly disables when input is invalid, preventing submission of bad data.
* Form validation (for empty value and max. length), which is relflected on the UI * Removed success/error toasts * Display errors on the form itself * Refresh the service account list after one is created * Hide "Add service account" button if the user isn't the owner of the team * Fix mobile device responsivity * Submit form via enter key press
VilppeRiskidev
force-pushed
the
create-service-account-qa-fixes
branch
from
097cdfd to
cd11832
Compare
