Skip to content

BA-1809 [FE] Simplify CurrentProfileProvider #140

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.

Sign up for GitHub

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

Closed
wants to merge 14 commits into from
Closed

BA-1809 [FE] Simplify CurrentProfileProvider #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fc564a8
BA-1809 Replace catalogs temporarily
tsl-ps2 Nov 19, 2024
a65a0d1
BA-1809 install jotai
tsl-ps2 Nov 19, 2024
3bb3b2f
Profile type
tsl-ps2 Nov 20, 2024
50d7436
update getCurrentProfile
tsl-ps2 Nov 20, 2024
59970ad
update references
tsl-ps2 Nov 20, 2024
2465eec
BA-1809 Use MinimalProfile consistently and update when user changes …
tsl-ps2 Nov 25, 2024
3a11154
BA-1809 updateProfileIfActive
tsl-ps2 Nov 26, 2024
b1e7d29
Revert "BA-1809 install jotai"
tsl-ps2 Nov 26, 2024
881b4d4
Revert "BA-1809 Replace catalogs temporarily"
tsl-ps2 Nov 26, 2024
25338fb
BA-1809 add jotai to dependencies
tsl-ps2 Nov 26, 2024
b6b77d0
BA-1809 Remove omitted avatar
tsl-ps2 Nov 26, 2024
2ec8f7e
BA-1809 modify storybooks
tsl-ps2 Nov 26, 2024
ee4bc9c
BA-1809 Jotai installation pnpm-lock
tsl-ps2 Nov 26, 2024
025aae0
BA-1809 Simplify MinimalProfile type
tsl-ps2 Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions packages/authentication/types/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import { MinimalProfile } from '@baseapp-frontend/utils'

export interface User {
id: number
email: string
isEmailVerified: boolean
newEmail: string
isNewEmailConfirmed: boolean
referralCode: string
avatar: {
fullSize: string
small: string
}
firstName: string
lastName: string
profile: MinimalProfile
Copy link

@coderabbitai coderabbitai bot Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the search results, I can now generate the final response:

Breaking change requires GraphQL schema updates and component modifications

The transition from avatar to profile.image affects GraphQL-related code:

  • GraphQL queries and fragments need to be updated to use profile.image instead of avatar
  • Generated types in packages/components/__generated__/ need to be regenerated
  • GraphQL schema already has profile field definitions in place

Please ensure:

  • Update all GraphQL queries/fragments that reference user.avatar
  • Regenerate GraphQL types after query updates
  • Update any component code that accesses avatar through destructuring
🔗 Analysis chain

Verify the impact of this breaking change across the application.

The transition from direct avatar property to profile.image is a significant architectural change that affects how profile data is accessed throughout the application.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for potential breaking changes in the codebase

# Check for direct avatar property access that needs updating
rg "user\.avatar\b" --type ts --type tsx -l

# Check for profile property access patterns
rg "user\.profile\.(image|name|urlPath)" --type ts --type tsx -l

# Check for destructuring patterns that might break
rg "const \{.*avatar.*\} = user" --type ts --type tsx -l

Length of output: 255

Script:

#!/bin/bash
# Description: Check for potential breaking changes in the codebase using correct file type flags

# Check for direct avatar property access that needs updating
rg "user\.avatar\b" -t ts -t typescript -t js -t jsx

# Check for profile property access patterns
rg "user\.profile\.(image|name|urlPath)" -t ts -t typescript -t js -t jsx

# Check for destructuring patterns that might break
rg "const \{.*avatar.*\} = user" -t ts -t typescript -t js -t jsx

# Additional check for avatar usage in test files
rg "avatar.*:" -t ts -t typescript -t test

Length of output: 383

Script:

#!/bin/bash
# Description: Check for potential breaking changes using simpler ripgrep commands

# Find all TypeScript/JavaScript files first
fd "\.(ts|js)$" > files.txt

# Check for direct avatar property access that needs updating
rg "user\.avatar\b" $(cat files.txt)

# Check for profile property access patterns
rg "user\.profile\." $(cat files.txt)

# Check for destructuring patterns that might break
rg "const.*\{.*avatar.*\}.*=.*user" $(cat files.txt)

# Clean up
rm files.txt

Length of output: 77631

Script:

#!/bin/bash
# Description: Check for GraphQL schema definitions and queries that might need updates

# Look for GraphQL type definitions containing avatar field
rg "type.*\{[^}]*avatar.*:[^}]*\}" -g "*.graphql"

# Look for GraphQL queries/fragments referencing avatar
rg "fragment.*\{[^}]*avatar" -g "*.graphql"
rg "query.*\{[^}]*avatar" -g "*.graphql"

# Check for avatar in GraphQL generated types
rg "avatar.*:" -g "*generated*.ts"

# Check for profile-related GraphQL fields
rg "profile.*:" -g "*.graphql"

Length of output: 2652

phoneNumber: string
preferredLanguage: string
}

