Send reminder message to both #vc-events and optional second channel

[ebanner]

Effects

Fixes #23

Fixes https://github.com/Virtual-Coffee/cms.virtualcoffee/issues/11

Changes

For each event, instead of choosing between DEFAULT_SLACK_EVENT_CHANNEL and event.eventSlackAnnouncementsChannelId (and preferring event.eventSlackAnnouncementsChannelId over DEFAULT_SLACK_EVENT_CHANNEL), we create a message for DEFAULT_SLACK_EVENT_CHANNEL and then if event.eventSlackAnnouncementsChannelId is set, then we also create a message for that channel.

We apply the same logic for the hourlyAdminMessage to generate a list of channels it got posted to e.g.

Viewing the changes

The diff does not look nice, but here are links to the main changes:

Doing a flatMap over events (because for one event we may generate two messages for both channels)

Parameterizing the message create logic to accept a channel argument

Create a DEFAULT_SLACK_EVENT_CHANNEL message then optionally a event.eventSlackAnnouncementsChannelId message (if it's set)

Get channel list for channels field in hourlyAdminMessage

Set the channels field in hourlyAdminMessage

[danieltott]

@ebanner this is looking pretty dope. Thanks for your patience.

I have a couple changes I've requested - see the other comments. Nothing major, in general this is looking nice.

The diff does not look nice

A big part of that is that you have automated formatting in your code editor running, so it changed a lot of lines that didn't need changing. We need to add prettier in here, but in general for OSS projects, if they don't have prettier installed, it's important to not apply your own so that the diff doesn't fill up with a bunch of whitespace changes that aren't relevant to the issue.

[danieltott]

Two things here:

1. Adjust to make sure event.eventSlackAnnouncementsChannelId !== DEFAULT_SLACK_EVENT_CHANNEL before adding
2. Let's abstract this in to one announcementChannels const at the top of this process (maybe just inside the try {} inside of handler). Then here we can do channels.map(channel=>createMessage(channel) and don't have to do the if (event.eventSlackAnnouncementsChannelId) again other places.

[ebanner]

1. done

Regarding the second point, I can't really see a nicer way to do it than how it is now. My first thought was also to put an announcementChannels at the top of the process. But because this function processes a list of events (eventsList) (each which may have two channels), we have to put the announcementChannels inside the eventsList.map()/eventsList.flatMap(), and I'm not sure that's any nicer than it is now.

Let me know if I'm misunderstanding you.

[danieltott]

How about this:

At top inside of first try:

const announcementsChannels = [DEFAULT_SLACK_EVENT_CHANNEL];

if (event.eventSlackAnnouncementsChannelId) {
  announcementsChannels.push(event.eventSlackAnnouncementsChannelId);
}

inside of the flatMap call

- const messages = [createMessage(DEFAULT_SLACK_EVENT_CHANNEL)];
- 
- if (event.eventSlackAnnouncementsChannelId) {
-   messages.push(
-     createMessage(event.eventSlackAnnouncementsChannelId)
-   );
- }
+ const messages = announcementsChannels.map(createMessage);

Inside of the hourlyAdminMessage:

text: {
  type: 'mrkdwn',
  text:
    `*Announcement posted to:* ` +
-     channels.map((channel) => `<#${channel}>`).join(' '),
+     announcementsChannels.map((channel) => `<#${channel}>`).join(' '),
},

[ebanner]

By "at top inside the first try" I believe you mean inside the try but outside of the flatMap. However, that won't work because event isn't defined yet (I think you may be mixing up the event that is passed into the handler() and the event variable that gets defined inside the flatMap)?

[ebanner]

Also, how did you format those code blocks with the green and red diffs?

[danieltott]

I think you may be mixing up the event that is passed into the handler() and the event variable that gets defined inside the flatMap

...shit you're right 😂

Let's rename one of those while you're in there so I don't make that mistake again.

Also, how did you format those code blocks with the green and red diffs?

Use the diff "language" in the code fence, and add - and + where needed:

Here is some code
- I'm removing this line
+ And adding this line

```diff
Here is some code
- I'm removing this line
+ And adding this line
```

[ebanner]

1. Adjust to make sure event.eventSlackAnnouncementsChannelId !== DEFAULT_SLACK_EVENT_CHANNEL before adding

done 👍

[ebanner]

Let's rename one of those while you're in there so I don't make that mistake again.

done 👍

[danieltott]

Small tweak - let's make that join(', ') here and earlier where we use join(' ')

[ebanner]

done 👍