Add destination delivery mode, cancel duplicate exports when in "once" mode #3009

rwfeather · 2022-02-25T21:48:06Z

Change description

This adds:

Delivery mode for destinations. Options are [once, continual]
- Continual - "Can deliver the same record multiple times" - default
- Once - "Guaranteed to only deliver a record at-most-once"
UI for the destination delivery mode

This changes:

Add a check in sendExports that will refuse to run any export whose destination is currently set to "once"

Checklists

Development

Application changes have been tested appropriately

Impact

Code follows company security practices and guidelines
Security impact of change has been considered
Performance impact of change has been considered
Possible migration needs considered (model migrations, config migrations, etc.)

Please explain any security, performance, migration, or other impacts if relevant:

Code review

Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached where applicable.
Relevant tags have been added to the PR (bug, enhancement, internal, etc.)

core/src/models/Destination.ts

…" mode

rwfeather · 2022-03-04T18:36:02Z

Test is broken in CI but green locally ☹️ . Tracking it down now.

evantahler · 2022-03-04T19:33:36Z

core/__tests__/models/destination/plugins/exportRecord.ts

@@ -955,6 +955,72 @@ describe("models/destination - with custom exportRecord plugin", () => {
      });
    });

+    describe("when destination delivery mode is", () => {
+      let record: GrouparooRecord = null;


TS note: You don't need to set it to null.

Suggested change

let record: GrouparooRecord = null;

let record: GrouparooRecord;

This makes the record start with a value of undefiend

evantahler · 2022-03-04T19:35:28Z

core/__tests__/models/destination/plugins/exportRecord.ts

+      afterEach(async () => {
+        await record.destroy();
+      });


You may want to Export.truncate() between tests as well, to ensure that there is no test polution

evantahler · 2022-03-04T19:36:20Z

core/src/actions/destinations.ts

@@ -175,6 +175,7 @@ export class DestinationEdit extends AuthenticatedAction {
    state: { required: false },
    options: { required: false, formatter: APIData.ensureObject },
    mapping: { required: false, formatter: APIData.ensureObject },
+    deliveryMode: { required: false },


This change implies that there should be an update to the Action's test as well, confirming that you can set the deliver mode

evantahler · 2022-03-04T19:36:40Z

core/src/migrations/000102-addDeliveryMode.ts

+    });
+    await queryInterface.changeColumn("destinations", "deliveryMode", {
+      type: DataTypes.STRING(191),
+      allowNull: false,


👍 we end up with a non-nullable column

evantahler · 2022-03-04T19:37:37Z

core/src/models/Destination.ts

+  @AllowNull(false)
+  @Default("continual")
+  @Column(DataType.ENUM(...DELIVERY_MODES))
+  deliveryMode: DestinationDeliveryMode;


There's probably a test to add that foo can't be used as a delivery mode. That's probably going to need a @BeforeSave validation check.

This code here is about setting the types, but it won't validate the data coming in... I think. The test will let us know for sure!

evantahler · 2022-03-04T19:42:57Z

core/src/modules/ops/destination.ts

+        const mostRecentExport = await Export.findOne({
+          where: {
+            destinationId: destination.id,
+            recordId: consideredExport.recordId,
+            state: "complete",
+          },
+          order: [["completedAt", "desc"]],
+        });


this method will run for every (const consideredExport of exportsWithChanges), which could be a lot of SQL statements. We'll want to load this data in one SQL statement before the for loop for every export we are considering.

evantahler · 2022-03-04T19:43:59Z

ui/ui-components/pages/model/[modelId]/destination/[destinationId]/edit.tsx

@@ -1,3 +1,4 @@
+import { DestinationDeliveryMode } from "@grouparoo/core/src/models/Destination";


We can't import models from core - this will cause compication problems. The best we can do is import API responses via import { Actions } from "../../../../../utils/apiData"

rwfeather added the enhancement New feature or request label Feb 25, 2022

rwfeather force-pushed the 181244598-destination-delivery-mode branch from 5f6c380 to d975021 Compare March 1, 2022 17:57

evantahler reviewed Mar 2, 2022

View reviewed changes

core/src/models/Destination.ts Outdated Show resolved Hide resolved

rwfeather force-pushed the 181244598-destination-delivery-mode branch from d442e31 to 61c56ef Compare March 4, 2022 09:16

rwfeather marked this pull request as ready for review March 4, 2022 15:00

rwfeather requested review from evantahler and pedroslopez March 4, 2022 16:18

Reina Feather added 3 commits March 4, 2022 10:27

Add destination delivery mode, cancel duplicate exports when in "once…

9beccd9

…" mode

Use as const to more strongly type the delivery mode enum

c3ca84a

Add new tests for delivery mode

80960d3

rwfeather force-pushed the 181244598-destination-delivery-mode branch from eb06d87 to 80960d3 Compare March 4, 2022 18:43

Clean up test method

d393497

evantahler suggested changes Mar 4, 2022

View reviewed changes

pedroslopez removed their request for review March 10, 2022 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add destination delivery mode, cancel duplicate exports when in "once" mode #3009

Add destination delivery mode, cancel duplicate exports when in "once" mode #3009

rwfeather commented Feb 25, 2022 •

edited

rwfeather commented Mar 4, 2022

evantahler Mar 4, 2022

evantahler Mar 4, 2022

evantahler Mar 4, 2022

evantahler Mar 4, 2022

evantahler Mar 4, 2022

evantahler Mar 4, 2022

evantahler Mar 4, 2022

	let record: GrouparooRecord = null;
	let record: GrouparooRecord;

		@@ -1,3 +1,4 @@
		import { DestinationDeliveryMode } from "@grouparoo/core/src/models/Destination";

Add destination delivery mode, cancel duplicate exports when in "once" mode #3009

Are you sure you want to change the base?

Add destination delivery mode, cancel duplicate exports when in "once" mode #3009

Conversation

rwfeather commented Feb 25, 2022 • edited

Change description

Checklists

Development

Impact

Code review

rwfeather commented Mar 4, 2022

evantahler Mar 4, 2022

Choose a reason for hiding this comment

evantahler Mar 4, 2022

Choose a reason for hiding this comment

evantahler Mar 4, 2022

Choose a reason for hiding this comment

evantahler Mar 4, 2022

Choose a reason for hiding this comment

evantahler Mar 4, 2022

Choose a reason for hiding this comment

evantahler Mar 4, 2022

Choose a reason for hiding this comment

evantahler Mar 4, 2022

Choose a reason for hiding this comment

rwfeather commented Feb 25, 2022 •

edited