export interface UserUpdateParams<TUser extends Partial<User>> {
userId: TUser['id']
data: Partial<Omit<TUser, 'avatar' | 'id'>> & {
data: Partial<Omit<TUser, 'id'>> & {
avatar?: File | string
}
Comment on lines +19 to 21
Copy link

@coderabbitai coderabbitai bot Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Resolve inconsistency between User and UserUpdateParams interfaces.

The avatar field in UserUpdateParams.data seems inconsistent with the new User interface structure where avatar-related data is now part of the profile.image. Consider updating this to align with the new structure:

export interface UserUpdateParams<TUser extends Partial<User>> {
  userId: TUser['id']
  data: Partial<Omit<TUser, 'id'>> & {
-   avatar?: File | string
+   profile?: Partial<MinimalProfile> & {
+     image?: File | string
+   }
  }
}
📝 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
data: Partial<Omit<TUser, 'id'>> & {
avatar?: File | string
}
data: Partial<Omit<TUser, 'id'>> & {
profile?: Partial<MinimalProfile> & {
image?: File | string
}
}

}
26 changes: 13 additions & 13 deletions packages/components/.storybook/decorators/withProviders.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import { NotificationProvider } from '@baseapp-frontend/utils'

import type { StoryContext, StoryFn } from '@storybook/react'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { Provider as JotaiProvider } from 'jotai'

import CurrentProfileProvider from '../../modules/profiles/context/CurrentProfileProvider'
import '../../styles/tailwind/globals.css'
import defaultTheme from '../__mocks__/theme'

Expand All @@ -34,19 +34,19 @@ const withProviders = (Story: StoryFn, context: StoryContext) => {
}, [mockData, resolveMostRecentOperation, queueOperationResolver, mockResolvers])

return (
<QueryClientProvider client={queryClient}>
<RelayTestProvider environment={environment}>
<React.Suspense fallback={<LoadingState />}>
<ThemeProvider {...defaultTheme}>
<NotificationProvider>
<CurrentProfileProvider>
<JotaiProvider>
<QueryClientProvider client={queryClient}>
<RelayTestProvider environment={environment}>
<React.Suspense fallback={<LoadingState />}>
<ThemeProvider {...defaultTheme}>
<NotificationProvider>
<Story />
</CurrentProfileProvider>
</NotificationProvider>
</ThemeProvider>
</React.Suspense>
</RelayTestProvider>
</QueryClientProvider>
</NotificationProvider>
</ThemeProvider>
</React.Suspense>
</RelayTestProvider>
</QueryClientProvider>
</JotaiProvider>
)
}

Expand Down
2 changes: 1 addition & 1 deletion packages/components/modules/messages/MessageItem/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { MessageItemContainer } from './styled'
import { MessageItemProps } from './types'

const MessageItem: FC<MessageItemProps> = ({ messageRef, isFirstGroupedMessage }) => {
const { profile: currentProfile } = useCurrentProfile()
const { currentProfile } = useCurrentProfile()
const message = useFragment(MessageItemFragment, messageRef)

const isOwnMessage = currentProfile?.id === message?.profile?.id
Expand Down
2 changes: 1 addition & 1 deletion packages/components/modules/messages/MessagesGroup/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const MessagesGroup: FC<MessagesGroupProps> = ({
MessageItem = DefaultMessageItem,
MessageItemProps = {},
}) => {
const { profile: currentProfile } = useCurrentProfile()
const { currentProfile } = useCurrentProfile()

const renderDateOnTopOfMessagesGroup = useCallback(
(index: number) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/modules/messages/SendMessage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ let nextClientMutationId = 0
*/
const SendMessage = forwardRef<HTMLInputElement, SendMessageProps>(
({ roomId, SocialInput = DefaultSocialInput, SocialInputProps = {} }, ref) => {
const { profile: currentProfile } = useCurrentProfile()
const { currentProfile } = useCurrentProfile()

const form = useForm<SocialUpsertForm>({
defaultValues: DEFAULT_SOCIAL_UPSERT_FORM_VALUES,
Expand Down
13 changes: 11 additions & 2 deletions ...omponents/modules/navigations/Header/AccountMenu/AccountPopover/__storybook__/stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import { User, UserApi } from '@baseapp-frontend/authentication'
import { createTestEnvironment } from '@baseapp-frontend/graphql'

import { Meta, StoryObj } from '@storybook/react'
import Cookies from 'js-cookie'

import AccountPopover from '..'
import { withTokenSetup } from '../../../../../../.storybook/decorators'
import { CURRENT_PROFILE_STORAGE_KEY } from '../../../../../profiles/context/CurrentProfileProvider/constants'
import { PROFILE_KEY } from '../../../../../profiles/useCurrentProfile/constants'
Copy link
Collaborator

@anicioalexandre anicioalexandre Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong import

import { AccountPopoverProps } from '../types'
import { mockResolvers } from './mockResolvers'

Expand All @@ -31,7 +32,15 @@ export default {
decorators: [
withTokenSetup,
(Story, context) => {
localStorage.removeItem(CURRENT_PROFILE_STORAGE_KEY)
Cookies.set(
PROFILE_KEY,
JSON.stringify({
id: '1234',
name: 'test',
image: null,
url: { path: 'path' },
}),
)
Comment on lines +35 to +43
Copy link

@coderabbitai coderabbitai bot Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Mock profile structure needs to be updated to match MinimalProfile type

The mock profile structure in the story doesn't match the MinimalProfile type definition:

  • url: { path: 'path' } should be urlPath: string | null

Update the mock data to match the type:

  Cookies.set(
    PROFILE_KEY,
    JSON.stringify({
      id: '1234',
      name: 'test',
      image: null,
-     url: { path: 'path' },
+     urlPath: '/path',
    }),
    COOKIE_OPTIONS
  )
🔗 Analysis chain

Consider enhancing cookie security and mock data management

While the implementation works, consider these improvements:

  1. Add cookie security options (httpOnly, secure, sameSite)
  2. Move mock profile data to a separate constants file
  3. Verify that the mock profile structure matches the MinimalProfile type

Consider updating the code like this:

+ import { MOCK_PROFILE_DATA } from './mockData'
+ const COOKIE_OPTIONS = {
+   secure: true,
+   sameSite: 'strict',
+   path: '/'
+ }

  Cookies.set(
    PROFILE_KEY,
-   JSON.stringify({
-     id: '1234',
-     name: 'test',
-     image: null,
-     url: { path: 'path' },
-   }),
+   JSON.stringify(MOCK_PROFILE_DATA),
+   COOKIE_OPTIONS
  )

Let's verify the profile structure matches the type definition:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for MinimalProfile type definition
ast-grep --pattern 'type MinimalProfile = {
  $$$
}'

Length of output: 374


const { queueOperationResolver } = context.parameters.relayMockEnvironment as ReturnType<
typeof createTestEnvironment
Expand Down
22 changes: 11 additions & 11 deletions ...navigations/Header/AccountMenu/AccountPopover/__tests__/__utils__/withProviders/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { RelayTestProvider } from '@baseapp-frontend/graphql'
import { NotificationProvider } from '@baseapp-frontend/utils'

import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import { Provider as JotaiProvider } from 'jotai'

import CurrentProfileProvider from '../../../../../../../profiles/context/CurrentProfileProvider'
import { AccountPopoverProps } from '../../../types'
import defaultTheme from '../../__mocks__/theme'
import { WithProvidersOptions } from './types'
Expand All @@ -16,17 +16,17 @@ const queryClient = new QueryClient()
const withProviders =
(Component: FC<AccountPopoverProps>) =>
({ environment, ...props }: WithProvidersOptions & AccountPopoverProps) => (
<QueryClientProvider client={queryClient}>
<RelayTestProvider environment={environment}>
<ThemeProvider {...defaultTheme}>
<NotificationProvider>
<CurrentProfileProvider>
<JotaiProvider>
<QueryClientProvider client={queryClient}>
<RelayTestProvider environment={environment}>
<ThemeProvider {...defaultTheme}>
<NotificationProvider>
<Component {...props} />
</CurrentProfileProvider>
</NotificationProvider>
</ThemeProvider>
</RelayTestProvider>
</QueryClientProvider>
</NotificationProvider>
</ThemeProvider>
</RelayTestProvider>
</QueryClientProvider>
</JotaiProvider>
)

export default withProviders
7 changes: 2 additions & 5 deletions packages/components/modules/navigations/Header/AccountMenu/AccountPopover/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

import { FC, useEffect, useState } from 'react'

import { User as BaseUser, useJWTUser } from '@baseapp-frontend/authentication'
import { ClickableAvatar, Popover, usePopover } from '@baseapp-frontend/design-system'
import { JWTContent } from '@baseapp-frontend/utils'

import Divider from '@mui/material/Divider'

Expand Down Expand Up @@ -35,8 +33,7 @@ const AccountPopover: FC<AccountPopoverProps> = ({
AddProfileMenuItemProps = {},
LogoutItemProps = {},
}) => {
const { user } = useJWTUser<BaseUser & JWTContent>()
const { profile } = useCurrentProfile()
const profile = useCurrentProfile({ noSSR: false }).currentProfile
const popover = usePopover()

const [openProfilesList, setOpenProfilesList] = useState(false)
Expand Down Expand Up @@ -69,7 +66,7 @@ const AccountPopover: FC<AccountPopoverProps> = ({
<>
<ClickableAvatar
color="secondary"
src={profile?.image?.url ?? user?.avatar?.small}
src={profile?.image ?? ''}
alt="User avatar"
onClick={popover.onOpen}
isOpen={Boolean(popover.open)}
Expand Down
10 changes: 5 additions & 5 deletions packages/components/modules/profiles/ProfilePopover/CurrentProfile/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import { AvatarWithPlaceholder } from '@baseapp-frontend/design-system'

import { Box, Typography } from '@mui/material'

import useCurrentProfile from '../../context/useCurrentProfile'
import useCurrentProfile from '../../useCurrentProfile'

const CurrentProfile: FC = () => {
const { profile } = useCurrentProfile()
const profile = useCurrentProfile().currentProfile

if (!profile) return null

Expand All @@ -16,7 +16,7 @@ const CurrentProfile: FC = () => {
<AvatarWithPlaceholder
width={40}
height={40}
src={profile.image?.url ?? ''}
src={profile.image ?? ''}
alt="Current profile avatar"
color="secondary"
/>
Expand All @@ -25,9 +25,9 @@ const CurrentProfile: FC = () => {
{profile.name}
</Typography>

{profile.urlPath?.path && (
{profile.urlPath && (
<Typography variant="body2" sx={{ color: 'text.secondary' }} noWrap>
{profile.urlPath?.path}
{profile.urlPath}
</Typography>
)}
</Box>
Expand Down
3 changes: 2 additions & 1 deletion packages/components/modules/profiles/ProfilePopover/ProfilesList/ProfileMenuItem/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { MinimalProfile } from '@baseapp-frontend/authentication'
import type { AvatarWithPlaceholderProps } from '@baseapp-frontend/design-system'

import { MenuItemProps } from '@mui/material'
Expand All @@ -10,7 +11,7 @@ import type {
export interface ProfileMenuItemProps {
profileRef: ProfileItemFragment$key
onProfileChange: (newProfile: ProfileItemFragment$data) => void
currentProfile?: ProfileItemFragment$data
currentProfile?: MinimalProfile | null
Copy link
Collaborator

@anicioalexandre anicioalexandre Nov 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's not pass currentProfile as props, instead ProfileMenuItem could get it from useCurrentProfile

avatarProps?: AvatarWithPlaceholderProps
width?: number
height?: number
Expand Down
6 changes: 3 additions & 3 deletions packages/components/modules/profiles/ProfilePopover/ProfilesList/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { useLazyLoadQuery } from 'react-relay'

import { ProfileItemFragment$data } from '../../../../__generated__/ProfileItemFragment.graphql'
import { ProfilesListQuery as ProfilesListQueryType } from '../../../../__generated__/ProfilesListQuery.graphql'
import useCurrentProfile from '../../context/useCurrentProfile'
import { ProfilesListQuery } from '../../graphql/queries/ProfilesList'
import useCurrentProfile, { getMinimalProfile } from '../../useCurrentProfile'
import LoadingState from './LoadingState'
import ProfileMenuItem from './ProfileMenuItem'
import { CancelMenuItem, StyledList } from './styled'
Expand All @@ -20,11 +20,11 @@ import { ProfilesListProps } from './types'
const ProfilesList: FC<ProfilesListProps> = ({ handleCloseSubmenu, MenuItemProps }) => {
const { me } = useLazyLoadQuery<ProfilesListQueryType>(ProfilesListQuery, {})
const { sendToast } = useNotification()
const { profile: currentProfile, setCurrentProfile } = useCurrentProfile()
const { currentProfile, setCurrentProfile } = useCurrentProfile()

const handleProfileChange = (profile: ProfileItemFragment$data) => {
if (currentProfile?.id !== profile.id) {
setCurrentProfile({ profile })
setCurrentProfile(getMinimalProfile(profile))
sendToast(`Switched to ${profile.name}`)
handleCloseSubmenu()
}
Expand Down
110 changes: 0 additions & 110 deletions ...modules/profiles/context/CurrentProfileProvider/__tests__/CurrentProfileProvider.test.tsx
View file
Open in desktop

This file was deleted.

7 changes: 0 additions & 7 deletions packages/components/modules/profiles/context/CurrentProfileProvider/constants.ts
View file
Open in desktop

This file was deleted.

Loading
Loading