feat(campaign-exports): send one email after multiple campaign exports #1661

henryk1229 · 2023-08-11T15:25:18Z

Description

This pr refactors the export-multiple-campaigns task so that when admins export multiple campaigns at once, only one email is sent to them, with links for each campaign exported. It branches off of #1641 - that should be reviewed and merged first, then we can rebase and review this pr more easily.

Motivation and Context

It closes #1660

How Has This Been Tested?

Locally, and with mailtrap

Screenshots (if appropriate):

Documentation Changes

Checklist:

My change requires a change to the documentation.
I have included updates for the documentation accordingly.

…ror messaging

…utils

…ted plumbing N

…tation input

… to campaign id for email

…paign exports process

src/server/tasks/export-multiple-campaigns.ts

bchrobot

Nice!

bchrobot · 2023-09-12T15:46:51Z

libs/gql-schema/schema.ts

@@ -215,6 +215,11 @@ const rootSchema = `
    vanOptions: ExportForVanInput
  }

+    input MultipleCampaignExportInput {


nit: fix indentation

Suggested change

input MultipleCampaignExportInput {

input MultipleCampaignExportInput {

bchrobot · 2023-09-12T15:56:56Z

src/components/ExportMultipleCampaignDataDialog.tsx

+
+  const [exportCampaignsMutation] = useExportCampaignsMutation();
+
+  const handleChange = (


Suggestion: rename to handleChangeFactory to make it clear that this creates handlers rather than handling change itself

bchrobot · 2023-09-12T15:57:14Z

src/components/ExportMultipleCampaignDataDialog.tsx

+  open: boolean;
+  onClose: () => void;
+  onError: (errorMessage: string) => void;
+  onComplete(): void;


nit: consistent function typing

Suggested change

onComplete(): void;

onComplete: () => void;

bchrobot · 2023-09-12T16:00:22Z

src/components/ExportMultipleCampaignDataDialog.tsx

+              return (
+                <div
+                  key={campaign.id}
+                  style={{
+                    display: "flex",
+                    alignItems: "center",
+                    margin: "4px"
+                  }}
+                >
+                  <Typography variant="body1" style={{ margin: "4px" }}>
+                    {campaign.title}
+                  </Typography>
+                  <Typography
+                    variant="body1"
+                    style={{ marginLeft: "4px", color: "#666666" }}
+                  >
+                    ID: {campaign.id}
+                  </Typography>
+                </div>
+              );


Suggestion: pull this up into its own component

bchrobot · 2023-09-12T16:00:38Z

src/components/ExportMultipleCampaignDataDialog.tsx

+              return (
+                <div
+                  key={campaign.id}
+                  style={{
+                    display: "flex",
+                    alignItems: "center",
+                    margin: "4px"
+                  }}
+                >
+                  <Typography variant="body1" style={{ margin: "4px" }}>
+                    {campaign.title}
+                  </Typography>
+                  <Typography
+                    variant="body1"
+                    style={{ marginLeft: "4px", color: "#666666" }}
+                  >
+                    ID: {campaign.id}
+                  </Typography>
+                </div>
+              );


Suggestion: pull this up into its own component

bchrobot · 2023-09-12T17:11:34Z

src/server/tasks/export-multiple-campaigns.ts

+    };
+  });
+
+  // map fetched export urls to  campaignMetaData


nit: remove extra space

Suggested change

// map fetched export urls to campaignMetaData

// map fetched export urls to campaignMetaData

bchrobot · 2023-09-12T17:16:20Z

src/server/tasks/export-multiple-campaigns.ts

+      campaignExport.exportUrls;
+  }
+
+  try {


Question: what is the try / finally here accomplishing? The finally block will be executed even if an error is thrown in try but in this case the finally block is logging a success?

bchrobot · 2023-09-12T17:18:05Z

src/server/tasks/export-multiple-campaigns.ts

+  // wait for all campaign exports to process
+  const exportsStillProcessing = rows.length !== campaignIds.length;


Suggestion: create a new issue for handling errors in bulk-export-campaigns child jobs and link here as future work?

My understanding is that if a bulk-export-campaigns job encounters a terminal failure and never reaches status = 100 then email-export-campaigns will just keep running every 4 minutes?

bchrobot · 2023-09-13T17:43:27Z

src/containers/AdminCampaignStats/components/CampaignExportModal.tsx

+          control={
+            <Switch
+              checked={exportCampaign}
+              onChange={handleChange(setExportCampaign)}


Required: move the handleChange() factory to the parent component and only pass down the setExport* callbacks as props

bchrobot · 2023-09-13T17:45:38Z

src/containers/CampaignList/components/CampaignListHeader.tsx

+            </InputAdornment>
+          )
+        }}
+        onChange={(ev) => debounceSearchTerm(ev.target.value)}


nit: prefer named callback function

hiemanshu · 2023-09-15T06:27:05Z

libs/spoke-codegen/src/graphql/campaign-stats.graphql

@@ -63,8 +63,22 @@ mutation ExportCampaign($options: CampaignExportInput!) {
  }
 }

-mutation CopyCampaigns($templateId: String!, $quantity: Int!, $targetOrgId: String) {
-  copyCampaigns(sourceCampaignId: $templateId, quantity: $quantity, targetOrgId: $targetOrgId) {
+mutation ExportCampaigns($options: MultipleCampaignExportInput!) {


nit: move mutations to be below queries

hiemanshu · 2023-09-15T06:28:15Z

src/components/ExportCampaignDataSnackbar.tsx

+import Alert from "@material-ui/lab/Alert";
+import React from "react";
+
+interface Props {


required: export props, and name ExportCampaignDataSnackbarProps, more details in https://github.com/politics-rewired/Spoke/blob/main/conventions.md

hiemanshu · 2023-09-15T06:28:48Z

src/components/ExportMultipleCampaignDataDialog.tsx

+  id: string;
+  title: string;
+};
+interface Props {


required: export props, and name ExportMultipleCampaignDataDialogProps, more details in https://github.com/politics-rewired/Spoke/blob/main/conventions.md

hiemanshu · 2023-09-15T06:31:21Z

src/containers/AdminCampaignList.jsx

@@ -4,6 +4,7 @@ import Dialog from "@material-ui/core/Dialog";
 import DialogActions from "@material-ui/core/DialogActions";


suggestion: update file to tsx, and functional component

hiemanshu · 2023-09-15T06:31:52Z

src/containers/AdminCampaignStats/components/CampaignExportModal.tsx

@@ -19,6 +19,79 @@ interface CampaignExportModalProps {
  onComplete(): void;
 }

+interface CampaignExportModalContentProps {


required: export props

hiemanshu · 2023-09-15T06:33:30Z

src/containers/CampaignList/components/CampaignHeader.tsx

+  status: string;
+};
+
+interface CampaignHeaderProps {


required: export props

hiemanshu · 2023-09-15T06:33:53Z

src/containers/CampaignList/components/CampaignListHeader.tsx

+
+const DEBOUNCE_INTERVAL = 500;
+
+interface Props {


required: export props, update name

hiemanshu · 2023-09-15T06:34:15Z

src/containers/CampaignList/components/CampaignListLoader.tsx


-interface Props {
+interface Props extends CampaignOperations {


required: export props, update name

hiemanshu · 2023-09-15T06:37:33Z

src/containers/CampaignList/utils.ts

 export const isCampaignGroupsPermissionError = (gqlError: GraphQLError) => {
  return (
    gqlError.path &&
    gqlError.path[gqlError.path.length - 1] === "campaignGroups" &&
    gqlError.extensions.code === "FORBIDDEN"
  );
 };
+
+type MakeCampaignTagsFn = (props: {


required: use interface for props, export, and update name

hiemanshu · 2023-09-15T06:38:09Z

src/server/lib/templates/export-multiple-campaigns.tsx

+  [id: string]: CampaignExportDetails;
+};
+
+interface Props {


required: export props, update name

henryk1229 added 30 commits July 13, 2023 14:55

feat: wip, routing and boilerplate react component

7dea763

feat: wip - add mutation, initial task

2977ef0

feat: graphql mutation

bb4ea78

feat: wip, frontend - use CampaignListMenu to select for export

a79b4ef

feat(export-multiple-campaigns): simple backend solution

3aae8c5

feat(campaignexportmodal): share ModalContent between components

506b02d

feat(admincampaignlist): show snackbar after kicking off exports

10a567e

feat(campaign-list-row): add color to export tag

99abef1

feat(export-campaign-snackbar): snackbar component for success and er…

ddc3d28

…ror messaging

feat: handle de-selecting for export in CampaignListMenu

7f19a3d

feat: reset idsForExport on complete

ee721c1

feat(campaign-list): initial card approach, break out components and …

cea4766

…utils

feat(campaign-list): hook up export campaign button

bef6de3

refactor: rm mvp solution

4add064

feat(admin-campaign-list): styling, search field

67ac4d3

feat(campaign-list): filter by campaign title

4807092

feat(campaign-export-modal): display camaign details for export, rela…

ebd991a

…ted plumbing N

feat(campaign-list): improve card styling

d14b20c

feat(campaign-list): use rounded icons, improve export dialog styling

e847c34

refactor: require campaign ids, spoke options, rm export type from mu…

abb78ff

…tation input

refactor: improve var names

7ef2246

feat: improve typing

b5bc27a

feat: destructure props in component declaration

45ea951

fix: revert campaign list menu changes

338b169

feat(campaign-list-row): improve typing

dd93365

refactor(campaign-list-row): styling changes

b8348d0

refactor(multi-export-dialog): styling changes

16c9467

feat(export-multiple-campaigns): wip, breakout tasks

3c541f1

feat(export-multiple-campaigns): wip, map campaign title, export urls…

f8c9de5

… to campaign id for email

feat(task-utils): add cleanup fn to progress job wrapper fn

1ed058b

feat(export-multiple-campaigns): wip, format and send email after cam…

46db8cc

…paign exports process

henryk1229 commented Aug 18, 2023

View reviewed changes

src/server/tasks/export-multiple-campaigns.ts Show resolved Hide resolved

henryk1229 added 2 commits August 29, 2023 14:57

feat(export-multiple-campaigns): add key to email template div

e6d2859

feat(email-multiple-campaigns): re-queue job if query returns no rows

9eb66c7

henryk1229 requested review from hiemanshu, bchrobot and ajohn25 September 12, 2023 15:41

henryk1229 marked this pull request as ready for review September 12, 2023 15:43

bchrobot requested changes Sep 13, 2023

View reviewed changes

hiemanshu requested changes Sep 15, 2023

View reviewed changes

ajohn25 approved these changes Sep 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(campaign-exports): send one email after multiple campaign exports #1661

feat(campaign-exports): send one email after multiple campaign exports #1661

henryk1229 commented Aug 11, 2023 •

edited

Loading

bchrobot left a comment

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 12, 2023

bchrobot Sep 13, 2023

bchrobot Sep 13, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

hiemanshu Sep 15, 2023

	input MultipleCampaignExportInput {
	input MultipleCampaignExportInput {


		const [exportCampaignsMutation] = useExportCampaignsMutation();

		const handleChange = (

	// map fetched export urls to campaignMetaData
	// map fetched export urls to campaignMetaData

		// wait for all campaign exports to process
		const exportsStillProcessing = rows.length !== campaignIds.length;

		@@ -4,6 +4,7 @@ import Dialog from "@material-ui/core/Dialog";
		import DialogActions from "@material-ui/core/DialogActions";


		interface Props {
		interface Props extends CampaignOperations {

feat(campaign-exports): send one email after multiple campaign exports #1661

Are you sure you want to change the base?

feat(campaign-exports): send one email after multiple campaign exports #1661

Conversation

henryk1229 commented Aug 11, 2023 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Documentation Changes

Checklist:

bchrobot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henryk1229 commented Aug 11, 2023 •

edited

Loading