Skip to content

Commit 5dd671e

Browse files
authored
Merge pull request #297 from ffmemes/codex/fix-admin-source-permissions
Preserve roles when users block the bot
2 parents d51ea82 + f184778 commit 5dd671e

10 files changed

Lines changed: 246 additions & 21 deletions

File tree

CONTEXT.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ Use these terms consistently:
3838
_Code aliases_: `UserType.MODERATOR`, `UserType.ADMIN`
3939
_Avoid_: админ, проверяющий загрузки
4040

41+
**Блокировка бота**:
42+
Транспортное состояние Telegram: пользователь запретил боту писать в личку.
43+
Не является ролью пользователя и не должно перетирать `UserType`.
44+
_Code aliases_: `user.blocked_bot_at`
45+
_Avoid_: `UserType.BLOCKED_BOT` как новое состояние, бан, потеря роли
46+
4147
**Модераторский чат**:
4248
Комьюнити-чат модераторов, где бот может запускать задания, обсуждения и голосования по новым источникам.
4349
_Code aliases_: `TELEGRAM_MODERATOR_CHAT_ID`, Moderator Chat
@@ -115,6 +121,7 @@ _Avoid_: router
115121
## Relationships
116122

117123
- **Модератор** может участвовать в **модераторском чате** и получать **разбор новых мемов** в своей ленте.
124+
- **Блокировка бота** влияет на возможность отправить пользователю сообщение, но не меняет роль **Модератора**.
118125
- **Дедупликация мемов** должна выполняться до того, как мем становится доступен обычной рекомендательной системе.
119126
- **Модераторский чат** используется для комьюнити-циклов; **чат проверки загрузок** используется для решений по пользовательским загрузкам.
120127
- **Проверяющий загрузки** определяется доступом к **чату проверки загрузок**, а не `UserType`.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
"""restore okhlopkov admin role
2+
3+
Revision ID: a9f0d6c2b1e3
4+
Revises: 8c2f4a6d9e10
5+
Create Date: 2026-05-25 12:45:00.000000
6+
7+
"""
8+
9+
import sqlalchemy as sa
10+
from alembic import op
11+
12+
# revision identifiers, used by Alembic.
13+
revision = "a9f0d6c2b1e3"
14+
down_revision = "8c2f4a6d9e10"
15+
branch_labels = None
16+
depends_on = None
17+
18+
OKHLOPKOV_USER_ID = 49820636
19+
TELEGRAM_MODERATOR_CHAT_ID = -1001305866294
20+
21+
22+
def upgrade() -> None:
23+
conn = op.get_bind()
24+
conn.execute(
25+
sa.text(
26+
"""
27+
INSERT INTO "user" (id, type, created_at, last_active_at, blocked_bot_at)
28+
VALUES (:user_id, 'admin', now(), now(), NULL)
29+
ON CONFLICT (id)
30+
DO UPDATE SET
31+
type = 'admin',
32+
blocked_bot_at = NULL
33+
"""
34+
),
35+
{"user_id": OKHLOPKOV_USER_ID},
36+
)
37+
conn.execute(
38+
sa.text(
39+
"""
40+
INSERT INTO user_tg_chat_membership (user_tg_id, chat_id, last_seen_at)
41+
SELECT id, :chat_id, now()
42+
FROM user_tg
43+
WHERE id = :user_id
44+
ON CONFLICT (user_tg_id, chat_id)
45+
DO UPDATE SET last_seen_at = now()
46+
"""
47+
),
48+
{"user_id": OKHLOPKOV_USER_ID, "chat_id": TELEGRAM_MODERATOR_CHAT_ID},
49+
)
50+
51+
52+
def downgrade() -> None:
53+
pass

