From de5ecf4fb2e7528e1da8a46219ce7c733ef3e6a8 Mon Sep 17 00:00:00 2001 From: chihlasm Date: Thu, 9 Apr 2026 05:15:10 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20Phase=201=20Group=203=20=E2=80=94=20add?= =?UTF-8?q?=20account=5Fid=20to=20step=5Fratings=20and=20step=5Fusage=5Flo?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...167e9374b0c_add_account_id_step_ratings.py | 46 +++++++++++ backend/app/models/step_library.py | 14 ++++ backend/tests/test_phase1_migrations.py | 77 +++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 backend/alembic/versions/7167e9374b0c_add_account_id_step_ratings.py diff --git a/backend/alembic/versions/7167e9374b0c_add_account_id_step_ratings.py b/backend/alembic/versions/7167e9374b0c_add_account_id_step_ratings.py new file mode 100644 index 00000000..e34ac86e --- /dev/null +++ b/backend/alembic/versions/7167e9374b0c_add_account_id_step_ratings.py @@ -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') diff --git a/backend/app/models/step_library.py b/backend/app/models/step_library.py index e93c1f75..cd627463 100644 --- a/backend/app/models/step_library.py +++ b/backend/app/models/step_library.py @@ -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"), diff --git a/backend/tests/test_phase1_migrations.py b/backend/tests/test_phase1_migrations.py index b90279a9..416ca4ab 100644 --- a/backend/tests/test_phase1_migrations.py +++ b/backend/tests/test_phase1_migrations.py @@ -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}" + )