fix: add processing page to file upload and reprocessing #650
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
![pr-agent-monadical[bot]](https://avatars.githubusercontent.com/u/29933591?s=60&v=4){kind=small}
Comment on lines
54
to
+77
| useEffect(() => { | ||
| if (waiting) { | ||
| const newUrl = "/transcripts/" + params.transcriptId + "/record"; | ||
| if (!waiting || !transcript.data) return; | ||
|
|
||
| const status = transcript.data.status; | ||
| let newUrl: string | null = null; | ||
|
|
||
| if (status === "processing" || status === "uploaded") { | ||
| newUrl = `/transcripts/${params.transcriptId}/processing`; | ||
| } else if (status === "recording") { | ||
| newUrl = `/transcripts/${params.transcriptId}/record`; | ||
| } else if (status === "idle") { | ||
| newUrl = | ||
| transcript.data.source_kind === "file" | ||
| ? `/transcripts/${params.transcriptId}/upload` | ||
| : `/transcripts/${params.transcriptId}/record`; | ||
| } | ||
|
|
||
| if (newUrl) { | ||
| // Shallow redirection does not work on NextJS 13 | ||
| // https://github.com/vercel/next.js/discussions/48110 | ||
| // https://github.com/vercel/next.js/discussions/49540 | ||
| router.replace(newUrl); | ||
| // history.replaceState({}, "", newUrl); | ||
| } | ||
| }, [waiting]); | ||
| }, [waiting, transcript.data?.status, transcript.data?.source_kind]); |
Contributor
There was a problem hiding this comment.
Suggestion: Add router and params to the dependency array of the useEffect hook to ensure the effect runs when these dependencies change. [possible issue, importance: 8]
Suggested change
| useEffect(() => { | |
| if (waiting) { | |
| const newUrl = "/transcripts/" + params.transcriptId + "/record"; | |
| if (!waiting || !transcript.data) return; | |
| const status = transcript.data.status; | |
| let newUrl: string | null = null; | |
| if (status === "processing" || status === "uploaded") { | |
| newUrl = `/transcripts/${params.transcriptId}/processing`; | |
| } else if (status === "recording") { | |
| newUrl = `/transcripts/${params.transcriptId}/record`; | |
| } else if (status === "idle") { | |
| newUrl = | |
| transcript.data.source_kind === "file" | |
| ? `/transcripts/${params.transcriptId}/upload` | |
| : `/transcripts/${params.transcriptId}/record`; | |
| } | |
| if (newUrl) { | |
| // Shallow redirection does not work on NextJS 13 | |
| // https://github.com/vercel/next.js/discussions/48110 | |
| // https://github.com/vercel/next.js/discussions/49540 | |
| router.replace(newUrl); | |
| // history.replaceState({}, "", newUrl); | |
| } | |
| }, [waiting]); | |
| }, [waiting, transcript.data?.status, transcript.data?.source_kind]); | |
| useEffect(() => { | |
| if (!waiting || !transcript.data) return; | |
| const status = transcript.data.status; | |
| let newUrl: string | null = null; | |
| if (status === "processing" || status === "uploaded") { | |
| newUrl = `/transcripts/${params.transcriptId}/processing`; | |
| } else if (status === "recording") { | |
| newUrl = `/transcripts/${params.transcriptId}/record`; | |
| } else if (status === "idle") { | |
| newUrl = | |
| transcript.data.source_kind === "file" | |
| ? `/transcripts/${params.transcriptId}/upload` | |
| : `/transcripts/${params.transcriptId}/record`; | |
| } | |
| if (newUrl) { | |
| // Shallow redirection does not work on NextJS 13 | |
| // https://github.com/vercel/next.js/discussions/48110 | |
| // https://github.com/vercel/next.js/discussions/49540 | |
| router.replace(newUrl); | |
| } | |
| }, [waiting, transcript.data?.status, transcript.data?.source_kind, router, params]); |
Comment on lines
+49
to
+55
| if (transcript.isLoading) { | ||
| return ( | ||
| <VStack align="center" py={8}> | ||
| <Heading size="lg">Loading transcript...</Heading> | ||
| </VStack> | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Suggestion: The loading state check should also handle the case when transcript.data is undefined, as the useEffect logic depends on it. [possible issue, importance: 7]
Suggested change
| if (transcript.isLoading) { | |
| return ( | |
| <VStack align="center" py={8}> | |
| <Heading size="lg">Loading transcript...</Heading> | |
| </VStack> | |
| ); | |
| } | |
| if (transcript.isLoading || !transcript.data) { | |
| return ( | |
| <VStack align="center" py={8}> | |
| <Heading size="lg">Loading transcript...</Heading> | |
| </VStack> | |
| ); | |
| } |
smankovsky
changed the title
tito
changed the title
| const status = transcript.data?.status; | ||
| if (!status) return; | ||
|
|
||
| if (status === "ended" || status === "error") { |
Contributor
There was a problem hiding this comment.
logic dupe with www/app/(app)/transcripts/[transcriptId]/page.tsx - only allow dupes when it's different business processes
|
|
||
| type FileUploadButton = { | ||
| transcriptId: string; | ||
| onUploadComplete?: () => void; |
Contributor
There was a problem hiding this comment.
nit: "onComplete" (we already know it's Upload from the name)
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.
User description
PR Type
Enhancement
Description
Add processing page for file uploads
Improve transcript status handling
Unify processing experience
Fix redirection logic
Changes walkthrough 📝
page.tsx
Improve transcript status handling and UIwww/app/(app)/transcripts/[transcriptId]/page.tsx
page.tsx
Add new processing page componentwww/app/(app)/transcripts/[transcriptId]/processing/page.tsx
page.tsx
Refactor upload page to use processing pagewww/app/(app)/transcripts/[transcriptId]/upload/page.tsx
fileUploadButton.tsx
Add completion callback to FileUploadButtonwww/app/(app)/transcripts/fileUploadButton.tsx