feat: add Recently Viewed Projects feature with tracking and display functionality #226
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds per-user recently viewed project tracking (capped at 10) on the backend, exposes enriched recently_viewed data in project list/detail APIs, extends the User model, and surfaces up to 5 recently viewed items in the dashboard via a new frontend component. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser as Client
participant Server
participant DB as Database
User->>Browser: Click project (open detail)
Browser->>Server: GET /projects/:id
Server->>DB: Fetch project
Server-->>Browser: Return project data
par Async track
Server->>Server: schedule track_project_view(user, project_id)
Server->>DB: Read user.recently_viewed_projects
Server->>DB: Remove existing entry for project_id
Server->>DB: Prepend new entry with timestamp
Server->>DB: Trim list to 10 and save user
end
User->>Browser: Load dashboard overview
Browser->>Server: GET /projects
Server->>DB: Fetch projects and user.recently_viewed_projects
Server->>Server: Enrich recently_viewed with project details, is_owner, ISO timestamps
Server-->>Browser: Return projects + recently_viewed
Browser->>Browser: Render RecentlyViewedProjects (top 5, "+N more")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
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 (1)
client/src/pages/Dashboard/useOverviewData.tsx (1)
78-113:refetchProjectsdoesn't updaterecentlyViewedstate.The initial fetch updates both
projectsandrecentlyViewed(lines 63, 66-67), butrefetchProjectsonly updatesprojects. This inconsistency means the recently viewed list becomes stale after a refetch, potentially showing outdated information.Apply this diff to maintain consistency:
const refetchProjects = useCallback(async () => { setIsLoadingProjects(true) try { const response = await api.get(`/v1/projects`) if (response.status !== 200) { throw new Error('Network response was not ok') } setProjects( response.data.project.map( (project: { id: string title: string description: string papers: unknown[] results: { id: string; finished: boolean; paper_id: string }[] }) => { const uniquePaperIds = Array.from( new Set(project.results.map((result) => result.paper_id)), ) return { id: project.id, name: project.title, description: project.description, paper_count: uniquePaperIds.length, results: project.results, } as Projects }, ), ) + + if (response.data.recently_viewed) { + setRecentlyViewed(response.data.recently_viewed) + } } catch (error) { console.error(error) } finally { setIsLoadingProjects(false) } }, [])
🧹 Nitpick comments (5)
server/database/models/users.py (1)
22-24: Consider using a typed model instead ofList[dict]for better type safety.The
recently_viewed_projectsfield uses an untypedList[dict], which loses compile-time type checking and IDE support. Consider defining a typed model:+from pydantic import BaseModel + +class RecentlyViewedProject(BaseModel): + project_id: str + viewed_at: datetime + class User(Document): ... - recently_viewed_projects: List[dict] = Field(default_factory=list) + recently_viewed_projects: List[RecentlyViewedProject] = Field(default_factory=list)This would provide validation, better IDE autocompletion, and catch schema mismatches early.
server/controllers/project.py (1)
196-200: Replaceprint()with proper logging.Using
print()for logging is not production-ready. The codebase already importsloggingin the routes file.+import logging + +logger = logging.getLogger(__name__) + async def track_project_view(user: User, project_id: str): ... - print(f"Tracked project view for user {user.email} and project {project_id}") + logger.info(f"Tracked project view for user {user.email} and project {project_id}") except Exception as e: # Log error but don't fail the request - print(f"Error tracking project view: {e}") + logger.warning(f"Error tracking project view: {e}", exc_info=True)Note: The broad
Exceptioncatch (Ruff BLE001) is acceptable here since the intent is to never fail the request, butlogger.warningwithexc_info=Truewill preserve the stack trace for debugging.client/src/pages/Dashboard/Overview.tsx (1)
43-44: Consider showing a loading state for recently viewed projects.While
RecentlyViewedProjectsreturnsnullfor empty arrays, users might see a brief layout shift when data loads. IfisLoadingProjectsalso applies to recently viewed data, consider conditionally rendering a skeleton.This is a minor UX consideration and can be deferred if the loading is fast enough to not cause noticeable shifts.
server/routes/v1/projects.py (1)
125-132: Add defensive handling forviewed_atserialization.The code assumes
view["viewed_at"]is always a datetime object. If the data is malformed (e.g., already a string from older data or corruption), calling.isoformat()will fail.+ viewed_at = view["viewed_at"] + if isinstance(viewed_at, datetime): + viewed_at_str = viewed_at.isoformat() + else: + viewed_at_str = str(viewed_at) + recently_viewed_enriched.append( { "project_id": project_id, - "viewed_at": view["viewed_at"].isoformat(), + "viewed_at": viewed_at_str, "project": project_data, "exists": project_data is not None, } )client/src/components/View/DataGrid/RecentlyViewedProjects.tsx (1)
69-69: Unsafe assumption that email contains '@' symbol.The
split('@')[0]will not throw but returns the full string if '@' is missing. While unlikely, malformed emails from the backend (or test data) could produce unexpected UI output.Apply this diff for safer extraction:
- view.project.owner_email.split('@')[0] + view.project.owner_email.split('@')[0] || view.project.owner_emailOr use a helper function:
const getEmailUsername = (email: string) => { const parts = email.split('@') return parts[0] || email }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
client/src/components/View/DataGrid/RecentlyViewedProjects.tsx(1 hunks)client/src/pages/Dashboard/Overview.tsx(2 hunks)client/src/pages/Dashboard/useOverviewData.tsx(4 hunks)server/controllers/project.py(1 hunks)server/database/models/projects.py(1 hunks)server/database/models/users.py(2 hunks)server/routes/v1/projects.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
client/src/pages/Dashboard/Overview.tsx (1)
client/src/components/View/DataGrid/RecentlyViewedProjects.tsx (1)
RecentlyViewedProjects(18-103)
client/src/pages/Dashboard/useOverviewData.tsx (2)
server/routes/v1/projects.py (1)
project(37-136)client/src/components/View/DataGrid/ProjectsTable.tsx (1)
Projects(39-49)
client/src/components/View/DataGrid/RecentlyViewedProjects.tsx (1)
client/src/pages/Dashboard/useOverviewData.tsx (1)
RecentlyViewedProject(7-17)
server/controllers/project.py (1)
server/database/models/users.py (1)
User(11-54)
🪛 Ruff (0.14.8)
server/controllers/project.py
198-198: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (6)
server/database/models/projects.py (1)
1-3: LGTM!The docstring correction from "Papers model." to "Projects model." accurately reflects the file's purpose.
server/database/models/users.py (1)
40-54: Verify ifrecently_viewed_projectsshould be included into_dict().The
to_dict()method does not include the newrecently_viewed_projectsfield. If this field needs to be serialized elsewhere (e.g., for API responses or debugging), consider adding it. If the omission is intentional for privacy reasons, this is fine as-is.client/src/pages/Dashboard/Overview.tsx (1)
6-10: LGTM!Clean integration of the
RecentlyViewedProjectscomponent. The component handles empty state gracefully by returningnull, making additional conditional rendering unnecessary.server/routes/v1/projects.py (2)
156-159: Good use of non-blocking task for view tracking.Using
request.app.add_task()to schedule the tracking asynchronously is the right approach—it prevents the view tracking from delaying the response to the user.
109-118: Verify if exposingowner_emailis intended.The
owner_emailfield is included for all recently viewed projects, which exposes other users' email addresses to the requesting user. Ensure this aligns with privacy requirements. If emails should be partially obscured, consider masking (e.g.,j***@example.com) or using display names instead.client/src/components/View/DataGrid/RecentlyViewedProjects.tsx (1)
47-51: Navigation pattern useswindow.openinstead of React Router.Both the row click and the external link button use
window.opento open projects in a new tab. This bypasses React Router and can create a jarring user experience. Verify this is intentional for cross-context navigation.If in-app navigation is preferred, consider using React Router's
navigate:import { useNavigate } from 'react-router-dom' // Inside component: const navigate = useNavigate() // For row click: onClick={() => { if (view.exists) { navigate(`/project/${view.project_id}`) } }} // For external link button, you can still use target="_blank" with Link: <Link to={`/project/${view.project_id}`} target="_blank"> <Button variant='ghost' size='sm' className='h-6 w-6 p-0'> <ExternalLink className='h-3.5 w-3.5' /> </Button> </Link>If opening in new tabs is intentional (e.g., for comparing projects), consider adding
rel="noopener noreferrer"for security.Also applies to: 84-87
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
server/routes/v1/projects.py (1)
96-111: Hardenrecently_viewed_projectsagainst malformedproject_idvalues.Directly doing
PydanticObjectId(view["project_id"])will raise if any storedproject_idis malformed (data corruption, manual edits, legacy data), causing the entire/projectsGET to fail instead of just skipping bad entries. Please catch and skip/log invalid ids so the endpoint remains robust. This was already flagged in a previous review and still applies.Suggested adjustment:
- recently_viewed_project_ids = [ - PydanticObjectId(view["project_id"]) - for view in user.recently_viewed_projects - ] + recently_viewed_project_ids: list[PydanticObjectId] = [] + for view in user.recently_viewed_projects: + project_id = view.get("project_id") + if not project_id: + continue + try: + recently_viewed_project_ids.append(PydanticObjectId(project_id)) + except Exception: + logger.warning( + "Skipping invalid project_id in recently_viewed for user %s: %r", + user.id, + project_id, + )If you prefer to hide such entries entirely, you can also
continuelater whenproject_data is Nonebefore appending torecently_viewed_enriched.
🧹 Nitpick comments (2)
server/routes/v1/projects.py (2)
10-16: Non-blockingtrack_project_viewusage is good; consider guarding background failures.Using
request.app.add_task(track_project_view(user, project_id))keeps the GET fast, which is appropriate. To avoid silent loss of tracking iftrack_project_viewthrows (DB/network issues, etc.), consider wrapping its body in a small wrapper coroutine withtry/except+ logging, or handling errors insidetrack_project_viewitself so background exceptions are always captured and logged explicitly.Also applies to: 160-162
123-135: Makeview["viewed_at"].isoformat()resilient to legacy/string data.This assumes
view["viewed_at"]is always adatetime. If any olderrecently_viewed_projectsentries storedviewed_atas a string (e.g., ISO string) orNone, this will raise and break the endpoint. If there’s any chance of mixed data, consider normalizing first, e.g.:viewed_at = view.get("viewed_at") if hasattr(viewed_at, "isoformat"): viewed_at_str = viewed_at.isoformat() elif isinstance(viewed_at, str): viewed_at_str = viewed_at # or parse/normalize if needed else: viewed_at_str = Noneand then use
viewed_at_strin the response.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
server/routes/v1/projects.py(6 hunks)
🔇 Additional comments (1)
server/routes/v1/projects.py (1)
42-44: ConsistentUsertyping forrequest.ctx.userlooks solid.Using
user: User = request.ctx.userin all handlers improves type checking and readability, and the downstream uses (user.id,user.email) are consistent with that type. No issues here.Also applies to: 152-153, 199-200, 314-315
…d improve project owner display
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.