mirror of
https://github.com/dataelement/bisheng.git
synced 2026-09-21 04:26:48 +08:00
fix(approval): activate PENDING channel membership on subscribe approval
find_membership defaulted to ACTIVE-only for channels, so the approval activation flow could not locate the applicant's PENDING membership and on_approved silently returned missing_membership — outbox/instance reported success while the membership stayed PENDING and no ReBAC grant was written, so the channel never appeared in the user's subscription list. - find_membership: add include_inactive flag (default keeps ACTIVE-only) - approval activation paths pass include_inactive=True - on_approved now raises on missing membership instead of faking success - add regression tests + repair script for stuck instances 248/250/251/252
This commit is contained in:
@@ -134,6 +134,7 @@ ApprovalCenterService.decide_task()
|
||||
- **入口**:`channel/domain/services/channel_service.py::subscribe_channel()`(`REVIEW` 可见性频道)
|
||||
- **Handler**:`ChannelSubscribeScenarioHandler`
|
||||
- 通过 / pass 路径调 `ChannelService.sync_direct_channel_user_permissions()` 写 ReBAC(OpenFGA) 关系(否则成员不出现在 ReBAC 成员列表)
|
||||
- `on_approved` 先把申请人的 **PENDING** membership 翻成 ACTIVE 再写 ReBAC;查 membership 必须用 `include_inactive=True`(CHANNEL 默认只查 ACTIVE),缺失时直接 raise。详见 [§11 调试指南](#11-调试指南) 的已知坑
|
||||
- PENDING 时调 `_send_channel_approval_notification()` 通知审批人
|
||||
|
||||
### 4.3 知识空间加入审批 (`knowledge_space_subscribe_request`)
|
||||
@@ -314,6 +315,18 @@ SELECT id, exception_type, status, detail FROM approval_exception WHERE instance
|
||||
### "频道/知识空间审批通过但成员列表看不到"
|
||||
检查对应 `sync_direct_channel_user_permissions` / `sync_direct_space_user_permissions` 是否在该激活路径被调用(写 ReBAC/OpenFGA 关系)。
|
||||
|
||||
**已知坑(频道场景,2026-06 修复)**:`on_approved` 第一步 `_get_membership` 要找到那条 **PENDING** 的 membership 才能翻成 ACTIVE。但 `SpaceChannelMemberRepositoryImpl.find_membership` 对 `business_type=CHANNEL` 默认**只查 ACTIVE**,导致查不到 PENDING → 返回 None。激活路径(`approval_runtime_handler_factory._AsyncSpaceChannelMembershipAdapter.find_membership`、旧 `channel_subscribe_approval_handler._get_membership`)**必须传 `include_inactive=True`**。知识空间场景用的是 `SpaceChannelMemberDao.async_find_member`(不过滤状态),所以不受影响——这也是"知识空间正常、频道异常"的原因。
|
||||
|
||||
> 该 bug 还会被一个静默分支掩盖:旧版 `on_approved` 在 membership 缺失时 `return {'status':'missing_membership'}` 不抛异常 → outbox 仍标 success、instance 仍 executed,但 membership 永远停在 PENDING、ReBAC 从未写。现已改为 **raise**,让 outbox 进 FAILED + `execute_failed` 异常暴露问题。排查时若看到 instance=executed 但 `space_channel_member.status=PENDING`,就是这个老数据。
|
||||
|
||||
排查 SQL:
|
||||
```sql
|
||||
SELECT i.id, i.status, m.id, m.status
|
||||
FROM approval_instance i JOIN space_channel_member m
|
||||
ON m.business_id=i.business_resource_id AND m.business_type='CHANNEL' AND m.user_id=i.applicant_user_id
|
||||
WHERE i.scenario_code='channel_subscribe_request' AND i.status IN ('executed','approved') AND m.status='PENDING';
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 12. 测试
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
"""One-off compensation for channel-subscribe approvals stuck by the
|
||||
find_membership ACTIVE-only filter bug.
|
||||
|
||||
For each affected instance the approval was marked EXECUTED but the applicant's
|
||||
membership stayed PENDING and no ReBAC grant was written, so the channel never
|
||||
appeared in the user's subscription list. This re-runs the (now fixed)
|
||||
ChannelSubscribeScenarioHandler.on_approved() to flip the membership to ACTIVE
|
||||
and write the OpenFGA relation.
|
||||
|
||||
Idempotent: re-running on an already-ACTIVE membership is a no-op upsert.
|
||||
|
||||
Run from src/backend with the same config the backend uses:
|
||||
config=config.yaml uv run python ../../scripts/repair_channel_subscribe_memberships.py
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
|
||||
logging.basicConfig(level=logging.INFO)
|
||||
logger = logging.getLogger("repair_channel_subscribe")
|
||||
|
||||
# Affected instance ids discovered via the §11 diagnostic query.
|
||||
INSTANCE_IDS = [248, 250, 251, 252]
|
||||
TENANT_ID = 1
|
||||
|
||||
|
||||
async def main() -> None:
|
||||
from bisheng.approval.domain.repositories.approval_instance_repository import ApprovalInstanceRepository
|
||||
from bisheng.approval.domain.services.approval_runtime_handler_factory import build_runtime_handler
|
||||
from bisheng.common.services.config_service import settings
|
||||
from bisheng.core.context import close_app_context, initialize_app_context
|
||||
from bisheng.core.context.tenant import set_current_tenant_id
|
||||
|
||||
await initialize_app_context(config=settings)
|
||||
set_current_tenant_id(TENANT_ID)
|
||||
|
||||
try:
|
||||
for instance_id in INSTANCE_IDS:
|
||||
instance = await ApprovalInstanceRepository.get_instance(instance_id)
|
||||
if instance is None:
|
||||
logger.warning("instance %s not found, skipping", instance_id)
|
||||
continue
|
||||
if instance.scenario_code != "channel_subscribe_request":
|
||||
logger.warning(
|
||||
"instance %s is %s, not channel_subscribe_request, skipping",
|
||||
instance_id, instance.scenario_code,
|
||||
)
|
||||
continue
|
||||
payload = instance.payload_snapshot or {}
|
||||
handler = await build_runtime_handler(instance.scenario_code)
|
||||
result = await handler.on_approved(instance_id, payload)
|
||||
logger.info(
|
||||
"repaired instance=%s channel_id=%s applicant=%s -> %s",
|
||||
instance_id, payload.get("channel_id"), payload.get("applicant_user_id"), result,
|
||||
)
|
||||
finally:
|
||||
await close_app_context()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
asyncio.run(main())
|
||||
@@ -35,10 +35,12 @@ class _AsyncSpaceChannelMembershipAdapter:
|
||||
async def find_membership(self, business_id: str, business_type: BusinessTypeEnum, user_id: int):
|
||||
async with get_async_db_session() as session:
|
||||
repository = SpaceChannelMemberRepositoryImpl(session)
|
||||
# Activation must locate the PENDING membership (not just ACTIVE) to flip it to ACTIVE.
|
||||
return await repository.find_membership(
|
||||
business_id=business_id,
|
||||
business_type=business_type,
|
||||
user_id=user_id,
|
||||
include_inactive=True,
|
||||
)
|
||||
|
||||
async def update(self, membership):
|
||||
|
||||
@@ -83,8 +83,15 @@ class ChannelSubscribeScenarioHandler:
|
||||
async def on_approved(self, instance_id: int, payload_snapshot: dict) -> dict:
|
||||
membership = await self._get_membership(payload_snapshot)
|
||||
if not membership:
|
||||
logger.warning('Channel membership not found when approval instance=%s approved', instance_id)
|
||||
return {'status': 'missing_membership'}
|
||||
# Fail loudly: the outbox executor turns this into a FAILED outbox +
|
||||
# execute_failed exception. Returning a benign dict here would mark the
|
||||
# instance EXECUTED while the membership stays PENDING and ReBAC is never
|
||||
# written, so the channel never appears in the user's subscription list.
|
||||
raise RuntimeError(
|
||||
f'Channel membership not found for approval instance={instance_id}, '
|
||||
f"channel_id={payload_snapshot.get('channel_id')}, "
|
||||
f"applicant_user_id={payload_snapshot.get('applicant_user_id')}"
|
||||
)
|
||||
membership.status = MembershipStatusEnum.ACTIVE
|
||||
await self.space_channel_member_repository.update(membership)
|
||||
if self.sync_permissions:
|
||||
|
||||
@@ -130,9 +130,14 @@ class ChannelSubscribeApprovalHandler(ApprovalHandler):
|
||||
)
|
||||
|
||||
async def _get_membership(self, channel_id: str, applicant_user_id: int):
|
||||
"""Load the applicant membership for the target channel."""
|
||||
"""Load the applicant membership for the target channel.
|
||||
|
||||
Includes inactive rows: the applicant's membership is PENDING while awaiting
|
||||
approval, and this handler must find it to flip it to ACTIVE/REJECTED.
|
||||
"""
|
||||
return await self.space_channel_member_repository.find_membership(
|
||||
business_id=channel_id,
|
||||
business_type=BusinessTypeEnum.CHANNEL,
|
||||
user_id=applicant_user_id,
|
||||
include_inactive=True,
|
||||
)
|
||||
|
||||
+8
-3
@@ -104,14 +104,19 @@ class SpaceChannelMemberRepositoryImpl(BaseRepositoryImpl[SpaceChannelMember, in
|
||||
return list(highest_by_channel.values())
|
||||
|
||||
async def find_membership(self, business_id: str, business_type: BusinessTypeEnum,
|
||||
user_id: int) -> Optional[SpaceChannelMember]:
|
||||
"""Find a specific membership by business ID, type, and user ID."""
|
||||
user_id: int, include_inactive: bool = False) -> Optional[SpaceChannelMember]:
|
||||
"""Find a specific membership by business ID, type, and user ID.
|
||||
|
||||
For channels the default only returns ACTIVE members. Pass
|
||||
``include_inactive=True`` to also match PENDING/REJECTED rows — the approval
|
||||
activation flow needs the PENDING membership in order to flip it to ACTIVE.
|
||||
"""
|
||||
query = select(SpaceChannelMember).where(
|
||||
SpaceChannelMember.business_id == business_id,
|
||||
SpaceChannelMember.business_type == business_type,
|
||||
SpaceChannelMember.user_id == user_id
|
||||
)
|
||||
if business_type == BusinessTypeEnum.CHANNEL:
|
||||
if business_type == BusinessTypeEnum.CHANNEL and not include_inactive:
|
||||
query = query.where(SpaceChannelMember.status == MembershipStatusEnum.ACTIVE)
|
||||
result = await self.session.exec(query)
|
||||
rows = list(result.all())
|
||||
|
||||
+8
-2
@@ -36,8 +36,14 @@ class SpaceChannelMemberRepository(BaseRepository[SpaceChannelMember, int], ABC)
|
||||
|
||||
@abstractmethod
|
||||
async def find_membership(self, business_id: str, business_type: BusinessTypeEnum,
|
||||
user_id: int) -> Optional[SpaceChannelMember]:
|
||||
"""Find a specific membership by business ID, type, and user ID."""
|
||||
user_id: int, include_inactive: bool = False) -> Optional[SpaceChannelMember]:
|
||||
"""Find a specific membership by business ID, type, and user ID.
|
||||
|
||||
For channels the default only returns ACTIVE members. Set
|
||||
``include_inactive=True`` to also match PENDING/REJECTED rows — required by
|
||||
the approval activation flow, which must locate the PENDING membership to
|
||||
flip it to ACTIVE.
|
||||
"""
|
||||
pass
|
||||
|
||||
@abstractmethod
|
||||
|
||||
@@ -151,3 +151,27 @@ async def test_channel_subscribe_scenario_handler_updates_membership_states():
|
||||
membership.status = MembershipStatusEnum.PENDING
|
||||
await handler.on_rejected(instance_id=1, payload_snapshot=payload, reason='reject')
|
||||
assert membership.status == MembershipStatusEnum.REJECTED
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_channel_subscribe_on_approved_raises_when_membership_missing():
|
||||
"""A missing membership must fail loudly so the outbox is marked FAILED and an
|
||||
execute_failed exception is raised — never silently reported as success."""
|
||||
from bisheng.approval.domain.services.channel_subscribe_scenario_handler import (
|
||||
ChannelSubscribeScenarioHandler,
|
||||
)
|
||||
|
||||
member_repository = SimpleNamespace(
|
||||
find_membership=AsyncMock(return_value=None),
|
||||
update=AsyncMock(),
|
||||
)
|
||||
sync_permissions = AsyncMock()
|
||||
handler = ChannelSubscribeScenarioHandler(
|
||||
space_channel_member_repository=member_repository,
|
||||
sync_permissions=sync_permissions,
|
||||
)
|
||||
|
||||
payload = {'channel_id': 'channel-1', 'applicant_user_id': 427}
|
||||
with pytest.raises(RuntimeError):
|
||||
await handler.on_approved(instance_id=99, payload_snapshot=payload)
|
||||
sync_permissions.assert_not_awaited()
|
||||
|
||||
@@ -250,3 +250,48 @@ async def test_remove_channel_subscription_members_preserves_authorized_grants(a
|
||||
).all()
|
||||
assert removed == 2
|
||||
assert sorted(remaining) == [1, 4, 5]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_find_membership_skips_pending_channel_by_default(async_db_session: AsyncSession):
|
||||
"""Default channel lookup only returns ACTIVE members (existing behavior)."""
|
||||
repo = SpaceChannelMemberRepositoryImpl(async_db_session)
|
||||
async_db_session.add(
|
||||
SpaceChannelMember(
|
||||
business_id='channel-pending',
|
||||
business_type=BusinessTypeEnum.CHANNEL,
|
||||
user_id=427,
|
||||
user_role=UserRoleEnum.MEMBER,
|
||||
status=MembershipStatusEnum.PENDING,
|
||||
relation=ChannelRelationEnum.VIEWER,
|
||||
)
|
||||
)
|
||||
await async_db_session.commit()
|
||||
|
||||
found = await repo.find_membership('channel-pending', BusinessTypeEnum.CHANNEL, 427)
|
||||
|
||||
assert found is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_find_membership_returns_pending_channel_when_include_inactive(async_db_session: AsyncSession):
|
||||
"""Approval activation must locate the PENDING membership to flip it to ACTIVE."""
|
||||
repo = SpaceChannelMemberRepositoryImpl(async_db_session)
|
||||
async_db_session.add(
|
||||
SpaceChannelMember(
|
||||
business_id='channel-pending',
|
||||
business_type=BusinessTypeEnum.CHANNEL,
|
||||
user_id=427,
|
||||
user_role=UserRoleEnum.MEMBER,
|
||||
status=MembershipStatusEnum.PENDING,
|
||||
relation=ChannelRelationEnum.VIEWER,
|
||||
)
|
||||
)
|
||||
await async_db_session.commit()
|
||||
|
||||
found = await repo.find_membership(
|
||||
'channel-pending', BusinessTypeEnum.CHANNEL, 427, include_inactive=True
|
||||
)
|
||||
|
||||
assert found is not None
|
||||
assert found.status == MembershipStatusEnum.PENDING
|
||||
|
||||
Reference in New Issue
Block a user