fix(dashboard): add missing payload on activity feed #7883
Conversation
❌ Deploy Preview for dashboard-v2-novu-staging failed. Why did it fail? →
|
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
djabarovgeorge
had a problem deploying
to
Linting
GitHub Actions
Failure
…ayload-in-activity-feed-item # Conflicts: # libs/application-generic/src/usecases/message-template/create-message-template/create-message-template.usecase.ts
There was a problem hiding this comment.
not sure why i got formatting changes on the template store template in this PR ..
djabarovgeorge
changed the title
| toast.error('Failed to trigger resend workflow', { | ||
| description: e instanceof Error ? e.message : 'There was an error triggering the resend workflow.', | ||
| }); |
There was a problem hiding this comment.
please use showErrorToast
| toast.success('Notification resent successfully', { | ||
| description: `A new notification has been triggered with transaction ID: ${newTransactionId}`, | ||
| }); |
There was a problem hiding this comment.
please use showSuccessToast
| const setIsFullscreenOpen = useCallback((isOpen: boolean) => { | ||
| if (isOpen && popoverCloseRef.current) { | ||
| popoverCloseRef.current.click(); | ||
| } | ||
|
|
||
| setIsFullscreenOpenState(isOpen); | ||
| }, []); | ||
|
|
||
| const closePopover = useCallback(() => { | ||
| if (popoverCloseRef.current) { | ||
| popoverCloseRef.current.click(); | ||
| } | ||
|
|
||
| setIsPopoverOpen(false); | ||
| }, []); | ||
|
|
||
| const handleResend = useCallback(async () => { |
There was a problem hiding this comment.
if you pass these handlers to "simple" components (without deep tree and useMemo), then there is not much sense in memoization and useCallback
| variant="secondary" | ||
| mode="ghost" | ||
| size="sm" | ||
| onClick={handleResend} |
There was a problem hiding this comment.
should not be allowed on deleted workflows, please do it in both places
| <span className="text-foreground-950 text-sm font-medium">Logs</span> | ||
| </div> | ||
|
|
||
| {payload && ( |
There was a problem hiding this comment.
isn't the payload always available?
| return showToast({ | ||
| variant: 'lg', | ||
| children: ({ close }) => ( | ||
| <> | ||
| <ToastIcon variant="error" /> | ||
| <div className="flex flex-col gap-2"> | ||
| <span className="font-medium">Test workflow failed</span> | ||
| <span className="text-foreground-600 inline"> | ||
| Workflow <span className="font-bold">{activity.template?.name}</span> cannot be triggered. Ensure that | ||
| it is active and requires not further actions. | ||
| </span> | ||
| </div> | ||
| <ToastClose onClick={close} /> | ||
| </> | ||
| ), | ||
| options: { | ||
| position: 'bottom-right', | ||
| }, | ||
| }); |
There was a problem hiding this comment.
this should be an error or warning toast, please check showErrorToast
| setIsPopoverOpen(false); | ||
| }, []); | ||
|
|
||
| const handleResend = useCallback(async () => { |
There was a problem hiding this comment.
please use the useMutation from @tanstack/react-query then you won't need to handle the loading flag and you will have onSuccess and onError callbacks to handle
| @@ -55,33 +57,48 @@ export const TestWorkflowLogsSidebar = ({ transactionId, workflow }: TestWorkflo | |||
| } | |||
| }, [activityId]); | |||
|
|
|||
| // Reset refetch when transaction ID changes | |||
| const handleTransactionIdChange = useCallback((newTransactionId: string) => { | |||
| setIsTransactionChanging(true); | |||
There was a problem hiding this comment.
I'm not sure that this flag is needed, because the useFetchActivities query should be triggered and areActivitiesPending should be true resulting in the skeleton
| <ActivityLogs | ||
| activity={activity} | ||
| onActivitySelect={setParentActivityId} | ||
| onTransactionIdChange={handleTransactionIdChange} |
There was a problem hiding this comment.
I'm missing the similar handling logic in the subscriber-activity-drawer.tsx
| } | ||
| }; | ||
|
|
||
| setTimeout(checkAndUpdateTransaction, 1000); |
There was a problem hiding this comment.
this one is a bit hacky, what if I have the other activities triggered at the same time? it might pick a wrong one
What changed? Why was the change needed?
Can be tested in preview deployment.
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer