Skip to content

docs: justify phone_collector as aiogram-dialog migration exception (#2055)#2075

Open
yastman wants to merge 1 commit into
devfrom
docs/2055-phone-collector-justification
Open

docs: justify phone_collector as aiogram-dialog migration exception (#2055)#2075
yastman wants to merge 1 commit into
devfrom
docs/2055-phone-collector-justification

Conversation

@yastman
Copy link
Copy Markdown
Owner

@yastman yastman commented May 22, 2026

This pull request was created by @kiro-agent on behalf of @yastman 👻

Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro autonomous agent


Summary

Per the issue body, CRM quick actions (#2053) and the demo handler (#2054) are migrating to aiogram-dialog, but phone_collector should stay on raw aiogram Router + FSMContext. Reason:

  • Telegram lead capture uses KeyboardButton(request_contact=True) inside ReplyKeyboardMarkup. One tap sends a verified Contact payload.
  • aiogram-dialog renders inline-keyboard Select/Button widgets and does not provide a request_contact widget.
  • A forced migration would replace one-tap contact share with manual phone typing, measurably reducing opt-in rate for lead capture.

This PR makes the exception explicit in two places, so a future reader doing a "consistency refactor" doesn't accidentally weaken the UX.

Changes

This is a docs-only PR — no behaviour change in phone_collector.py.

Verification

uv run python -c 'from telegram_bot.handlers import phone_collector'
  → OK
uv run pytest tests/unit/handlers/test_phone_collector.py \
              tests/unit/handlers/test_phone_crm_integration.py -q
  → 47 passed
uv run python scripts/check_markdown_links.py
  → All relative Markdown links OK.
uvx ruff check telegram_bot/handlers/phone_collector.py
  → All checks passed!

Closes #2055
Parent: #1232

…2055)

Parent: #1232 (Replace custom aiogram FSM with aiogram-dialog).

Per the issue body, CRM quick actions (#2053) and the demo handler
(#2054) are migrating to aiogram-dialog, but the phone collector should
stay on raw aiogram Router + FSMContext. Reason:

- Telegram lead capture uses KeyboardButton(request_contact=True) inside
  ReplyKeyboardMarkup. One tap sends a verified Contact payload.
- aiogram-dialog renders inline-keyboard Select/Button widgets and does
  NOT provide a request_contact widget.
- A forced migration would replace one-tap contact share with manual
  phone typing, measurably reducing opt-in rate for lead capture.

The exception is now documented in two places:

1. telegram_bot/handlers/phone_collector.py — module docstring expands
   from a single-line '#628' note into a full design-exception block
   that names #1232 and #2055, lists the trade-off, and tells future
   readers when the exception can be dropped (product accepts weaker UX
   or aiogram-dialog ships a contact-share widget).

2. docs/engineering/sdk-registry.md — adds a new bullet at the end of
   the aiogram-dialog gotchas section calling out phone_collector as
   the SINGLE intentional exception to the 'no custom FSM' rule, with
   cross-references to #2053 and #2054.

This closes the documentation deliverable from #2055; the actual code
of phone_collector.py is intentionally unchanged.

Verification:
  uv run python -c 'from telegram_bot.handlers import phone_collector'
    OK
  uv run pytest tests/unit/handlers/test_phone_collector.py
                tests/unit/handlers/test_phone_crm_integration.py -q
    47 passed
  uv run python scripts/check_markdown_links.py
    All relative Markdown links OK.
  uvx ruff check telegram_bot/handlers/phone_collector.py
    All checks passed!

Closes #2055
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.

dialog: document phone collector as justified FSM/router exception (#1232)

2 participants