INJIWEB-1667 Success message will appear after downloading VC's#501
INJIWEB-1667 Success message will appear after downloading VC's#501sanehema9 wants to merge 3 commits intoinji:developfrom
Conversation
WalkthroughReworked RedirectionPage to remove automatic navigation and surface download outcomes via localized toast notifications; both logged-in and guest download flows now mark completion on both success and error, triggering a toast effect when Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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)
inji-web/src/pages/RedirectionPage.tsx (1)
29-30: Prevent duplicate/incorrect toasts by persisting the outcome at the time of response.The concern is valid. When
vcDownloadApi.fetchData()completes, it returns the response immediately but queues state updates asynchronously. InhandleLoggedInDownloadFlowandhandleGuestDownloadFlow,setCompletedDownload(true)is called before React processes thesetState()calls fromuseApi, meaningvcDownloadApi.statedoesn't reflect the actual outcome yet.The useEffect (lines 111–123) depends on both
completedDownloadandvcDownloadApi.state, so it will fire again whenvcDownloadApi.stateupdates asynchronously, potentially showing a duplicate or incorrect toast. Capture the outcome ('success' | 'error' | null) immediately when the response resolves, use it solely for the toast decision, then clear it to prevent re-runs.Suggested fix
- const [completedDownload, setCompletedDownload] = useState<boolean>(false); + const [completedDownload, setCompletedDownload] = useState<boolean>(false); + const [downloadOutcome, setDownloadOutcome] = useState<'success' | 'error' | null>(null); const handleLoggedInDownloadFlow = async (issuerId: string, requestBody: TokenRequestBody) => { const downloadId = addSession(credentialTypeDisplayObj, RequestStatus.LOADING); const credentialDownloadResponse = await vcDownloadApi.fetchData({ body: requestBody, apiConfig: api.downloadVCInloginFlow, headers: api.downloadVCInloginFlow.headers(language) }); if (credentialDownloadResponse.ok()) { updateSession(downloadId, RequestStatus.DONE) + setDownloadOutcome('success'); setCompletedDownload(true); } else { updateSession(downloadId, RequestStatus.ERROR) + setDownloadOutcome('error'); setCompletedDownload(true); } } const handleGuestDownloadFlow = async (requestBody: TokenRequestBody) => { const urlState = searchParams.get('state') ?? ''; const credentialDownloadResponse = await vcDownloadApi.fetchData({ body: requestBody, apiConfig: api.fetchTokenAnddownloadVc }) if (credentialDownloadResponse.state !== RequestStatus.ERROR) { await downloadCredentialPDF( credentialDownloadResponse.data, credentialType + ".pdf" ); + setDownloadOutcome('success'); setCompletedDownload(true); } else { + setDownloadOutcome('error'); setCompletedDownload(true); } ... } useEffect(() => { - if (completedDownload) { - const isError = vcDownloadApi.state === RequestStatus.ERROR; + if (downloadOutcome) { + const isError = downloadOutcome === 'error'; showToast({ message: i18next.t(`VCDownload.${isError ? 'error' : 'success'}`, { ns: 'Layout', cardType: credentialTypeDisplayObj?.[0]?.name || credentialType }), type: isError ? 'error' : 'success', testId: `download-${isError ? 'error' : 'success'}-toast` }); + setDownloadOutcome(null); } - }, [completedDownload, vcDownloadApi.state, credentialType, credentialTypeDisplayObj]); + }, [downloadOutcome, credentialType, credentialTypeDisplayObj]);Applies to: lines 37–51, 54–69, 111–123
. I’ve updated the VC download flow to capture the outcome immediately (success or error) and refactored the useEffect to depend only on that outcome. This ensures the toast is shown exactly once, at the right time, and prevents duplicates or delayed messages. |
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| if (completedDownload) { |
There was a problem hiding this comment.
Please add unit tests for the newly added code
| if (completedDownload) { | ||
| const isError = vcDownloadApi.state === RequestStatus.ERROR; | ||
| showToast({ | ||
| message: i18next.t(`VCDownload.${isError ? 'error' : 'success'}`, { |
There was a problem hiding this comment.
can we please use useTranslation hook instead of i18next ?
|
@sanchi-singh24 can you please confirm on the UI design |
Signed-off-by: sanehema9 <sanehema9@gmail.com>
Signed-off-by: sanehema9 <sanehema9@gmail.com>
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)
inji-web/src/pages/RedirectionPage.tsx (1)
37-69:⚠️ Potential issue | 🟠 MajorToast can fire with the wrong status (or twice) due to async state updates.
completedDownloadis set immediately afterfetchDataresolves, butvcDownloadApi.statecan lag. The effect readsvcDownloadApi.stateand re-runs when it updates, so you can show a success toast on an error (then later an error toast), or duplicate toasts when state changes. Consider storing the outcome from the response and driving the toast off that, then clearing it after display.✅ Suggested fix (capture outcome + show toast once)
- const [completedDownload, setCompletedDownload] = useState<boolean>(false); + const [completedDownload, setCompletedDownload] = useState<boolean>(false); + const [downloadOutcome, setDownloadOutcome] = useState<'success' | 'error' | null>(null); @@ - if (credentialDownloadResponse.ok()) { + if (credentialDownloadResponse.ok()) { updateSession(downloadId, RequestStatus.DONE) - setCompletedDownload(true); + setDownloadOutcome('success'); + setCompletedDownload(true); } else { updateSession(downloadId, RequestStatus.ERROR) - setCompletedDownload(true); + setDownloadOutcome('error'); + setCompletedDownload(true); } @@ - if (credentialDownloadResponse.state !== RequestStatus.ERROR) { + if (credentialDownloadResponse.state !== RequestStatus.ERROR) { await downloadCredentialPDF( credentialDownloadResponse.data, credentialType + ".pdf" ); - setCompletedDownload(true); + setDownloadOutcome('success'); + setCompletedDownload(true); } else { - setCompletedDownload(true); + setDownloadOutcome('error'); + setCompletedDownload(true); } @@ - useEffect(() => { - if (completedDownload) { - const isError = vcDownloadApi.state === RequestStatus.ERROR; + useEffect(() => { + if (downloadOutcome) { + const isError = downloadOutcome === 'error'; showToast({ message: tLayout(`VCDownload.${isError ? 'error' : 'success'}`, { cardType: credentialTypeDisplayObj?.[0]?.name || credentialType }), type: isError ? 'error' : 'success', testId: `download-${isError ? 'error' : 'success'}-toast` }); + setDownloadOutcome(null); } - }, [completedDownload, vcDownloadApi.state, credentialType, credentialTypeDisplayObj]); + }, [downloadOutcome, credentialType, credentialTypeDisplayObj, tLayout]);Also applies to: 111-122
Description
Fixed an issue where the VC download success message was not displayed immediately after logging in with Google. The success toast now appears as soon as the VC download is completed, without requiring navigation to the Home or Stored Cards page.
Files Changed
RedirectionPage
Issue TicketNumber and Link
INJIWEB-1667(https://mosip.atlassian.net/browse/INJIWEB-1667)
Video
INJIWEB-1667.mp4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.