✨ Implement DocsDetailPage component and related functionality

[MH4GF]

Issue

• resolve:

Why is this change needed?

• Created DocsDetailPage component to display document content based on project ID, branch or commit, and file path.
• Implemented getProjectRepository function to fetch repository information and integrated it with the document fetching logic.
  • Added tests for getProjectRepository to ensure correct functionality and data structure.

What would you like reviewers to focus on?

Testing Verification

What was done

🤖 Generated by PR Agent at 1c85bb9

• Added DocsDetailPage component for displaying document content.
• Implemented getDocumentContent and getProjectRepository functions.
• Added tests for getProjectRepository to ensure functionality.
• Created CSS module for styling document detail page.

Detailed Changes

	Relevant files
Enhancement	getDocumentContent.ts Add `getDocumentContent` function for document fetching    frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/getDocumentContent.ts Added getDocumentContent function to fetch document content. Integrated repository and file path handling. Included error handling for missing repository or content. +38/-0    getProjectRepository.ts Implement `getProjectRepository` for repository details    frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/getProjectRepository.ts Implemented getProjectRepository to fetch repository details. Integrated database queries for project-repository mapping. Added error handling for missing repository data. +51/-0    DocsDetailPage.module.css Add CSS module for `DocsDetailPage` styling                            frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/DocsDetailPage.module.css Added CSS styles for document detail page. Included responsive and monospace font styling. Styled container and content elements. +18/-0    DocsDetailPage.tsx Create `DocsDetailPage` component for document rendering  frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/DocsDetailPage.tsx Created DocsDetailPage component for rendering document content. Integrated getDocumentContent for fetching content. Added error handling for missing content. +36/-0    page.tsx Add page component for document routing                                    frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/page.tsx Added page component for routing and parameter validation. Integrated DocsDetailPage for document display. Used valibot for schema validation of parameters. +25/-0
Tests	getProjectRepository.test.ts Add tests for `getProjectRepository` function                        frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/getProjectRepository.test.ts Added tests for getProjectRepository function. Verified repository structure and null scenarios. Included database setup for test cases. +58/-0

Additional Notes

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
liam-app	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 28, 2025 0:46am
liam-erd-sample	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 28, 2025 0:46am

4 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
liam-docs	⬜️ Ignored (Inspect)	Visit Preview		Mar 28, 2025 0:46am
test-liam-app	⬜️ Ignored (Inspect)			Mar 28, 2025 0:46am
test-liam-docs	⬜️ Ignored (Inspect)			Mar 28, 2025 0:46am
test-liam-erd-sample	⬜️ Ignored (Inspect)			Mar 28, 2025 0:46am

[changeset-bot]

⚠️ No Changeset found

Latest commit: 277f50d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[qodo-free-for-open-source-projects]

PR Reviewer Guide 🔍

(Review updated until commit 1c85bb9)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The function returns null for any error but doesn't distinguish between different error types (repository not found, file not found, permission issues). Consider returning more specific error information to improve user experience. } catch (error) { console.error('Error fetching document content:', error) return null } Error Handling The component throws notFound() for all errors, which might not be appropriate for temporary issues like network problems. Consider implementing more granular error handling with appropriate user feedback. } catch (error) { console.error('Error fetching document:', error) throw notFound() }

[qodo-free-for-open-source-projects]

PR Code Suggestions ✨

Latest suggestions up to 1c85bb9
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix error handling pattern Instead of throwing notFound(), which is meant to be used as a return value, you should return it. Throwing it can lead to unexpected behavior in error handling. frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/DocsDetailPage.tsx [16-35] try { const content = await getDocumentContent({ projectId, branchOrCommit, docFilePath, }) if (!content) { return notFound() } return ( <div className={styles.container}> <pre className={styles.content}>{content}</pre> </div> ) } catch (error) { console.error('Error fetching document:', error) - throw notFound() + return notFound() } Apply this suggestion Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential issue with error handling. Using throw notFound() is incorrect as Next.js's notFound() function is designed to be returned, not thrown. This change would prevent unexpected behavior in the error handling flow.	Medium
General	Add test data cleanup The test is creating actual database records without cleaning them up afterward. This can lead to test pollution and database growth over time. Consider adding cleanup code or using database transactions that can be rolled back. frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/getProjectRepository.test.ts [9-29] // Create test repository const { data: repository } = await supabase .from('Repository') .insert({ name: 'test-repo', owner: 'test-owner', installationId: 12345, updatedAt: new Date().toISOString(), }) .select() .single() // Create test project const { data: project } = await supabase .from('Project') .insert({ name: 'Test Project', updatedAt: new Date().toISOString(), }) .select() .single() + +// Ensure cleanup after test +afterEach(async () => { + if (project?.id) { + await supabase.from('ProjectRepositoryMapping').delete().eq('projectId', project.id); + await supabase.from('Project').delete().eq('id', project.id); + } + if (repository?.id) { + await supabase.from('Repository').delete().eq('id', repository.id); + } +}); Apply this suggestion Suggestion importance[1-10]: 7 __ Why: The suggestion addresses an important testing best practice by adding cleanup code to remove test data after tests run. This prevents test pollution and database growth over time, which could lead to unreliable tests and unnecessary database bloat.	Medium
Learned best practice	Remove unnecessary await when accessing route parameters in Next.js page components The code is incorrectly treating params as a Promise by using await params, but in Next.js route parameters are directly available as an object, not a Promise. Remove the await keyword when parsing the params to follow the correct Next.js pattern for handling route parameters. frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/page.tsx [13-16] export default async function Page({ params }: PageProps) { - const parsedParams = v.safeParse(paramsSchema, await params) + const parsedParams = v.safeParse(paramsSchema, params) if (!parsedParams.success) throw notFound() const { projectId, branchOrCommit, docFilePath } = parsedParams.output } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6	Low
Update

---

Previous suggestions

Suggestions up to commit 1c85bb9

Category	Suggestion	Impact
Possible issue	Fix notFound usage Instead of throwing notFound(), which is meant to be used directly as a return value, you should return it. The current implementation will cause a runtime error. frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/DocsDetailPage.tsx [16-35] try { const content = await getDocumentContent({ projectId, branchOrCommit, docFilePath, }) if (!content) { return notFound() } return ( <div className={styles.container}> <pre className={styles.content}>{content}</pre> </div> ) } catch (error) { console.error('Error fetching document:', error) - throw notFound() + return notFound() } Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a critical issue where notFound() is being thrown instead of returned in the catch block. This would cause a runtime error as Next.js's notFound() is designed to be returned, not thrown. Fixing this prevents a potential application crash.	High
General	Add test cleanup The test creates real database records but doesn't clean them up after the test. This can lead to test pollution and database growth over time. Consider mocking the database calls or implementing proper cleanup in an afterEach/afterAll block. frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/getProjectRepository.test.ts [9-29] -// Create test repository -const { data: repository } = await supabase - .from('Repository') - .insert({ - name: 'test-repo', - owner: 'test-owner', - installationId: 12345, - updatedAt: new Date().toISOString(), - }) - .select() - .single() +// Mock database responses instead of creating real records +vi.mock('@/libs/db/server', () => ({ + createClient: vi.fn().mockResolvedValue({ + from: vi.fn().mockReturnValue({ + select: vi.fn().mockReturnThis(), + insert: vi.fn().mockReturnThis(), + eq: vi.fn().mockReturnThis(), + single: vi.fn().mockImplementation((table) => { + if (table === 'Repository') { + return Promise.resolve({ + data: { + id: 1, + name: 'test-repo', + owner: 'test-owner', + installationId: 12345, + }, + error: null, + }); + } else if (table === 'Project') { + return Promise.resolve({ + data: { + id: 1, + name: 'Test Project', + ProjectRepositoryMapping: [{ + Repository: { + name: 'test-repo', + owner: 'test-owner', + installationId: 12345, + } + }] + }, + error: null, + }); + } + }), + }), + }), +})); -// Create test project -const { data: project } = await supabase - .from('Project') - .insert({ - name: 'Test Project', - updatedAt: new Date().toISOString(), - }) - .select() - .single() - Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue with test pollution due to creating real database records without cleanup. The proposed mocking approach would prevent database growth and make tests more isolated and reliable, which is an important testing best practice.	Medium
Learned best practice	Remove unnecessary awaiting of route parameters in Next.js page components In Next.js route components, the params object is already available as a plain object and doesn't need to be awaited. Using await params is incorrect and could lead to unexpected behavior. Remove the await keyword when parsing the params object. frontend/apps/app/app/(app)/app/projects/[projectId]/ref/[branchOrCommit]/docs/[...docFilePath]/page.tsx [13-14] -const parsedParams = v.safeParse(paramsSchema, await params) +const parsedParams = v.safeParse(paramsSchema, params) if (!parsedParams.success) throw notFound() Suggestion importance[1-10]: 6	Low

[supabase]

Updates to Preview Branch (show-github-doc-file-path) ↗︎

Deployments	Status	Updated
Database	✅	Fri, 28 Mar 2025 00:38:45 UTC
Services	✅	Fri, 28 Mar 2025 00:38:45 UTC
APIs	✅	Fri, 28 Mar 2025 00:38:45 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks	Status	Updated
Configurations	✅	Fri, 28 Mar 2025 00:38:45 UTC
Migrations	⚠️	Fri, 28 Mar 2025 00:38:48 UTC
Seeding	✅	Fri, 28 Mar 2025 00:38:48 UTC
Edge Functions	✅	Fri, 28 Mar 2025 00:38:48 UTC

---

⚠️ Warning — Applied out-of-order migrations: [frontend/packages/db/supabase/migrations/20250327034651_add_branch_name_to_overall_review.sql]

---

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

[hoshinotsuyoshi]

nits

It looks like running the test command ($ pnpm --filter @liam-hq/app test) requires resetting the database beforehand ($ pnpm --filter @liam-hq/db supabase:reset).
This is because the second run will fail due to a unique constraint violation when inserting into the Repository table.
But this is ok for CI. 👍

[MH4GF]

@hoshinotsuyoshi You're right, the second run will fail. I'd like to do something about it!

[hoshinotsuyoshi]

confirmed in preview https://liam-app-git-show-github-doc-file-path-route-06-core.vercel.app/app/projects/1/ref/main/docs/README.md

[hoshinotsuyoshi]

LGTM!