Mailgun event sources and actions

[sergioeliot2039]

Hi,

This is an initial merge of the Mailgun actions that were marge to PipedreamHQ/master while we had Mailgun sources on review.

I believe there are chances of further merging, for example. the method that creates a Mailgun SDK object could be shared both between actions and sources, however, doing so would require further modifications on either action and/or sources. So looking forward to discussing on this as needed.

The previous PR for Mailgun sources is here. Furthermore, this PR doesn't touch files from reddit, as the previously unintendedly had.

Resolves #1024 and #1471

[vercel[bot]]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

pipedream-docs-redirect-do-not-edit – ./docs

🔍 Inspect: https://vercel.com/pipedreamers/pipedream-docs-redirect-do-not-edit/6d9sPkb8og47ykFtNxcVyocuJ5gH
✅ Preview: https://pipedream-docs-redirect-do-not-edit-git-for-d42481-pipedreamers.vercel.app

pipedream-docs – ./docs

🔍 Inspect: https://vercel.com/pipedreamers/pipedream-docs/3UN2x7VyskEVyDFtjqdnGVPqdKZq
✅ Preview: https://pipedream-docs-git-fork-sergioeliot2039-mai-7c7a2b-pipedreamers.vercel.app

[sergioeliot2039]

Hi @compwright , looks good to me. When it is ready for me to review please just ping me. Thanks!

[compwright]

@sergioeliot2039 haven't tested yet, but you can give this a review now.

[dannyroosevelt]

Not sure if I'm missing something but I can't get any of the actions I tried testing working:

-Add a Mailing List Member

-List Domains

-List a Mailing List Member

[sergioeliot2039]

Most actions were failing because they leverage from a method withErrorHandler in the common file to avoid duplicating boiler plate code which had this issues: the app object was undefined, props values weren't being passed properly to the code segment actually hitting mailgun api. once those fixed there was an underlying issue that even if the code didn't break nor returned an error, it would return null and shown a workflow output message saying that return value is not serializable.

the fixups for the issues described above included: sending the mailgun app object from withErrorHandler in the common file to the delegate function hitting the api, the props were passed as additinal parameters to withErrorHandler which are then relayed to the delegate function hitting the api. the output message about return value not being serializable was fixed by importing withErrorHandler to the action file via the methods component section instead of doing a direct import at the top of the file as a declaration.

while some extra boilerplate code was added for passing the variables between the coupled withErrorHandler in the common file with the deleage function hitting the api in the related component file, or doing imports via the methods component section, i think the ethos of using withErrorHandler for handling errors instead of using repetitives try/catch in the component files is preserved.

finally, some actions had other logic errors on its own, i tried to fix all but here are the outstanding: verify-email -- Validations are not enabled. create-route-- Routes quota (0) is exceeded for these two i assumed it's due to my using a mailgun free tier account or just missing configuration. Since those didn't look like a problem with the request config itself i stopped there but i can look further if required. send-email -- is not including the cc nor bcc parameters. this issue is extremely interesting because with the mailgun sdk it won't accept parameters from the UI for cc nor bcc because of the in memory handling of the arrays. if i hardcoded an array and did a direct assignment to these parameters with double quote they would work (i.e. data.cc = ["test@mail.com"]), i consoled out the parameters received from the UI and the console out as ['test@mail.com'], i tried to JSON.stringify the params, do a replace from ' to " and JSON.parse back but this didn't work. I had to remove cc and bcc. Odd because assigning an array from the UI on the to prop works.

yes -- there are ESLint issues in this PR commit but those are in the sources. I didn't touch sources files just this time, but let me know if should tackle them.

[sergioeliot2039]

forgot to mention, i stripped off pagination on action list-mailinglist-members because the related sdk method is buggy -- it always gets the first page and doesn't honor the input page/skip parameters, so it goes on a infinite loop until the workflows times out. i set the code to return the first page only.

[dannyroosevelt]

