Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for multi-post messages #45

Merged
merged 8 commits into from
Nov 19, 2023

Conversation

ThorntonMatthewD
Copy link
Collaborator

@ThorntonMatthewD ThorntonMatthewD commented Oct 24, 2023

Fixes #11

Summary

Adds the ability to split events over multiple Slack messages to skirt around Slack's message size restrictions.

  • Each message can have a maximum text length of 3000 characters. This is 3/4 of Slack's recommended max of 4k. I wanted to play it safe, but am open to tweaking this. This should keep us well below the block count limit also.
  • If a new event is added after initial posts are made for the week that cause there to be a need for additional messages (called "spillover" in the code/comments), then for each Slack channel that doesn't have posts yet for the following week a new message is created. The events will still be sorted in their usual order and spread across the n+1 messages.
    • If a Slack channel DOES have messages for the following week posted already and this scenario is hit, then an error will be logged. The new event will not be posted to Slack. This is because there isn't a way to add messages before existing ones. Maybe in the future we can send out separate messages for any of these stragglers to let viewers know about them.
chunked_messages2.mp4

Testing

Step Instructions Expected Results
1 Download the following test data files: part_1.json part_2.json1 Test data files are downloaded to your project root.
2 Change https://events.openupstate.org/api/gtc on this line of code to the following: http://localhost:5544/events The URL for where to grab events from is updated.
3 Start json-server with the contents of part_1.json as the data source: npx json-server --port 5544 --watch part_1.json json-server is running. Navigating to http://localhost:5544/events should show all of the events from the JSON file.
4 Load up your local instance of the Slack bot You should see two messages appear in your subscribed channel(s). See the video attachment above for a sampling of roughly what you should see.
5 Shut down json-server. Reload it with part_2.json this time. Restart your Slack bot. You should see the second message having been updated, and a third one added, in your channel from before.
6 Change some of the value in part_2.json for events that are set to take place this week. Restart the Slack bot Your changes should take effect the messages that had been previously posted.
7 Delete (rm slack-events-bot.db) or move (mv slack-events-bot.db something-different.db) your sqlite DB. The DB is reset.
8 Repeat steps 3 and 4 Two new messages, with the same contents as before, have been posted into your Slack channel.
9 Add a message to your messages table with a week value that's the datetime of the next week.2 You should have a new message in your messages table for the next week.
10 Repeat step 5 You should see an error reported to STDERR.

1 curl commands for convenience:

curl -L https://raw.githubusercontent.com/ThorntonMatthewD/slack-events-bot/chunk-messages-test-samples/tests/data/part_1.json -o part_1.json

curl -L https://raw.githubusercontent.com/ThorntonMatthewD/slack-events-bot/chunk-messages-test-samples/tests/data/part_2.json -o part_2.json

2 Query for adding a new message for next week (at the time of posting this):

INSERT INTO messages (
  week, message, message_timestamp, channel_id, sequence_position
) VALUES (
  '2023-10-29 00:00:00+00:00',
  'I am here to be an impediment',
  '1698591600.954199',
  1,
  1
 )

Deployment Considerations

A new column has been added to the messages table. The following query will need to be executed.

ALTER TABLE messages ADD sequence_position INTEGER DEFAULT 0 NOT NULL;

@ThorntonMatthewD ThorntonMatthewD marked this pull request as ready for review October 25, 2023 02:24
Copy link
Member

@oliviasculley oliviasculley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, but we'll need to hold off on merging this until we can reinitialize the db on Jim's instance for a proper deploy

@ThorntonMatthewD
Copy link
Collaborator Author

This looks great, but we'll need to hold off on merging this until we can reinitialize the db on Jim's instance for a proper deploy

Thank you so much! 😄

@allella
Copy link
Member

allella commented Oct 27, 2023

If you want to drop some commands to run on the production app, then I can run them whenever.

@ThorntonMatthewD
Copy link
Collaborator Author

ThorntonMatthewD commented Oct 27, 2023

@allella Thank you!

Does the VM already have sqlite3 installed?

If not, and if it's running a Debian-based OS, then sudo apt install sqlite3 should take care of that.

Opening the SQLite console:

sqlite3 path/to/slack-events-bot.db

Queries to execute

-- Add the new sequence_position column to the messages table.
ALTER TABLE messages ADD sequence_position INTEGER DEFAULT 0 NOT NULL;

-- Backfill sequence_position values for messages created prior to this release.
UPDATE messages SET sequence_position = 0 WHERE sequence_position IS NULL;

After those queries are run then these set of changes should be good to deploy! I think watchtower should pick up on them automatically once they are merged.

@allella
Copy link
Member

allella commented Oct 31, 2023

Yes, we have SQLite3 installed.

It sounds like Olivia said we'd do this in person at the HG Labs event, but I'll double-check in the Slack.

@ThorntonMatthewD
Copy link
Collaborator Author

If this works with you all, I think next week may be an opportune time to deploy this piece. Something that may occur whenever these changes are shipped is that any weeks still in range could have their events posts "chunked", which could cause things to become slightly out of order (a post 2 of 2 being newly placed at the end of the channel). Since next week should be light on events due to Thanksgiving it should make it less jarring.

@allella
Copy link
Member

allella commented Nov 18, 2023

I can run the SQLite commands now, or early tomorrow, if that's the pre-requisite for the merge and deployment of the image.

@ThorntonMatthewD
Copy link
Collaborator Author

@allella Thank you! That would be wonderful. Adding the new column will not affect the bot as it stands currently, and we could then merge in these changes maybe Sunday morning? I think if it were merged prior then the second half of the events for the week of the 12th would be reposted.

@allella
Copy link
Member

allella commented Nov 19, 2023

@ThorntonMatthewD I've run the two SQL commands above.

@ThorntonMatthewD
Copy link
Collaborator Author

@allella Thank you! I will proceed with the merge. 🙌

@ThorntonMatthewD ThorntonMatthewD merged commit c1e5c43 into hackgvl:dev Nov 19, 2023
1 check passed
@ThorntonMatthewD ThorntonMatthewD deleted the chunk-messages branch November 19, 2023 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for multi-post messages
3 participants