From 37784d3781f4868d40b3884ca9dfcc009c51b64a Mon Sep 17 00:00:00 2001 From: yashoza19 Date: Wed, 2 Sep 2026 18:46:33 -0400 Subject: [PATCH 1/2] fix(slack): harden M4 edit and regenerate flows (#22) --- src/status/db/confirm.py | 31 +++ src/status/db/draft.py | 163 ------------ src/status/db/edit.py | 172 ++++++++++++ src/status/skills/drafter.py | 10 +- src/status/slack/blocks.py | 300 +++++++++++++-------- src/status/slack/handlers.py | 469 +++++++++++++++++---------------- tests/test_slack_edit.py | 153 +++++++++++ tests/test_slack_regenerate.py | 72 +++++ 8 files changed, 869 insertions(+), 501 deletions(-) create mode 100644 src/status/db/edit.py create mode 100644 tests/test_slack_edit.py create mode 100644 tests/test_slack_regenerate.py diff --git a/src/status/db/confirm.py b/src/status/db/confirm.py index 6b8c6eb..55498bf 100644 --- a/src/status/db/confirm.py +++ b/src/status/db/confirm.py @@ -57,6 +57,37 @@ def latest_confirmed_week(session: Session, person_id: str) -> date | None: return session.scalars(stmt).first() +def record_regeneration( + session: Session, + person_id: str, + week_ending: date, + *, + reason: str, + notes: str | None = None, +) -> Participation: + """Record that the user regenerated their draft for this week.""" + now = datetime.now(timezone.utc) + row = session.get(Participation, (person_id, week_ending)) + if row is None: + row = Participation( + person_id=person_id, + week_ending=week_ending, + status="sent", + draft_sent_at=now, + ) + session.add(row) + + row.regenerated = True + reason_text = reason + if notes and notes.strip(): + reason_text = f"{reason}: {notes.strip()}" + row.regenerate_reason = reason_text + if notes and notes.strip(): + row.note = notes.strip() + session.flush() + return row + + def record_draft_sent(session: Session, person_id: str, week_ending: date) -> Participation: now = datetime.now(timezone.utc) row = session.get(Participation, (person_id, week_ending)) diff --git a/src/status/db/draft.py b/src/status/db/draft.py index 9c1630e..323a495 100644 --- a/src/status/db/draft.py +++ b/src/status/db/draft.py @@ -301,166 +301,3 @@ def get_current_drafts(session: Session, person_id: str, week_ending: date) -> l StatusEntry.confirmed_at.is_(None), ) return list(session.scalars(stmt).all()) - - -def persist_edited_entries( - session: Session, - person_id: str, - week_ending: date, - *, - edited_outcomes: dict[int, str], # index -> new outcome - dropped_indices: set[int], - unticketed_work: str | None, - leadership_asks: str | None, -) -> list[StatusEntry]: - """Create new 'drafted_edited' revisions for changed entries. - - Args: - session: Database session - person_id: Person ID - week_ending: Week ending date - edited_outcomes: Map of entry index to new outcome text - dropped_indices: Set of indices to remove - unticketed_work: Optional unticketed work description - leadership_asks: Optional leadership asks - - Returns: - List of newly created/updated entries - """ - # Get current drafts in order - access attributes BEFORE we supersede them - current_entries = get_current_drafts(session, person_id, week_ending) - if not current_entries: - return [] - - # Extract all data we need from current entries BEFORE modifying them - entry_data = [] - for entry in current_entries: - entry_data.append({ - 'epic_key': entry.epic_key, - 'epic_name_snapshot': entry.epic_name_snapshot, - 'project': entry.project, - 'state': entry.state, - 'outcome': entry.outcome, - 'blocker': entry.blocker, - 'ask': entry.ask, - 'draft_outcome': entry.draft_outcome, - 'confidence': entry.confidence, - 'prompt_version': entry.prompt_version, - 'evidence': entry.evidence, - 'extra': entry.extra, - 'revision': entry.revision, - 'entry_id': entry.entry_id, - 'drafted_at': entry.drafted_at, - }) - - # Supersede all current drafts using UPDATE for immediate database-level change - session.execute( - update(StatusEntry) - .where( - StatusEntry.person_id == person_id, - StatusEntry.week_ending == week_ending, - StatusEntry.is_current.is_(True), - StatusEntry.confirmed_at.is_(None), - ) - .values(is_current=False) - ) - # Flush immediately to ensure partial unique index is updated before inserts - session.flush() - - edited_at = datetime.now(timezone.utc) - new_entries: list[StatusEntry] = [] - - # Process each entry using the extracted data - for idx, data in enumerate(entry_data): - # Skip dropped entries - if idx in dropped_indices: - continue - - # Check if outcome was edited - new_outcome = edited_outcomes.get(idx) - if new_outcome and new_outcome.strip() != data['outcome'].strip(): - # Create edited revision - new_entry = StatusEntry( - week_ending=week_ending, - person_id=person_id, - epic_key=data['epic_key'], - epic_name_snapshot=data['epic_name_snapshot'], - project=data['project'], - state=data['state'], - outcome=new_outcome.strip(), - blocker=data['blocker'], - ask=leadership_asks if leadership_asks and leadership_asks.strip() else data['ask'], - draft_outcome=data['draft_outcome'], # Preserve original draft - source=EntrySource.DRAFTED_EDITED.value, - confidence=data['confidence'], - needs_human=False, # Human just reviewed it - prompt_version=data['prompt_version'], - evidence=data['evidence'], - extra=data['extra'], - revision=data['revision'] + 1, - supersedes_entry_id=data['entry_id'], - is_current=True, - drafted_at=data['drafted_at'], - confirmed_at=None, - ) - session.add(new_entry) - new_entries.append(new_entry) - else: - # No change, keep original entry as current but update asks if provided - # Re-create the entry with is_current=True - unchanged_entry = StatusEntry( - week_ending=week_ending, - person_id=person_id, - epic_key=data['epic_key'], - epic_name_snapshot=data['epic_name_snapshot'], - project=data['project'], - state=data['state'], - outcome=data['outcome'], - blocker=data['blocker'], - ask=leadership_asks.strip() if leadership_asks and leadership_asks.strip() else data['ask'], - draft_outcome=data['draft_outcome'], - source=EntrySource.DRAFTED.value, - confidence=data['confidence'], - needs_human=False, - prompt_version=data['prompt_version'], - evidence=data['evidence'], - extra=data['extra'], - revision=data['revision'], - supersedes_entry_id=None, - is_current=True, - drafted_at=data['drafted_at'], - confirmed_at=None, - ) - session.add(unchanged_entry) - new_entries.append(unchanged_entry) - - # Add unticketed work as a new entry if provided - if unticketed_work and unticketed_work.strip(): - unticketed_entry = StatusEntry( - week_ending=week_ending, - person_id=person_id, - epic_key=None, - epic_name_snapshot=None, - project="Unticketed", - state="progressing", - outcome=unticketed_work.strip(), - blocker=None, - ask=None, - draft_outcome=None, - source=EntrySource.HUMAN_WRITTEN.value, - confidence="high", - needs_human=False, - prompt_version="manual", - evidence=[], - extra={}, - revision=1, - supersedes_entry_id=None, - is_current=True, - drafted_at=edited_at, - confirmed_at=None, - ) - session.add(unticketed_entry) - new_entries.append(unticketed_entry) - - session.flush() - return new_entries diff --git a/src/status/db/edit.py b/src/status/db/edit.py new file mode 100644 index 0000000..9b9a7a1 --- /dev/null +++ b/src/status/db/edit.py @@ -0,0 +1,172 @@ +"""Ledger revisions for human-edited draft entries.""" + +from __future__ import annotations + +from datetime import date, datetime, timezone +from sqlalchemy.orm import Session + +from status.db.draft import get_current_drafts +from status.db.models import EntrySource, StatusEntry + + +class EditValidationError(ValueError): + """Raised when edit payload would violate ledger constraints.""" + + +def _supersede_entry(session: Session, entry: StatusEntry) -> None: + if not entry.is_current: + return + entry.is_current = False + session.flush() + + +def _clone_edited_entry( + entry: StatusEntry, + *, + outcome: str, + ask: str | None, + source: str, +) -> StatusEntry: + return StatusEntry( + week_ending=entry.week_ending, + person_id=entry.person_id, + epic_key=entry.epic_key, + epic_name_snapshot=entry.epic_name_snapshot, + project=entry.project, + state=entry.state, + outcome=outcome, + blocker=entry.blocker, + ask=ask, + draft_outcome=entry.draft_outcome, + source=source, + confidence=entry.confidence, + needs_human=False, + prompt_version=entry.prompt_version, + evidence=entry.evidence, + extra=entry.extra, + revision=entry.revision + 1, + supersedes_entry_id=entry.entry_id, + is_current=True, + drafted_at=entry.drafted_at, + confirmed_at=None, + ) + + +def persist_edited_entries( + session: Session, + person_id: str, + week_ending: date, + *, + edited_outcomes: dict[str, str], + unticketed_work: str | None, + existing_unticketed_entry_id: str | None, + leadership_asks: str | None, +) -> list[StatusEntry]: + """Apply per-entry edits without touching unchanged current rows.""" + current_entries = get_current_drafts(session, person_id, week_ending) + if not current_entries: + return [] + + by_id = {str(entry.entry_id): entry for entry in current_entries} + new_entries: list[StatusEntry] = [] + aggregate_ask = leadership_asks.strip() if leadership_asks and leadership_asks.strip() else None + aggregate_applied = False + + for entry_id, raw_outcome in edited_outcomes.items(): + entry = by_id.get(entry_id) + if entry is None or not entry.is_current: + continue + + outcome = raw_outcome.strip() + if not outcome: + _supersede_entry(session, entry) + continue + + if outcome == entry.outcome.strip(): + continue + + _supersede_entry(session, entry) + new_entry = _clone_edited_entry( + entry, + outcome=outcome, + ask=entry.ask, + source=EntrySource.DRAFTED_EDITED.value, + ) + session.add(new_entry) + new_entries.append(new_entry) + + if aggregate_ask and not aggregate_applied: + for entry in current_entries: + if not entry.is_current or entry.epic_key is None: + continue + if entry.ask == aggregate_ask: + continue + _supersede_entry(session, entry) + new_entry = _clone_edited_entry( + entry, + outcome=entry.outcome, + ask=aggregate_ask, + source=EntrySource.DRAFTED_EDITED.value, + ) + session.add(new_entry) + new_entries.append(new_entry) + break + + unticketed_text = unticketed_work.strip() if unticketed_work else "" + existing_unticketed: StatusEntry | None = None + if existing_unticketed_entry_id: + candidate = by_id.get(existing_unticketed_entry_id) + if candidate is not None and candidate.epic_key is None: + existing_unticketed = candidate + + if existing_unticketed is not None: + if not unticketed_text: + if existing_unticketed.is_current: + _supersede_entry(session, existing_unticketed) + elif unticketed_text != existing_unticketed.outcome.strip(): + if existing_unticketed.is_current: + _supersede_entry(session, existing_unticketed) + new_entries.append( + _add_unticketed_entry(session, person_id, week_ending, unticketed_text) + ) + elif unticketed_text: + new_entries.append(_add_unticketed_entry(session, person_id, week_ending, unticketed_text)) + + session.flush() + return new_entries + + +def _add_unticketed_entry( + session: Session, + person_id: str, + week_ending: date, + outcome: str, +) -> StatusEntry: + if not outcome.strip(): + raise EditValidationError("unticketed work outcome cannot be blank") + edited_at = datetime.now(timezone.utc) + row = StatusEntry( + week_ending=week_ending, + person_id=person_id, + epic_key=None, + epic_name_snapshot=None, + project="Unticketed", + state="progressing", + outcome=outcome.strip(), + blocker=None, + ask=None, + draft_outcome=None, + source=EntrySource.HUMAN_WRITTEN.value, + confidence="high", + needs_human=False, + prompt_version="manual", + evidence=[], + extra={}, + revision=1, + supersedes_entry_id=None, + is_current=True, + drafted_at=edited_at, + confirmed_at=None, + ) + session.add(row) + return row diff --git a/src/status/skills/drafter.py b/src/status/skills/drafter.py index d6cf04d..cc1702c 100644 --- a/src/status/skills/drafter.py +++ b/src/status/skills/drafter.py @@ -180,10 +180,18 @@ def run_drafter(payload: dict[str, Any], *, dry_run: bool = False) -> DraftOutpu version=settings.drafter_skill_version, ) + instruction = DRAFTER_INSTRUCTION + regeneration_notes = str(payload.get("regeneration_notes") or "").strip() + if regeneration_notes: + instruction = ( + f"{instruction}\n\nThe user asked to regenerate this draft with this guidance: " + f"{regeneration_notes}" + ) + last_error: SkillError | None = None for attempt in range(2): try: - result = client.invoke_json(skill, payload, DRAFTER_INSTRUCTION, DraftOutput) + result = client.invoke_json(skill, payload, instruction, DraftOutput) assert isinstance(result, DraftOutput) normalized = _normalize_draft(result, payload) labeled = attach_evidence_labels(normalized, payload) diff --git a/src/status/slack/blocks.py b/src/status/slack/blocks.py index 373a6b7..c0b0101 100644 --- a/src/status/slack/blocks.py +++ b/src/status/slack/blocks.py @@ -13,6 +13,17 @@ ACTION_EDIT = "status_edit" ACTION_REGENERATE = "status_regenerate" +# Slack allows at most 10 input blocks per modal view. +EDIT_MODAL_MAX_TICKETED = 8 # reserve two inputs for unticketed + leadership asks + +REGENERATE_REASON_LABELS: dict[str, str] = { + "missed_work": "Draft missed important work", + "wrong_grouping": "Epic grouping was wrong", + "inaccurate": "Outcomes were inaccurate", + "new_activity": "New Jira/GitHub activity since draft", + "other": "Other reason", +} + STATE_LABELS: dict[str, str] = { "shipped": "Shipped", "progressing": "In progress", @@ -171,156 +182,217 @@ def draft_fallback_text(display_name: str, week_ending: date, *, confirmed: bool return f"{prefix} status for {display_name}, week ending {week_ending.isoformat()}" +def _unticketed_prefill(entries: list[StatusEntry], flags: list[Flag]) -> str: + for entry in entries: + if entry.epic_key is None: + return entry.outcome + for flag in flags: + if flag.flag_type == "unticketed": + return flag.message + return "" + + +def _find_unticketed_entry(entries: list[StatusEntry]) -> StatusEntry | None: + for entry in entries: + if entry.epic_key is None: + return entry + return None + + def build_edit_modal( *, person_id: str, week_ending: date, entries: list[StatusEntry], + flags: list[Flag], + channel: str, + message_ts: str, + page_offset: int = 0, ) -> dict[str, Any]: """Build a Slack modal for editing draft status entries. - Slack modal constraints: - - Maximum ~100 blocks total - - Each input block counts as multiple blocks - - Limit to ~20 entries to stay under limit + One input block per ticketed epic (blank outcome removes the entry). + Slack allows at most 10 input blocks per modal. """ - week_label = week_ending.strftime("%b %d, %Y") + ticketed = [entry for entry in entries if entry.epic_key is not None] + page_entries = ticketed[page_offset : page_offset + EDIT_MODAL_MAX_TICKETED] + unticketed_entry = _find_unticketed_entry(entries) + unticketed_initial = _unticketed_prefill(entries, flags) - # Modal view structure - blocks: list[dict[str, Any]] = [] - - # Add entry fields (one per epic/project) - entries_to_show = entries[:20] # Slack modal limit + blocks: list[dict[str, Any]] = [ + { + "type": "section", + "text": { + "type": "mrkdwn", + "text": ( + f"*Week ending {week_ending.strftime('%b %d, %Y')}* — " + "edit outcomes below. Leave a field blank to remove that entry." + ), + }, + } + ] - for idx, entry in enumerate(entries_to_show): + entry_ids: list[str] = [] + for entry in page_entries: + entry_id = str(entry.entry_id) + entry_ids.append(entry_id) entry_title = _entry_title(entry) - block_id = f"entry_{idx}" - - # Text input for the outcome - blocks.append({ - "type": "input", - "block_id": f"{block_id}_outcome", - "label": { - "type": "plain_text", - "text": f"{entry_title} ({entry.state})"[:75], # Slack limit - }, - "element": { - "type": "plain_text_input", - "action_id": "outcome_value", - "multiline": True, - "initial_value": entry.outcome, - "placeholder": { + blocks.append( + { + "type": "input", + "block_id": f"entry_{entry_id}", + "label": { "type": "plain_text", - "text": "Describe what happened this week...", + "text": f"{entry_title} ({entry.state})"[:75], }, - }, - "optional": False, - }) + "element": { + "type": "plain_text_input", + "action_id": "outcome_value", + "multiline": True, + "initial_value": entry.outcome[:3000], + "placeholder": { + "type": "plain_text", + "text": "Describe what happened this week...", + }, + }, + "optional": True, + } + ) - # Checkbox to drop this entry - blocks.append({ - "type": "input", - "block_id": f"{block_id}_drop", - "label": { - "type": "plain_text", - "text": "Options", - }, - "element": { - "type": "checkboxes", - "action_id": "drop_entry", - "options": [ + if len(ticketed) > page_offset + len(page_entries): + remaining = len(ticketed) - page_offset - len(page_entries) + blocks.append( + { + "type": "context", + "elements": [ { - "text": {"type": "plain_text", "text": "Remove this entry"}, - "value": "drop", + "type": "mrkdwn", + "text": ( + f"_{remaining} more entr{'y' if remaining == 1 else 'ies'} on the next page. " + "Save, then click Edit again to continue._" + ), } ], - }, - "optional": True, - }) - - # Show count if we hit the limit - if len(entries) > 20: - blocks.append({ - "type": "context", - "elements": [ - { - "type": "mrkdwn", - "text": f"_Showing 20 of {len(entries)} entries. Edit the rest in a second pass._", - } - ], - }) + } + ) + elif page_offset > 0: + blocks.append( + { + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": f"_Editing entries {page_offset + 1}–{page_offset + len(page_entries)} of {len(ticketed)}._", + } + ], + } + ) - # Additional unticketed work field - blocks.append({ - "type": "input", - "block_id": "unticketed_work", - "label": { + unticketed_element: dict[str, Any] = { + "type": "plain_text_input", + "action_id": "unticketed_value", + "multiline": True, + "placeholder": { "type": "plain_text", - "text": "Additional work not listed above", + "text": "Meetings, reviews, or other work without a Jira ticket...", }, - "element": { - "type": "plain_text_input", - "action_id": "unticketed_value", - "multiline": True, - "placeholder": { + } + if unticketed_initial: + unticketed_element["initial_value"] = unticketed_initial[:3000] + + blocks.append( + { + "type": "input", + "block_id": "unticketed_work", + "label": { "type": "plain_text", - "text": "Meetings, reviews, or other work without a Jira ticket...", + "text": "Additional work not listed above", }, - }, - "optional": True, - }) - - # Leadership asks field - blocks.append({ - "type": "input", - "block_id": "leadership_asks", - "label": { - "type": "plain_text", - "text": "Asks for leadership", - }, - "element": { - "type": "plain_text_input", - "action_id": "asks_value", - "multiline": True, - "placeholder": { + "element": unticketed_element, + "optional": True, + } + ) + + blocks.append( + { + "type": "input", + "block_id": "leadership_asks", + "label": { "type": "plain_text", - "text": "Decisions needed, blockers requiring escalation...", + "text": "Ask for leadership (applies to first epic entry)", }, - }, - "optional": True, - }) + "element": { + "type": "plain_text_input", + "action_id": "asks_value", + "multiline": True, + "placeholder": { + "type": "plain_text", + "text": "Decisions needed, blockers requiring escalation...", + }, + }, + "optional": True, + } + ) - # Build the modal view - modal = { + return { "type": "modal", "callback_id": "edit_status_modal", - "private_metadata": json.dumps({ - "person_id": person_id, - "week_ending": week_ending.isoformat(), - "entry_count": len(entries_to_show), - }), - "title": { - "type": "plain_text", - "text": "Edit Status", - }, - "submit": { - "type": "plain_text", - "text": "Save Changes", - }, - "close": { - "type": "plain_text", - "text": "Cancel", - }, + "private_metadata": json.dumps( + { + "person_id": person_id, + "week_ending": week_ending.isoformat(), + "channel": channel, + "message_ts": message_ts, + "page_offset": page_offset, + "entry_ids": entry_ids, + "existing_unticketed_entry_id": ( + str(unticketed_entry.entry_id) if unticketed_entry is not None else None + ), + "total_ticketed": len(ticketed), + } + ), + "title": {"type": "plain_text", "text": "Edit Status"}, + "submit": {"type": "plain_text", "text": "Save Changes"}, + "close": {"type": "plain_text", "text": "Cancel"}, "blocks": blocks, } - return modal + +def parse_edit_submission_values( + values: dict[str, Any], + *, + entry_ids: list[str], +) -> tuple[dict[str, str], str | None, str | None]: + """Return edited outcomes, unticketed work, and leadership asks from modal state.""" + edited_outcomes: dict[str, str] = {} + for entry_id in entry_ids: + block = values.get(f"entry_{entry_id}") + if block is None: + continue + outcome = block["outcome_value"].get("value") + if outcome is None: + continue + edited_outcomes[entry_id] = outcome + + unticketed_work = None + unticketed_block = values.get("unticketed_work") + if unticketed_block: + unticketed_work = unticketed_block["unticketed_value"].get("value") + + leadership_asks = None + asks_block = values.get("leadership_asks") + if asks_block: + leadership_asks = asks_block["asks_value"].get("value") + + return edited_outcomes, unticketed_work, leadership_asks def build_regenerate_modal( *, person_id: str, week_ending: date, + channel: str, + message_ts: str, ) -> dict[str, Any]: """Build a Slack modal for regenerating draft status entries. @@ -332,6 +404,8 @@ def build_regenerate_modal( "private_metadata": json.dumps({ "person_id": person_id, "week_ending": week_ending.isoformat(), + "channel": channel, + "message_ts": message_ts, }), "title": { "type": "plain_text", diff --git a/src/status/slack/handlers.py b/src/status/slack/handlers.py index 4e7ecae..ae1f8f3 100644 --- a/src/status/slack/handlers.py +++ b/src/status/slack/handlers.py @@ -4,6 +4,7 @@ import json import logging +import threading from datetime import date from typing import Any @@ -15,23 +16,112 @@ get_unacknowledged_flags, latest_confirmed_week, latest_unconfirmed_week, + record_regeneration, ) -from status.db.draft import get_current_drafts, persist_edited_entries +from status.db.draft import get_current_drafts +from status.db.edit import EditValidationError, persist_edited_entries from status.db.repo import get_person +from status.db.models import Person from status.slack.blocks import ( ACTION_CONFIRM, ACTION_EDIT, ACTION_REGENERATE, + REGENERATE_REASON_LABELS, build_draft_blocks, build_edit_modal, build_regenerate_modal, draft_fallback_text, + parse_edit_submission_values, ) from status.slack.send import send_status_review log = logging.getLogger(__name__) +def _authorize_person(session: Any, person_id: str, slack_user_id: str) -> Person | None: + person = get_person(session, person_id) + if person is None: + return None + if person.slack_user_id and person.slack_user_id != slack_user_id: + return None + return person + + +def _post_dm(client: Any, slack_user_id: str, text: str) -> None: + dm_response = client.conversations_open(users=[slack_user_id]) + channel_id = dm_response["channel"]["id"] + client.chat_postMessage(channel=channel_id, text=text) + + +def _run_regenerate_background( + *, + person_id: str, + week_ending: date, + reason: str, + notes: str | None, + channel: str, + message_ts: str, + slack_user_id: str, + display_name: str, + client: Any, +) -> None: + try: + from status.collectors import run_collect + from status.skills.drafter import draft_and_persist + + with get_session() as session: + record_regeneration( + session, + person_id, + week_ending, + reason=reason, + notes=notes, + ) + session.commit() + + payload = run_collect(person_id, week_ending) + if notes and notes.strip(): + payload["regeneration_notes"] = notes.strip() + + result = draft_and_persist(payload, dry_run=False, persist=True) + log.info( + "regenerated draft for %s week %s: %s entries, superseded %s", + person_id, + week_ending, + len(result.persisted_entry_ids), + result.superseded_count, + ) + + _update_message( + client, + channel=channel, + ts=message_ts, + person_id=person_id, + display_name=display_name, + week_ending=week_ending, + confirmed=False, + ) + reason_text = REGENERATE_REASON_LABELS.get(reason, reason) + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text=( + f"Draft regenerated ({len(result.persisted_entry_ids)} entries). " + f"Reason: {reason_text}" + ), + ) + except Exception: + log.exception("regenerate failed for %s week %s", person_id, week_ending) + try: + _post_dm( + client, + slack_user_id, + "Could not regenerate your draft. Try again or use `/weekly-status`.", + ) + except Exception: + log.exception("could not send regenerate failure DM") + + def parse_action_value(raw: str) -> tuple[str, date]: data = json.loads(raw) person_id = str(data["person_id"]) @@ -131,167 +221,136 @@ def on_confirm(ack: Any, body: dict[str, Any], client: Any) -> None: @app.action(ACTION_EDIT) def on_edit(ack: Any, body: dict[str, Any], client: Any) -> None: - """Open the edit modal when user clicks Edit button. - - Critical: Must call views.open within 3 seconds of interaction. - """ + """Open the edit modal when user clicks Edit button.""" ack() - log.info("=== Edit button clicked ===") + slack_user_id = body["user"]["id"] + channel = body["channel"]["id"] + message_ts = body["message"]["ts"] try: action = body["actions"][0] person_id, week_ending = parse_action_value(action["value"]) trigger_id = body.get("trigger_id") - - log.info(f"Edit params: person={person_id}, week={week_ending}, has_trigger_id={bool(trigger_id)}") - if not trigger_id: - log.error("No trigger_id found in body!") client.chat_postEphemeral( - channel=body["channel"]["id"], - user=body["user"]["id"], + channel=channel, + user=slack_user_id, text="Missing trigger ID. Cannot open modal.", ) return - # Fetch entries - must be fast (< 3 seconds) - log.info("Querying database for current drafts...") with get_session() as session: - entries = get_current_drafts(session, person_id, week_ending) - - log.info(f"Found {len(entries)} draft entries") + person = _authorize_person(session, person_id, slack_user_id) + if person is None: + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text="Could not open edit — person record not found or not authorized.", + ) + return + entries = get_current_drafts(session, person_id, week_ending) if not entries: - log.warning("No draft entries to edit") client.chat_postEphemeral( - channel=body["channel"]["id"], - user=body["user"]["id"], + channel=channel, + user=slack_user_id, text="No draft entries found to edit.", ) return - # Build modal INSIDE the session so we can access entry attributes - log.info("Building modal...") + flags = get_unacknowledged_flags(session, person_id, week_ending) modal = build_edit_modal( person_id=person_id, week_ending=week_ending, entries=entries, + flags=flags, + channel=channel, + message_ts=message_ts, ) - log.info(f"Modal has {len(modal.get('blocks', []))} blocks") - log.info("Calling Slack views.open API...") - response = client.views_open(trigger_id=trigger_id, view=modal) + if not response.get("ok"): + log.error("views.open failed: %s", response.get("error")) - log.info(f"views.open response: ok={response.get('ok')}") - if not response.get('ok'): - log.error(f"Slack API error: {response.get('error')}") - - except Exception as e: - log.exception(f"Failed to open edit modal: {type(e).__name__}: {str(e)}") + except Exception: + log.exception("failed to open edit modal for %s", slack_user_id) client.chat_postEphemeral( - channel=body["channel"]["id"], - user=body["user"]["id"], - text=f"Error: {type(e).__name__}: {str(e)[:100]}", + channel=channel, + user=slack_user_id, + text="Could not open the edit modal. Try again in a moment.", ) @app.action(ACTION_REGENERATE) def on_regenerate(ack: Any, body: dict[str, Any], client: Any) -> None: """Open the regenerate modal when user clicks Regenerate button.""" ack() - log.info("=== Regenerate button clicked ===") + slack_user_id = body["user"]["id"] + channel = body["channel"]["id"] + message_ts = body["message"]["ts"] try: action = body["actions"][0] person_id, week_ending = parse_action_value(action["value"]) trigger_id = body.get("trigger_id") - - log.info(f"Regenerate request: person={person_id}, week={week_ending}") - if not trigger_id: - log.error("No trigger_id found!") client.chat_postEphemeral( - channel=body["channel"]["id"], - user=body["user"]["id"], + channel=channel, + user=slack_user_id, text="Missing trigger ID. Cannot open modal.", ) return - # Build and open regenerate modal + with get_session() as session: + if _authorize_person(session, person_id, slack_user_id) is None: + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text="Could not open regenerate — person record not found or not authorized.", + ) + return + modal = build_regenerate_modal( person_id=person_id, week_ending=week_ending, + channel=channel, + message_ts=message_ts, ) - response = client.views_open(trigger_id=trigger_id, view=modal) - log.info(f"Regenerate modal opened: ok={response.get('ok')}") + if not response.get("ok"): + log.error("regenerate views.open failed: %s", response.get("error")) - except Exception as e: - log.exception(f"Failed to open regenerate modal: {type(e).__name__}: {str(e)}") + except Exception: + log.exception("failed to open regenerate modal for %s", slack_user_id) client.chat_postEphemeral( - channel=body["channel"]["id"], - user=body["user"]["id"], - text=f"Error: {type(e).__name__}: {str(e)[:100]}", + channel=channel, + user=slack_user_id, + text="Could not open the regenerate modal. Try again in a moment.", ) @app.view("edit_status_modal") def handle_edit_submission(ack: Any, body: dict[str, Any], view: dict[str, Any], client: Any) -> None: """Handle modal submission when user saves edited status.""" ack() - log.info("=== Edit modal submitted ===") + slack_user_id = body["user"]["id"] + metadata = json.loads(view["private_metadata"]) + person_id = metadata["person_id"] + week_ending = date.fromisoformat(metadata["week_ending"]) + channel = metadata["channel"] + message_ts = metadata["message_ts"] + entry_ids: list[str] = metadata["entry_ids"] + existing_unticketed_entry_id = metadata.get("existing_unticketed_entry_id") try: - # Parse metadata - metadata = json.loads(view["private_metadata"]) - person_id = metadata["person_id"] - week_ending = date.fromisoformat(metadata["week_ending"]) - entry_count = metadata["entry_count"] - slack_user_id = body["user"]["id"] - - log.info(f"Processing edit submission: person={person_id}, week={week_ending}, entries={entry_count}") - - # Extract form values values = view["state"]["values"] - edited_outcomes: dict[int, str] = {} - dropped_indices: set[int] = set() - - for idx in range(entry_count): - # Get outcome - outcome_block = values.get(f"entry_{idx}_outcome") - if outcome_block: - outcome = outcome_block["outcome_value"]["value"] - if outcome: - edited_outcomes[idx] = outcome - - # Check if dropped - drop_block = values.get(f"entry_{idx}_drop") - if drop_block: - selected = drop_block["drop_entry"].get("selected_options", []) - if selected: - dropped_indices.add(idx) - log.info(f"Entry {idx} marked for deletion") - - # Get unticketed work and asks - unticketed_work = None - unticketed_block = values.get("unticketed_work") - if unticketed_block: - unticketed_work = unticketed_block["unticketed_value"].get("value") - if unticketed_work: - log.info("Unticketed work added") - - leadership_asks = None - asks_block = values.get("leadership_asks") - if asks_block: - leadership_asks = asks_block["asks_value"].get("value") - if leadership_asks: - log.info("Leadership asks added") - - # Persist changes to database - log.info("Persisting edited entries to database...") + edited_outcomes, unticketed_work, leadership_asks = parse_edit_submission_values( + values, + entry_ids=entry_ids, + ) + with get_session() as session: - person = get_person(session, person_id) - if not person: - log.warning("edit submission for unknown person %s", person_id) + person = _authorize_person(session, person_id, slack_user_id) + if person is None: + log.warning("edit submission for unauthorized person %s", person_id) return new_entries = persist_edited_entries( @@ -299,155 +358,117 @@ def handle_edit_submission(ack: Any, body: dict[str, Any], view: dict[str, Any], person_id, week_ending, edited_outcomes=edited_outcomes, - dropped_indices=dropped_indices, unticketed_work=unticketed_work, + existing_unticketed_entry_id=existing_unticketed_entry_id, leadership_asks=leadership_asks, ) session.commit() - display_name = person.display_name - log.info(f"Persisted {len(new_entries)} entries") - - # Send updated draft as a NEW message - # (We could update the original message if we tracked its ts, but new message is clearer) - log.info("Sending updated draft message...") - result = send_status_review( - person_id, - week_ending, - bot_token=bot_token, + _update_message( + client, + channel=channel, + ts=message_ts, + person_id=person_id, + display_name=display_name, + week_ending=week_ending, confirmed=False, ) - - log.info(f"Updated draft sent: channel={result.get('channel')}, ts={result.get('ts')}") - - # Send a regular message (not ephemeral) so user can see it - try: - dm_response = client.conversations_open(users=[slack_user_id]) - channel_id = dm_response["channel"]["id"] - - client.chat_postMessage( - channel=channel_id, - text=f"✅ *Changes saved!* {len(new_entries)} entries updated. See the new draft message above.", - ) - log.info("Confirmation message sent") - except Exception: - log.exception("Could not send confirmation message after edit") - - except Exception as e: - log.exception(f"Failed to process edit submission: {type(e).__name__}: {str(e)}") - # Modal is already closed, can't show error in modal - # Send DM with error + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text=f"Changes saved ({len(new_entries)} updated entries).", + ) + except EditValidationError as exc: + log.warning("edit validation failed for %s: %s", person_id, exc) + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text=f"Could not save changes: {exc}", + ) + except Exception: + log.exception("edit submission failed for %s week %s", person_id, week_ending) try: - dm_response = client.conversations_open(users=[slack_user_id]) - channel_id = dm_response["channel"]["id"] - client.chat_postMessage( - channel=channel_id, - text=f"❌ Error saving changes: {str(e)[:200]}", + _post_dm( + client, + slack_user_id, + "Could not save your edits. Try again or use `/weekly-status`.", ) except Exception: - log.exception("Could not send error message") + log.exception("could not send edit failure DM") @app.view("regenerate_status_modal") def handle_regenerate_submission(ack: Any, body: dict[str, Any], view: dict[str, Any], client: Any) -> None: """Handle modal submission when user regenerates status.""" ack() - log.info("=== Regenerate modal submitted ===") + slack_user_id = body["user"]["id"] + metadata = json.loads(view["private_metadata"]) + person_id = metadata["person_id"] + week_ending = date.fromisoformat(metadata["week_ending"]) + channel = metadata["channel"] + message_ts = metadata["message_ts"] + + values = view["state"]["values"] + reason = None + reason_block = values.get("regenerate_reason") + if reason_block: + selected = reason_block["reason_select"].get("selected_option") + if selected: + reason = selected["value"] + + notes = None + notes_block = values.get("regenerate_notes") + if notes_block: + notes = notes_block["notes_value"].get("value") + + if not reason: + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text="Select a reason before regenerating.", + ) + return try: - # Parse metadata - metadata = json.loads(view["private_metadata"]) - person_id = metadata["person_id"] - week_ending = date.fromisoformat(metadata["week_ending"]) - slack_user_id = body["user"]["id"] - - log.info(f"Processing regenerate request: person={person_id}, week={week_ending}") - - # Extract form values - values = view["state"]["values"] + with get_session() as session: + person = _authorize_person(session, person_id, slack_user_id) + if person is None: + log.warning("regenerate submission for unauthorized person %s", person_id) + return + display_name = person.display_name - # Get reason - reason_block = values.get("regenerate_reason") - reason = None - if reason_block: - selected = reason_block["reason_select"].get("selected_option") - if selected: - reason = selected["value"] - - # Get optional notes - notes = None - notes_block = values.get("regenerate_notes") - if notes_block: - notes = notes_block["notes_value"].get("value") - - log.info(f"Regenerate reason: {reason}, has_notes: {bool(notes)}") - - # Import collector and drafter functions - from status.collectors import run_collect - from status.skills.drafter import draft_and_persist - - # Re-collect data - log.info("Re-collecting activity data...") - payload = run_collect(person_id, week_ending) - - # If user provided notes, add to collection errors so drafter sees them - collection_errors = list(payload.get("collection_errors") or []) - if notes and notes.strip(): - collection_errors.append(f"User regeneration note: {notes.strip()}") - payload["collection_errors"] = collection_errors - - # Re-run drafter - log.info("Re-running drafter...") - result = draft_and_persist(payload, dry_run=False, persist=True) - - log.info(f"Regenerated {len(result.persisted_entry_ids)} entries, superseded {result.superseded_count}") - - # Send updated draft message - log.info("Sending regenerated draft message...") - from status.slack.send import send_status_review - send_result = send_status_review( - person_id, - week_ending, - bot_token=bot_token, - confirmed=False, + client.chat_postEphemeral( + channel=channel, + user=slack_user_id, + text="Regenerating your draft — this may take a minute...", ) - log.info(f"Regenerated draft sent: channel={send_result.get('channel')}, ts={send_result.get('ts')}") - - # Send confirmation message - try: - dm_response = client.conversations_open(users=[slack_user_id]) - channel_id = dm_response["channel"]["id"] - - reason_text = { - "missed_work": "Draft missed important work", - "wrong_grouping": "Epic grouping was wrong", - "inaccurate": "Outcomes were inaccurate", - "new_activity": "New Jira/GitHub activity since draft", - "other": "Other reason", - }.get(reason, "Unknown reason") - - client.chat_postMessage( - channel=channel_id, - text=f"✅ *Draft regenerated!* {len(result.persisted_entry_ids)} entries created from fresh data. Reason: {reason_text}", - ) - log.info("Confirmation message sent") - except Exception: - log.exception("Could not send confirmation message after regenerate") - - except Exception as e: - log.exception(f"Failed to process regenerate submission: {type(e).__name__}: {str(e)}") - # Modal is already closed, can't show error in modal - # Send DM with error + thread = threading.Thread( + target=_run_regenerate_background, + kwargs={ + "person_id": person_id, + "week_ending": week_ending, + "reason": reason, + "notes": notes, + "channel": channel, + "message_ts": message_ts, + "slack_user_id": slack_user_id, + "display_name": display_name, + "client": client, + }, + daemon=True, + ) + thread.start() + except Exception: + log.exception("regenerate submission failed for %s week %s", person_id, week_ending) try: - dm_response = client.conversations_open(users=[slack_user_id]) - channel_id = dm_response["channel"]["id"] - client.chat_postMessage( - channel=channel_id, - text=f"❌ Error regenerating draft: {str(e)[:200]}", + _post_dm( + client, + slack_user_id, + "Could not start regeneration. Try again in a moment.", ) except Exception: - log.exception("Could not send error message") + log.exception("could not send regenerate failure DM") @app.command("/weekly-status") def on_weekly_status_command(ack: Any, command: dict[str, Any], client: Any) -> None: diff --git a/tests/test_slack_edit.py b/tests/test_slack_edit.py new file mode 100644 index 0000000..3c88736 --- /dev/null +++ b/tests/test_slack_edit.py @@ -0,0 +1,153 @@ +from __future__ import annotations + +from datetime import date +from unittest.mock import MagicMock, patch +from uuid import uuid4 + +from status.db.edit import persist_edited_entries +from status.db.models import EntrySource, Flag, StatusEntry +from status.slack.blocks import ( + EDIT_MODAL_MAX_TICKETED, + build_edit_modal, + parse_edit_submission_values, +) + + +def _entry( + *, + epic_key: str | None = "EET-1", + outcome: str = "Original outcome.", + ask: str | None = None, +) -> StatusEntry: + return StatusEntry( + entry_id=uuid4(), + week_ending=date(2026, 8, 14), + person_id="pilot", + epic_key=epic_key, + epic_name_snapshot="Pipeline" if epic_key else None, + project="EET", + state="progressing", + outcome=outcome, + ask=ask, + source=EntrySource.DRAFTED.value, + is_current=True, + revision=1, + ) + + +def test_build_edit_modal_respects_slack_input_block_limit() -> None: + entries = [_entry(epic_key=f"EET-{idx}") for idx in range(EDIT_MODAL_MAX_TICKETED + 4)] + modal = build_edit_modal( + person_id="pilot", + week_ending=date(2026, 8, 14), + entries=entries, + flags=[], + channel="C123", + message_ts="1234.5678", + ) + input_blocks = [block for block in modal["blocks"] if block["type"] == "input"] + assert len(input_blocks) <= 10 + assert len(input_blocks) == EDIT_MODAL_MAX_TICKETED + 2 + + +def test_build_edit_modal_prefills_unticketed_from_flag() -> None: + flag = Flag( + flag_id=uuid4(), + week_ending=date(2026, 8, 14), + person_id="pilot", + flag_type="unticketed", + message="Meetings and design reviews.", + acknowledged=False, + ) + modal = build_edit_modal( + person_id="pilot", + week_ending=date(2026, 8, 14), + entries=[_entry()], + flags=[flag], + channel="C123", + message_ts="1234.5678", + ) + unticketed_block = next( + block for block in modal["blocks"] if block.get("block_id") == "unticketed_work" + ) + assert unticketed_block["element"]["initial_value"] == "Meetings and design reviews." + + +def test_parse_edit_submission_values_maps_entry_ids() -> None: + entry_id = str(uuid4()) + values = { + f"entry_{entry_id}": {"outcome_value": {"value": "Updated outcome."}}, + "unticketed_work": {"unticketed_value": {"value": "Side project"}}, + "leadership_asks": {"asks_value": {"value": "Need a decision"}}, + } + edited, unticketed, asks = parse_edit_submission_values(values, entry_ids=[entry_id]) + assert edited[entry_id] == "Updated outcome." + assert unticketed == "Side project" + assert asks == "Need a decision" + + +def test_persist_edited_entries_creates_drafted_edited_revision() -> None: + entry = _entry() + session = MagicMock() + session.flush = MagicMock() + + with patch("status.db.edit.get_current_drafts", return_value=[entry]): + new_rows = persist_edited_entries( + session, + "pilot", + date(2026, 8, 14), + edited_outcomes={str(entry.entry_id): "Edited outcome."}, + unticketed_work=None, + existing_unticketed_entry_id=None, + leadership_asks=None, + ) + + assert entry.is_current is False + assert len(new_rows) == 1 + assert new_rows[0].source == EntrySource.DRAFTED_EDITED.value + assert new_rows[0].outcome == "Edited outcome." + assert new_rows[0].supersedes_entry_id == entry.entry_id + assert new_rows[0].revision == 2 + session.add.assert_called_once() + + +def test_persist_edited_entries_drop_epic_supersedes_without_reinsert() -> None: + entry = _entry() + session = MagicMock() + session.flush = MagicMock() + + with patch("status.db.edit.get_current_drafts", return_value=[entry]): + new_rows = persist_edited_entries( + session, + "pilot", + date(2026, 8, 14), + edited_outcomes={str(entry.entry_id): ""}, + unticketed_work=None, + existing_unticketed_entry_id=None, + leadership_asks=None, + ) + + assert entry.is_current is False + assert new_rows == [] + session.add.assert_not_called() + + +def test_persist_edited_entries_leaves_unchanged_rows_current() -> None: + entry = _entry() + session = MagicMock() + session.flush = MagicMock() + + with patch("status.db.edit.get_current_drafts", return_value=[entry]): + new_rows = persist_edited_entries( + session, + "pilot", + date(2026, 8, 14), + edited_outcomes={str(entry.entry_id): entry.outcome}, + unticketed_work=None, + existing_unticketed_entry_id=None, + leadership_asks=None, + ) + + assert entry.is_current is True + assert new_rows == [] + session.add.assert_not_called() diff --git a/tests/test_slack_regenerate.py b/tests/test_slack_regenerate.py new file mode 100644 index 0000000..8e2bcdb --- /dev/null +++ b/tests/test_slack_regenerate.py @@ -0,0 +1,72 @@ +from __future__ import annotations + +from datetime import date +from unittest.mock import MagicMock, patch + +from status.db.confirm import record_regeneration +from status.db.models import Participation +from status.slack.blocks import build_regenerate_modal + + +def test_build_regenerate_modal_includes_message_metadata() -> None: + modal = build_regenerate_modal( + person_id="pilot", + week_ending=date(2026, 8, 14), + channel="C123", + message_ts="1234.5678", + ) + metadata = modal["private_metadata"] + assert '"channel": "C123"' in metadata + assert '"message_ts": "1234.5678"' in metadata + + +def test_record_regeneration_sets_participation_fields() -> None: + session = MagicMock() + session.get.return_value = None + + row = record_regeneration( + session, + "pilot", + date(2026, 8, 14), + reason="missed_work", + notes="Include the cluster bot work.", + ) + + assert isinstance(row, Participation) + assert row.regenerated is True + assert row.regenerate_reason == "missed_work: Include the cluster bot work." + assert row.note == "Include the cluster bot work." + session.add.assert_called_once() + + +def test_run_regenerate_background_updates_message_and_records_participation() -> None: + from status.slack.handlers import _run_regenerate_background + + client = MagicMock() + draft_result = MagicMock(persisted_entry_ids=["a"], superseded_count=2) + + with ( + patch("status.slack.handlers.get_session") as mock_session_ctx, + patch("status.slack.handlers.record_regeneration") as mock_record, + patch("status.collectors.run_collect", return_value={"person": "pilot", "week_end": "2026-08-14"}), + patch("status.skills.drafter.draft_and_persist", return_value=draft_result), + patch("status.slack.handlers._update_message") as mock_update, + ): + session = MagicMock() + mock_session_ctx.return_value.__enter__.return_value = session + + _run_regenerate_background( + person_id="pilot", + week_ending=date(2026, 8, 14), + reason="wrong_grouping", + notes="Group by initiative", + channel="C123", + message_ts="1234.5678", + slack_user_id="U123", + display_name="Pilot User", + client=client, + ) + + mock_record.assert_called_once() + mock_update.assert_called_once() + client.chat_postEphemeral.assert_called_once() From bf6e5af556061065631504f6c95236464ca831d5 Mon Sep 17 00:00:00 2001 From: yashoza19 Date: Wed, 9 Sep 2026 13:55:03 -0400 Subject: [PATCH 2/2] fix(slack): harden edit modal drops and simplify missed-work field. Treat cleared Slack inputs as epic removals, remove the leadership-asks field, and relabel unticketed work so engineers can add missed activity. Co-authored-by: Cursor --- src/status/db/edit.py | 20 ------------ src/status/slack/blocks.py | 49 +++++++--------------------- src/status/slack/handlers.py | 20 ++++++++++-- tests/test_slack_edit.py | 62 ++++++++++++++++++++++++++++++++---- 4 files changed, 84 insertions(+), 67 deletions(-) diff --git a/src/status/db/edit.py b/src/status/db/edit.py index 9b9a7a1..c2a6c9b 100644 --- a/src/status/db/edit.py +++ b/src/status/db/edit.py @@ -60,7 +60,6 @@ def persist_edited_entries( edited_outcomes: dict[str, str], unticketed_work: str | None, existing_unticketed_entry_id: str | None, - leadership_asks: str | None, ) -> list[StatusEntry]: """Apply per-entry edits without touching unchanged current rows.""" current_entries = get_current_drafts(session, person_id, week_ending) @@ -69,8 +68,6 @@ def persist_edited_entries( by_id = {str(entry.entry_id): entry for entry in current_entries} new_entries: list[StatusEntry] = [] - aggregate_ask = leadership_asks.strip() if leadership_asks and leadership_asks.strip() else None - aggregate_applied = False for entry_id, raw_outcome in edited_outcomes.items(): entry = by_id.get(entry_id) @@ -95,23 +92,6 @@ def persist_edited_entries( session.add(new_entry) new_entries.append(new_entry) - if aggregate_ask and not aggregate_applied: - for entry in current_entries: - if not entry.is_current or entry.epic_key is None: - continue - if entry.ask == aggregate_ask: - continue - _supersede_entry(session, entry) - new_entry = _clone_edited_entry( - entry, - outcome=entry.outcome, - ask=aggregate_ask, - source=EntrySource.DRAFTED_EDITED.value, - ) - session.add(new_entry) - new_entries.append(new_entry) - break - unticketed_text = unticketed_work.strip() if unticketed_work else "" existing_unticketed: StatusEntry | None = None if existing_unticketed_entry_id: diff --git a/src/status/slack/blocks.py b/src/status/slack/blocks.py index c0b0101..a6d4d3e 100644 --- a/src/status/slack/blocks.py +++ b/src/status/slack/blocks.py @@ -14,7 +14,7 @@ ACTION_REGENERATE = "status_regenerate" # Slack allows at most 10 input blocks per modal view. -EDIT_MODAL_MAX_TICKETED = 8 # reserve two inputs for unticketed + leadership asks +EDIT_MODAL_MAX_TICKETED = 9 # reserve one input for missed/additional work REGENERATE_REASON_LABELS: dict[str, str] = { "missed_work": "Draft missed important work", @@ -226,7 +226,8 @@ def build_edit_modal( "type": "mrkdwn", "text": ( f"*Week ending {week_ending.strftime('%b %d, %Y')}* — " - "edit outcomes below. Leave a field blank to remove that entry." + "edit outcomes below. Leave a field blank to remove that entry. " + "Use the field at the bottom to add work the draft missed." ), }, } @@ -294,7 +295,7 @@ def build_edit_modal( "multiline": True, "placeholder": { "type": "plain_text", - "text": "Meetings, reviews, or other work without a Jira ticket...", + "text": "Meetings, side projects, epics not listed above — add Jira links if you have them", }, } if unticketed_initial: @@ -306,34 +307,13 @@ def build_edit_modal( "block_id": "unticketed_work", "label": { "type": "plain_text", - "text": "Additional work not listed above", + "text": "Missed or additional work this week", }, "element": unticketed_element, "optional": True, } ) - blocks.append( - { - "type": "input", - "block_id": "leadership_asks", - "label": { - "type": "plain_text", - "text": "Ask for leadership (applies to first epic entry)", - }, - "element": { - "type": "plain_text_input", - "action_id": "asks_value", - "multiline": True, - "placeholder": { - "type": "plain_text", - "text": "Decisions needed, blockers requiring escalation...", - }, - }, - "optional": True, - } - ) - return { "type": "modal", "callback_id": "edit_status_modal", @@ -362,29 +342,24 @@ def parse_edit_submission_values( values: dict[str, Any], *, entry_ids: list[str], -) -> tuple[dict[str, str], str | None, str | None]: - """Return edited outcomes, unticketed work, and leadership asks from modal state.""" +) -> tuple[dict[str, str], str | None]: + """Return edited outcomes and missed/additional work from modal state.""" edited_outcomes: dict[str, str] = {} for entry_id in entry_ids: block = values.get(f"entry_{entry_id}") if block is None: + # Optional inputs cleared in Slack may omit the whole block from state.values. + edited_outcomes[entry_id] = "" continue - outcome = block["outcome_value"].get("value") - if outcome is None: - continue - edited_outcomes[entry_id] = outcome + # Cleared optional inputs may omit "value" or send null. + edited_outcomes[entry_id] = block["outcome_value"].get("value") or "" unticketed_work = None unticketed_block = values.get("unticketed_work") if unticketed_block: unticketed_work = unticketed_block["unticketed_value"].get("value") - leadership_asks = None - asks_block = values.get("leadership_asks") - if asks_block: - leadership_asks = asks_block["asks_value"].get("value") - - return edited_outcomes, unticketed_work, leadership_asks + return edited_outcomes, unticketed_work def build_regenerate_modal( diff --git a/src/status/slack/handlers.py b/src/status/slack/handlers.py index ae1f8f3..df7333a 100644 --- a/src/status/slack/handlers.py +++ b/src/status/slack/handlers.py @@ -342,10 +342,17 @@ def handle_edit_submission(ack: Any, body: dict[str, Any], view: dict[str, Any], try: values = view["state"]["values"] - edited_outcomes, unticketed_work, leadership_asks = parse_edit_submission_values( + edited_outcomes, unticketed_work = parse_edit_submission_values( values, entry_ids=entry_ids, ) + log.info( + "edit submission for %s week %s: %s field(s), %s cleared", + person_id, + week_ending, + len(edited_outcomes), + sum(1 for text in edited_outcomes.values() if not text.strip()), + ) with get_session() as session: person = _authorize_person(session, person_id, slack_user_id) @@ -360,7 +367,6 @@ def handle_edit_submission(ack: Any, body: dict[str, Any], view: dict[str, Any], edited_outcomes=edited_outcomes, unticketed_work=unticketed_work, existing_unticketed_entry_id=existing_unticketed_entry_id, - leadership_asks=leadership_asks, ) session.commit() display_name = person.display_name @@ -374,10 +380,18 @@ def handle_edit_submission(ack: Any, body: dict[str, Any], view: dict[str, Any], week_ending=week_ending, confirmed=False, ) + dropped = sum(1 for text in edited_outcomes.values() if not text.strip()) + updated = len(new_entries) + parts: list[str] = [] + if updated: + parts.append(f"{updated} updated") + if dropped: + parts.append(f"{dropped} removed") + detail = f" ({', '.join(parts)})" if parts else "" client.chat_postEphemeral( channel=channel, user=slack_user_id, - text=f"Changes saved ({len(new_entries)} updated entries).", + text=f"Changes saved{detail}.", ) except EditValidationError as exc: log.warning("edit validation failed for %s: %s", person_id, exc) diff --git a/tests/test_slack_edit.py b/tests/test_slack_edit.py index 3c88736..1516e75 100644 --- a/tests/test_slack_edit.py +++ b/tests/test_slack_edit.py @@ -47,7 +47,7 @@ def test_build_edit_modal_respects_slack_input_block_limit() -> None: ) input_blocks = [block for block in modal["blocks"] if block["type"] == "input"] assert len(input_blocks) <= 10 - assert len(input_blocks) == EDIT_MODAL_MAX_TICKETED + 2 + assert len(input_blocks) == EDIT_MODAL_MAX_TICKETED + 1 def test_build_edit_modal_prefills_unticketed_from_flag() -> None: @@ -73,17 +73,46 @@ def test_build_edit_modal_prefills_unticketed_from_flag() -> None: assert unticketed_block["element"]["initial_value"] == "Meetings and design reviews." +def test_parse_edit_submission_values_treats_cleared_field_as_drop() -> None: + entry_id = str(uuid4()) + values = { + f"entry_{entry_id}": {"outcome_value": {}}, + } + edited, _ = parse_edit_submission_values(values, entry_ids=[entry_id]) + assert edited[entry_id] == "" + + +def test_parse_edit_submission_values_treats_missing_block_as_drop() -> None: + entry_id = str(uuid4()) + edited, _ = parse_edit_submission_values({}, entry_ids=[entry_id]) + assert edited[entry_id] == "" + + def test_parse_edit_submission_values_maps_entry_ids() -> None: entry_id = str(uuid4()) values = { f"entry_{entry_id}": {"outcome_value": {"value": "Updated outcome."}}, "unticketed_work": {"unticketed_value": {"value": "Side project"}}, - "leadership_asks": {"asks_value": {"value": "Need a decision"}}, } - edited, unticketed, asks = parse_edit_submission_values(values, entry_ids=[entry_id]) + edited, unticketed = parse_edit_submission_values(values, entry_ids=[entry_id]) assert edited[entry_id] == "Updated outcome." assert unticketed == "Side project" - assert asks == "Need a decision" + + +def test_build_edit_modal_has_missed_work_field() -> None: + modal = build_edit_modal( + person_id="pilot", + week_ending=date(2026, 8, 14), + entries=[_entry()], + flags=[], + channel="C123", + message_ts="1234.5678", + ) + missed_block = next( + block for block in modal["blocks"] if block.get("block_id") == "unticketed_work" + ) + assert missed_block["label"]["text"] == "Missed or additional work this week" + assert "leadership_asks" not in str(modal) def test_persist_edited_entries_creates_drafted_edited_revision() -> None: @@ -99,7 +128,6 @@ def test_persist_edited_entries_creates_drafted_edited_revision() -> None: edited_outcomes={str(entry.entry_id): "Edited outcome."}, unticketed_work=None, existing_unticketed_entry_id=None, - leadership_asks=None, ) assert entry.is_current is False @@ -124,7 +152,6 @@ def test_persist_edited_entries_drop_epic_supersedes_without_reinsert() -> None: edited_outcomes={str(entry.entry_id): ""}, unticketed_work=None, existing_unticketed_entry_id=None, - leadership_asks=None, ) assert entry.is_current is False @@ -132,6 +159,28 @@ def test_persist_edited_entries_drop_epic_supersedes_without_reinsert() -> None: session.add.assert_not_called() +def test_persist_edited_entries_adds_missed_work() -> None: + entry = _entry() + session = MagicMock() + session.flush = MagicMock() + + with patch("status.db.edit.get_current_drafts", return_value=[entry]): + new_rows = persist_edited_entries( + session, + "pilot", + date(2026, 8, 14), + edited_outcomes={str(entry.entry_id): entry.outcome}, + unticketed_work="Partner sync and design review.", + existing_unticketed_entry_id=None, + ) + + assert len(new_rows) == 1 + assert new_rows[0].epic_key is None + assert new_rows[0].outcome == "Partner sync and design review." + assert new_rows[0].source == EntrySource.HUMAN_WRITTEN.value + session.add.assert_called_once() + + def test_persist_edited_entries_leaves_unchanged_rows_current() -> None: entry = _entry() session = MagicMock() @@ -145,7 +194,6 @@ def test_persist_edited_entries_leaves_unchanged_rows_current() -> None: edited_outcomes={str(entry.entry_id): entry.outcome}, unticketed_work=None, existing_unticketed_entry_id=None, - leadership_asks=None, ) assert entry.is_current is True