feat: Phase 1 Group 3 — add account_id to step_ratings and step_usage_log
Backfill from rater/user's account_id (not the step's account_id). This is an explicit design decision — step rating data is attributed to the account that performed the rating. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,46 @@
|
||||
"""add account_id to step_ratings and step_usage_log
|
||||
|
||||
Revision ID: 7167e9374b0c
|
||||
Revises: 478c159e5654
|
||||
Create Date: 2026-04-09 00:00:00.000000
|
||||
"""
|
||||
from typing import Sequence, Union
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision: str = '7167e9374b0c'
|
||||
down_revision: Union[str, None] = '478c159e5654'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
for table in ('step_ratings', 'step_usage_log'):
|
||||
op.add_column(table, sa.Column('account_id', sa.UUID(), nullable=True))
|
||||
op.create_foreign_key(
|
||||
f'fk_{table}_account_id', table, 'accounts',
|
||||
['account_id'], ['id'], ondelete='CASCADE',
|
||||
)
|
||||
# Backfill: from the RATER/LOGGER user's account (not the step's account)
|
||||
op.execute(f"""
|
||||
UPDATE {table} t
|
||||
SET account_id = u.account_id
|
||||
FROM users u
|
||||
WHERE t.user_id = u.id
|
||||
AND t.account_id IS NULL
|
||||
""")
|
||||
result = op.get_bind().execute(
|
||||
sa.text(f"SELECT COUNT(*) FROM {table} WHERE account_id IS NULL")
|
||||
)
|
||||
count = result.scalar()
|
||||
if count > 0:
|
||||
raise RuntimeError(f"ROLLBACK: {count} NULL account_id rows in {table}.")
|
||||
op.alter_column(table, 'account_id', nullable=False)
|
||||
op.create_index(f'ix_{table}_account_id', table, ['account_id'])
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
for table in ('step_ratings', 'step_usage_log'):
|
||||
op.drop_index(f'ix_{table}_account_id', table_name=table)
|
||||
op.drop_constraint(f'fk_{table}_account_id', table, type_='foreignkey')
|
||||
op.drop_column(table, 'account_id')
|
||||
@@ -143,6 +143,13 @@ class StepRating(Base):
|
||||
ForeignKey("users.id", ondelete="CASCADE"),
|
||||
nullable=False
|
||||
)
|
||||
account_id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("accounts.id", ondelete="CASCADE"),
|
||||
nullable=False,
|
||||
index=True,
|
||||
comment="Account of the RATER (not the step owner).",
|
||||
)
|
||||
rating: Mapped[Optional[int]] = mapped_column(Integer, nullable=True)
|
||||
was_helpful: Mapped[Optional[bool]] = mapped_column(Boolean, nullable=True)
|
||||
review_text: Mapped[Optional[str]] = mapped_column(String(500), nullable=True)
|
||||
@@ -187,6 +194,13 @@ class StepUsageLog(Base):
|
||||
ForeignKey("users.id", ondelete="CASCADE"),
|
||||
nullable=False
|
||||
)
|
||||
account_id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("accounts.id", ondelete="CASCADE"),
|
||||
nullable=False,
|
||||
index=True,
|
||||
comment="Account of the user who logged this usage.",
|
||||
)
|
||||
session_id: Mapped[uuid.UUID] = mapped_column(
|
||||
UUID(as_uuid=True),
|
||||
ForeignKey("sessions.id", ondelete="CASCADE"),
|
||||
|
||||
@@ -221,3 +221,80 @@ async def test_ai_suggestion_account_id_matches_user(test_db: AsyncSession):
|
||||
)
|
||||
row = result.scalar_one()
|
||||
assert row.account_id == account.id
|
||||
|
||||
|
||||
# ── Group 3: Steps & ratings ──────────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_step_rating_account_id_is_rater_account(test_db: AsyncSession):
|
||||
"""step_ratings.account_id must be the RATER's account, not the step's account."""
|
||||
from app.models.step_library import StepLibrary, StepRating
|
||||
|
||||
account_a, user_a = await _make_account_and_user(test_db, "sr-rater")
|
||||
account_b, user_b = await _make_account_and_user(test_db, "sr-step-owner")
|
||||
|
||||
# Step owned by account_b
|
||||
step = StepLibrary(
|
||||
title="A step",
|
||||
step_type="action",
|
||||
content={"text": "do something"},
|
||||
created_by=user_b.id,
|
||||
account_id=account_b.id,
|
||||
visibility="public",
|
||||
)
|
||||
test_db.add(step)
|
||||
await test_db.flush()
|
||||
|
||||
# user_a (account_a) rates the step
|
||||
rating = StepRating(
|
||||
step_id=step.id,
|
||||
user_id=user_a.id,
|
||||
account_id=account_a.id, # rater's account, not step owner's
|
||||
was_helpful=True,
|
||||
is_verified_use=False,
|
||||
is_visible=True,
|
||||
)
|
||||
test_db.add(rating)
|
||||
await test_db.commit()
|
||||
|
||||
result = await test_db.execute(select(StepRating).where(StepRating.id == rating.id))
|
||||
row = result.scalar_one()
|
||||
assert row.account_id == account_a.id, (
|
||||
f"account_id should be rater's account ({account_a.id}), got {row.account_id}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_step_usage_log_account_id_is_logger_account(test_db: AsyncSession):
|
||||
"""step_usage_log.account_id must be the LOGGER's account (user who used the step)."""
|
||||
from app.models.step_library import StepLibrary, StepUsageLog
|
||||
|
||||
account, user = await _make_account_and_user(test_db, "sul1")
|
||||
tree = await _make_tree(test_db, account, user)
|
||||
session = await _make_session(test_db, account, user, tree)
|
||||
|
||||
step = StepLibrary(
|
||||
title="A usage step",
|
||||
step_type="action",
|
||||
content={"text": "do something"},
|
||||
created_by=user.id,
|
||||
account_id=account.id,
|
||||
visibility="team",
|
||||
)
|
||||
test_db.add(step)
|
||||
await test_db.flush()
|
||||
|
||||
log = StepUsageLog(
|
||||
step_id=step.id,
|
||||
user_id=user.id,
|
||||
account_id=account.id,
|
||||
session_id=session.id,
|
||||
)
|
||||
test_db.add(log)
|
||||
await test_db.commit()
|
||||
|
||||
result = await test_db.execute(select(StepUsageLog).where(StepUsageLog.id == log.id))
|
||||
row = result.scalar_one()
|
||||
assert row.account_id == account.id, (
|
||||
f"account_id should be logger's account ({account.id}), got {row.account_id}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user