[MM-69381] Add check/uncheck chip to playbook checklist items#9959
[MM-69381] Add check/uncheck chip to playbook checklist items#9959vish9812 wants to merge 5 commits into
Conversation
Documentation Impact Analysis — updates neededDocumentation Impact AnalysisOverall Assessment: Documentation Updates Recommended Changes SummaryThis PR adds a "task activity" feature to playbook run checklist items on mobile: when a task is checked or unchecked, a compact chip now appears on the item showing who performed the action and when (relative time), and the task detail bottom sheet gains a new section showing the actor name, relative time, and absolute timestamp. Internally, the server database schema advances to version 21 to persist timeline events on the Documentation Impact Details
Recommended Actions
ConfidenceMedium — The new
|
Coverage Comparison Report |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change persists playbook-run timeline events, derives checklist task activity and actors, and displays activity chips and detail rows with localized relative and absolute times. Checklist components now receive timeline events and military-time preferences, while friendly-date formatting supports narrow relative-time output. ChangesPlaybook run activity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant PlaybookRun
participant ChecklistList
participant Checklist
participant ChecklistItem
participant TaskActivityIndicator
PlaybookRun->>ChecklistList: pass timelineEvents
ChecklistList->>Checklist: forward timelineEvents
Checklist->>ChecklistItem: forward timelineEvents
ChecklistItem->>ChecklistItem: derive activity and activityActor
ChecklistItem->>TaskActivityIndicator: render activity chip
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/database/migration/server/index.test.ts (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply the required test naming convention consistently.
app/database/migration/server/index.test.ts#L9-L9: rename the case to start withshould.app/products/playbooks/database/models/playbook_run.test.ts#L28-L45: rename both cases to start withshould; use object cases and$labelto avoid the unused callback parameter.As per coding guidelines, tests should use
it('should...')naming and unused parameters must be removed completely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/database/migration/server/index.test.ts` at line 9, Rename the test in app/database/migration/server/index.test.ts:9 to start with “should”. In app/products/playbooks/database/models/playbook_run.test.ts:28-45, rename both cases to start with “should”, convert their cases to objects using $label for descriptions, and remove the unused callback parameter.Source: Coding guidelines
app/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity.test.ts (1)
37-102: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest titles don't follow the
it('should...')convention.None of the titles in this file (e.g., Line 37, Line 55, Line 86) start with "should". As per path instructions,
app/**/*.test.tsguideline states: "Tests should use it('should...') naming, not test('happy path')."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity.test.ts` around lines 37 - 102, Rename every test title in the checklist activity suite to follow the it('should...') convention, including the parameterized tests and collision cases. Preserve each test’s existing meaning and assertions while prefixing titles with clear “should” wording.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/products/playbooks/database/models/playbook_run.ts`:
- Around line 39-43: Update the TimelineEvent type guard around the event
validation to verify every required TimelineEvent property, including id,
creator_user_id, and playbook_run_id, alongside the existing checks before
narrowing the value. Ensure only objects satisfying the complete TimelineEvent
contract are returned as valid, or narrow the guard’s declared type if those
fields are not required.
In
`@app/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity_indicator.tsx`:
- Around line 119-176: The outer accessible View in the chip and detail-row
branches currently wraps tappable BaseChip or PressableOpacity content. Remove
that accessibility container behavior, place the accessibility label on the
non-interactive content, and assign the tappable chip/avatar its own appropriate
accessibility role and label while preserving the existing press handlers.
---
Nitpick comments:
In `@app/database/migration/server/index.test.ts`:
- Line 9: Rename the test in app/database/migration/server/index.test.ts:9 to
start with “should”. In
app/products/playbooks/database/models/playbook_run.test.ts:28-45, rename both
cases to start with “should”, convert their cases to objects using $label for
descriptions, and remove the unused callback parameter.
In
`@app/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity.test.ts`:
- Around line 37-102: Rename every test title in the checklist activity suite to
follow the it('should...') convention, including the parameterized tests and
collision cases. Preserve each test’s existing meaning and assertions while
prefixing titles with clear “should” wording.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e6b35a8-eaa9-47d0-b736-4e46a3ab092f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
app/components/friendly_date/friendly_date.test.tsxapp/components/friendly_date/index.tsxapp/database/migration/server/index.test.tsapp/database/migration/server/index.tsapp/database/schema/server/index.tsapp/database/schema/server/test.tsapp/products/playbooks/database/models/playbook_run.test.tsapp/products/playbooks/database/models/playbook_run.tsapp/products/playbooks/database/operators/transformers/index.test.tsapp/products/playbooks/database/operators/transformers/index.tsapp/products/playbooks/database/schema/playbook_run.tsapp/products/playbooks/screens/playbook_run/checklist/checklist.test.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item.test.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.test.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/checklist_item_bottom_sheet.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/index.test.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/checklist_item_bottom_sheet/index.tsapp/products/playbooks/screens/playbook_run/checklist/checklist_item/index.test.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/index.tsapp/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity.test.tsapp/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity.tsapp/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity_indicator.test.tsxapp/products/playbooks/screens/playbook_run/checklist/checklist_item/task_activity_indicator.tsxapp/products/playbooks/screens/playbook_run/checklist/index.test.tsxapp/products/playbooks/screens/playbook_run/checklist_list.test.tsxapp/products/playbooks/screens/playbook_run/checklist_list.tsxapp/products/playbooks/screens/playbook_run/playbook_run.test.tsxapp/products/playbooks/screens/playbook_run/playbook_run.tsxapp/products/playbooks/types/api.d.tsapp/products/playbooks/types/database/models/playbook_run.tstest/test_helper.ts
…ents Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Adds task activity support to playbook run checklist items:
task_activitymodel/schema field onPlaybookRun).Ticket Link
JIRA - https://mattermost.atlassian.net/browse/MM-69381
Related Web PR - mattermost/mattermost-plugin-playbooks#2307
Checklist
Device Information
This PR was tested on: ios and android.
Screenshots