Skip to content

Add SlackCampaignSummaryMessage schema to track posted summaries#485

Draft
neal-bpm wants to merge 3 commits intomainfrom
vn/store-campaign-summary-messages
Draft

Add SlackCampaignSummaryMessage schema to track posted summaries#485
neal-bpm wants to merge 3 commits intomainfrom
vn/store-campaign-summary-messages

Conversation

@neal-bpm
Copy link
Copy Markdown
Contributor

#476

Describe your changes

Introduces a new schema and migration to store records of campaign summary messages posted to Slack. This enables tracking which campaigns have already had summaries sent.

Schema fields:

  • slack_channel_id: Target Slack channel
  • raw_message: The formatted message content
  • sent_at: When the message was posted
  • campaign_id: Reference to the associated campaign

The migration includes indexes on campaign_id and sent_at for efficient lookups when checking if a summary was already posted or querying by time range.

Checklist before requesting a review

  • I have performed a self-review of my code
  • If it is a core feature, I have added tests.
  • Are there other PRs or Issues that I should link to here?
  • Will this be part of a product update? If yes, please write one phrase
    about this update in the description above.

  #476

  Introduces a new schema and migration to store records of campaign summary messages posted to Slack. This enables tracking which campaigns have already had summaries sent.

  Schema fields:
  - slack_channel_id: Target Slack channel
  - raw_message: The formatted message content
  - sent_at: When the message was posted
  - campaign_id: Reference to the associated campaign

  The migration includes indexes on campaign_id and sent_at for efficient lookups when checking if a summary was already posted or querying by time range.
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a6e0ce82-452b-4e55-bb9c-5586559072d8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch vn/store-campaign-summary-messages

Comment @coderabbitai help to get the list of available commands and usage tips.

The "monday + week" combined filter test was failing because campaigns were created with delivery_start at 12:00 UTC, which is in the future when tests run before noon UTC.
The riders_latest_campaigns view excludes campaigns where delivery_start >= NOW(), so the rider's latest_campaign was NULL.
This caused the "week" filter to exclude the rider even though they had a valid Monday campaign.
Change the test helper to use 00:00:00 instead of 12:00:00 for delivery_start, ensuring campaigns are always in the past relative to test execution time.
Convert the existing campaign_id index to a unique constraint to support atomic inserts with on_conflict: :nothing.
    - Prevents duplicate campaign summaries
    - Enables race-condition-free inserts
end

defp create_campaign_for_date(date) do
datetime = DateTime.new!(date, ~T[12:00:00], "Etc/UTC")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whats up with this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The "monday + week" combined filter test was failing because campaigns were created with delivery_start at 12:00 UTC, which is in the future when tests run before noon UTC. To address the test, changed the campaign start window.

Copy link
Copy Markdown
Member

@mveytsman mveytsman left a comment

Choose a reason for hiding this comment

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

LGTM, though there's a stray change to a test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants