Closed
Conversation
Contributor
WalkthroughThis PR implements custom branding functionality for spaces, allowing users to enable branding display and configure primary colors through new settings UI components. Changes include localization strings, branding-gated UI components with paywall integration, EventCard prop refactoring for branding configuration, TRPC route consolidation, and new UI primitives (ColorPicker, Field components, AlertAction). Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant SettingsUI as Settings Page
participant Switch as CustomBrandingSwitch
participant Billing as useBilling()
participant TRPC as TRPC Mutation
participant Cache as Query Cache
participant DB as Database
User->>SettingsUI: Navigate to branding settings
User->>Switch: Toggle "Show Branding"
Switch->>Billing: Check plan (isFree)
alt Free Plan
Billing-->>Switch: isFree = true
Switch->>User: Trigger showPayWall()
else Pro Plan
Billing-->>Switch: isFree = false
Switch->>TRPC: updateShowBranding({ showBranding: true })
TRPC->>DB: Update space.showBranding
DB-->>TRPC: Success
TRPC->>Cache: Invalidate spaces.getCurrent
Cache-->>SettingsUI: Refetch space data
Switch->>User: Show success toast
end
sequenceDiagram
actor User
participant AdminPage as Admin Page
participant Alert as CustomBrandingAlert
participant Billing as useBilling()
participant Dialog as PayWallDialog
User->>AdminPage: View poll admin page
AdminPage->>Alert: Render CustomBrandingAlert
Alert->>Billing: Check plan
alt Free Plan + Branding Disabled
Billing-->>Alert: Show alert
Alert->>User: Display upsell alert with palette icon
User->>Alert: Click "Upgrade" button
Alert->>Dialog: Trigger showPayWall()
Dialog->>User: Show upgrade dialog with branding benefit
else Other Conditions
Alert-->>AdminPage: Return null (no render)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
apps/web/src/app/[locale]/(space)/settings/general/page.tsx (1)
5-6: Drop unnecessaryasyncfromGeneralSettingsPage(Line 5).No awaited work is performed in this function.
♻️ Suggested simplification
-export default async function GeneralSettingsPage() { +export default function GeneralSettingsPage() { return <GeneralSettingsPageClient />; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[locale]/(space)/settings/general/page.tsx around lines 5 - 6, Remove the unnecessary async from the GeneralSettingsPage function declaration: update the export default function GeneralSettingsPage() to be a plain synchronous function that simply returns <GeneralSettingsPageClient /> (no await usage), keeping the component name GeneralSettingsPage and the client component GeneralSettingsPageClient unchanged.packages/ui/src/field.tsx (2)
5-5: UseReact.useMemoinstead of standalone import.Per coding guidelines, prefer
React.useMemoover standaloneuseMemoimport.♻️ Proposed fix
-import { useMemo } from "react"; +import React from "react";Then at line 185:
- const content = useMemo(() => { + const content = React.useMemo(() => {As per coding guidelines: "Prefer
React.useState,React.useEffect, etc. over standalone imports (useState,useEffect)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/field.tsx` at line 5, The file imports useMemo directly; update to use React.useMemo per guidelines by replacing the standalone import (remove useMemo from the named imports) and ensure all calls to useMemo (e.g., inside the Field component or any function where useMemo is referenced at/around the existing usage) are changed to React.useMemo; locate references by the symbol useMemo and the Field component or the block around line ~185 and switch them to React.useMemo after importing React as the default.
78-79: Minor typo in biome-ignore comments."defualt" should be "default" in the biome-ignore comments at lines 78 and 206.
✏️ Proposed fix
- // biome-ignore lint/a11y/useSemanticElements: avoid messing with shadcn defualt implementation + // biome-ignore lint/a11y/useSemanticElements: avoid messing with shadcn default implementation- // biome-ignore lint/suspicious/noArrayIndexKey: avoid messing with shadcn defualt implementation + // biome-ignore lint/suspicious/noArrayIndexKey: avoid messing with shadcn default implementationAlso applies to: 206-207
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/field.tsx` around lines 78 - 79, Fix the typo in the biome-ignore comments: change "defualt" to "default" in the comment immediately before the <div> element (the biome-ignore lint/a11y/useSemanticElements comment) and the matching comment later in the file (the second biome-ignore lint/a11y/useSemanticElements instance near the other <div>), ensuring both comments read "avoid messing with shadcn default implementation".apps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsx (1)
74-96: Consider explicit disabled state on Button.The Button at line 87-93 relies on
pointer-events-nonefrom the parent div when branding is disabled. For better semantics and accessibility, consider explicitly disabling the button:♻️ Suggested change
<Button onClick={handleSave} - disabled={!isDirty} + disabled={!space.showBranding || !isDirty} loading={updateSpace.isPending} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx around lines 74 - 96, The Button relies on the parent div's pointer-events for disabling which is not semantic or accessible; update the Button in the Custom Branding section to explicitly set disabled when branding is turned off (use space.showBranding to compute the prop), keep the existing disabled={!isDirty} behavior combined (e.g. disabled={!space.showBranding || !isDirty}), preserve loading={updateSpace.isPending}, and add an aria-disabled attribute if the Button component supports it so screen readers get the correct state; ensure handleSave remains the click handler but will not run when the explicit disabled is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-switch.tsx:
- Around line 21-41: The onChange callback in handleToggle fires immediately
instead of after the mutation settles, causing possible UI-server state
mismatch; move the onChange?.(checked) call into the success path of the
mutation (e.g., chain it inside the promise .then() after
updateShowBranding.mutateAsync and utils.spaces.getCurrent.invalidate()), or
remove the call entirely and rely on utils.spaces.getCurrent.invalidate() to
refresh state—update handleToggle to only invoke onChange after the mutation and
invalidation complete.
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/space-settings-form.tsx:
- Around line 66-85: The form is being reset immediately because
form.reset(data) is called right after starting toast.promise; change the flow
so the reset happens only after updateSpace.mutateAsync succeeds: inside the
onSubmit handler (wrapped by form.handleSubmit) await updateSpace.mutateAsync({
name: data.name }) (or use the Promise returned by
updateSpace.mutateAsync/.then) and call form.reset(data) in the success path,
keeping toast.promise around the same mutation promise to show UI notifications;
ensure form.reset is not executed on mutation failure.
In `@apps/web/src/app/`[locale]/(space)/settings/general/page-client.tsx:
- Line 68: CustomBrandingSection is currently rendered without admin gating so
non-admins can interact then hit FORBIDDEN; update the usage and component to
mirror SpaceSettingsForm's pattern by passing a disabled prop (e.g.,
disabled={!isAdmin}) where <CustomBrandingSection /> is rendered and update
CustomBrandingSection to respect that prop by disabling the color picker and
save button when disabled is true (preserve backend checks in spaces.update and
spaces.updateShowBranding). Ensure the prop name matches how SpaceSettingsForm
uses it (disabled) and that handlers early-return or UI controls are
uninteractive when disabled.
In `@apps/web/src/components/event-card.tsx`:
- Around line 59-63: The branding block currently renders only when logoUrl is
truthy, hiding the space name if branding is enabled but no image exists; update
the JSX so that when showBranding (or the prop controlling branding) is true the
space name (variable name) is always rendered and SpaceIcon (component
SpaceIcon) is shown only if logoUrl (derived from image) exists — i.e., move the
<p> that displays name out of the logoUrl conditional (or add a fallback branch)
so name appears even when image/logoUrl is undefined.
---
Nitpick comments:
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-section.tsx:
- Around line 74-96: The Button relies on the parent div's pointer-events for
disabling which is not semantic or accessible; update the Button in the Custom
Branding section to explicitly set disabled when branding is turned off (use
space.showBranding to compute the prop), keep the existing disabled={!isDirty}
behavior combined (e.g. disabled={!space.showBranding || !isDirty}), preserve
loading={updateSpace.isPending}, and add an aria-disabled attribute if the
Button component supports it so screen readers get the correct state; ensure
handleSave remains the click handler but will not run when the explicit disabled
is true.
In `@apps/web/src/app/`[locale]/(space)/settings/general/page.tsx:
- Around line 5-6: Remove the unnecessary async from the GeneralSettingsPage
function declaration: update the export default function GeneralSettingsPage()
to be a plain synchronous function that simply returns
<GeneralSettingsPageClient /> (no await usage), keeping the component name
GeneralSettingsPage and the client component GeneralSettingsPageClient
unchanged.
In `@packages/ui/src/field.tsx`:
- Line 5: The file imports useMemo directly; update to use React.useMemo per
guidelines by replacing the standalone import (remove useMemo from the named
imports) and ensure all calls to useMemo (e.g., inside the Field component or
any function where useMemo is referenced at/around the existing usage) are
changed to React.useMemo; locate references by the symbol useMemo and the Field
component or the block around line ~185 and switch them to React.useMemo after
importing React as the default.
- Around line 78-79: Fix the typo in the biome-ignore comments: change "defualt"
to "default" in the comment immediately before the <div> element (the
biome-ignore lint/a11y/useSemanticElements comment) and the matching comment
later in the file (the second biome-ignore lint/a11y/useSemanticElements
instance near the other <div>), ensuring both comments read "avoid messing with
shadcn default implementation".
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 79ee3fef-04a4-48ec-86d6-4ed6d5091b50
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
apps/web/public/locales/en/app.jsonapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/admin-page.tsxapps/web/src/app/[locale]/(optional-space)/poll/[urlId]/custom-branding-alert.tsxapps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-section.tsxapps/web/src/app/[locale]/(space)/settings/general/components/custom-branding-switch.tsxapps/web/src/app/[locale]/(space)/settings/general/components/space-branding-form.tsxapps/web/src/app/[locale]/(space)/settings/general/components/space-settings-form.tsxapps/web/src/app/[locale]/(space)/settings/general/page-client.tsxapps/web/src/app/[locale]/(space)/settings/general/page.tsxapps/web/src/app/[locale]/invite/[urlId]/invite-page.tsxapps/web/src/components/event-card.tsxapps/web/src/components/pro-badge.tsxapps/web/src/components/stacked-list.tsxapps/web/src/features/billing/components/pay-wall-dialog.tsxapps/web/src/features/space/client.tsxapps/web/src/trpc/routers/spaces.tspackages/tailwind-config/shared-styles.csspackages/ui/package.jsonpackages/ui/src/alert.tsxpackages/ui/src/button-variants.tspackages/ui/src/color-picker.tsxpackages/ui/src/field.tsxpackages/ui/src/sidebar.tsxpackages/ui/src/switch.tsx
Comment on lines
+21
to
+41
| const handleToggle = (checked: boolean) => { | ||
| if (isFree) { | ||
| showPayWall(); | ||
| return; | ||
| } | ||
| toast.promise( | ||
| updateShowBranding | ||
| .mutateAsync({ showBranding: checked }) | ||
| .then(() => utils.spaces.getCurrent.invalidate()), | ||
| { | ||
| loading: t("savingBranding", { defaultValue: "Saving..." }), | ||
| success: checked | ||
| ? t("brandingEnabled", { defaultValue: "Custom branding enabled" }) | ||
| : t("brandingDisabled", { defaultValue: "Custom branding disabled" }), | ||
| error: t("brandingSaveError", { | ||
| defaultValue: "Failed to save branding", | ||
| }), | ||
| }, | ||
| ); | ||
| onChange?.(checked); | ||
| }; |
Contributor
There was a problem hiding this comment.
onChange callback fires before mutation completes.
onChange?.(checked) on line 40 executes immediately after starting the mutation, not after it succeeds. If the mutation fails, the parent component's state may be inconsistent with the server state.
Consider moving onChange into the .then() chain or removing it entirely since utils.spaces.getCurrent.invalidate() already refreshes the data.
🐛 Proposed fix
const handleToggle = (checked: boolean) => {
if (isFree) {
showPayWall();
return;
}
toast.promise(
updateShowBranding
.mutateAsync({ showBranding: checked })
- .then(() => utils.spaces.getCurrent.invalidate()),
+ .then(() => {
+ utils.spaces.getCurrent.invalidate();
+ onChange?.(checked);
+ }),
{
loading: t("savingBranding", { defaultValue: "Saving..." }),
success: checked
? t("brandingEnabled", { defaultValue: "Custom branding enabled" })
: t("brandingDisabled", { defaultValue: "Custom branding disabled" }),
error: t("brandingSaveError", {
defaultValue: "Failed to save branding",
}),
},
);
- onChange?.(checked);
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/custom-branding-switch.tsx
around lines 21 - 41, The onChange callback in handleToggle fires immediately
instead of after the mutation settles, causing possible UI-server state
mismatch; move the onChange?.(checked) call into the success path of the
mutation (e.g., chain it inside the promise .then() after
updateShowBranding.mutateAsync and utils.spaces.getCurrent.invalidate()), or
remove the call entirely and rely on utils.spaces.getCurrent.invalidate() to
refresh state—update handleToggle to only invoke onChange after the mutation and
invalidation complete.
Comment on lines
+66
to
+85
| onSubmit={form.handleSubmit(async (data) => { | ||
| toast.promise( | ||
| updateSpace.mutateAsync({ | ||
| name: data.name, | ||
| }), | ||
| { | ||
| loading: t("updatingSpace", { | ||
| defaultValue: "Updating space...", | ||
| }), | ||
| { | ||
| loading: t("updatingSpace", { | ||
| defaultValue: "Updating space...", | ||
| }), | ||
| success: t("spaceUpdatedSuccess", { | ||
| defaultValue: "Space updated successfully", | ||
| }), | ||
| error: t("spaceUpdatedError", { | ||
| defaultValue: "Failed to update space", | ||
| }), | ||
| }, | ||
| ); | ||
| success: t("spaceUpdatedSuccess", { | ||
| defaultValue: "Space updated successfully", | ||
| }), | ||
| error: t("spaceUpdatedError", { | ||
| defaultValue: "Failed to update space", | ||
| }), | ||
| }, | ||
| ); | ||
|
|
||
| form.reset(data); | ||
| })} | ||
| className="space-y-6" | ||
| > | ||
| <div> | ||
| <FormLabel> | ||
| <Trans i18nKey="image" defaults="Image" /> | ||
| </FormLabel> | ||
| <ImageUpload className="mt-2"> | ||
| <ImageUploadPreview> | ||
| <SpaceIcon name={space.name} src={space.image} size="xl" /> | ||
| </ImageUploadPreview> | ||
| <ImageUploadControl | ||
| getUploadUrl={(input) => getImageUploadUrl.mutateAsync(input)} | ||
| onUploadSuccess={handleImageUploadSuccess} | ||
| onRemoveSuccess={handleImageRemoveSuccess} | ||
| hasCurrentImage={!!space.image} | ||
| /> | ||
| </ImageUpload> | ||
| </div> | ||
| <FormField | ||
| control={form.control} | ||
| name="name" | ||
| render={({ field }) => ( | ||
| <FormItem> | ||
| <FormLabel> | ||
| <Trans i18nKey="name" defaults="Name" /> | ||
| </FormLabel> | ||
| <FormControl> | ||
| <Input | ||
| className="w-72" | ||
| placeholder={t("spaceNamePlaceholder", { | ||
| defaultValue: "My Team", | ||
| })} | ||
| disabled={disabled} | ||
| {...field} | ||
| form.reset(data); | ||
| })} |
Contributor
There was a problem hiding this comment.
form.reset(data) executes before mutation completes.
form.reset(data) on line 84 runs immediately after calling toast.promise, not after the mutation succeeds. If the mutation fails, the form state will already be reset, losing the user's changes.
🐛 Proposed fix: move reset into promise chain
onSubmit={form.handleSubmit(async (data) => {
toast.promise(
updateSpace.mutateAsync({
name: data.name,
- }),
+ }).then(() => {
+ form.reset(data);
+ }),
{
loading: t("updatingSpace", {
defaultValue: "Updating space...",
}),
success: t("spaceUpdatedSuccess", {
defaultValue: "Space updated successfully",
}),
error: t("spaceUpdatedError", {
defaultValue: "Failed to update space",
}),
},
);
-
- form.reset(data);
})}📝 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
| onSubmit={form.handleSubmit(async (data) => { | |
| toast.promise( | |
| updateSpace.mutateAsync({ | |
| name: data.name, | |
| }), | |
| { | |
| loading: t("updatingSpace", { | |
| defaultValue: "Updating space...", | |
| }), | |
| { | |
| loading: t("updatingSpace", { | |
| defaultValue: "Updating space...", | |
| }), | |
| success: t("spaceUpdatedSuccess", { | |
| defaultValue: "Space updated successfully", | |
| }), | |
| error: t("spaceUpdatedError", { | |
| defaultValue: "Failed to update space", | |
| }), | |
| }, | |
| ); | |
| success: t("spaceUpdatedSuccess", { | |
| defaultValue: "Space updated successfully", | |
| }), | |
| error: t("spaceUpdatedError", { | |
| defaultValue: "Failed to update space", | |
| }), | |
| }, | |
| ); | |
| form.reset(data); | |
| })} | |
| className="space-y-6" | |
| > | |
| <div> | |
| <FormLabel> | |
| <Trans i18nKey="image" defaults="Image" /> | |
| </FormLabel> | |
| <ImageUpload className="mt-2"> | |
| <ImageUploadPreview> | |
| <SpaceIcon name={space.name} src={space.image} size="xl" /> | |
| </ImageUploadPreview> | |
| <ImageUploadControl | |
| getUploadUrl={(input) => getImageUploadUrl.mutateAsync(input)} | |
| onUploadSuccess={handleImageUploadSuccess} | |
| onRemoveSuccess={handleImageRemoveSuccess} | |
| hasCurrentImage={!!space.image} | |
| /> | |
| </ImageUpload> | |
| </div> | |
| <FormField | |
| control={form.control} | |
| name="name" | |
| render={({ field }) => ( | |
| <FormItem> | |
| <FormLabel> | |
| <Trans i18nKey="name" defaults="Name" /> | |
| </FormLabel> | |
| <FormControl> | |
| <Input | |
| className="w-72" | |
| placeholder={t("spaceNamePlaceholder", { | |
| defaultValue: "My Team", | |
| })} | |
| disabled={disabled} | |
| {...field} | |
| form.reset(data); | |
| })} | |
| onSubmit={form.handleSubmit(async (data) => { | |
| toast.promise( | |
| updateSpace.mutateAsync({ | |
| name: data.name, | |
| }).then(() => { | |
| form.reset(data); | |
| }), | |
| { | |
| loading: t("updatingSpace", { | |
| defaultValue: "Updating space...", | |
| }), | |
| success: t("spaceUpdatedSuccess", { | |
| defaultValue: "Space updated successfully", | |
| }), | |
| error: t("spaceUpdatedError", { | |
| defaultValue: "Failed to update space", | |
| }), | |
| }, | |
| ); | |
| })} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/web/src/app/`[locale]/(space)/settings/general/components/space-settings-form.tsx
around lines 66 - 85, The form is being reset immediately because
form.reset(data) is called right after starting toast.promise; change the flow
so the reset happens only after updateSpace.mutateAsync succeeds: inside the
onSubmit handler (wrapped by form.handleSubmit) await updateSpace.mutateAsync({
name: data.name }) (or use the Promise returned by
updateSpace.mutateAsync/.then) and call form.reset(data) in the success path,
keeping toast.promise around the same mutation promise to show UI notifications;
ensure form.reset is not executed on mutation failure.
| <SpaceSettingsForm space={space} disabled={!isAdmin} /> | ||
| </PageSectionContent> | ||
| </PageSection> | ||
| <CustomBrandingSection /> |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate custom branding settings components"
fd -i "custom-branding.*\\.tsx$" apps/web/src
echo
echo "2) Inspect custom branding section implementation"
fd -i "custom-branding-section.tsx" apps/web/src -0 | xargs -0 -I{} sh -c 'echo "### {}"; sed -n "1,260p" "{}"'
echo
echo "3) Inspect branding mutations / authorization checks"
rg -n -C3 --type=ts --type=tsx '\bshowBranding\b|\bprimaryColor\b|\bisAdmin\b|\bability\b|\bcan\(|\btrpc\b|\bmutation\b|\bupdate\b' apps/web/srcRepository: lukevella/rallly
Length of output: 4041
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Check parent page-client.tsx to see how CustomBrandingSection is rendered"
sed -n '60,80p' apps/web/src/app/[locale]\(space\)/settings/general/page-client.tsx
echo
echo "2) Check if there's admin gating at page level"
grep -n "isAdmin\|CustomBrandingSection" apps/web/src/app/[locale]\(space\)/settings/general/page-client.tsx
echo
echo "3) Check trpc spaces.update mutation handler for authorization"
fd "spaces" apps/web/src -type f -name "*.ts" | xargs grep -l "\.update" | head -5
echo
echo "4) Search for space router/procedure definitions"
find apps/web/src -name "*router*" -o -name "*procedure*" | grep -i space | head -10Repository: lukevella/rallly
Length of output: 262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find the page-client file"
find apps/web/src -name "page-client.tsx" | grep "settings/general"
echo
echo "2) Read the page-client file around the CustomBrandingSection usage"
FILE=$(find apps/web/src -name "page-client.tsx" | grep "settings/general")
if [ -n "$FILE" ]; then
cat -n "$FILE" | sed -n '50,85p'
fi
echo
echo "3) Check for conditional rendering of CustomBrandingSection"
if [ -n "$FILE" ]; then
grep -n "CustomBrandingSection\|isAdmin" "$FILE"
fi
echo
echo "4) Find server-side trpc router for spaces"
find apps -name "*router*" -type f | xargs grep -l "spaces" | grep -E "\.(ts|tsx)$" | head -3Repository: lukevella/rallly
Length of output: 2264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find trpc router files"
find apps -type d -name "router" | grep -E "server|api" | head -5
echo
echo "2) Find spaces router"
fd ".*router.*" apps -type f -name "*.ts" | xargs grep -l "spaces" 2>/dev/null | head -5
echo
echo "3) Find spaces procedure or router definition"
find apps -type f -name "*.ts" | xargs grep -l "spaces.*Router\|export.*spaces" 2>/dev/null | head -5Repository: lukevella/rallly
Length of output: 85
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) List directory structure to understand trpc organization"
find apps/web/src -type d -name "trpc" -o -type d -name "server"
echo
echo "2) Find all files related to spaces in server context"
find apps -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "spaces.*update\|spaces\.Router" 2>/dev/null | head -10
echo
echo "3) Search for procedures or routers with 'update' in spaces context"
rg "spaces\." apps -t ts -A 5 | grep -A 5 "update" | head -40
echo
echo "4) Find where mutation handlers check authorization"
rg "isAdmin\|role.*admin\|ability" apps -t ts --context=3 | grep -B 3 -A 3 "update" | head -50Repository: lukevella/rallly
Length of output: 4709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) List files in trpc/server"
find apps/web/src/trpc/server -type f -name "*.ts" | head -20
echo
echo "2) Look for spaces router"
find apps/web/src/trpc -type f -name "*.ts" | xargs grep -l "spaces" | head -10
echo
echo "3) Check the main router structure"
cat apps/web/src/trpc/server/routers/_app.ts 2>/dev/null | head -50Repository: lukevella/rallly
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Read spaces router to find update procedure"
cat apps/web/src/trpc/routers/spaces.ts | head -200
echo
echo "2) Look for update procedure specifically"
grep -n "\.update\|export const\|procedure" apps/web/src/trpc/routers/spaces.ts | head -30Repository: lukevella/rallly
Length of output: 5604
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find the update procedure definition"
grep -n "update:" apps/web/src/trpc/routers/spaces.ts
echo
echo "2) Read the update procedure around line 280-350"
sed -n '280,350p' apps/web/src/trpc/routers/spaces.ts
echo
echo "3) Check what spaceProcedure is and if it has authorization"
grep -n "spaceProcedure\|export.*spaceProcedure" apps/web/src/trpc/trpc.tsRepository: lukevella/rallly
Length of output: 2479
Add admin-gating UI to CustomBrandingSection for consistency.
Backend authorization is properly enforced via memberAbility checks in spaces.update and spaces.updateShowBranding mutations. However, non-admins can still see and interact with this section (color picker, save button) only to receive a FORBIDDEN error on save. For UX consistency with SpaceSettingsForm which displays disabled={!isAdmin}, add the same admin-gating pattern to CustomBrandingSection to prevent unnecessary interaction and error states.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/`[locale]/(space)/settings/general/page-client.tsx at line
68, CustomBrandingSection is currently rendered without admin gating so
non-admins can interact then hit FORBIDDEN; update the usage and component to
mirror SpaceSettingsForm's pattern by passing a disabled prop (e.g.,
disabled={!isAdmin}) where <CustomBrandingSection /> is rendered and update
CustomBrandingSection to respect that prop by disabling the color picker and
save button when disabled is true (preserve backend checks in spaces.update and
spaces.updateShowBranding). Ensure the prop name matches how SpaceSettingsForm
uses it (disabled) and that handlers early-return or UI controls are
uninteractive when disabled.
Comment on lines
+59
to
+63
| {logoUrl ? ( | ||
| <div className="mb-2"> | ||
| <SpaceIcon | ||
| name={poll.space.name} | ||
| src={poll.space.image} | ||
| size="lg" | ||
| /> | ||
| <SpaceIcon name={name} src={logoUrl} size="lg" /> | ||
| <p className="mt-2 font-medium text-muted-foreground text-sm"> | ||
| {poll.space.name} | ||
| {name} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Inspect EventCard branding branch"
sed -n '40,120p' apps/web/src/components/event-card.tsx
echo
echo "2) Inspect EventCard callsites and showBranding gating"
rg -n -C3 --type=tsx '\bEventCard\b|\bshowBranding\b|\blogoUrl\b|\bimage\b' apps/web/src/app
echo
echo "3) Inspect whether space image/logo is optional in schema/types"
rg -n -C3 --type=ts '\bimage\b|\blogo\b|\bshowBranding\b' apps/web/src/features/spaceRepository: lukevella/rallly
Length of output: 2359
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) EventCard callsites - search all TypeScript/TSX files ==="
rg -n -C3 'EventCard' apps/web/src/app --type ts --type tsx || rg -n -C3 'EventCard' apps/web/src/app
echo
echo "=== 2) Space type definitions and logoUrl/image field ==="
fd -e ts -e tsx apps/web/src/features/space -o -type f | head -10
echo "---"
rg -n 'interface.*Space|type.*Space|image|logo' apps/web/src/features/space -A3 -B1
echo
echo "=== 3) Check if showBranding is used elsewhere ==="
rg -n 'showBranding|logo' apps/web/src --type ts --type tsx | head -20Repository: lukevella/rallly
Length of output: 3287
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== EventCard callsite 1: invite-page.tsx ==="
sed -n '60,70p' apps/web/src/app/[locale]/invite/[urlId]/invite-page.tsx
echo
echo "=== EventCard callsite 2: admin-page.tsx ==="
sed -n '26,36p' apps/web/src/app/[locale]/\(optional-space\)/poll/[urlId]/admin-page.tsx
echo
echo "=== Search for Space type definition ==="
rg -n 'interface Space|type Space' apps/web/src/features/space --max-count 5
echo
echo "=== Search for showBranding and logo/image fields ==="
rg -n 'showBranding' apps/web/src --max-count 20Repository: lukevella/rallly
Length of output: 3920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Space type definition ==="
cat -n apps/web/src/features/space/types.ts
echo
echo "=== Check Prisma schema for Space model image field ==="
rg -n 'model Space|image' apps/web --max-count 30 -B2 -A5 | grep -E '(model Space|image|String|Boolean)' | head -20Repository: lukevella/rallly
Length of output: 2759
Consider showing space name independently of logo upload.
The branding section only renders when logoUrl is truthy, which requires both showBranding: true AND an uploaded image. This means a space with branding enabled but no logo uploaded will display no branding header at all. The name should display even when the logo is missing to maintain UX clarity.
Suggested fix: render the space name outside the logo condition, or create a separate fallback when showBranding is true but image is undefined.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/event-card.tsx` around lines 59 - 63, The branding
block currently renders only when logoUrl is truthy, hiding the space name if
branding is enabled but no image exists; update the JSX so that when
showBranding (or the prop controlling branding) is true the space name (variable
name) is always rendered and SpaceIcon (component SpaceIcon) is shown only if
logoUrl (derived from image) exists — i.e., move the <p> that displays name out
of the logoUrl conditional (or add a fallback branch) so name appears even when
image/logoUrl is undefined.
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.
Summary by CodeRabbit
Release Notes
New Features
UI & Styling