Remove unassigned deliveries section from campaign delivery summary#494
Merged
Remove unassigned deliveries section from campaign delivery summary#494
Conversation
The total count now reflects only assigned delivery tasks. - Remove `unassigned` field from CampaignDeliverySummary struct - Remove unassigned deliveries block from Slack message payloads - Update total count logic to only increment for assigned tasks - Remove related tests
📝 WalkthroughWalkthroughThe PR removes unassigned delivery tracking from the campaign delivery summary system. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/bike_brigade/delivery/campaign_delivery_summary.ex">
<violation number="1" location="lib/bike_brigade/delivery/campaign_delivery_summary.ex:50">
P2: `completed` can now include unassigned completed tasks while `total` excludes unassigned tasks, causing inconsistent campaign summary counts.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| def add_task(cds, task) do | ||
| cds | ||
| |> Map.update!(:total, &(&1 + 1)) | ||
| |> total_assigned_task(task) |
There was a problem hiding this comment.
P2: completed can now include unassigned completed tasks while total excludes unassigned tasks, causing inconsistent campaign summary counts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/bike_brigade/delivery/campaign_delivery_summary.ex, line 50:
<comment>`completed` can now include unassigned completed tasks while `total` excludes unassigned tasks, causing inconsistent campaign summary counts.</comment>
<file context>
@@ -50,7 +47,7 @@ defmodule BikeBrigade.Delivery.CampaignDeliverySummary do
def add_task(cds, task) do
cds
- |> Map.update!(:total, &(&1 + 1))
+ |> total_assigned_task(task)
|> completed(task)
|> delivery(task)
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/bike_brigade/slack_api/payload_builder.ex (1)
110-116: Avoid rendering a trailing divider when there are no rider blocks.This is a minor Slack formatting edge case; when
cds.assignedis empty, the divider is left hanging.Suggested refactor
- blocks = - [ - %{type: "section", text: %{type: "mrkdwn", text: header}}, - %{type: "section", text: %{type: "mrkdwn", text: summary}}, - %{type: "divider"} - ] ++ rider_blocks + blocks = + [ + %{type: "section", text: %{type: "mrkdwn", text: header}}, + %{type: "section", text: %{type: "mrkdwn", text: summary}} + ] ++ + if rider_blocks == [] do + [] + else + [%{type: "divider"} | rider_blocks] + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/bike_brigade/slack_api/payload_builder.ex` around lines 110 - 116, The blocks array is always adding a trailing divider before rider_blocks which leaves a hanging divider when cds.assigned (and thus rider_blocks) is empty; update the payload_builder logic that builds blocks so the divider is appended only when rider_blocks is non-empty (e.g., create base_blocks = [%{type: "section", text: %{type: "mrkdwn", text: header}}, %{type: "section", text: %{type: "mrkdwn", text: summary}}] and then if rider_blocks != [] do base_blocks ++ [%{type: "divider"}] ++ rider_blocks else use base_blocks), referencing the blocks and rider_blocks variables (and the source of rider_blocks from cds.assigned) to locate and change the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/bike_brigade/slack_api/payload_builder.ex`:
- Around line 110-116: The blocks array is always adding a trailing divider
before rider_blocks which leaves a hanging divider when cds.assigned (and thus
rider_blocks) is empty; update the payload_builder logic that builds blocks so
the divider is appended only when rider_blocks is non-empty (e.g., create
base_blocks = [%{type: "section", text: %{type: "mrkdwn", text: header}},
%{type: "section", text: %{type: "mrkdwn", text: summary}}] and then if
rider_blocks != [] do base_blocks ++ [%{type: "divider"}] ++ rider_blocks else
use base_blocks), referencing the blocks and rider_blocks variables (and the
source of rider_blocks from cds.assigned) to locate and change the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 11df28ef-f26e-461b-a91b-0f574d068630
📒 Files selected for processing (4)
lib/bike_brigade/delivery/campaign_delivery_summary.exlib/bike_brigade/slack_api/payload_builder.extest/bike_brigade/delivery/campaign_delivery_summary_test.exstest/bike_brigade/slack_api/payload_builder_test.exs
💤 Files with no reviewable changes (1)
- test/bike_brigade/slack_api/payload_builder_test.exs
github-project-automation
Bot
moved this from In Progress
to Done
in Bike Brigade all things tech
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes
Data Model (CampaignDeliverySummary)
Delivery Summary Logic
Slack Integration
Tests
Impact
Users will no longer see a separate "Unassigned Deliveries" section in campaign delivery summary Slack messages. The total delivery count reflects only assigned tasks, making the summary cleaner and more focused on actual rider assignments.
AI can make mistakes, so double-check responses
Checklist before requesting a review
about this update in the description above.
Summary by cubic
Removed the "Unassigned Deliveries" section from Slack campaign delivery summaries and stopped counting unassigned tasks in totals. The total now reflects only rider-assigned deliveries for a cleaner summary.
unassignedfromBikeBrigade.Delivery.CampaignDeliverySummaryand updated docs.total_assigned_task/2;add_task/2only incrementstotalfor assigned tasks.delivery/2to ignore unassigned tasks; removedappend/2and the unassigned block inPayloadBuilder.Written for commit 1bd488a. Summary will update on new commits.
Summary by CodeRabbit