[WIKI-556] fix : invert tracking logic #7668
Conversation
WalkthroughThe backend now defaults track_visit to true in PageViewSet.retrieve. The web store’s fetchPageDetails signature changes to accept an optional options object and defaults trackVisit to true. The page component stops passing the options object, relying on the new default behavior. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant P as PageDetailsPage (SWR)
participant S as ProjectPageStore
participant API as Page API
participant Q as recent_visited_task Queue
U->>P: Navigate to Page Details
P->>S: fetchPageDetails(wsSlug, projectId, pageId)
Note right of S: Derive trackVisit = options?.trackVisit ?? true
S->>API: GET /pages/{pageId}?track_visit=true
alt 200 OK
API-->>S: Page data
API-->>Q: Enqueue recent_visited_task
S-->>P: Page data
P-->>U: Render page
else Error
API-->>S: Error
S-->>P: Propagate error
P-->>U: Show error state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
VipinDevelops
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/plane/app/views/page/base.py (1)
198-227: Fix None-check ordering to prevent AttributeError when page doesn’t exist.
pageis dereferenced in the guest-permission gate before verifying it isn’tNone. Ifpkis invalid (or filtered out byget_queryset()), this will raise.def retrieve(self, request, slug, project_id, pk=None): page = self.get_queryset().filter(pk=pk).first() project = Project.objects.get(pk=project_id) track_visit = request.query_params.get("track_visit", "true").strip().lower() == "true" """ if the role is guest and guest_view_all_features is false and owned by is not the requesting user then dont show the page """ - if ( + if page is None: + return Response( + {"error": "Page not found"}, status=status.HTTP_404_NOT_FOUND + ) + + if ( ProjectMember.objects.filter( workspace__slug=slug, project_id=project_id, member=request.user, role=5, is_active=True, ).exists() and not project.guest_view_all_features - and not page.owned_by == request.user + and page.owned_by != request.user ): return Response( {"error": "You are not allowed to view this page"}, status=status.HTTP_400_BAD_REQUEST, ) - if page is None: - return Response( - {"error": "Page not found"}, status=status.HTTP_404_NOT_FOUND - ) - else: + else: issue_ids = PageLog.objects.filter( page_id=pk, entity_name="issue" ).values_list("entity_identifier", flat=True) data = PageDetailSerializer(page).data data["issue_ids"] = issue_ids if track_visit: recent_visited_task.delay( slug=slug, entity_name="page", entity_identifier=pk, user_id=request.user.id, project_id=project_id, ) return Response(data, status=status.HTTP_200_OK)
🧹 Nitpick comments (2)
apps/api/plane/app/views/page/base.py (1)
201-201: Default-on tracking implemented; trim input to avoid false negatives.Safer to tolerate incidental whitespace in the query param.
- track_visit = request.query_params.get("track_visit", "true").lower() == "true" + track_visit = request.query_params.get("track_visit", "true").strip().lower() == "true"apps/web/core/store/pages/project-page.store.ts (1)
247-260: Prefer explicit parameters over variadic tuple for readability and DX.Using
(...args: Parameters<IProjectPageStore["fetchPageDetails"]>)obscures the method signature at call sites and hampers inline docs. Keep the explicit signature and defaulttrackVisitvia destructuring.- fetchPageDetails = async (...args: Parameters<IProjectPageStore["fetchPageDetails"]>) => { - const [workspaceSlug, projectId, pageId, options] = args; - const { trackVisit } = options || {}; + fetchPageDetails = async ( + workspaceSlug: string, + projectId: string, + pageId: string, + options?: { trackVisit?: boolean } + ) => { + const { trackVisit = true } = options || {}; @@ - const page = await this.service.fetchById(workspaceSlug, projectId, pageId, trackVisit ?? true); + const page = await this.service.fetchById(workspaceSlug, projectId, pageId, trackVisit);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/api/plane/app/views/page/base.py(1 hunks)apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx(1 hunks)apps/web/core/store/pages/project-page.store.ts(3 hunks)
⏰ 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: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/web/core/store/pages/project-page.store.ts (2)
52-57: API shape change to optional options is sensible and backward compatible.Calls that previously passed
{ trackVisit: ... }continue to work; omitting the arg now defaults to tracking. Looks good.
247-260: No boolean 4th-arg or missing trackVisit calls found—ready to merge.apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx (1)
59-63: Fetcher call simplified to rely on default tracking.Matches the store/API defaults; no functional regressions expected.
Description
this PR invert the tracking logic we have for pages
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Refactor