I was able to successfully QA everything except Create a Route (not because that action is broken, just because I didn't go deep in setting up the account / use case properly).

[sergioeliot2039]

    generateMeta(payload) {
      const ts = +new Date(payload.created_at);
      return {
        id: `${ts}`,

• Could this potentially dedupe two different mailing lists that have the same created_at? @js07 No, I'm leaving the "greatest" dedupe strategy. If both mailing list are created at the same time (have the same created_at) they both will trigger a new event, as long as their timestamp is greatest than the last emitted.

[sergioeliot2039]

@js07 this is ready for another review. I made adjustments to the sources, so that they work properly with the sdk. I also refactored the common-webhooks file into a base, http-based, and timer-based files. this is to take off the webhook signature off from timer based sources, which don't use this prop. i followed your suggestions of doing a check on event type/subtype in the common file (now http-based), as of the suggestion for checking "opened" event types, i left both spellings "OPENED" and "opened" because originally malign was sending it on uppercase, so just in case. looking forward to your reviews.

[sergioeliot2039]

Is there a reason to stringify vars and add it to data separately from the other opts?

Yes, an JSON formatted object is expected for this param. Since the prop is defined as an object, we have to stringify it. As per Mailgun API:

vars | JSON-encoded dictionary string with arbitrary parameters, e.g. {"gender":"female","age":27}

[sergioeliot2039]

Do you know if you could instead use this.mailgun.api("webhooks").destroy(domain, webhook);, so the Mailgun client handles building the endpoint URL?

this worked. i applied this change.

[sergioeliot2039]

It looks like the events emitted in deploy() are the earliest 5 events after 1 day ago. Instead, I think we want to emit the latest (most recent) 5 events on deploy.

To fetch the 5 most recent events, you could use these query options:

this.mailgun.api("events").get(this.domain,{ begin: Math.floor(Date.now() / 1000), ascending: "no", limit: 5, });

Hey, I had to use this logic to make ascending: "no" work. Essentially, the descending records had to be emitted from the current date and earlier. On the other hand, the ascending: "yes" records, are emitted using the current day minus one date. If I didn't use the current date for ascending: "no", then no record are returned by Mailgun. THis is the relevant code:

      const date = new Date();
      if ([
        "yes",
      ].includes(ascending)) {
        date.setDate(date.getDate() - 1);
      } else {
        date.setDate(date.getDate());
      }
      const config = {
        begin: Math.floor(date.valueOf() / 1000),
        ascending,
        limit,
      };

[sergioeliot2039]

Since this.mailgun.api("lists").list() returns mailing lists for all domains, I think events for incorrect/unselected domains (domain !== this.domain) could also be emitted?

To fix this issue, I'd suggest filtering sortedList the way mailing lists are sorted in the run method.

I confirmed the issue. The source was emitting new mailing lists for all domains on deploy. I committed your changes, tested and it works like a charm. Great catch!

[vercel[bot]]

@sergioeliot2039 is attempting to deploy a commit to the Pipedreamers Team on Vercel.

A member of the Team first needs to authorize it.

[sergioeliot2039]

It looks like we're fetching all events created after 1 day ago each time this source runs. If there are more than 100 events in 1 day, this source could emit the same event multiple times (since it's using the unique deduping strategy)?

To avoid refetching events (beginning 1 day ago) on each run, could we store the timestamp of the last seen event using $.service.db and request only events after that timestamp? E.g., Google Drive New or Modified Comments, Twilio common-polling

I implemented your suggestion to avoid refetching events as described by checking for the timestamp of the last seen event and leveraging from $.service.db to do so.

[sergioeliot2039]

@js07 @dannyroosevelt this is ready for another review!

[sergioeliot2039]

@js07 this is ready for a new review! I addressed the older review comments that were on hidden conversations and the lastest from newest review round. Thanks

[sergioeliot2039]

Proceeding to squash and merge as this PR, its changes have been approved.