Fix critical functionality: pagination, search, real-time chat, guest guard, activity feed #320
Conversation
motirebuma
changed the title
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Addresses several P0 regressions from the Next.js migration by wiring up missing pagination/search UX, restoring real-time chat behavior, guarding guest interactions, expanding admin controls, and replacing the profile Activity placeholder with a functional paginated feed (per #319).
Changes:
- Integrates
PaginatedPostsListacross Explore tabs with configurable query + response key support. - Adds
@username-driven user search dropdown + “selected user” filter chip in Explore. - Switches post discussion chat updates from polling to GraphQL subscriptions and implements Activity tab filters + paginated feed; adds guest guard for notification interactions; enables admin delete for chat messages.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| quotevote-frontend/src/components/SearchContainer/UsernameResults.tsx | Adds selectable-mode behavior for username dropdown results (supports click-to-select vs navigation). |
| quotevote-frontend/src/components/Profile/ProfileView.tsx | Replaces Activity placeholder with filters + PaginatedActivityList. |
| quotevote-frontend/src/components/Post/PaginatedPostsList.tsx | Allows passing a custom query and response dataKey for reuse across tabs. |
| quotevote-frontend/src/components/Notifications/NotificationLists.tsx | Adds guest guard to notification click handling. |
| quotevote-frontend/src/components/Chat/MessageItem.tsx | Allows admins to delete chat messages (in addition to message owner). |
| quotevote-frontend/src/app/dashboard/post/[group]/[title]/[postId]/page.tsx | Replaces chat polling with a NEW_MESSAGE_SUBSCRIPTION subscription-triggered refresh. |
| quotevote-frontend/src/app/dashboard/explore/ExploreContent.tsx | Implements URL-synced pagination usage, username search dropdown, and selected-user filtering UX. |
| quotevote-frontend/src/tests/components/Profile/ProfileView.test.tsx | Updates Activity tab test expectation to reflect new UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| // Don't sync username searches or when filtering by user | ||
| if (isUsernameSearch || selectedUser) return | ||
|
|
There was a problem hiding this comment.
When a user filter is active (selectedUser) or the input is in @username mode, the URL-sync effect bails out early. That means any existing q query param is left intact, but the UI (chip) implies the results are filtered only by user. This can unintentionally combine an old q search with the selected user filter. Consider explicitly clearing q from the URL when entering @ mode and/or inside handleUserSelect, so PaginatedPostsList doesn’t receive a stale searchKey while userId is set.
| // Username search dropdown — triggered by @prefix or general search | ||
| const searchQueryForUsers = isUsernameSearch ? usernameQuery : debouncedQuery | ||
| const { | ||
| loading: usersLoading, | ||
| data: usersData, | ||
| error: usersError, | ||
| } = useQuery<{ searchUser: UsernameSearchUser[] }>(SEARCH_USERNAMES, { | ||
| variables: { query: debouncedQuery }, | ||
| skip: !debouncedQuery, | ||
| variables: { query: searchQueryForUsers }, | ||
| skip: !searchQueryForUsers, | ||
| }) |
There was a problem hiding this comment.
searchQueryForUsers uses the non-debounced usernameQuery when the input starts with @, so username lookups will fire on every keystroke (while normal search uses debouncedQuery). If the intent is to debounce username search as well, consider debouncing usernameQuery (or just using debouncedQuery and stripping the leading @), to avoid unnecessary network requests.
| <Link | ||
| key={user._id} | ||
| href={`/${user.username}`} | ||
| href={onUserSelect ? '#' : `/dashboard/profile/${user.username}`} | ||
| onClick={handleClick} |
There was a problem hiding this comment.
When onUserSelect is provided, each result is rendered as a Next.js Link with href="#" and navigation is prevented. This is semantically a button/action rather than a link, and href="#" can still be problematic for assistive tech and for users opening the item in a new tab/window. Consider conditionally rendering a <button> (or an element with role="button") when onUserSelect is set, and only render a Link when actual navigation is intended.
| if (pagination.currentPage > 1 && (!data || !(data as unknown as Record<string, unknown>)[dataKey])) { | ||
| refetch() | ||
| } | ||
| }, [pagination.currentPage, data, refetch]) |
There was a problem hiding this comment.
The useEffect that checks dataKey uses dataKey from props but doesn’t include it in the dependency array. If dataKey changes (e.g., the component is reused with a different query/key), the effect can run with a stale dataKey value. Add dataKey to the dependency list (and ensure any other referenced props are included) to keep the behavior correct and satisfy hook linting.
| }, [pagination.currentPage, data, refetch]) | |
| }, [pagination.currentPage, data, refetch, dataKey]) |
| // Fetch data | ||
| const { loading, error, data, refetch } = useQuery<PaginatedPostsListData>(GET_TOP_POSTS, { | ||
| const { loading, error, data, refetch } = useQuery<PaginatedPostsListData>(query || GET_TOP_POSTS, { | ||
| variables, |
There was a problem hiding this comment.
useQuery is typed as PaginatedPostsListData even when a different query/dataKey is passed (e.g., featuredPosts). That defeats type-safety and forces broad casts later (Record<string, unknown>), making it easier to ship a mismatched dataKey or response shape without TS catching it. Consider making PaginatedPostsList generic over the query result type, or typing data as Record<string, { entities?: Post[]; pagination?: ... }> keyed by dataKey to avoid unsafe casts.
| it('shows activity filters when Activity tab is clicked', async () => { | ||
| const user = userEvent.setup(); | ||
| await act(async () => { | ||
| render(<ProfileView profileUser={mockProfileUser} />); | ||
| }); | ||
| const activityTab = screen.getByRole('tab', { name: 'Activity' }); | ||
| await user.click(activityTab); | ||
| await waitFor(() => { | ||
| expect(screen.getByText('Activity feed coming soon')).toBeInTheDocument(); | ||
| expect(screen.getByText('All')).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
This test now renders the real PaginatedActivityList when the Activity tab is selected, which triggers the GET_USER_ACTIVITY query. Since the test doesn’t provide Apollo mocks, this can produce MockLink “no more mocked responses” errors/noise (and can become flaky if the component behavior changes). Consider mocking PaginatedActivityList in this test (similar to Activity.test.tsx) or passing an explicit mocks array to render(...) for the activity query.
motirebuma
changed the title
PR Title:
Summary
PaginatedPostsListinto all Explore tabs with URL-synced pagination and dynamic GraphQL offset/limitUsernameResultsdropdown with@prefix detection, user selection filtering, and dismissable badgepollInterval: 3000) withgraphql-wsWebSocket subscriptions (NEW_MESSAGE_SUBSCRIPTION)useGuestGuard()to notification click/delete handlers, showing auth modal for unauthenticated usersPaginatedActivityListincluding POSTED/VOTED/COMMENTED/QUOTED filtersCloses #319