feature/BA-2717 update profile interaction accuracy #300
Conversation
rf2tsl
requested review from
anicioalexandre and
priscilladeroode
as code owners
|
WalkthroughComment and messaging code now use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI Components
participant Fetch as baseAppFetch
participant Storage as TokenStore (SSR / client)
participant API as GraphQL/API
UI->>Fetch: request(url, options)
Fetch->>Storage: getTokenSSR/getToken(CURRENT_PROFILE_KEY_NAME)
alt profile found
Storage-->>Fetch: "stringified MinimalProfile"
Fetch->>Fetch: parseString<MinimalProfile>()
Fetch->>API: request with header "Current-Profile": profile.id
else no profile
Storage-->>Fetch: null
Fetch->>API: request without Current-Profile header
end
API-->>Fetch: response
Fetch-->>UI: response
sequenceDiagram
autonumber
participant ProfilesUI as ProfilesList
participant Browser as Browser
ProfilesUI->>ProfilesUI: switch profile + show toast
ProfilesUI->>Browser: window.location.reload()
Note right of Browser: full reload to rehydrate app under new profile
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/modules/comments/web/CommentItem/index.tsx (1)
90-100: Reply name will be undefined if profile data is missing.Line 96 sets the reply name from
comment.profile?.name, but the function only checkshasUser(line 91). If a comment was created before the profile migration or if profile data is unavailable, users will see "Replying to undefined" in the UI.Consider adding fallback logic:
const replyToComment = () => { if (hasUser) { onReplyClick?.() setCommentReply({ commentItemRef, inReplyToId: comment.id, - name: comment.profile?.name, + name: comment.profile?.name ?? comment.user?.fullName ?? 'User', }) } showReplies() }packages/components/modules/messages/web/AllChatRoomsList/index.tsx (1)
111-111: Add null safety check for data.id access.Accessing
data.idwithout a null check will throw a runtime error ifdatais undefined or null. Given the transition to profile-based data fetching and the potential for loading/error states, this is a critical issue.Apply this diff to add a safe fallback:
- useRoomListSubscription({ profileId: data.id, connections: [] }) + useRoomListSubscription({ profileId: data?.id, connections: [] })Alternatively, consider whether the subscription should be conditional on data availability:
useRoomListSubscription({ profileId: data?.id ?? '', connections: [] })Or skip the subscription entirely when data is unavailable:
if (data?.id) { useRoomListSubscription({ profileId: data.id, connections: [] }) }
🧹 Nitpick comments (2)
packages/utils/functions/fetch/baseAppFetch/index.ts (1)
94-101: Consider refactoring the window check for better readability.The pattern
typeof window === typeof undefinedworks but is less conventional thantypeof window === 'undefined'. While this matches the existing pattern in lines 125 and 161, consider refactoring all occurrences for improved clarity.Apply this diff to improve readability:
- if (typeof window === typeof undefined) { + if (typeof window === 'undefined') { const { getTokenSSR } = await import('../../token/getTokenSSR') currentProfile = await getTokenSSR('CurrentProfile') } else {packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx (1)
42-49: Image path workaround should be addressed.The TODO comment indicates this image path handling is temporary and should be moved to the backend. This client-side URL construction is fragile and duplicates logic that belongs in the API layer.
Consider creating a backend ticket to return absolute image URLs from the API. This would eliminate the need for environment variable access and URL manipulation on the client.
If you'd like, I can help create an issue to track moving this logic to the backend.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/components/modules/comments/common/graphql/queries/CommentItem.ts(1 hunks)packages/components/modules/comments/web/CommentItem/index.tsx(3 hunks)packages/components/modules/messages/common/graphql/queries/ChatRoomsQuery.ts(1 hunks)packages/components/modules/messages/web/AllChatRoomsList/index.tsx(1 hunks)packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx(1 hunks)packages/utils/functions/fetch/baseAppFetch/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/utils/functions/fetch/baseAppFetch/index.ts (3)
packages/utils/functions/token/getTokenSSR/index.ts (1)
getTokenSSR(3-8)packages/utils/functions/token/getToken/index.ts (1)
getToken(9-17)packages/utils/functions/fetch/baseAppFetch/types.ts (1)
RequestOptions(39-42)
⏰ 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: Analyze (javascript)
- GitHub Check: Component Test Packages
🔇 Additional comments (4)
packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx (1)
40-61: Authentication headers are properly updated via the Current-Profile cookie header.The profile switching mechanism correctly aligns with PR objectives. When
setCurrentProfile()is called, it updates the Zustand store and persists the profile to cookies. ThebaseAppFetchutility then includes theCurrent-Profileheader on every subsequent request by reading from cookies. This ensures subsequent interactions use the newly active profile as required.However, the
window.location.reload()on line 58 suggests the full page reload is being used as a workaround to reinitialize app state. Since profile changes are already handled reactively through the Zustand store and properly propagated via headers for backend requests, consider whether the reload is truly necessary or if it can be removed to provide a smoother user experience.packages/components/modules/comments/common/graphql/queries/CommentItem.ts (1)
12-22: Profile fields added correctly to support profile-based attribution.The profile field block includes all necessary data for UI rendering and is actively consumed for display (name, image URL, profile URL path). The user field serves as a deleted-user indicator only—it checks account existence via
Boolean(comment?.user)to conditionally render "Deleted User" and control reply permissions, but no user data fields (fullName, avatar) are consumed. This design correctly separates primary data (profile) from deletion status (user existence).packages/components/modules/messages/web/AllChatRoomsList/index.tsx (1)
30-46: All callers ofAllChatRoomsListhave been properly updated to match the newtargetRefshape.Only one consumer of
AllChatRoomsListexists in the codebase:ChatRoomsComponent(line 105 ofpackages/components/modules/messages/web/ChatRoomsComponent/index.tsx). It correctly passestargetRef={chatRoomsQueryData}where both the prop and the passed value are typed asChatRoomsQuery$data. The types are consistently aligned with the component's internal usage oftargetRef?.profile(line 45).packages/components/modules/messages/common/graphql/queries/ChatRoomsQuery.ts (1)
3-11: Based on my verification through web search and codebase analysis, I cannot conclusively determine where the ChatRoomsQuery is executed and how the profileId parameter is validated. Here's what I found:Verified:
- The query requires authentication early, tenant isolation in the data layer, and verification that the requesting user has access to the requested profile
- Suspense boundaries are in place in the messages module for error handling
- The query definition correctly enforces
$profileId: ID!as a required parameter- Component types are properly defined to handle the query data
Unable to verify:
- Where ChatRoomsQuery is actually executed (useQueryLoader/useLazyLoadQuery call site)
- How profileId is obtained and validated before query execution
- Whether backend authorization checks are implemented for the profile lookup
The query receives
chatRoomsQueryDataas a prop in ChatRoomsComponent but the actual query loading happens at a parent layer not visible in the component files. The ChatRoomProvider context might be involved but needs manual verification.
packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx
Show resolved
Hide resolved
packages/components/modules/messages/common/graphql/queries/ChatRoomsQuery.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/authentication/modules/access/useLogout/index.ts(1 hunks)packages/authentication/modules/profile/useCurrentProfile/__tests__/useCurrentProfile.test.tsx(1 hunks)packages/authentication/modules/profile/useCurrentProfile/constants.ts(0 hunks)packages/authentication/modules/profile/useCurrentProfile/store.ts(1 hunks)packages/authentication/modules/profile/utils.ts(1 hunks)packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx(1 hunks)packages/graphql/config/environment.ts(1 hunks)packages/utils/constants/profile.ts(1 hunks)packages/utils/functions/fetch/baseAppFetch/index.ts(2 hunks)packages/utils/types/profile.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/authentication/modules/profile/useCurrentProfile/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/utils/functions/fetch/baseAppFetch/index.ts
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
📚 Learning: 2025-10-29T13:48:38.345Z
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
Applied to files:
packages/authentication/modules/profile/useCurrentProfile/store.ts
⏰ 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: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (9)
packages/utils/constants/profile.ts (1)
1-1: LGTM! Centralized constant improves maintainability.Centralizing
CURRENT_PROFILE_KEY_NAMEinto a shared constants module is a best practice that reduces duplication and ensures consistency across the codebase.packages/authentication/modules/tests/utils/providers/withAuthenticationTestProviders/index.tsx (1)
3-3: LGTM! Import path updated for centralized constant.The import path change aligns with the centralization of
CURRENT_PROFILE_KEY_NAME. Functionality remains unchanged.packages/authentication/modules/profile/useCurrentProfile/__tests__/useCurrentProfile.test.tsx (1)
5-5: LGTM! Import path correctly updated.The import path change for
CURRENT_PROFILE_KEY_NAMEis consistent with the centralization effort. The test logic remains unchanged and should continue to function properly.packages/graphql/config/environment.ts (2)
103-115: LGTM! Websocket connection properly includes current profile.The logic correctly:
- Retrieves the current profile token
- Safely parses it with error handling
- Only includes the
Current-Profileheader when both authentication and a valid profile exist- Properly extracts just the profile ID for the header
This ensures websocket connections are properly associated with the active profile.
1-1: Current import is correct—no changes needed.The
MinimalProfileimport from@baseapp-frontend/authenticationinpackages/graphql/config/environment.tsis the proper pattern. While@baseapp-frontend/utilsmaintains its own internal copy ofMinimalProfile, it does not export it publicly. Only@baseapp-frontend/authenticationre-exports it in its index.ts (export type * from './types/profile'), making it the canonical public import. Internal code within the utils package uses relative imports to their own copy, while external packages correctly source from the authentication package's public API.Likely an incorrect or invalid review comment.
packages/authentication/modules/profile/useCurrentProfile/store.ts (1)
1-2: LGTM! Import path updated for centralized constant.The import path change for
CURRENT_PROFILE_KEY_NAMEaligns with the centralization effort. All usages throughout the file remain functionally unchanged.packages/authentication/modules/access/useLogout/index.ts (2)
8-8: LGTM! Import path updated for centralized constant.The import path change aligns with the centralization of profile-related constants.
24-34: Excellent addition! Current profile cleared on logout.Adding
removeTokenAsync(CURRENT_PROFILE_KEY_NAME)to the logout flow ensures that the active profile selection is properly cleared when a user logs out. This prevents stale profile data from persisting and aligns with the PR's objective of accurate profile attribution.packages/authentication/modules/profile/utils.ts (1)
2-2: LGTM! Import path updated for centralized constant.The import path change for
CURRENT_PROFILE_KEY_NAMEis consistent with the centralization effort. The function behavior remains unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/components/modules/messages/web/GroupChatEdit/index.tsx(1 hunks)packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
🧬 Code graph analysis (1)
packages/components/modules/messages/web/GroupChatEdit/index.tsx (1)
packages/components/modules/messages/web/__shared__/GroupChatMembersList/types.ts (1)
GroupChatMembersListProps(14-37)
⏰ 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: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (1)
packages/components/modules/messages/web/GroupChatEdit/index.tsx (1)
173-190: Layout change unrelated to PR objectives.The PR objectives focus on ensuring interactions use the currently active profile. This layout wrapper change doesn't affect profile handling or interaction accuracy. Consider whether this UI change belongs in a separate PR focused on layout improvements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/utils/functions/fetch/baseAppFetch/index.ts (1)
105-105: Consider optional chaining for safer property access.If
parseStringreceives malformed JSON (e.g., a plain string), it returns that string cast asMinimalProfile. Accessing.idon a string returnsundefined, which would set the header toundefinedrather than an empty string. While this likely works in practice, optional chaining provides clearer intent and guarantees the fallback.Apply this diff for improved type safety:
- const parsedCurrentProfile = parseString<MinimalProfile>(currentProfile ?? undefined) + const parsedCurrentProfile = parseString<MinimalProfile>(currentProfile ?? undefined) const fetchOptions: RequestOptions = { ...options, headers: { Accept: 'application/json, text/plain, */*', - 'Current-Profile': parsedCurrentProfile ? parsedCurrentProfile.id : '', + 'Current-Profile': parsedCurrentProfile?.id || '', ...options.headers, }, }Also applies to: 111-111
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/utils/functions/fetch/baseAppFetch/__tests__/baseAppFetch.test.ts(1 hunks)packages/utils/functions/fetch/baseAppFetch/index.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
🧬 Code graph analysis (1)
packages/utils/functions/fetch/baseAppFetch/index.ts (6)
packages/utils/functions/token/getTokenSSR/index.ts (1)
getTokenSSR(3-8)packages/utils/constants/profile.ts (1)
CURRENT_PROFILE_KEY_NAME(1-1)packages/utils/functions/token/getToken/index.ts (1)
getToken(9-17)packages/utils/functions/string/parseString/index.ts (1)
parseString(1-10)packages/utils/types/profile.ts (1)
MinimalProfile(1-6)packages/utils/functions/fetch/baseAppFetch/types.ts (1)
RequestOptions(39-42)
⏰ 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). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (3)
packages/utils/functions/fetch/baseAppFetch/index.ts (2)
7-8: LGTM: Clean imports following established patterns.The imports for profile constant, type, and parsing utility are well-organized and follow the centralized import pattern requested by the team.
Also applies to: 11-11
96-103: LGTM: Environment-aware token retrieval is correctly implemented.The SSR vs client detection and dynamic imports for token retrieval properly handle both server-side and client-side execution contexts.
packages/utils/functions/fetch/baseAppFetch/__tests__/baseAppFetch.test.ts (1)
428-428: Test correctly accounts for new profile token retrieval.The added mock value ensures the test passes with the new
getTokenSSR(CURRENT_PROFILE_KEY_NAME)call in the implementation. Note that the string value'Current-Profile'(not valid JSON) effectively tests the error-handling path where malformed profile data is gracefully handled.
… feature/BA-2717-ba-multiple-user-interaction-profile-accuracy
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/utils/CHANGELOG.md (1)
7-7: Capitalize the changelog entry and consider adding more descriptive detail.Changelog conventions typically capitalize verbs in bullet points. Additionally, based on the PR scope, this entry could benefit from more detail about what was updated and why, particularly since the change involves SSR/client-aware profile header injection and introduces new infrastructure (constants, types).
Consider updating the entry:
- update to baseAppFetch and tests to include 'Current-Profile' in request headers + Updated `baseAppFetch` to inject `Current-Profile` header from stored MinimalProfile (SSR and client-aware); centralized `CURRENT_PROFILE_KEY_NAME` constant; updated related testsThis provides more context about the infrastructure changes and why they matter to consumers of this package.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/authentication/CHANGELOG.md(1 hunks)packages/authentication/package.json(1 hunks)packages/components/CHANGELOG.md(1 hunks)packages/components/package.json(1 hunks)packages/design-system/CHANGELOG.md(1 hunks)packages/design-system/package.json(1 hunks)packages/graphql/CHANGELOG.md(1 hunks)packages/graphql/package.json(1 hunks)packages/provider/CHANGELOG.md(1 hunks)packages/provider/package.json(1 hunks)packages/utils/CHANGELOG.md(1 hunks)packages/utils/package.json(1 hunks)packages/wagtail/CHANGELOG.md(1 hunks)packages/wagtail/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- packages/provider/package.json
- packages/utils/package.json
- packages/components/CHANGELOG.md
- packages/components/package.json
- packages/graphql/package.json
- packages/design-system/package.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
⏰ 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: Component Test Packages
- GitHub Check: Build and Lint Packages
🔇 Additional comments (7)
packages/provider/CHANGELOG.md (1)
3-8: Changelog entry looks good.The new version 2.0.18 entry is properly formatted, follows the established changelog pattern, and correctly documents the dependency update to @baseapp-frontend/utils@4.0.4.
packages/graphql/CHANGELOG.md (1)
3-10: Changelog entry is accurate and appropriately formatted.The new 1.3.6 entry correctly documents the import refactoring and dependency updates. The version bump is appropriate for a patch-level change, and the dependency versions align with the coordinated updates across the monorepo (moving
CURRENT_PROFILE_KEY_NAMEfrom authentication to a centralized utils module). Based on learnings.packages/authentication/package.json (1)
4-4: Patch version bump is appropriate for the refactoring work.The version bump from 5.0.4 to 5.0.5 is semantically correct for the internal refactoring of moving
CURRENT_PROFILE_KEY_NAMEto the utils package, with updated imports and peer dependency declaration remaining compatible viaworkspace:*.packages/authentication/CHANGELOG.md (1)
3-9: Changelog entry is accurate and complete.The 5.0.5 patch entry correctly documents the constant relocation and coordinated dependency update. Version numbering and formatting are consistent with the existing changelog.
packages/wagtail/package.json (1)
4-4: Version bump looks good.Clean patch-level version increment (1.0.39 → 1.0.40) consistent with dependency updates documented in CHANGELOG.md.
packages/wagtail/CHANGELOG.md (1)
3-10: Changelog entry is well-formatted and consistent.Version 1.0.40 entry correctly documents the three dependency updates in proper format, matching the established pattern from previous releases.
packages/design-system/CHANGELOG.md (1)
3-8: The changelog entry is correct and complete.Design-system received only a dependency update (utils 4.0.3 → 4.0.4); no component source files were modified in this package. The changelog accurately documents this change. The AI summary's mention of "Comment and messaging components now use profile-centric fields" refers to changes in other packages, not design-system.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx (2)
96-103: Critical: Profile list interaction test disabled.Step 2's test assertions are partially disabled—the test claims to verify profile list scrolling and interaction, but the menu click and operation resolution are commented out. This compounds the test coverage gap.
The same stubbing approach from the previous comment would resolve this. Without these assertions, the scrolling verification (lines 105-109) may be testing against stale or incorrect data.
125-128: Critical: Profile switch verification incomplete.Step 4's test title states "should not show the success toast when profile is not changed," but the profile switch action is commented out, making the assertion impossible to verify.
This is the third instance of disabled profile switching tests in this file. The pattern suggests the test approach needs to be reconsidered rather than leaving critical assertions disabled.
Recommendation: Either stub
window.location.reload()as suggested earlier, or convert these to E2E tests if component tests cannot adequately handle the reload behavior. Leaving the assertions commented out undermines the value of the test suite.
🧹 Nitpick comments (1)
packages/components/modules/comments/web/Comments/__tests__/__mocks__/constants.ts (1)
1-12: MOCK_PROFILE shape looks good; consider making it typed/readonly.The structure matches the way
profileis used in the mocks; if you have a shared Profile/MinimalProfile type for comments, you might optionally annotateMOCK_PROFILEwith it or useas constto keep the shape aligned with the schema over time.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/components/CHANGELOG.md(1 hunks)packages/components/modules/comments/web/Comments/__tests__/__mocks__/constants.ts(1 hunks)packages/components/modules/comments/web/Comments/__tests__/__mocks__/requests.ts(8 hunks)packages/components/modules/comments/web/Comments/__tests__/__mocks__/resolvers.ts(3 hunks)packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx(2 hunks)packages/components/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/components/package.json
- packages/components/modules/comments/web/Comments/tests/mocks/resolvers.ts
- packages/components/CHANGELOG.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-29T13:48:38.345Z
Learnt from: nossila
Repo: silverlogic/baseapp-frontend PR: 300
File: packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx:57-59
Timestamp: 2025-10-29T13:48:38.345Z
Learning: In packages/components/modules/profiles/web/profile-popover/ProfilesList/index.tsx, the full page reload via window.location.reload() when switching profiles is intentional and required due to websockets connections and data loading patterns that don't yet support reactive profile switching. This is a known technical debt that will be addressed with a larger refactoring in the future.
Applied to files:
packages/components/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx
🧬 Code graph analysis (1)
packages/components/modules/comments/web/Comments/__tests__/__mocks__/requests.ts (1)
packages/components/modules/comments/web/Comments/__tests__/__mocks__/constants.ts (1)
MOCK_PROFILE(1-12)
🔇 Additional comments (1)
packages/components/modules/comments/web/Comments/__tests__/__mocks__/requests.ts (1)
1-1: Import andprofile: MOCK_PROFILEwiring look correct; verify coverage vs API shape.The new import and all added
profile: MOCK_PROFILEfields are consistent and should help exercise the active‑profile behavior in tests. One thing to double‑check: several other comment nodes in this file still have noprofile(onlyuser). If the updated GraphQL/API contract always includes aprofilefor comments created under any profile, you may want mocks to reflect that uniformly; if some comments are intentionally “no profile” (fallback scenarios), this asymmetry is fine.Also applies to: 17-17, 39-39, 168-168, 201-201, 284-284, 317-317, 965-965
...ts/modules/navigations/web/Header/AccountMenu/AccountPopover/__tests__/AccountPopover.cy.tsx
Show resolved
Hide resolved
|
rf2tsl
deleted the
feature/BA-2717-ba-multiple-user-interaction-profile-accuracy
branch
Acceptance Criteria
As a user with multiple profiles, I want my interactions (such as comments) to reflect the currently active profile, so that others can correctly identify which profile performed the action.
Acceptance Criteria
When a user with multiple profiles performs an action (e.g., posts a comment), the system should associate the interaction with the currently active profile, not the main account.
The profile name and avatar displayed in the UI should correspond to the active profile that performed the action.
The system must validate that a secondary profile is active at the moment of the interaction to associate it properly.
If no profile is explicitly active, default to the main user profile.
Backend APIs should store and return the correct profile ID associated with each interaction.
The change should apply to all user-generated interactions across the platform where profile attribution is relevant.
Examples: comments, posts, messages, likes.
When switching profiles, the system must update the session or context accordingly, ensuring all subsequent interactions use the new profile that they switched to.
Approvd
https://app.approvd.io/projects/BA/stories/43026
Summary by CodeRabbit
New Features
Improvements
Tests
Chores