-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat(app): redeem flow fix & invite page #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
For some reason I don't land in the Invitation Dialog. When opening an invite url in a private tab I see the Invitation for very short moment and than I end up in the SignIn Dialog. After login in there is no relation established |
…you are already following
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements the QR invite flow functionality, allowing users to invite others to the platform and automatically create follow relationships. The changes span frontend invite handling, profile creation, and backend configuration for the relations feature.
Key changes:
- Implemented invite validation and redemption flow with profile display
- Added automatic profile creation for new users accepting invites
- Introduced relations (following/followers) feature with UI configuration
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
lib/src/types/FullItemsApi.d.ts |
New interface extending ItemsApi with required CRUD methods including getItem returning nullable values |
lib/src/Components/Onboarding/InvitePage.tsx |
Complete rewrite of invite flow with validation, authentication handling, and confirmation UI for both logged-in and logged-out users |
lib/src/Components/Map/hooks/useStoredInviteCode.ts |
New hook to redirect users to invite page after authentication when invite code exists in localStorage |
lib/src/Components/Map/hooks/useMyProfile.ts |
Added profile creation capability and refined profile loading detection logic |
lib/src/Components/Map/UtopiaMap.tsx |
Integrated useStoredInviteCode hook to handle stored invites |
backend/directus-config/development/sql/type-ui-components.sql |
Added relations components to user profile template configuration |
backend/directus-config/development/seed/types.json |
Added QR button configuration to user profile type |
backend/directus-config/development/seed/relations.json |
New seed data defining following/follower relation types |
backend/directus-config/development/collections/permissions.json |
Added read permissions for relations collection across multiple policies |
app/src/api/itemsApi.ts |
Changed implementation from ItemsApi to FullItemsApi interface |
app/src/App.tsx |
Created itemsApiInstance and passed it to InvitePage component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| itemsApi, | ||
| isRedeemingDone, | ||
| isValidationDone, | ||
| createEmptyProfile, |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useEffect hook has an exhaustive dependency array that includes function references like 'inviteApi', 'navigate', 'itemsApi', and 'createEmptyProfile'. These functions are likely recreated on every render, which will cause this effect to run repeatedly. This could lead to infinite loops or unexpected behavior. Consider using useCallback to memoize these functions or restructure the effect to avoid depending on them directly.
| createEmptyProfile, |
| } | ||
|
|
||
| if (!isMyProfileLoaded) return | ||
| const invitingProfile = await itemsApi.getItem(invitingProfileId) | ||
|
|
||
| if (!myProfile) { | ||
| toast.error('Could not find your profile to redeem the invite.') | ||
| if (invitingProfileId === myProfile?.id) { | ||
| toast.error('You cannot invite yourself') | ||
| localStorage.removeItem('inviteCode') | ||
| // Navigate to own profile | ||
| navigate('/item/' + myProfile.id) | ||
| return | ||
| } | ||
|
|
||
| const invitingProfileId = await inviteApi.redeemInvite(id, myProfile.id) | ||
| if ( | ||
| myProfile?.relations?.some( | ||
| (r) => r.type === 'is_following' && r.related_items_id === invitingProfileId, | ||
| ) | ||
| ) { | ||
| toast.error('You are already following this profile') | ||
| localStorage.removeItem('inviteCode') | ||
| navigate('/item/' + invitingProfileId) | ||
| return | ||
| } | ||
|
|
||
| if (invitingProfileId) { | ||
| toast.success('Invite redeemed successfully!') | ||
| navigate(`/item/${invitingProfileId}`) | ||
| } else { | ||
| toast.error('Failed to redeem invite') | ||
| if (!invitingProfile) { | ||
| toast.error('Inviting profile not found') | ||
| localStorage.removeItem('inviteCode') | ||
| navigate('/') |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation logic checks if the invitingProfile exists after checking if the user is already following the profile. However, if itemsApi.getItem returns null, the code will pass the null check on line 102-108 and 110-119 because those checks use invitingProfileId (a string). The null check should occur immediately after fetching the profile to prevent potential issues with the subsequent checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
| import { useEffect } from 'react' | ||
| import { useNavigate } from 'react-router-dom' | ||
|
|
||
| import { useAuth } from '#components/Auth/useAuth' | ||
|
|
||
| export const useStoredInviteCode = () => { | ||
| const inviteCode = localStorage.getItem('inviteCode') |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useStoredInviteCode hook reads from localStorage on every render (line 7) which could cause performance issues. Consider using useState with lazy initialization or useMemo to cache this value, and only read from localStorage once when the component mounts.
| import { useEffect } from 'react' | |
| import { useNavigate } from 'react-router-dom' | |
| import { useAuth } from '#components/Auth/useAuth' | |
| export const useStoredInviteCode = () => { | |
| const inviteCode = localStorage.getItem('inviteCode') | |
| import { useEffect, useState } from 'react' | |
| import { useNavigate } from 'react-router-dom' | |
| import { useAuth } from '#components/Auth/useAuth' | |
| export const useStoredInviteCode = () => { | |
| const [inviteCode] = useState(() => localStorage.getItem('inviteCode')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that breaks the functionality
| if (isAuthenticated) { | ||
| return ( | ||
| <MapOverlayPage backdrop className='tw:max-w-xs tw:h-fit'> | ||
| <h2 className='tw:text-2xl tw:font-semibold tw:mb-2 tw:text-center'>Confirmation</h2> | ||
| {invitingProfile ? ( | ||
| <> | ||
| <p className='tw:text-sm tw:text-base-content/70 tw:text-center'> | ||
| Do you want to follow <strong>{invitingProfile.name}</strong>? | ||
| </p> | ||
| <div className='tw:card-actions tw:justify-between'> | ||
| <button className='tw:btn tw:btn-ghost' onClick={goToStart}> | ||
| No | ||
| </button> | ||
| <button className='tw:btn tw:btn-primary' onClick={confirmFollow}> | ||
| Yes | ||
| </button> | ||
| </div> | ||
| </> | ||
| ) : ( | ||
| <div className='tw:flex tw:justify-center'> | ||
| <span className='tw:loading tw:loading-spinner tw:loading-md'></span> | ||
| </div> | ||
| )} | ||
| </MapOverlayPage> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <MapOverlayPage backdrop className='tw:max-w-xs tw:h-fit'> | ||
| <h2 className='tw:text-2xl tw:font-semibold tw:mb-2 tw:text-center'>Invitation</h2> | ||
| {invitingProfile ? ( | ||
| <> | ||
| <h2 className='tw:text-2xl tw:font-semibold tw:mb-2 tw:text-center'> | ||
| Welcome{appName && <> to {appName}</>}! | ||
| </h2> | ||
| <p className='tw:text-sm tw:text-base-content/70 tw:text-center'> | ||
| You have been invited by <strong>{invitingProfile.name}</strong> to join{' '} | ||
| {appName || 'the community'}. | ||
| </p> | ||
| <div className='tw:card-actions tw:justify-between'> | ||
| <button className='tw:btn tw:btn-ghost' onClick={goToLogin}> | ||
| Login | ||
| </button> | ||
| <button className='tw:btn tw:btn-primary' onClick={goToSignup}> | ||
| Sign Up | ||
| </button> | ||
| </div> | ||
| </> |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent Tailwind CSS class prefix usage. The same file uses both 'tw:' (lines 174, 181, 198, 199, 207) and 'tw-' (lines 175, 177, 178, 182, 185, 186, 191, 201, 202, 203, 207, 208, 211, 217) prefixes. This mixing of prefixes is inconsistent and could indicate a configuration issue. The codebase should use one consistent prefix throughout.
| } | ||
|
|
||
| await redeemInvite(id, myActualProfile.id) | ||
|
|
||
| // Add new relation to local state | ||
| updateItem({ | ||
| ...myActualProfile, | ||
| relations: [ | ||
| ...(myActualProfile.relations ?? []), | ||
| { | ||
| type: 'is_following', | ||
| direction: 'outgoing', | ||
| related_items_id: invitingProfile.id, | ||
| }, | ||
| ], | ||
| }) |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The local state update (lines 70-80) occurs after redeemInvite completes, but redeemInvite navigates away on both success and failure. This means the updateItem call will execute even when the redemption fails and the user is navigated to '/'. This could lead to inconsistent local state. The updateItem should only be called when redemption succeeds, or the logic should wait for redeemInvite to return a success indicator before updating local state.
| const reloadItemWithSecret = async (itemId: string, layer: LayerProps, baseItem: Item) => { | ||
| const maxRetries = 3 | ||
| const retryDelay = 500 // ms | ||
|
|
||
| for (let i = 0; i < maxRetries; i++) { | ||
| await new Promise((resolve) => setTimeout(resolve, retryDelay)) |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The createEmptyProfile function generates a UUID client-side (line 31) and sends it to the server. This could lead to ID conflicts if the server also generates IDs. Verify that the server's createItem method accepts and uses client-generated IDs, or remove the id field and let the server generate it. Additionally, the server response should be trusted as the source of truth for the item's ID.
| async getItem(id: string): Promise<T> { | ||
| try { | ||
| const result = await directusClient.request(readItem(this.collectionName as never, id)) | ||
| const result = await directusClient.request( | ||
| readItem(this.collectionName, id, { | ||
| fields: [ | ||
| '*', | ||
| 'secrets.*', | ||
| 'to.*', | ||
| 'relations.*', | ||
| 'user_created.*', |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type of getItem changed from Promise<T> to Promise<T> (no change in signature), but the FullItemsApi interface specifies Promise<T | null>. The implementation should handle cases where the item doesn't exist and return null instead of throwing an error, or the interface definition should be updated to match the implementation.
| } | ||
|
|
||
| const confirmFollowAsync = async () => { | ||
| if (!isAuthenticated) return | ||
|
|
||
| if (!isUserProfileLayerLoaded || isRedeemingDone) return | ||
|
|
||
| const myActualProfile = myProfile ?? (await createEmptyProfile()) | ||
|
|
||
| if (!myActualProfile) { | ||
| toast.error('Failed to create profile') | ||
| return | ||
| } | ||
|
|
||
| if (!invitingProfile) { | ||
| toast.error('Inviting profile not found') | ||
| return | ||
| } | ||
|
|
||
| await redeemInvite(id, myActualProfile.id) | ||
|
|
||
| // Add new relation to local state | ||
| updateItem({ | ||
| ...myActualProfile, | ||
| relations: [ | ||
| ...(myActualProfile.relations ?? []), | ||
| { | ||
| type: 'is_following', | ||
| direction: 'outgoing', | ||
| related_items_id: invitingProfile.id, | ||
| }, | ||
| ], | ||
| }) | ||
|
|
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential race condition in the confirmFollowAsync function. The function checks isRedeemingDone at line 53, but then proceeds to call redeemInvite and set isRedeemingDone to true. If this function is called multiple times before the first call completes (e.g., due to rapid button clicks), multiple redemptions could be initiated. Consider adding a loading state or disabling the button during the async operation.
| // Helper function for background reload with retry | ||
| const reloadItemWithSecret = async (itemId: string, layer: LayerProps, baseItem: Item) => { | ||
| const maxRetries = 3 | ||
| const retryDelay = 500 // ms |
Copilot
AI
Dec 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function uses user.first_name with a nullish fallback to 'User', but doesn't verify that user exists before accessing its property. While there's a guard on line 23 that returns early if !user, TypeScript might not recognize this as a sufficient guard depending on the configuration. Consider using optional chaining: user?.first_name ?? 'User' for defensive programming.
|
@antontranelis I've opened a new pull request, #641, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This reverts commit e147822.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 20 out of 22 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (2)
app/src/api/itemsApi.ts:91
- The error handling in getItem is inconsistent with other methods. Line 89 accesses error.errors[0] without optional chaining, which could throw if the errors array is empty or undefined. In contrast, getItems uses optional chaining (error.errors?.[0]?.message). Use consistent error handling across all methods.
} catch (error: any) {
console.log(error)
if (error.errors[0]?.message) throw error.errors[0].message
else throw error
}
app/src/api/itemsApi.ts:92
- The return type of getItem is Promise, but according to the FullItemsApi interface, it should be Promise<T | null> to handle cases where an item is not found. The current implementation will throw an error instead of returning null when an item doesn't exist, which is inconsistent with the interface definition and how it's used in InvitePage.tsx (line 99, where the code checks if invitingProfile is null).
async getItem(id: string): Promise<T> {
try {
const result = await directusClient.request(
readItem(this.collectionName, id, {
fields: [
'*',
'secrets.*',
'to.*',
'relations.*',
'user_created.*',
'markerIcon.*',
{ offers: ['*'], needs: ['*'], gallery: ['*.*'] } as any,
],
}),
)
return result as T
} catch (error: any) {
console.log(error)
if (error.errors[0]?.message) throw error.errors[0].message
else throw error
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| if (!inviteCode || !user) return | ||
|
|
||
| navigate(`/invite/${inviteCode}`) | ||
| }, [inviteCode, navigate, user]) |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The navigate function is missing from the useEffect dependency array. This can cause the effect to use a stale reference to navigate. Add navigate to the dependency array to fix this issue.
| }) | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [user, isUserProfileLayerLoaded, myProfile]) |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useEffect at line 98 is missing dependencies from its dependency array. The effect uses layers, addItem, and updateItem but these are not included in the dependencies. The eslint disable comment suggests this is intentional, but it could lead to the effect using stale references or not running when these values change. Consider adding these to the dependency array or documenting why they're intentionally excluded.
| }, [user, isUserProfileLayerLoaded, myProfile]) | |
| }, [user, isUserProfileLayerLoaded, myProfile, layers, addItem, updateItem]) |
|
|
||
| function RelationCard({ item }: { item: Item }) { | ||
| const appState = useAppState() | ||
| const avatar = item.image ? appState.assetsApi.url + item.image : null |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The avatar URL construction is missing a separator between the base URL and the image path. Line 18 concatenates appState.assetsApi.url + item.image, but this will likely result in URLs like 'https://example.comimage.jpg' instead of 'https://example.com/image.jpg'. Add a '/' separator between the URL components, similar to how it was done in the previous implementation.
| const avatar = item.image ? appState.assetsApi.url + item.image : null | |
| const avatar = item.image ? `${appState.assetsApi.url}/${item.image}` : null |
| { | ||
| "_sync_id": "2", | ||
| "relation": "is_following", | ||
| "heading": "Follower", |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The heading "Follower" should be plural "Followers" to match the "Following" heading and to be grammatically correct when displaying multiple followers.
| "heading": "Follower", | |
| "heading": "Followers", |
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [myProfile?.id, hasSecrets]) |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useEffect at line 54 is missing several dependencies from its dependency array. The effect uses myProfile.layer and myProfile (the whole object), but only includes myProfile?.id and hasSecrets. This could lead to stale closures. Either add the missing dependencies or use a ref for the reloadItemWithSecret function to avoid unnecessary re-runs.
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [myProfile?.id, hasSecrets]) | |
| }, [myProfile, hasSecrets]) |
| const userLayer = layers.find((l) => l.userProfileLayer === true) | ||
| if (!userLayer?.api?.createItem) { | ||
| throw new Error('User profile layer or create API not available') | ||
| } | ||
|
|
||
| const newProfile = { | ||
| id: crypto.randomUUID(), | ||
| name: user.first_name ?? 'User', | ||
| } | ||
|
|
||
| const serverResponse = await userLayer.api.createItem(newProfile) | ||
|
|
||
| const newItem = { | ||
| ...serverResponse, | ||
| user_created: user, // eslint-disable-line camelcase | ||
| layer: userLayer, | ||
| public_edit: false, // eslint-disable-line camelcase | ||
| } | ||
|
|
||
| // Add item immediately (without secret) | ||
| addItem(newItem) | ||
|
|
||
| // Reload in background to get server-generated fields (like secrets) | ||
| if (userLayer.api.getItem) { | ||
| void reloadItemWithSecret(serverResponse.id, userLayer, newItem) | ||
| } | ||
|
|
||
| return newItem | ||
| } | ||
|
|
||
| // Auto-create profile when user is logged in but has no profile | ||
| useEffect(() => { | ||
| if (user && isUserProfileLayerLoaded && !myProfile && !isCreatingProfileRef.current) { | ||
| isCreatingProfileRef.current = true | ||
| void createEmptyProfile().finally(() => { | ||
| isCreatingProfileRef.current = false | ||
| }) |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential race condition in the auto-create profile logic. The isCreatingProfileRef is used to prevent concurrent profile creation, but if the effect runs multiple times before createEmptyProfile completes, the ref won't prevent the race. The ref is only set to true inside the useEffect, so if the effect triggers again before the async function starts, it could create multiple profiles. Consider setting the ref before calling createEmptyProfile or using a state variable instead of a ref.
| const userLayer = layers.find((l) => l.userProfileLayer === true) | |
| if (!userLayer?.api?.createItem) { | |
| throw new Error('User profile layer or create API not available') | |
| } | |
| const newProfile = { | |
| id: crypto.randomUUID(), | |
| name: user.first_name ?? 'User', | |
| } | |
| const serverResponse = await userLayer.api.createItem(newProfile) | |
| const newItem = { | |
| ...serverResponse, | |
| user_created: user, // eslint-disable-line camelcase | |
| layer: userLayer, | |
| public_edit: false, // eslint-disable-line camelcase | |
| } | |
| // Add item immediately (without secret) | |
| addItem(newItem) | |
| // Reload in background to get server-generated fields (like secrets) | |
| if (userLayer.api.getItem) { | |
| void reloadItemWithSecret(serverResponse.id, userLayer, newItem) | |
| } | |
| return newItem | |
| } | |
| // Auto-create profile when user is logged in but has no profile | |
| useEffect(() => { | |
| if (user && isUserProfileLayerLoaded && !myProfile && !isCreatingProfileRef.current) { | |
| isCreatingProfileRef.current = true | |
| void createEmptyProfile().finally(() => { | |
| isCreatingProfileRef.current = false | |
| }) | |
| // Prevent concurrent profile creation | |
| if (isCreatingProfileRef.current) { | |
| return | |
| } | |
| isCreatingProfileRef.current = true | |
| try { | |
| const userLayer = layers.find((l) => l.userProfileLayer === true) | |
| if (!userLayer?.api?.createItem) { | |
| throw new Error('User profile layer or create API not available') | |
| } | |
| const newProfile = { | |
| id: crypto.randomUUID(), | |
| name: user.first_name ?? 'User', | |
| } | |
| const serverResponse = await userLayer.api.createItem(newProfile) | |
| const newItem = { | |
| ...serverResponse, | |
| user_created: user, // eslint-disable-line camelcase | |
| layer: userLayer, | |
| public_edit: false, // eslint-disable-line camelcase | |
| } | |
| // Add item immediately (without secret) | |
| addItem(newItem) | |
| // Reload in background to get server-generated fields (like secrets) | |
| if (userLayer.api.getItem) { | |
| void reloadItemWithSecret(serverResponse.id, userLayer, newItem) | |
| } | |
| return newItem | |
| } finally { | |
| isCreatingProfileRef.current = false | |
| } | |
| } | |
| // Auto-create profile when user is logged in but has no profile | |
| useEffect(() => { | |
| if (user && isUserProfileLayerLoaded && !myProfile) { | |
| void createEmptyProfile() |
| void navigate('/login') | ||
| } | ||
|
|
||
| if (!isUserProfileLayerLoaded) return |
Copilot
AI
Dec 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a user is not authenticated, the code saves the invite code to localStorage (line 138) but then returns early at line 141 because isUserProfileLayerLoaded will be false for unauthenticated users. This means validateInvite is never called, and invitingProfile remains null, causing the UI to show a loading spinner indefinitely (lines 217-220). For unauthenticated users, you should either skip the isUserProfileLayerLoaded check or fetch the inviting profile before checking authentication status.
| if (!isUserProfileLayerLoaded) return | |
| if (isAuthenticated && !isUserProfileLayerLoaded) return |
🍰 Pullrequest
Implements more of QR invite flow.
Issues
Todo