Skip to content

feat: integrate profile completeness check into captcha verification#9

Open
rezhajulio wants to merge 4 commits into
mainfrom
feat/captcha-profile-check
Open

feat: integrate profile completeness check into captcha verification#9
rezhajulio wants to merge 4 commits into
mainfrom
feat/captcha-profile-check

Conversation

@rezhajulio

@rezhajulio rezhajulio commented Mar 19, 2026

Copy link
Copy Markdown
Owner

Summary

Combines captcha verification with a mandatory profile completeness check (photo + username) before unrestricting new members.

Changes

Captcha + Profile Check Integration

  • When a user clicks the captcha button, their profile is checked first
  • If incomplete, an alert shows what's missing (photo, username, or both) — user can retry
  • Timeout job is preserved until profile check passes, preventing users from getting stuck
  • Welcome message updated to inform users about profile requirements upfront

Handler Hardening

  • Added parse guard for malformed callback data (ValueError/IndexError)
  • Moved timeout job cancellation after successful unrestrict + DB cleanup (fixes bug where failed unrestrict would leave user stuck with no timeout)
  • Wrapped DB finalization in try/except to prevent unanswered callbacks

Test Improvements

  • Replaced fragile AsyncMock(is_complete=...) with real ProfileCheckResult dataclass in all mocks
  • Added tests: malformed callback data, unrestrict failure preserving timeout, incomplete profile (single/both missing), profile check API exception

Flow

  1. User joins → restricted → captcha message sent
  2. User clicks button → profile checked
  3. If incomplete → alert with missing items, timeout stays active
  4. If complete → unrestrict → cleanup DB → cancel timeout → success message

Tests: 555 passed, ~99.9% coverage
@rezhajulio
feat: combine captcha verification with profile photo/username check
63257a4 
- Update captcha welcome message to inform users about profile requirements
- Add profile completeness check (photo + username) in captcha callback handler
- Reject verification with alert if profile is incomplete, keeping timeout active
- Fix double query.answer() bug — each code path now answers exactly once
- Add CAPTCHA_INCOMPLETE_PROFILE_MESSAGE constant for rejection alert
- Update and add tests for new captcha+profile flow (553 tests, 99% coverage)
@rezhajulio
fix: harden captcha callback handler and improve test quality
60009ac 
- Add parse guard for malformed callback data (ValueError/IndexError)
- Move timeout job cancellation after successful unrestrict + DB cleanup
- Wrap DB finalization in try/except to prevent unanswered callbacks
- Replace AsyncMock with ProfileCheckResult dataclass in all test mocks
- Add tests for malformed callback data and unrestrict failure preserving timeout
@rezhajulio
fix(captcha): reorder DB finalization before unrestrict + tighten tests
0f929fa 
Apply review feedback to feat/captcha-profile-check. The previous order
(unrestrict first, then DB cleanup) left Telegram state ahead of DB
state on a DB write failure: user unrestricted in Telegram, pending
captcha still in DB, timeout still armed. When the timeout fired,
handle_captcha_expiration re-interpreted the inconsistency as a
timeout event, sending contradictory success+timeout messages and a
false-positive "unrestricted" DM notification.

Reorder so the irreversible Telegram side effect happens LAST:
- DB finalization (remove_pending_captcha + start_new_user_probation)
  first. Reversible. If it fails, user stays restricted, DB row
  preserved, timeout armed, user can retry.
- unrestrict_user second. Irreversible. If it fails, DB row is gone
  but user is still restricted on Telegram; verify button is gone;
  user waits for admin. State is consistent.

Tests
- Replace hard-coded "Gagal memverifikasi..." literal with
  CAPTCHA_FAILED_VERIFICATION_MESSAGE constant in two tests
  (test_unknown_group_in_callback_rejects,
  test_malformed_callback_data_rejected) to remove drift risk.
- Rename test_unrestrict_failure_stops_execution ->
  test_unrestrict_failure_keeps_user_restricted and invert assertions
  for the new order (DB row is now None, edit_message_text NOT called).
- Add test_db_finalization_failure_keeps_user_restricted covering the
  DB-cleanup-failure path: unrestrict_user NOT called, DB row
  preserved, query shows failure alert.
- Delete redundant test_profile_check_exception_shows_error (strict
  subset of test_captcha_callback_profile_check_exception).
- Delete test_unrestrict_failure_preserves_timeout_job (its assertion
  was wrong for the new order; the renamed test covers the intent).
@rezhajulio rezhajulio force-pushed the feat/captcha-profile-check branch from 37a3c8c to 8c429e3 Compare June 17, 2026 14:37
@rezhajulio
test(captcha): apply deferred mechanical cleanups
6a42c16 
Round-1 follow-ups on feat/captcha-profile-check. All items are
mechanical cleanups; no behavior change.

Constants
- Add CAPTCHA_PROFILE_CHECK_FAILED_MESSAGE. The previous behavior
  conflated "Telegram API failed" with "user failed verification"
  by showing the same generic CAPTCHA_FAILED_VERIFICATION_MESSAGE
  for both. The new message tells the user to retry in a few
  seconds, which is the right UX for a transient API error.

Handler
- captcha_callback_handler uses CAPTCHA_PROFILE_CHECK_FAILED_MESSAGE
  in the check_user_profile exception path. Other failure modes
  (parse error, missing pending captcha, unrestrict failure, DB
  finalization failure) still use CAPTCHA_FAILED_VERIFICATION_MESSAGE
  because those are deterministic verification failures, not API
  blips.

Tests
- Add _make_callback_query helper to TestCaptchaCallbackHandler.
  Refactor 10 tests to use it instead of repeating the 7-line
  MagicMock setup. Net diff: -55 lines.
- Parametrize test_captcha_callback_incomplete_profile_rejected
  across (photo-only-missing, both-missing, username-only-missing)
  using missing_items as the parameter and computing the expected
  CAPTCHA_INCOMPLETE_PROFILE_MESSAGE via MISSING_ITEMS_SEPARATOR.
  The username-only-missing case is new coverage.
- Parametrize test_malformed_callback_data_rejected across 4
  variants: non-numeric single token, missing parts (IndexError),
  int parse failure (ValueError), unparseable second part. Drop
  the empty-string case (CallbackQueryHandler's regex filters it
  before the handler sees it).
- Strengthen test_incomplete_profile_blocks_verification: replace
  "Lengkapi" + "username" substring assertions with a full-string
  match against CAPTCHA_INCOMPLETE_PROFILE_MESSAGE.format(missing_text="username").
- Add edit_message_text.assert_not_called() to the parametrized
  malformed test (Fix 5).
@rezhajulio rezhajulio force-pushed the feat/captcha-profile-check branch from 5308d10 to 6a42c16 Compare June 22, 2026 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant

@rezhajulio