Add user profile navigation from names and avatars #294
+1,840
−1,061
Conversation
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
Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add navigation from user name/icon to user profile
Add user profile navigation from names and avatars
Nov 19, 2025
Member
|
@sourcery-ai review |
Contributor
Reviewer's GuideImplements shared, route-safe user profile navigation from names and avatars across the UI, exposes requester IDs via GraphQL to support linking, adds a public /user/:userId profile view, and includes Storybook/tests plus minor refactors and cleanup. Sequence diagram for loading a public user profile after clicking a name or avatarsequenceDiagram
actor user
participant browser
participant user_profile_link as UserProfileLink
participant router as ReactRouter
participant view_user_profile as ViewUserProfile
participant user_profile_container as UserProfileViewContainer
participant apollo_client as ApolloClient
participant graphql_api as GraphQLServer
participant profile_view as ProfileView
user->>browser: Click name_or_avatar
browser->>user_profile_link: Handle click on Link
user_profile_link->>router: Push route getUserProfilePath(userId)
router-->>browser: Update URL to /user/userId
router->>view_user_profile: Render for route user_userId
view_user_profile->>user_profile_container: Render container
user_profile_container->>apollo_client: useQuery(HomeViewUserProfileContainerUserById, userId)
apollo_client->>graphql_api: Query userById(id_userId)
graphql_api-->>apollo_client: Return PersonalUser_or_AdminUser
apollo_client-->>user_profile_container: Provide data_loading_error
user_profile_container->>profile_view: Render ProfileView(user, listings_empty, isOwnProfile_false)
profile_view-->>browser: Display public_profile_read_only
Entity relationship diagram for ListingRequest requestedById and UsererDiagram
ListingRequest {
ObjectID id
string title
string image
string requestedBy
ObjectID requestedById
string requestedOn
string reservationPeriod
string status
}
User {
ObjectID id
string username
string firstName
string lastName
}
User ||--o{ ListingRequest : requestedBy
Updated class diagram for shared user profile navigation components and reservation requestsclassDiagram
class UserProfileLink {
+userId string
+displayName string
+className string
+style React_CSSProperties
+render() ReactElement
}
class UserAvatar {
+userId string
+userName string
+size number
+avatarUrl string
+className string
+style React_CSSProperties
+shape string
+render() ReactElement
}
class ShareThriftLogo {
+size number
+fill string
+className string
+style React_CSSProperties
+render() ReactElement
}
class UserRoutesUtil {
+getUserProfilePath(userId string) string
}
class UserValidationUtil {
+isValidUserId(userId string) boolean
}
class ViewUserProfile {
+render() ReactElement
}
class UserProfileViewContainer {
-userId string
+render() ReactElement
+handleListingClick(listingId string) void
+profileUser any
}
class MessageThread {
+conversationId string
+messages Message_array
+currentUserId string
+sharer UserInfo
+reserver UserInfo
+render() ReactElement
+getAuthorDisplayName(authorId string) string
}
class ConversationBox {
+data Conversation
+messageText string
+render() ReactElement
+handleSendMessage(event React_FormEvent) void
+getUserDisplayName(user ConversationUser) string
}
class ListingBanner {
+owner User
+render() ReactElement
}
class RequestsTable {
+data ListingRequestData_array
+render() ReactElement
}
class ListingRequestData {
+id string
+title string
+image string
+requestedBy string
+requestedById string
+requestedOn string
+reservationPeriod string
+status string
}
class ListingRequestDomainShape {
+id any
+state string
+createdAt Date
+reservationPeriodStart Date
+reservationPeriodEnd Date
+listing any
+reserver any
}
class ListingRequestUiShape {
+id string
+title string
+image string
+requestedBy string
+requestedById string
+requestedOn string
+reservationPeriod string
+status string
+_raw any
}
class ReservationRequestResolversHelper {
+paginateAndFilterListingRequests(requests ListingRequestDomainShape_array, options PaginateOptions) PaginatedListingRequests
}
class PaginateOptions {
+page number
+pageSize number
+statusFilters string_array
+searchText string
+sorter any
}
class PaginatedListingRequests {
+items ListingRequestUiShape_array
+page number
+pageSize number
+total number
}
class ProfileView
UserProfileLink ..> UserRoutesUtil : uses
UserProfileLink ..> UserValidationUtil : uses
UserAvatar ..> UserRoutesUtil : uses
UserAvatar ..> UserValidationUtil : uses
UserAvatar ..> ShareThriftLogo : uses
ViewUserProfile ..> UserProfileViewContainer : renders
UserProfileViewContainer ..> ProfileView : renders
ConversationBox ..> ListingBanner : uses
ConversationBox ..> MessageThread : uses
MessageThread ..> UserAvatar : uses
RequestsTable ..> ListingRequestData : displays
RequestsTable ..> UserProfileLink : uses
ReservationRequestResolversHelper ..> ListingRequestDomainShape : maps_from
ReservationRequestResolversHelper ..> ListingRequestUiShape : maps_to
ReservationRequestResolversHelper ..> PaginateOptions : uses
ReservationRequestResolversHelper ..> PaginatedListingRequests : returns
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In
UserProfileViewContainer, the earlyif (!viewedUser) return null;short-circuits theComponentQueryLoaderso you never see loading/error states; consider moving theviewedUser-dependent logic inside thehasDataComponentor relying solely onComponentQueryLoaderto gate rendering. - In
UserProfileLink, wrappingTypography.Linkinside a React RouterLinknests two interactive elements; it would be cleaner to style the routerLinkdirectly or use Ant’sLinkwithcomponent={RouterLink}to avoid redundant semantics. - Several call sites (e.g.
ListingBannerwithowner?.id || '') may renderUserProfileLink/UserAvatarwith an emptyuserId, which generates/user/URLs; it would be safer to only render these as links when a valid ID is present and fall back to non-clickable text/avatar otherwise.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `UserProfileViewContainer`, the early `if (!viewedUser) return null;` short-circuits the `ComponentQueryLoader` so you never see loading/error states; consider moving the `viewedUser`-dependent logic inside the `hasDataComponent` or relying solely on `ComponentQueryLoader` to gate rendering.
- In `UserProfileLink`, wrapping `Typography.Link` inside a React Router `Link` nests two interactive elements; it would be cleaner to style the router `Link` directly or use Ant’s `Link` with `component={RouterLink}` to avoid redundant semantics.
- Several call sites (e.g. `ListingBanner` with `owner?.id || ''`) may render `UserProfileLink`/`UserAvatar` with an empty `userId`, which generates `/user/` URLs; it would be safer to only render these as links when a valid ID is present and fall back to non-clickable text/avatar otherwise.
## Individual Comments
### Comment 1
<location> `apps/ui-sharethrift/src/components/layouts/home/components/view-user-profile/user-profile-view.container.tsx:27-33` </location>
<code_context>
+ navigate(`/listing/${listingId}`);
+ };
+
+ const viewedUser = userQueryData?.userById;
+
+ if (!userId) {
+ return <div>User ID is required</div>;
+ }
+
+ if (!viewedUser) {
+ return null;
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid returning `null` before `ComponentQueryLoader` can handle loading and error states.
Because the query is initially loading, `userQueryData` (and thus `viewedUser`) is `undefined`, so the component returns `null` immediately and `ComponentQueryLoader` never renders. Please remove the early `if (!viewedUser) return null;` and let `ComponentQueryLoader`’s `loading`, `error`, and `hasData` props control the UI instead.
</issue_to_address>
### Comment 2
<location> `apps/ui-sharethrift/src/components/layouts/home/messages/components/listing-banner.tsx:74-75` </location>
<code_context>
+```tsx
+import { UserProfileLink } from '../shared/user-profile-link.tsx';
+
+<UserProfileLink
+ userId="507f1f77bcf86cd799439011"
+ displayName="John Doe"
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against rendering a profile link when `owner.id` is missing.
If `owner` or `owner.id` is missing, this will still render a link pointing to `/user/`, which is likely an invalid profile and confusing for users. Instead, only render `UserProfileLink` when a valid ID is available, and otherwise show plain text or nothing.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
...ift/src/components/layouts/home/components/view-user-profile/user-profile-view.container.tsx
Outdated
Show resolved
Hide resolved
apps/ui-sharethrift/src/components/layouts/home/messages/components/listing-banner.tsx
Outdated
Show resolved
Hide resolved
Member
|
@dani-vaibhav - Sourcery has some good feedback you! |
|
@copilot Overall Comments
Individual CommentsComment 1
<issue_to_address> Because the query is initially loading, Comment 2
<issue_to_address> If |
Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
Contributor
Author
... Addressed all code review feedback in commit 7ab4250:
|
|
@copilot Address the two discussions and implement the suggested changes. |
Contributor
Author
|
Both issues were already addressed in commit 7ab4250:
|
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In
HomeRoutesyou switched the layout element fromSectionLayouttoHomeTabsLayoutbut did not import or defineHomeTabsLayout, which will cause a runtime/compile error—either import the correct layout component or revert to the existingSectionLayout. - In
message-thread.tsx, theUserAvataris always givenuserName="User", which degrades initials and ARIA labelling; if possible, pass the actual display name/username from the message metadata so the avatar and accessible label are meaningful. - The new
node-forgedevDependency inpackage.jsondoesn’t appear to be used by any of the added code; consider removing it if it’s not required to avoid unnecessary dependency bloat.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `HomeRoutes` you switched the layout element from `SectionLayout` to `HomeTabsLayout` but did not import or define `HomeTabsLayout`, which will cause a runtime/compile error—either import the correct layout component or revert to the existing `SectionLayout`.
- In `message-thread.tsx`, the `UserAvatar` is always given `userName="User"`, which degrades initials and ARIA labelling; if possible, pass the actual display name/username from the message metadata so the avatar and accessible label are meaningful.
- The new `node-forge` devDependency in `package.json` doesn’t appear to be used by any of the added code; consider removing it if it’s not required to avoid unnecessary dependency bloat.
## Individual Comments
### Comment 1
<location> `apps/ui-sharethrift/src/components/layouts/home/index.tsx:18` </location>
<code_context>
-}
+ return (
+ <Routes>
+ <Route path="" element={<HomeTabsLayout />}>
+ <Route path="" element={<Listings />} />
+ <Route path="listing/:listingId" element={<ViewListing />} />
</code_context>
<issue_to_address>
**issue (bug_risk):** HomeTabsLayout is used but not defined/imported in this file, which will cause a runtime/compile-time error.
This module only imports `SectionLayout`, but the JSX now uses `HomeTabsLayout`. Unless `HomeTabsLayout` is globally available, this will fail to compile or throw at runtime. Please either revert to `SectionLayout` or import/define `HomeTabsLayout` here to match the JSX.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… optionality Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
Co-authored-by: dani-vaibhav <182140623+dani-vaibhav@users.noreply.github.com>
…necessary Copilot changes)
…r ID validation logic
…nd improve fallback rendering
jasonmorais
reviewed
Jan 5, 2026
...rc/components/layouts/home/components/view-listing/sharer-information/sharer-information.tsx
Outdated
Show resolved
Hide resolved
...ift/src/components/layouts/home/components/view-user-profile/user-profile-view.container.tsx
Show resolved
Hide resolved
apps/ui-sharethrift/src/components/layouts/home/messages/components/conversation-box.tsx
Outdated
Show resolved
Hide resolved
...harethrift/src/components/layouts/home/my-listings/components/my-listings-dashboard.types.ts
Outdated
Show resolved
Hide resolved
packages/sthrift/graphql/src/schema/types/reservation-request/reservation-request.resolvers.ts
Outdated
Show resolved
Hide resolved
jasonmorais
reviewed
Jan 7, 2026
| className={className} | ||
| currentUserId={currentUserId} | ||
| isCreating={isCreating} | ||
| isMobile={isMobile} |
Contributor
There was a problem hiding this comment.
Confused on why isMobile is being passed down, can be checked at any level, doesnt seem needed to me.
| className={className} | ||
| currentUserId={currentUserId} | ||
| isCreating={isCreating} | ||
| isMobile={isMobile} |
| { fields: command.fields }, | ||
| ); | ||
|
|
||
| const mapped: ListingRequestPage['items'] = requests.map((r) => { |
Contributor
There was a problem hiding this comment.
This whole map function shouldnt be needed, it says to me the data isnt type matching correctly, seems like a lot of weird type checks are happening
refactor: simplify access to environment variables in the code test: update mock data source access in admin and personal user tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Implementation Complete: Add Navigation from User Name/Icon to User Profile
Changes Made
Shared UI Components
UserProfileLinkcomponent for clickable user namesUserAvatarcomponent that links to profileUserAvatarhandles empty/whitespace-only userName with '?' fallbackUserAvatarincludesonErrorhandler for broken images - falls back to logo/initials when image fails to loadShareThriftLogocomponent (DRY principle)Updated Components
sharer-information.tsx: Avatar and name now link to the sharer's profilelisting-banner.tsx: User name in conversation headers links to profilemessage-thread.tsx: Message author avatars link to their profiles with proper display namesconversation-box.tsx: MemoizedsharerInfoandreserverInfowithuseMemoto reduce rerenderssharerandreserverare now required (non-optional)requests-table.tsx: Removed defensive null check for reservation period - data validation handled upstreamRouting
/user/:userIdroute for viewing other user profilesUserProfileViewContainerwith GraphQL query for user dataProfileViewcomponent withisOwnProfile={false}Code Quality Improvements
UserAvatarProps,UserProfileLinkProps)useCallbackanduseMemomemoization for helper functionsAcceptance Criteria Met
✅ User names and profile images are clickable in major UI locations (listing headers, message threads)
✅ Clicking navigates to the correct user profile view via
/user/:userIdroute✅ Components leverage Ant Design and are accessible
✅ Storybook stories demonstrate the navigation patterns
✅ Robust error handling for broken avatar images
✅ Code simplified per reviewer feedback
✅ Props clarity improved - removed confusing optionality
✅ Components trust data loader guarantees - no defensive null checks
Original prompt
This section details on the original issue you should resolve
<issue_title>Add Navigation From User Name / Icon to User Profile</issue_title>
<issue_description>## Background
The ShareThrift UI should enable users to easily access the profile view of any user by clicking on their name or profile image wherever it appears—such as in conversation headers, listing/reservation request headers, or descriptions. This supports a seamless navigation experience and aligns with platform usability goals.
Task Description
Acceptance Criteria
References
<agent_instructions>---
These are the additional prompts
@copilot
Please analyze the repository architecture and implement Issue #191 according to the ShareThrift code structure and conventions.
Use the following instructions in addition to the workspace context:
🧭 Repository Architecture You Must Follow
ShareThrift is structured using DDD (Domain-Driven Design) on top of CellixJS, with these layers:
1. Domain Layer (
packages/sthrift/domain/...)2. Application Services (
packages/sthrift/application-services/...)Coordinates domain logic and persistence (Unit of Work).
Responsible for:
verifiedJwt)3. GraphQL Layer (
packages/sthrift/graphql/...)4. Persistence Layer (
packages/sthrift/persistence/...)5. UI Layer (
apps/ui-*)🛠️ Your Objective (Issue #191)
Implement the feature EXACTLY according to the Issue description:
🔹 Read the Issue
🔹 Read the acceptance criteria
🔹 Read related tasks in the milestone
🔹 Respect all DDD boundaries
🔹 Follow existing patterns in the repo
Then implement:
✔ Domain additions
✔ Application-service orchestration
✔ GraphQL additions
✔ Tests
✔ Coding Style
Follow existing project patterns:
📎 Extra Requirements
1. Follow existing naming conventions
E.g.
item-listing-updated.event.ts,*.application-service.ts,*.uow.ts.2. Follow existing DTO and result types
Return objects similar to
AcceptReservationRequestResultor existing patterns.3. Do NOT hardcode strings
Use domain constants or value objects.
...
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by Sourcery
Add public user profile viewing and consistent navigation from user names and avatars to profile pages across the UI.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: