feat(core): simple placeholder send refferal ui#2277
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new SolidJS invite modal component with email parsing, referral-link copy/send actions, a module-level open signal, a minor sidebar hotkey-badge styling tweak, and a new global hotkey token. Changes
Possibly related PRs
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/packages/app/component/app-sidebar/invite-modal.tsx`:
- Around line 11-16: The current parseEmails function only checks for an '@' and
should be tightened to perform basic email format validation: in parseEmails
replace the simple .filter((s) => s.includes('@')) with a test against a
lightweight email regex (e.g., HTML5 or RFC5322-lite) so only syntactically
valid addresses are returned; keep the existing splitting and trimming logic
(split(/[,\n\s]/) and .map(s=>s.trim())) and ensure empty strings are filtered
out.
- Line 70: Fix the typo in the Invite Modal: change the prop name on
Dialog.CloseButton that currently passes "varian" to the Button component to the
correct "variant" prop (look for the Dialog.CloseButton as={Button} usage in
invite-modal.tsx and update the prop key to variant with the intended value,
e.g., variant="ghost").
In `@js/app/packages/app/component/app-sidebar/sidebar.tsx`:
- Around line 286-294: The JSX uses a nested ternary to choose tooltip content
(checking props.tooltip? then props.hotkeyToken ? <LabelAndHotKey .../> :
props.label : undefined); replace the nested ternary with a small helper
function (eg. getTooltipContent or renderTooltipContent) that returns undefined,
props.label, or <LabelAndHotKey label={props.label}
hotkeyToken={props.hotkeyToken}/> via clear if/else logic; update the tooltip
usage to call that helper instead, referencing LabelAndHotKey,
props.hotkeyToken, and props.label so the conditional is no longer nested in the
JSX.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fc905b7d-8e02-4adb-8a50-d53762c03384
📒 Files selected for processing (3)
js/app/packages/app/component/app-sidebar/invite-modal.tsxjs/app/packages/app/component/app-sidebar/sidebar.tsxjs/app/packages/core/hotkey/tokens.ts
| import { createSignal, Show } from 'solid-js'; | ||
| import { useReferralCode } from '@core/context/user'; | ||
| import { getWebOrigin } from '@core/util/webOrigin'; | ||
| import { authServiceClient } from '@service-auth/client'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if a TanStack Query mutation for referral invites already exists
rg -n "sendReferralInvite|referral.*invite" --type ts -g '*/queries/*'Repository: macro-inc/macro
Length of output: 207
🏁 Script executed:
#!/bin/bash
# Read the target file to verify the specific lines flagged
cat -n js/app/packages/app/component/app-sidebar/invite-modal.tsx | head -80Repository: macro-inc/macro
Length of output: 3222
🏁 Script executed:
#!/bin/bash
# Search for TanStack Query mutations related to referral/invite more broadly
# Try multiple search strategies since the initial glob pattern failed
find . -type f -path "*/queries/*" -name "*.ts" -o -path "*/queries/*" -name "*.tsx" | head -20Repository: macro-inc/macro
Length of output: 1040
🏁 Script executed:
#!/bin/bash
# Search for sendReferralInvite across the codebase to understand usage patterns
rg "sendReferralInvite" --type ts --type tsx -A 2 -B 2Repository: macro-inc/macro
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Check the auth mutations file
cat js/app/packages/queries/auth/mutations.tsRepository: macro-inc/macro
Length of output: 1801
🏁 Script executed:
#!/bin/bash
# Better search for sendReferralInvite - try ts and tsx files separately
rg "sendReferralInvite" --type ts -A 3 -B 1Repository: macro-inc/macro
Length of output: 1417
Move service client call to TanStack Query mutation.
The authServiceClient.sendReferralInvite call on line 5 and line 47 violates the guideline that all network calls must go through TanStack Query in the queries package. No mutation currently exists for this operation.
Additionally, the async calls are fire-and-forget—promises are not awaited, so errors are silently swallowed. The success toast displays immediately regardless of whether requests succeed or fail.
Finally, line 70 has a typo: varian="ghost" should be variant="ghost".
🐛 Proposed fix
Create a mutation in queries/auth/mutations.ts:
+type SendReferralInviteCallbacks = MutationCallbacks<void, Error, string>;
+
+export function useSendReferralInviteMutation(
+ callbacks?: SendReferralInviteCallbacks
+) {
+ return useMutation(() => ({
+ mutationFn: async (email: string) => {
+ await throwOnErr(
+ async () => await authServiceClient.sendReferralInvite(email)
+ );
+ },
+ ...withCallbacks<void, Error, string>({}, callbacks),
+ }));
+}Then update the component:
-import { authServiceClient } from '@service-auth/client';
+import { useSendReferralInviteMutation } from '@queries/auth';
export const InviteModal = () => {
const [value, setValue] = createSignal('');
const [copied, setCopied] = createSignal(false);
const [sending, setSending] = createSignal(false);
const referralCode = useReferralCode();
+ const sendInvite = useSendReferralInviteMutation();
// ...
const handleSend = async () => {
const emails = parseEmails(value());
if (!emails.length) return;
setSending(true);
- for (const email of emails) {
- authServiceClient.sendReferralInvite(email);
- }
+ try {
+ const results = await Promise.allSettled(
+ emails.map((email) => sendInvite.mutateAsync(email))
+ );
+ const succeeded = results.filter((r) => r.status === 'fulfilled').length;
+ const failed = results.filter((r) => r.status === 'rejected').length;
+
+ if (failed > 0) {
+ toast.error(`${failed} invite(s) failed to send`);
+ }
+ if (succeeded > 0) {
+ toast.success(
+ succeeded === 1
+ ? 'Invite sent successfully'
+ : `${succeeded} invites sent successfully`
+ );
+ }
+ } catch (error) {
+ toast.error('Failed to send invites');
+ }
setValue('');
setSending(false);
setInviteModalOpen(false);
};
// ...
- <Dialog.CloseButton as={Button} varian="ghost" size="icon-sm">
+ <Dialog.CloseButton as={Button} variant="ghost" size="icon-sm">
<CloseIcon />
</Dialog.CloseButton>| function parseEmails(raw: string): string[] { | ||
| return raw | ||
| .split(/[,\n\s]/) | ||
| .map((s) => s.trim()) | ||
| .filter((s) => s.includes('@')); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Email parsing is minimal but acceptable for placeholder UI.
The parseEmails function only checks for @ presence. For a production-ready feature, consider adding basic email format validation to provide earlier user feedback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/packages/app/component/app-sidebar/invite-modal.tsx` around lines 11 -
16, The current parseEmails function only checks for an '@' and should be
tightened to perform basic email format validation: in parseEmails replace the
simple .filter((s) => s.includes('@')) with a test against a lightweight email
regex (e.g., HTML5 or RFC5322-lite) so only syntactically valid addresses are
returned; keep the existing splitting and trimming logic (split(/[,\n\s]/) and
.map(s=>s.trim())) and ensure empty strings are filtered out.
…vite-anytime-button
No description provided.