grassrootsgrocery / admin-portal

GNU Affero General Public License v3.0
10 stars 5 forks source link

Make blueprint for "Send Driver Info" changed, causing an error in frontend #118

Closed mattsahn closed 1 year ago

mattsahn commented 1 year ago

This /api/messaging/locations-to-drivers-text GET message assumes that the text of the message that gets sent exists in a specific place in the Make scenario blueprint, but since it changed significantly, it is no longer there. And, there is no longer a 5th element, so the mapper() call errors out.

The frontend shows missing: image and the console and server logs will have an error like this:

[2023-08-12T21:14:32.342Z] ERROR: [200]: Cannot read properties of undefined (reading 'mapper') TypeError: Cannot read properties of undefined (reading 'mapper')
at /app/dist/routes/messaging.js:230:83
at step (/app/dist/routes/messaging.js:33:23)
at Object.next (/app/dist/routes/messaging.js:14:53)
at fulfilled (/app/dist/routes/messaging.js:5:58)
at runMicrotasks (<anonymous>)
at processTicksAndRejections (node:internal/process/task_queues:96:5)

code is here: https://github.com/grassrootsgrocery/admin-portal/blob/main/server/routes/messaging.ts#L177C1-L194C3

Since this is a fragile method, given Make scenarios can change, I think we need to reconsider pulling this directly from the Make scenario dynamically. Maybe hard code the text message or just reference the name of the Make scenario which is static/repliable in the blueprint.

/**
 * @description Get the "[NEW] Send Locations and POC details to volunteer drivers (only text, no email)" text message blueprint from Make
 * @route  GET /api/messaging/locations-to-drivers-text
 * @access
 */
router.route("/api/messaging/locations-to-drivers-text").get(
  protect,
  asyncHandler(async (req: Request, res: Response) => {
    const driverLocationInfoTextBlueprintId = 329564;
    const url = `https://us1.make.com/api/v2/scenarios/${driverLocationInfoTextBlueprintId}/blueprint`;
    const data = await makeRequest(
      url,
      res,
      "There was a problem fetching the '[NEW] Send Locations and POC details to volunteer drivers (only text, no email)' text."
    );
    res.status(OK).json(data.response.blueprint.flow[5].mapper.body);
  })
);
mattsahn commented 1 year ago

Re-opening since this broke again. Will need to do a permanent fix this time.

jasoncavanaugh commented 1 year ago

I remember looking over the Make API docs to try and find an endpoint that would give us the text message exactly, but I don't believe I found anything. We could look into it again, but it might not be worth the effort considering that the text message display isn't super critical. If the body of the text messages doesn't change that often, then maybe we should consider just hardcoding. Or maybe we could change the UI somehow to link to Make to show the specific automation being triggered. Open to hearing other people's thoughts.

mattsahn commented 1 year ago

@jasoncavanaugh / @hgreenhut - We could store a sample text message in a "note" in the Make scenario and reliably retrieve it regardless of how the scenario changes over time. And, the Make scenario editor could update it at will.

In Make, notes can be added anywhere like this: image

The notes always show up in the scenario json in the same place, response.blueprint.metadata.designer.notes, but there can be multiple notes so the order or id number aren't known. But, you could iterate/search all the notes' text and choose the one that begins with "(Grassroots Grocery)" as a convention, then return that one as the text to display in the app:

image

If someone hasn't specified a note, then the fallback could be to use response.blueprint.name which is just the actual scenario name: image

This would allow the text sample to be stored directly in Make with the scenario that's being used with a reasonable fallback display without having to hardcode anything. what do you think?

jasoncavanaugh commented 1 year ago

I think this is a good solution considering that it probably wouldn't be too difficult to implement. I guess we'll have to remember to update the note if the text message body ever changes.

mattsahn commented 1 year ago

Ok, i'm going to close this and start as a new issue enhancement