src/broadcasts/service.py

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ async def get_user_ids_active_minutes_ago(
2525
id
2626
FROM "user"
2727
WHERE 1=1
28+
AND blocked_bot_at IS NULL
2829
AND type NOT IN ('waitlist', 'blocked_bot')
2930
AND last_active_at BETWEEN
3031
NOW() - INTERVAL '{to_minutes_ago} MINUTES'
@@ -51,10 +52,14 @@ async def get_users_to_broadcast_meme_from_tgchannelru(
5152
AND UMR.meme_id = {meme_id}
5253
LEFT JOIN user_tg_chat_membership UTCM
5354
ON UTCM.user_tg_id = UL.user_id
55+
JOIN "user" U
56+
ON U.id = UL.user_id
5457
WHERE 1=1
5558
AND UL.language_code = 'ru'
5659
AND UMR.user_id IS NULL
5760
AND UTCM.user_tg_id IS NULL
61+
AND U.blocked_bot_at IS NULL
62+
AND U.type NOT IN ('waitlist', 'blocked_bot')
5863
"""
5964

6065
return await fetch_all(text(select_query))
@@ -70,9 +75,13 @@ async def get_users_to_broadcast_post_from_tgchannelru():
7075
FROM user_language UL
7176
LEFT JOIN user_tg_chat_membership UTCM
7277
ON UTCM.user_tg_id = UL.user_id
78+
JOIN "user" U
79+
ON U.id = UL.user_id
7380
WHERE 1=1
7481
AND UL.language_code = 'ru'
7582
AND UTCM.user_tg_id IS NULL
83+
AND U.blocked_bot_at IS NULL
84+
AND U.type NOT IN ('waitlist', 'blocked_bot')
7685
"""
7786

7887
return await fetch_all(text(select_query))
@@ -82,9 +91,13 @@ async def get_users_with_language(
8291
language_code: str,
8392
):
8493
select_query = f"""
85-
SELECT user_id
86-
FROM user_language
87-
WHERE language_code = '{language_code}'
94+
SELECT UL.user_id
95+
FROM user_language UL
96+
JOIN "user" U
97+
ON U.id = UL.user_id
98+
WHERE UL.language_code = '{language_code}'
99+
AND U.blocked_bot_at IS NULL
100+
AND U.type NOT IN ('waitlist', 'blocked_bot')
88101
"""
89102
return await fetch_all(text(select_query))
90103

@@ -96,6 +109,7 @@ async def get_users_active_more_than_days_ago(
96109
SELECT id
97110
FROM "user"
98111
WHERE last_active_at < NOW() - INTERVAL '{days_ago} DAYS'
112+
AND blocked_bot_at IS NULL
99113
AND type != 'blocked_bot'
100114
"""
101115
return await fetch_all(text(select_query))
@@ -119,7 +133,8 @@ async def get_all_non_blocked_users() -> list[dict]:
119133
) AS language_code
120134
FROM "user" u
121135
LEFT JOIN user_tg ut ON ut.id = u.id
122-
WHERE u.type NOT IN ('waitlist', 'blocked_bot')
136+
WHERE u.blocked_bot_at IS NULL
137+
AND u.type NOT IN ('waitlist', 'blocked_bot')
123138
ORDER BY u.last_active_at DESC
124139
"""
125140
)

src/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ async def lifespan(_application: FastAPI) -> AsyncGenerator:
6262
),
6363
],
6464
ignore_errors=[
65-
"telegram.error.Forbidden", # handled by error.py → marks user as blocked_bot
65+
"telegram.error.Forbidden", # handled by error.py -> records blocked_bot_at
6666
],
6767
include_local_variables=False,
6868
before_send=before_send,

src/tgbot/handlers/chat/send_tokens.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,11 @@ async def send_tokens_to_reply(update: Update, context: ContextTypes.DEFAULT_TYP
6868
return # no need to send tokens to yourself
6969

7070
to_user = await get_user_by_id(to_user_id)
71-
if not to_user or to_user["type"] == UserType.BLOCKED_BOT:
71+
if (
72+
not to_user
73+
or to_user["blocked_bot_at"] is not None
74+
or to_user["type"] == UserType.BLOCKED_BOT
75+
):
7276
return await _reply_and_delete(
7377
update.message,
7478
f"Не вижу {to_user_tg.name} в боте! ай-яй-яй 😿",

src/tgbot/handlers/deep_link.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,12 @@ async def handle_invited_user(
4848
# set inviter id
4949
await update_user(invited_user["id"], inviter_id=invitor_user_id)
5050

51-
# dont reward if a user blocked the bot
52-
if invitor_user["type"] == UserType.BLOCKED_BOT:
51+
# Don't reward if the inviter cannot receive the bot notification.
52+
if invitor_user["blocked_bot_at"] is not None or invitor_user["type"] == UserType.BLOCKED_BOT:
5353
return await log(
5454
f"""
5555
{invited_user_name} was invited by #{invitor_user_id}
56-
but his type is {invitor_user["type"]}
56+
but his bot access is blocked (type={invitor_user["type"]})
5757
"""
5858
)
5959

src/tgbot/service.py

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -467,9 +467,10 @@ async def mark_user_blocked(
467467
) -> dict[str, Any] | None:
468468
"""Mark a user as having blocked the bot.
469469
470-
Idempotent. Preserves privileged roles (moderator/admin/super_user) —
471-
still records blocked_bot_at for retention analysis, but does not
472-
demote their type. Invalidates user_info cache on success.
470+
Idempotent. Preserves the user's role in `type` and records the Telegram
471+
transport state in blocked_bot_at. The legacy `blocked_bot` user type can
472+
still exist on old rows, but new block events must not overwrite roles.
473+
Invalidates user_info cache on success.
473474
474475
`source` is a free-form label for observability
475476
("my_chat_member", "forbidden_send_meme", ...).
@@ -483,7 +484,10 @@ async def mark_user_blocked(
483484
return None
484485

485486
ts = _blocked_bot_at_timestamp(when)
486-
current_type = UserType(current["type"]) if current["type"] else None
487+
try:
488+
current_type = UserType(current["type"]) if current["type"] else None
489+
except ValueError:
490+
current_type = None
487491

488492
if current_type and current_type.is_moderator:
489493
logging.warning(
@@ -493,13 +497,8 @@ async def mark_user_blocked(
493497
source,
494498
current_type.value,
495499
)
496-
updated = await update_user(user_id, blocked_bot_at=ts)
497-
else:
498-
updated = await update_user(
499-
user_id,
500-
type=UserType.BLOCKED_BOT,
501-
blocked_bot_at=ts,
502-
)
500+
501+
updated = await update_user(user_id, blocked_bot_at=ts)
503502

504503
if updated is not None:
505504
try:

tests/broadcasts/test_service.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
from datetime import datetime
2+
3+
import pytest
4+
import pytest_asyncio
5+
from sqlalchemy import delete
6+
from tests.factories import create_user, create_user_language
7+
8+
from src.broadcasts.service import get_all_non_blocked_users, get_users_with_language
9+
from src.database import engine, user, user_language, user_tg
10+
11+
ACTIVE_USER_ID = 311001
12+
BLOCKED_ADMIN_ID = 311002
13+
LEGACY_BLOCKED_USER_ID = 311003
14+
WAITLIST_USER_ID = 311004
15+
ACTIVE_ADMIN_ID = 311005
16+
TEST_USER_IDS = (
17+
ACTIVE_USER_ID,
18+
BLOCKED_ADMIN_ID,
19+
LEGACY_BLOCKED_USER_ID,
20+
WAITLIST_USER_ID,
21+
ACTIVE_ADMIN_ID,
22+
)
23+
24+
25+
@pytest_asyncio.fixture()
26+
async def cleanup_broadcast_users():
27+
await _cleanup()
28+
yield
29+
await _cleanup()
30+
31+
32+
async def _cleanup() -> None:
33+
async with engine.connect() as conn:
34+
await conn.execute(delete(user_language).where(user_language.c.user_id.in_(TEST_USER_IDS)))
35+
await conn.execute(delete(user_tg).where(user_tg.c.id.in_(TEST_USER_IDS)))
36+
await conn.execute(delete(user).where(user.c.id.in_(TEST_USER_IDS)))
37+
await conn.commit()
38+
39+
40+
@pytest.mark.asyncio
41+
async def test_get_users_with_language_skips_blocked_transport_state(
42+
cleanup_broadcast_users,
43+
) -> None:
44+
async with engine.connect() as conn:
45+
await create_user(conn, id=ACTIVE_USER_ID)
46+
await create_user(conn, id=BLOCKED_ADMIN_ID, type="admin")
47+
await create_user(conn, id=LEGACY_BLOCKED_USER_ID, type="blocked_bot")
48+
await create_user(conn, id=WAITLIST_USER_ID, type="waitlist")
49+
await create_user(conn, id=ACTIVE_ADMIN_ID, type="admin")
50+
for user_id in TEST_USER_IDS:
51+
await create_user_language(conn, user_id=user_id, language_code="ru")
52+
await conn.execute(
53+
user.update()
54+
.where(user.c.id == BLOCKED_ADMIN_ID)
55+
.values(blocked_bot_at=datetime(2026, 5, 25, 12, 0, 0))
56+
)
57+
await conn.commit()
58+
59+
rows = await get_users_with_language("ru")
60+
61+
assert {row["user_id"] for row in rows if row["user_id"] in TEST_USER_IDS} == {
62+
ACTIVE_USER_ID,
63+
ACTIVE_ADMIN_ID,
64+
}
65+
66+
67+
@pytest.mark.asyncio
68+
async def test_get_all_non_blocked_users_skips_blocked_transport_state(
69+
cleanup_broadcast_users,
70+
) -> None:
71+
async with engine.connect() as conn:
72+
await create_user(conn, id=ACTIVE_USER_ID)
73+
await create_user(conn, id=BLOCKED_ADMIN_ID, type="admin")
74+
await create_user(conn, id=LEGACY_BLOCKED_USER_ID, type="blocked_bot")
75+
await create_user(conn, id=ACTIVE_ADMIN_ID, type="admin")
76+
for user_id in (
77+
ACTIVE_USER_ID,
78+
BLOCKED_ADMIN_ID,
79+
LEGACY_BLOCKED_USER_ID,
80+
ACTIVE_ADMIN_ID,
81+
):
82+
await create_user_language(conn, user_id=user_id, language_code="ru")
83+
await conn.execute(
84+
user.update()
85+
.where(user.c.id == BLOCKED_ADMIN_ID)
86+
.values(blocked_bot_at=datetime(2026, 5, 25, 12, 0, 0))
87+
)
88+
await conn.commit()
89+
90+
rows = await get_all_non_blocked_users()
91+
92+
assert {row["user_id"] for row in rows if row["user_id"] in TEST_USER_IDS} == {
93+
ACTIVE_USER_ID,
94+
ACTIVE_ADMIN_ID,
95+
}

tests/tgbot/test_block_tracking.py

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ async def test_mark_user_blocked_sends_naive_timestamp_to_db(monkeypatch) -> Non
2424
update_user = AsyncMock(
2525
return_value={
2626
"id": 10001,
27-
"type": UserType.BLOCKED_BOT.value,
27+
"type": UserType.USER.value,
2828
"blocked_bot_at": datetime(2026, 4, 27, 20, 0, 27),
2929
}
3030
)
@@ -48,11 +48,44 @@ async def test_mark_user_blocked_sends_naive_timestamp_to_db(monkeypatch) -> Non
4848
)
4949

5050
update_user.assert_awaited_once()
51+
assert "type" not in update_user.await_args.kwargs
5152
blocked_bot_at = update_user.await_args.kwargs["blocked_bot_at"]
5253
assert blocked_bot_at == datetime(2026, 4, 27, 20, 0, 27)
5354
assert blocked_bot_at.tzinfo is None
5455

5556

57+
@pytest.mark.asyncio
58+
async def test_mark_user_blocked_preserves_admin_type(monkeypatch) -> None:
59+
update_user = AsyncMock(
60+
return_value={
61+
"id": 49820636,
62+
"type": UserType.ADMIN.value,
63+
"blocked_bot_at": datetime(2026, 4, 27, 20, 0, 27),
64+
}
65+
)
66+
monkeypatch.setattr(
67+
service,
68+
"get_user_by_id",
69+
AsyncMock(return_value={"id": 49820636, "type": UserType.ADMIN.value}),
70+
)
71+
monkeypatch.setattr(service, "update_user", update_user)
72+
73+
monkeypatch.setitem(
74+
sys.modules,
75+
"src.tgbot.user_info",
76+
SimpleNamespace(update_user_info_cache=AsyncMock()),
77+
)
78+
79+
await service.mark_user_blocked(
80+
user_id=49820636,
81+
source="my_chat_member",
82+
when=datetime(2026, 4, 27, 20, 0, 27, tzinfo=timezone.utc),
83+
)
84+
85+
update_user.assert_awaited_once()
86+
assert update_user.await_args.kwargs == {"blocked_bot_at": datetime(2026, 4, 27, 20, 0, 27)}
87+
88+
5689
@pytest.mark.asyncio
5790
async def test_handle_user_blocked_bot_escapes_admin_log_html(monkeypatch) -> None:
5891
messages = []

0 commit comments

Comments
 (0)