Skip to content

AIT-385: dedupe alert phone commands — alerts phone is the one surface - #54

Merged
ord669 merged 2 commits into
mainfrom
ait-385-dedupe-phone-command
Aug 11, 2026
Merged

AIT-385: dedupe alert phone commands — alerts phone is the one surface#54
ord669 merged 2 commits into
mainfrom
ait-385-dedupe-phone-command

Conversation

@ord669

@ord669 ord669 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes AIT-385

CLI main registered two parallel alert-phone surfaces. The top-level hookmyapp phone (never published to npm) still shipped per-category consent flags (--no-operational/--product/--marketing) from before the consent model changed, duplicating alerts phone wholesale.

  • Delete src/commands/phone.ts and its registration; alerts phone {status,set,verify,remove} is the one surface.
  • Port the opt-out path as alerts phone consents (PATCH /auth/phone/consents), same flags and local validation.
  • Drop the stale product?/marketing? fields from the alerts phone set action type.
  • Tests: consents cases moved to alerts.test.ts; phone.test.ts becomes org-profile.test.ts (org profile tests kept unchanged).

1092 tests green, tsc clean. No release yet — rides the pending v0.14.12 tag, so the stale flags never reach npm.

Summary by CodeRabbit

  • New Features

    • Added an alerts phone consents command for updating operational, product, and marketing alert preferences.
    • Supports delivery-channel preferences with validation and table or JSON output.
    • Added usage examples for managing alert phone consent settings.
  • Changes

    • Removed the standalone phone verification and status command. Phone-related consent management is now handled through the alerts command.
  • Documentation

    • Updated organization profile guidance to reference the alerts command for personal alert phone settings.

Delete the top-level `hookmyapp phone` command (never published): its set
still carried per-category consent flags from before the consent model
changed, and it duplicated `alerts phone` wholesale. The consents
subcommand survives as `alerts phone consents` (the opt-out path), and the
stale consent fields in the alerts set action type are gone.
@ord669

ord669 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c3047dec-6c34-480f-a985-01ba6e1d78c4

📥 Commits

Reviewing files that changed from the base of the PR and between 82324f5 and b3c6c18.

📒 Files selected for processing (1)
  • src/commands/org-profile.ts

📝 Walkthrough

Walkthrough

The CLI adds alerts phone consents for consent and delivery updates, adds organization profile tests, and removes the standalone phone command and its CLI registration.

Changes

Alert phone CLI changes

Layer / File(s) Summary
Consent command implementation
src/commands/alerts.ts
Adds alertPhoneConsents, consent and delivery validation, PATCH requests, output formatting, command registration, and examples.
Consent command validation
src/commands/__tests__/alerts.test.ts
Tests selective payloads, invalid values, and empty updates without API calls.
Profile coverage and command wiring
src/commands/__tests__/org-profile.test.ts, src/index.ts, src/commands/org-profile.ts
Tests organization profile retrieval and updates. Removes standalone phone command registration. Updates profile references to hookmyapp alerts phone.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant alertPhoneConsents
  participant APIClient
  CLI->>alertPhoneConsents: provide consent and delivery flags
  alertPhoneConsents->>APIClient: PATCH /auth/phone/consents
  APIClient-->>alertPhoneConsents: return consent status
  alertPhoneConsents-->>CLI: render JSON or table output
Loading

Possibly related PRs

Suggested reviewers: ordvir

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: removing duplicate alert phone commands and consolidating them under the alerts phone surface.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ait-385-dedupe-phone-command

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@src/commands/__tests__/org-profile.test.ts`:
- Around line 44-51: Update the profile help text configured by
registerOrgProfileCommand to reference hookmyapp alerts phone instead of the
removed hookmyapp phone command, and add a regression test in the org profile
command tests that verifies the generated help output contains the corrected
guidance.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 71fb6f50-b4ab-4eeb-9065-c7b7b18da772

📥 Commits

Reviewing files that changed from the base of the PR and between 69d3fb3 and 82324f5.

📒 Files selected for processing (6)
  • src/commands/__tests__/alerts.test.ts
  • src/commands/__tests__/org-profile.test.ts
  • src/commands/__tests__/phone.test.ts
  • src/commands/alerts.ts
  • src/commands/phone.ts
  • src/index.ts
💤 Files with no reviewable changes (3)
  • src/index.ts
  • src/commands/tests/phone.test.ts
  • src/commands/phone.ts

Comment thread src/commands/__tests__/org-profile.test.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82324f5192

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/index.ts
@ord669

ord669 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: b3c6c18211

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@ord669
ord669 merged commit cd2a6d6 into main Aug 11, 2026
3 checks passed
@ord669
ord669 deleted the ait-385-dedupe-phone-command branch August 11, 2026 15:04
ord669 added a commit that referenced this pull request Aug 12, 2026
AIT-385: dedupe alert phone commands — alerts phone is the one surface
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.

1 participant