feat(arch): implement soft delete for business-critical models
Adds SoftDeleteMixin (deleted_at + deleted_by_id) with automatic query
filtering via do_orm_execute event. Soft-deleted records are invisible
by default; bypass with execution_options={"include_deleted": True}.
Models: User, Merchant, Store, StoreUser, Customer, Order, Product,
LoyaltyProgram, LoyaltyCard.
Infrastructure:
- SoftDeleteMixin in models/database/base.py
- Auto query filter registered on SessionLocal and test sessions
- soft_delete(), restore(), soft_delete_cascade() in app/core/soft_delete.py
- Alembic migration adding columns to 9 tables
- Partial unique indexes on users.email/username, stores.store_code/subdomain
Service changes:
- admin_service: delete_user, delete_store → soft_delete/soft_delete_cascade
- merchant_service: delete_merchant → soft_delete_cascade (stores→children)
- store_team_service: remove_team_member → soft_delete (fixes is_active bug)
- product_service: delete_product → soft_delete
- program_service: delete_program → soft_delete_cascade
Admin API:
- include_deleted/only_deleted query params on admin list endpoints
- PUT restore endpoints for users, merchants, stores
Tests: 9 unit tests for soft-delete infrastructure.
Docs: docs/backend/soft-delete.md + follow-up proposals.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
118
alembic/versions/softdelete_001_add_soft_delete.py
Normal file
118
alembic/versions/softdelete_001_add_soft_delete.py
Normal file
@@ -0,0 +1,118 @@
|
||||
"""Add soft delete columns (deleted_at, deleted_by_id) to business-critical tables.
|
||||
|
||||
Also converts unique constraints on users.email, users.username,
|
||||
stores.store_code, stores.subdomain to partial unique indexes
|
||||
that only apply to non-deleted rows.
|
||||
|
||||
Revision ID: softdelete_001
|
||||
Revises: remove_is_primary_001, customers_002, dev_tools_002, orders_002, tenancy_004
|
||||
Create Date: 2026-03-28
|
||||
"""
|
||||
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision = "softdelete_001"
|
||||
down_revision = (
|
||||
"remove_is_primary_001",
|
||||
"customers_002",
|
||||
"dev_tools_002",
|
||||
"orders_002",
|
||||
"tenancy_004",
|
||||
)
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
# Tables receiving soft-delete columns
|
||||
SOFT_DELETE_TABLES = [
|
||||
"users",
|
||||
"merchants",
|
||||
"stores",
|
||||
"customers",
|
||||
"store_users",
|
||||
"orders",
|
||||
"products",
|
||||
"loyalty_programs",
|
||||
"loyalty_cards",
|
||||
]
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
# ======================================================================
|
||||
# Step 1: Add deleted_at and deleted_by_id to all soft-delete tables
|
||||
# ======================================================================
|
||||
for table in SOFT_DELETE_TABLES:
|
||||
op.add_column(table, sa.Column("deleted_at", sa.DateTime(), nullable=True))
|
||||
op.add_column(
|
||||
table,
|
||||
sa.Column(
|
||||
"deleted_by_id",
|
||||
sa.Integer(),
|
||||
sa.ForeignKey("users.id", ondelete="SET NULL"),
|
||||
nullable=True,
|
||||
),
|
||||
)
|
||||
op.create_index(f"ix_{table}_deleted_at", table, ["deleted_at"])
|
||||
|
||||
# ======================================================================
|
||||
# Step 2: Replace simple unique constraints with partial unique indexes
|
||||
# (only enforce uniqueness among non-deleted rows)
|
||||
# ======================================================================
|
||||
|
||||
# users.email: drop old unique index, create partial
|
||||
op.drop_index("ix_users_email", table_name="users")
|
||||
op.execute(
|
||||
'CREATE UNIQUE INDEX uq_users_email_active ON users (email) '
|
||||
'WHERE deleted_at IS NULL'
|
||||
)
|
||||
# Keep a non-unique index for lookups on all rows (including deleted)
|
||||
op.create_index("ix_users_email", "users", ["email"])
|
||||
|
||||
# users.username: drop old unique index, create partial
|
||||
op.drop_index("ix_users_username", table_name="users")
|
||||
op.execute(
|
||||
'CREATE UNIQUE INDEX uq_users_username_active ON users (username) '
|
||||
'WHERE deleted_at IS NULL'
|
||||
)
|
||||
op.create_index("ix_users_username", "users", ["username"])
|
||||
|
||||
# stores.store_code: drop old unique index, create partial
|
||||
op.drop_index("ix_stores_store_code", table_name="stores")
|
||||
op.execute(
|
||||
'CREATE UNIQUE INDEX uq_stores_store_code_active ON stores (store_code) '
|
||||
'WHERE deleted_at IS NULL'
|
||||
)
|
||||
op.create_index("ix_stores_store_code", "stores", ["store_code"])
|
||||
|
||||
# stores.subdomain: drop old unique index, create partial
|
||||
op.drop_index("ix_stores_subdomain", table_name="stores")
|
||||
op.execute(
|
||||
'CREATE UNIQUE INDEX uq_stores_subdomain_active ON stores (subdomain) '
|
||||
'WHERE deleted_at IS NULL'
|
||||
)
|
||||
op.create_index("ix_stores_subdomain", "stores", ["subdomain"])
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
# Reverse partial unique indexes back to simple unique indexes
|
||||
op.drop_index("ix_stores_subdomain", table_name="stores")
|
||||
op.execute("DROP INDEX IF EXISTS uq_stores_subdomain_active")
|
||||
op.create_index("ix_stores_subdomain", "stores", ["subdomain"], unique=True)
|
||||
|
||||
op.drop_index("ix_stores_store_code", table_name="stores")
|
||||
op.execute("DROP INDEX IF EXISTS uq_stores_store_code_active")
|
||||
op.create_index("ix_stores_store_code", "stores", ["store_code"], unique=True)
|
||||
|
||||
op.drop_index("ix_users_username", table_name="users")
|
||||
op.execute("DROP INDEX IF EXISTS uq_users_username_active")
|
||||
op.create_index("ix_users_username", "users", ["username"], unique=True)
|
||||
|
||||
op.drop_index("ix_users_email", table_name="users")
|
||||
op.execute("DROP INDEX IF EXISTS uq_users_email_active")
|
||||
op.create_index("ix_users_email", "users", ["email"], unique=True)
|
||||
|
||||
# Remove soft-delete columns from all tables
|
||||
for table in reversed(SOFT_DELETE_TABLES):
|
||||
op.drop_index(f"ix_{table}_deleted_at", table_name=table)
|
||||
op.drop_column(table, "deleted_by_id")
|
||||
op.drop_column(table, "deleted_at")
|
||||
@@ -12,8 +12,8 @@ Note: This project uses PostgreSQL only. SQLite is not supported.
|
||||
|
||||
import logging
|
||||
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import declarative_base, sessionmaker
|
||||
from sqlalchemy import create_engine, event
|
||||
from sqlalchemy.orm import declarative_base, sessionmaker, with_loader_criteria
|
||||
from sqlalchemy.pool import QueuePool
|
||||
|
||||
from .config import settings, validate_database_url
|
||||
@@ -38,6 +38,45 @@ Base = declarative_base()
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Soft-delete automatic query filter
|
||||
# ---------------------------------------------------------------------------
|
||||
# Any model that inherits SoftDeleteMixin will automatically have
|
||||
# `WHERE deleted_at IS NULL` appended to SELECT queries.
|
||||
# Bypass with: db.execute(stmt, execution_options={"include_deleted": True})
|
||||
# or db.query(Model).execution_options(include_deleted=True).all()
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def register_soft_delete_filter(session_factory):
|
||||
"""Register the soft-delete query filter on a session factory.
|
||||
|
||||
Call this for any sessionmaker that should auto-exclude soft-deleted records.
|
||||
Used for both the production SessionLocal and test session factories.
|
||||
"""
|
||||
|
||||
@event.listens_for(session_factory, "do_orm_execute")
|
||||
def _soft_delete_filter(orm_execute_state):
|
||||
if (
|
||||
orm_execute_state.is_select
|
||||
and not orm_execute_state.execution_options.get("include_deleted", False)
|
||||
):
|
||||
from models.database.base import SoftDeleteMixin
|
||||
|
||||
orm_execute_state.statement = orm_execute_state.statement.options(
|
||||
with_loader_criteria(
|
||||
SoftDeleteMixin,
|
||||
lambda cls: cls.deleted_at.is_(None),
|
||||
include_aliases=True,
|
||||
)
|
||||
)
|
||||
|
||||
return _soft_delete_filter
|
||||
|
||||
|
||||
# Register on the production session factory
|
||||
register_soft_delete_filter(SessionLocal)
|
||||
|
||||
|
||||
def get_db():
|
||||
"""
|
||||
Database session dependency for FastAPI routes.
|
||||
|
||||
143
app/core/soft_delete.py
Normal file
143
app/core/soft_delete.py
Normal file
@@ -0,0 +1,143 @@
|
||||
# app/core/soft_delete.py
|
||||
"""
|
||||
Soft-delete utility functions.
|
||||
|
||||
Provides helpers for soft-deleting, restoring, and cascade soft-deleting
|
||||
records that use the SoftDeleteMixin.
|
||||
|
||||
Usage:
|
||||
from app.core.soft_delete import soft_delete, restore, soft_delete_cascade
|
||||
|
||||
# Simple soft delete
|
||||
soft_delete(db, user, deleted_by_id=admin.id)
|
||||
|
||||
# Cascade soft delete (merchant + all stores + their children)
|
||||
soft_delete_cascade(db, merchant, deleted_by_id=admin.id, cascade_rels=[
|
||||
("stores", [("products", []), ("customers", []), ("orders", []), ("store_users", [])]),
|
||||
])
|
||||
|
||||
# Restore a soft-deleted record
|
||||
from app.modules.tenancy.models import User
|
||||
restore(db, User, entity_id=42, restored_by_id=admin.id)
|
||||
"""
|
||||
|
||||
import logging
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def soft_delete(db: Session, entity, deleted_by_id: int | None = None) -> None:
|
||||
"""
|
||||
Mark an entity as soft-deleted.
|
||||
|
||||
Sets deleted_at to now and deleted_by_id to the actor.
|
||||
Does NOT call db.commit() — caller is responsible.
|
||||
|
||||
Args:
|
||||
db: Database session.
|
||||
entity: SQLAlchemy model instance with SoftDeleteMixin.
|
||||
deleted_by_id: ID of the user performing the deletion.
|
||||
"""
|
||||
entity.deleted_at = datetime.now(UTC)
|
||||
entity.deleted_by_id = deleted_by_id
|
||||
db.flush()
|
||||
|
||||
logger.info(
|
||||
f"Soft-deleted {entity.__class__.__name__} id={entity.id} "
|
||||
f"by user_id={deleted_by_id}"
|
||||
)
|
||||
|
||||
|
||||
def restore(
|
||||
db: Session,
|
||||
model_class,
|
||||
entity_id: int,
|
||||
restored_by_id: int | None = None,
|
||||
):
|
||||
"""
|
||||
Restore a soft-deleted entity.
|
||||
|
||||
Queries with include_deleted=True to find the record, then clears
|
||||
deleted_at and deleted_by_id.
|
||||
|
||||
Args:
|
||||
db: Database session.
|
||||
model_class: SQLAlchemy model class.
|
||||
entity_id: ID of the entity to restore.
|
||||
restored_by_id: ID of the user performing the restore (for logging).
|
||||
|
||||
Returns:
|
||||
The restored entity.
|
||||
|
||||
Raises:
|
||||
ValueError: If entity not found.
|
||||
"""
|
||||
entity = db.execute(
|
||||
select(model_class).filter(model_class.id == entity_id),
|
||||
execution_options={"include_deleted": True},
|
||||
).scalar_one_or_none()
|
||||
|
||||
if entity is None:
|
||||
raise ValueError(f"{model_class.__name__} with id={entity_id} not found")
|
||||
|
||||
if entity.deleted_at is None:
|
||||
raise ValueError(f"{model_class.__name__} with id={entity_id} is not deleted")
|
||||
|
||||
entity.deleted_at = None
|
||||
entity.deleted_by_id = None
|
||||
db.flush()
|
||||
|
||||
logger.info(
|
||||
f"Restored {model_class.__name__} id={entity_id} "
|
||||
f"by user_id={restored_by_id}"
|
||||
)
|
||||
return entity
|
||||
|
||||
|
||||
def soft_delete_cascade(
|
||||
db: Session,
|
||||
entity,
|
||||
deleted_by_id: int | None = None,
|
||||
cascade_rels: list[tuple[str, list]] | None = None,
|
||||
) -> int:
|
||||
"""
|
||||
Soft-delete an entity and recursively soft-delete its children.
|
||||
|
||||
Args:
|
||||
db: Database session.
|
||||
entity: SQLAlchemy model instance with SoftDeleteMixin.
|
||||
deleted_by_id: ID of the user performing the deletion.
|
||||
cascade_rels: List of (relationship_name, child_cascade_rels) tuples.
|
||||
Example: [("stores", [("products", []), ("customers", [])])]
|
||||
|
||||
Returns:
|
||||
Total number of records soft-deleted (including the root entity).
|
||||
"""
|
||||
count = 0
|
||||
|
||||
# Soft-delete the entity itself
|
||||
soft_delete(db, entity, deleted_by_id)
|
||||
count += 1
|
||||
|
||||
# Recursively soft-delete children
|
||||
if cascade_rels:
|
||||
for rel_name, child_cascade in cascade_rels:
|
||||
children = getattr(entity, rel_name, None)
|
||||
if children is None:
|
||||
continue
|
||||
|
||||
# Handle both collections and single items (uselist=False)
|
||||
if not isinstance(children, list):
|
||||
children = [children]
|
||||
|
||||
for child in children:
|
||||
if hasattr(child, "deleted_at") and child.deleted_at is None:
|
||||
count += soft_delete_cascade(
|
||||
db, child, deleted_by_id, child_cascade
|
||||
)
|
||||
|
||||
return count
|
||||
@@ -26,10 +26,10 @@ from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from app.utils.money import cents_to_euros, euros_to_cents
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class Product(Base, TimestampMixin):
|
||||
class Product(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""Store-specific product.
|
||||
|
||||
Products can be created from marketplace imports or directly by stores.
|
||||
|
||||
@@ -192,9 +192,11 @@ class ProductService:
|
||||
True if deleted
|
||||
"""
|
||||
try:
|
||||
from app.core.soft_delete import soft_delete
|
||||
|
||||
product = self.get_product(db, store_id, product_id)
|
||||
|
||||
db.delete(product)
|
||||
soft_delete(db, product, deleted_by_id=None)
|
||||
|
||||
logger.info(f"Deleted product {product_id} from store {store_id} catalog")
|
||||
return True
|
||||
|
||||
@@ -17,10 +17,10 @@ from sqlalchemy import (
|
||||
from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class Customer(Base, TimestampMixin):
|
||||
class Customer(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""Customer model with store isolation."""
|
||||
|
||||
__tablename__ = "customers"
|
||||
|
||||
@@ -29,7 +29,7 @@ from sqlalchemy import (
|
||||
from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
def generate_card_number() -> str:
|
||||
@@ -48,7 +48,7 @@ def generate_apple_auth_token() -> str:
|
||||
return secrets.token_urlsafe(32)
|
||||
|
||||
|
||||
class LoyaltyCard(Base, TimestampMixin):
|
||||
class LoyaltyCard(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""
|
||||
Customer's loyalty card (PassObject).
|
||||
|
||||
|
||||
@@ -33,7 +33,7 @@ from sqlalchemy.dialects.sqlite import JSON
|
||||
from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class LoyaltyType(str, enum.Enum):
|
||||
@@ -44,7 +44,7 @@ class LoyaltyType(str, enum.Enum):
|
||||
HYBRID = "hybrid" # Both stamps and points
|
||||
|
||||
|
||||
class LoyaltyProgram(Base, TimestampMixin):
|
||||
class LoyaltyProgram(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""
|
||||
Merchant's loyalty program configuration.
|
||||
|
||||
|
||||
@@ -568,19 +568,23 @@ class ProgramService:
|
||||
return program
|
||||
|
||||
def delete_program(self, db: Session, program_id: int) -> None:
|
||||
"""Delete a loyalty program and all associated data."""
|
||||
"""Soft-delete a loyalty program and associated cards."""
|
||||
from app.core.soft_delete import soft_delete_cascade
|
||||
|
||||
program = self.require_program(db, program_id)
|
||||
merchant_id = program.merchant_id
|
||||
|
||||
# Also delete merchant settings
|
||||
# Hard delete merchant settings (config data, not business records)
|
||||
db.query(MerchantLoyaltySettings).filter(
|
||||
MerchantLoyaltySettings.merchant_id == merchant_id
|
||||
).delete()
|
||||
|
||||
db.delete(program)
|
||||
soft_delete_cascade(db, program, deleted_by_id=None, cascade_rels=[
|
||||
("cards", []),
|
||||
])
|
||||
db.commit()
|
||||
|
||||
logger.info(f"Deleted loyalty program {program_id} for merchant {merchant_id}")
|
||||
logger.info(f"Soft-deleted loyalty program {program_id} for merchant {merchant_id}")
|
||||
|
||||
# =========================================================================
|
||||
# Merchant Settings
|
||||
|
||||
@@ -37,10 +37,10 @@ from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from app.utils.money import cents_to_euros, euros_to_cents
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class Order(Base, TimestampMixin):
|
||||
class Order(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""
|
||||
Unified order model for all sales channels.
|
||||
|
||||
|
||||
@@ -10,10 +10,10 @@ from sqlalchemy import Boolean, Column, ForeignKey, Integer, String, Text
|
||||
from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class Merchant(Base, TimestampMixin):
|
||||
class Merchant(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""
|
||||
Represents a merchant (business entity) in the system.
|
||||
|
||||
@@ -74,7 +74,7 @@ class Merchant(Base, TimestampMixin):
|
||||
# ========================================================================
|
||||
# Relationships
|
||||
# ========================================================================
|
||||
owner = relationship("User", back_populates="owned_merchants")
|
||||
owner = relationship("User", foreign_keys="[Merchant.owner_user_id]", back_populates="owned_merchants")
|
||||
"""The user who owns this merchant."""
|
||||
|
||||
stores = relationship(
|
||||
|
||||
@@ -14,6 +14,7 @@ from sqlalchemy import (
|
||||
Column,
|
||||
DateTime,
|
||||
ForeignKey,
|
||||
Index,
|
||||
Integer,
|
||||
String,
|
||||
Text,
|
||||
@@ -24,13 +25,17 @@ from app.core.config import settings
|
||||
|
||||
# Import Base from the central database module instead of creating a new one
|
||||
from app.core.database import Base
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class Store(Base, TimestampMixin):
|
||||
class Store(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""Represents a store in the system."""
|
||||
|
||||
__tablename__ = "stores" # Name of the table in the database
|
||||
__table_args__ = (
|
||||
Index("uq_stores_store_code_active", "store_code", unique=True, postgresql_where="deleted_at IS NULL"),
|
||||
Index("uq_stores_subdomain_active", "subdomain", unique=True, postgresql_where="deleted_at IS NULL"),
|
||||
)
|
||||
|
||||
id = Column(
|
||||
Integer, primary_key=True, index=True
|
||||
@@ -42,11 +47,11 @@ class Store(Base, TimestampMixin):
|
||||
) # Foreign key to the parent merchant
|
||||
|
||||
store_code = Column(
|
||||
String, unique=True, index=True, nullable=False
|
||||
) # Unique, indexed, non-nullable store code column
|
||||
String, index=True, nullable=False
|
||||
) # Indexed, non-nullable store code column (unique among non-deleted)
|
||||
subdomain = Column(
|
||||
String(100), unique=True, nullable=False, index=True
|
||||
) # Unique, non-nullable subdomain column with indexing
|
||||
String(100), nullable=False, index=True
|
||||
) # Non-nullable subdomain column (unique among non-deleted)
|
||||
name = Column(
|
||||
String, nullable=False
|
||||
) # Non-nullable name column for the store (brand name)
|
||||
@@ -418,7 +423,7 @@ class Store(Base, TimestampMixin):
|
||||
}
|
||||
|
||||
|
||||
class StoreUser(Base, TimestampMixin):
|
||||
class StoreUser(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""
|
||||
Represents a user's team membership in a store.
|
||||
|
||||
|
||||
@@ -15,11 +15,11 @@ ROLE SYSTEM (Phase 1 — Consolidated 4-value enum):
|
||||
|
||||
import enum
|
||||
|
||||
from sqlalchemy import Boolean, Column, DateTime, Integer, String
|
||||
from sqlalchemy import Boolean, Column, DateTime, Index, Integer, String
|
||||
from sqlalchemy.orm import relationship
|
||||
|
||||
from app.core.database import Base
|
||||
from models.database.base import TimestampMixin
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
|
||||
class UserRole(str, enum.Enum):
|
||||
@@ -31,14 +31,18 @@ class UserRole(str, enum.Enum):
|
||||
STORE_MEMBER = "store_member" # Team member on specific store(s)
|
||||
|
||||
|
||||
class User(Base, TimestampMixin):
|
||||
class User(Base, TimestampMixin, SoftDeleteMixin):
|
||||
"""Represents a platform user (admins, merchant owners, and store team)."""
|
||||
|
||||
__tablename__ = "users"
|
||||
__table_args__ = (
|
||||
Index("uq_users_email_active", "email", unique=True, postgresql_where="deleted_at IS NULL"),
|
||||
Index("uq_users_username_active", "username", unique=True, postgresql_where="deleted_at IS NULL"),
|
||||
)
|
||||
|
||||
id = Column(Integer, primary_key=True, index=True)
|
||||
email = Column(String, unique=True, index=True, nullable=False)
|
||||
username = Column(String, unique=True, index=True, nullable=False)
|
||||
email = Column(String, index=True, nullable=False)
|
||||
username = Column(String, index=True, nullable=False)
|
||||
first_name = Column(String)
|
||||
last_name = Column(String)
|
||||
hashed_password = Column(String, nullable=False)
|
||||
@@ -57,7 +61,7 @@ class User(Base, TimestampMixin):
|
||||
# Relationships
|
||||
# NOTE: marketplace_import_jobs relationship removed - owned by marketplace module
|
||||
# Use: MarketplaceImportJob.query.filter_by(user_id=user.id) instead
|
||||
owned_merchants = relationship("Merchant", back_populates="owner")
|
||||
owned_merchants = relationship("Merchant", foreign_keys="[Merchant.owner_user_id]", back_populates="owner")
|
||||
store_memberships = relationship(
|
||||
"StoreUser", foreign_keys="[StoreUser.user_id]", back_populates="user"
|
||||
)
|
||||
|
||||
@@ -124,6 +124,8 @@ def get_all_merchants(
|
||||
search: str | None = Query(None, description="Search by merchant name"),
|
||||
is_active: bool | None = Query(None),
|
||||
is_verified: bool | None = Query(None),
|
||||
include_deleted: bool = Query(False, description="Include soft-deleted merchants"),
|
||||
only_deleted: bool = Query(False, description="Show only soft-deleted merchants (trash view)"),
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_admin_api),
|
||||
):
|
||||
@@ -135,6 +137,8 @@ def get_all_merchants(
|
||||
search=search,
|
||||
is_active=is_active,
|
||||
is_verified=is_verified,
|
||||
include_deleted=include_deleted,
|
||||
only_deleted=only_deleted,
|
||||
)
|
||||
|
||||
return MerchantListResponse(
|
||||
@@ -403,3 +407,24 @@ def delete_merchant(
|
||||
db.commit() # ✅ ARCH: Commit at API level for transaction control
|
||||
|
||||
return {"message": f"Merchant {merchant_id} deleted successfully"}
|
||||
|
||||
|
||||
@admin_merchants_router.put("/{merchant_id}/restore")
|
||||
def restore_merchant(
|
||||
merchant_id: int = Path(..., description="Merchant ID"),
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_admin_api),
|
||||
):
|
||||
"""
|
||||
Restore a soft-deleted merchant (Admin only).
|
||||
|
||||
This only restores the merchant record itself.
|
||||
Stores and their children must be restored separately.
|
||||
"""
|
||||
from app.core.soft_delete import restore
|
||||
from app.modules.tenancy.models import Merchant
|
||||
|
||||
restored = restore(db, Merchant, merchant_id, restored_by_id=current_admin.id)
|
||||
db.commit()
|
||||
logger.info(f"Merchant {merchant_id} restored by admin {current_admin.username}")
|
||||
return {"message": f"Merchant '{restored.name}' restored successfully", "merchant_id": merchant_id}
|
||||
|
||||
@@ -87,6 +87,8 @@ def get_all_stores_admin(
|
||||
is_active: bool | None = Query(None),
|
||||
is_verified: bool | None = Query(None),
|
||||
merchant_id: int | None = Query(None, description="Filter by merchant ID"),
|
||||
include_deleted: bool = Query(False, description="Include soft-deleted stores"),
|
||||
only_deleted: bool = Query(False, description="Show only soft-deleted stores (trash view)"),
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_admin_api),
|
||||
):
|
||||
@@ -99,6 +101,8 @@ def get_all_stores_admin(
|
||||
is_active=is_active,
|
||||
is_verified=is_verified,
|
||||
merchant_id=merchant_id,
|
||||
include_deleted=include_deleted,
|
||||
only_deleted=only_deleted,
|
||||
)
|
||||
return StoreListResponse(stores=stores, total=total, skip=skip, limit=limit)
|
||||
|
||||
@@ -309,3 +313,24 @@ def delete_store(
|
||||
message = admin_service.delete_store(db, store.id)
|
||||
db.commit()
|
||||
return {"message": message}
|
||||
|
||||
|
||||
@admin_stores_router.put("/{store_id}/restore")
|
||||
def restore_store(
|
||||
store_id: int,
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_admin_api),
|
||||
):
|
||||
"""
|
||||
Restore a soft-deleted store (Admin only).
|
||||
|
||||
This only restores the store record itself.
|
||||
Child records (products, customers, etc.) must be restored separately.
|
||||
"""
|
||||
from app.core.soft_delete import restore
|
||||
from app.modules.tenancy.models import Store
|
||||
|
||||
restored = restore(db, Store, store_id, restored_by_id=current_admin.id)
|
||||
db.commit()
|
||||
logger.info(f"Store {store_id} restored by admin {current_admin.username}")
|
||||
return {"message": f"Store '{restored.name}' restored successfully", "store_id": store_id}
|
||||
|
||||
@@ -143,6 +143,8 @@ def list_admin_users(
|
||||
skip: int = Query(0, ge=0),
|
||||
limit: int = Query(100, ge=1, le=500),
|
||||
include_super_admins: bool = Query(True),
|
||||
include_deleted: bool = Query(False, description="Include soft-deleted users"),
|
||||
only_deleted: bool = Query(False, description="Show only soft-deleted users (trash view)"),
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_super_admin),
|
||||
):
|
||||
@@ -156,6 +158,8 @@ def list_admin_users(
|
||||
skip=skip,
|
||||
limit=limit,
|
||||
include_super_admins=include_super_admins,
|
||||
include_deleted=include_deleted,
|
||||
only_deleted=only_deleted,
|
||||
)
|
||||
|
||||
admin_responses = [_build_admin_response(admin) for admin in admins]
|
||||
@@ -395,3 +399,26 @@ def delete_admin_user(
|
||||
"message": "Admin user deleted successfully",
|
||||
"user_id": user_id,
|
||||
}
|
||||
|
||||
|
||||
@admin_users_router.put("/{user_id}/restore")
|
||||
def restore_admin_user(
|
||||
user_id: int = Path(...),
|
||||
db: Session = Depends(get_db),
|
||||
current_admin: UserContext = Depends(get_current_super_admin_api),
|
||||
):
|
||||
"""
|
||||
Restore a soft-deleted admin user.
|
||||
|
||||
Super admin only.
|
||||
"""
|
||||
from app.core.soft_delete import restore
|
||||
from app.modules.tenancy.models import User
|
||||
|
||||
restored = restore(db, User, user_id, restored_by_id=current_admin.id)
|
||||
db.commit()
|
||||
|
||||
return {
|
||||
"message": f"User '{restored.username}' restored successfully",
|
||||
"user_id": user_id,
|
||||
}
|
||||
|
||||
@@ -443,6 +443,8 @@ class AdminPlatformService:
|
||||
include_super_admins: bool = True,
|
||||
is_active: bool | None = None,
|
||||
search: str | None = None,
|
||||
include_deleted: bool = False,
|
||||
only_deleted: bool = False,
|
||||
) -> tuple[list[User], int]:
|
||||
"""
|
||||
List all admin users with optional filtering.
|
||||
@@ -454,6 +456,8 @@ class AdminPlatformService:
|
||||
include_super_admins: Whether to include super admins
|
||||
is_active: Filter by active status
|
||||
search: Search term for username/email/name
|
||||
include_deleted: Include soft-deleted users
|
||||
only_deleted: Show only soft-deleted users
|
||||
|
||||
Returns:
|
||||
Tuple of (list of User objects, total count)
|
||||
@@ -462,6 +466,12 @@ class AdminPlatformService:
|
||||
User.role.in_(["super_admin", "platform_admin"])
|
||||
)
|
||||
|
||||
# Soft-delete visibility
|
||||
if include_deleted or only_deleted:
|
||||
query = query.execution_options(include_deleted=True)
|
||||
if only_deleted:
|
||||
query = query.filter(User.deleted_at.isnot(None))
|
||||
|
||||
if not include_super_admins:
|
||||
query = query.filter(User.role == "platform_admin")
|
||||
|
||||
|
||||
@@ -322,8 +322,10 @@ class AdminService:
|
||||
owned_count=len(user.owned_merchants),
|
||||
)
|
||||
|
||||
from app.core.soft_delete import soft_delete
|
||||
|
||||
username = user.username
|
||||
db.delete(user)
|
||||
soft_delete(db, user, deleted_by_id=current_admin_id)
|
||||
|
||||
logger.info(f"Admin {current_admin_id} deleted user {username}")
|
||||
return f"User {username} deleted successfully"
|
||||
@@ -477,12 +479,20 @@ class AdminService:
|
||||
is_active: bool | None = None,
|
||||
is_verified: bool | None = None,
|
||||
merchant_id: int | None = None,
|
||||
include_deleted: bool = False,
|
||||
only_deleted: bool = False,
|
||||
) -> tuple[list[Store], int]:
|
||||
"""Get paginated list of all stores with filtering."""
|
||||
try:
|
||||
# Eagerly load merchant relationship to avoid N+1 queries
|
||||
query = db.query(Store).options(joinedload(Store.merchant))
|
||||
|
||||
# Soft-delete visibility
|
||||
if include_deleted or only_deleted:
|
||||
query = query.execution_options(include_deleted=True)
|
||||
if only_deleted:
|
||||
query = query.filter(Store.deleted_at.isnot(None))
|
||||
|
||||
# Filter by merchant
|
||||
if merchant_id is not None:
|
||||
query = query.filter(Store.merchant_id == merchant_id)
|
||||
@@ -506,6 +516,10 @@ class AdminService:
|
||||
|
||||
# Get total count (without joinedload for performance)
|
||||
count_query = db.query(Store)
|
||||
if include_deleted or only_deleted:
|
||||
count_query = count_query.execution_options(include_deleted=True)
|
||||
if only_deleted:
|
||||
count_query = count_query.filter(Store.deleted_at.isnot(None))
|
||||
if merchant_id is not None:
|
||||
count_query = count_query.filter(Store.merchant_id == merchant_id)
|
||||
if search:
|
||||
@@ -596,17 +610,16 @@ class AdminService:
|
||||
store = self._get_store_by_id_or_raise(db, store_id)
|
||||
|
||||
try:
|
||||
from app.core.soft_delete import soft_delete_cascade
|
||||
|
||||
store_code = store.store_code
|
||||
|
||||
# TODO: Delete associated data in correct order
|
||||
# - Delete orders
|
||||
# - Delete customers
|
||||
# - Delete products
|
||||
# - Delete team members
|
||||
# - Delete roles
|
||||
# - Delete import jobs
|
||||
|
||||
db.delete(store)
|
||||
soft_delete_cascade(db, store, deleted_by_id=None, cascade_rels=[
|
||||
("products", []),
|
||||
("customers", []),
|
||||
("orders", []),
|
||||
("store_users", []),
|
||||
])
|
||||
|
||||
logger.warning(f"Store {store_code} and all associated data deleted")
|
||||
return f"Store {store_code} successfully deleted"
|
||||
|
||||
@@ -148,6 +148,8 @@ class MerchantService:
|
||||
search: str | None = None,
|
||||
is_active: bool | None = None,
|
||||
is_verified: bool | None = None,
|
||||
include_deleted: bool = False,
|
||||
only_deleted: bool = False,
|
||||
) -> tuple[list[Merchant], int]:
|
||||
"""
|
||||
Get paginated list of merchants with optional filters.
|
||||
@@ -159,15 +161,25 @@ class MerchantService:
|
||||
search: Search term for merchant name
|
||||
is_active: Filter by active status
|
||||
is_verified: Filter by verified status
|
||||
include_deleted: Include soft-deleted merchants
|
||||
only_deleted: Show only soft-deleted merchants (trash view)
|
||||
|
||||
Returns:
|
||||
Tuple of (merchants list, total count)
|
||||
"""
|
||||
exec_opts = {}
|
||||
if include_deleted or only_deleted:
|
||||
exec_opts["include_deleted"] = True
|
||||
|
||||
query = select(Merchant).options(
|
||||
joinedload(Merchant.stores),
|
||||
joinedload(Merchant.owner),
|
||||
)
|
||||
|
||||
# Soft-delete filter
|
||||
if only_deleted:
|
||||
query = query.where(Merchant.deleted_at.isnot(None))
|
||||
|
||||
# Apply filters
|
||||
if search:
|
||||
query = query.where(Merchant.name.ilike(f"%{search}%"))
|
||||
@@ -178,13 +190,13 @@ class MerchantService:
|
||||
|
||||
# Get total count
|
||||
count_query = select(func.count()).select_from(query.subquery())
|
||||
total = db.execute(count_query).scalar()
|
||||
total = db.execute(count_query, execution_options=exec_opts).scalar()
|
||||
|
||||
# Apply pagination and order
|
||||
query = query.order_by(Merchant.name).offset(skip).limit(limit)
|
||||
|
||||
# Use unique() when using joinedload with collections to avoid duplicate rows
|
||||
merchants = list(db.execute(query).scalars().unique().all())
|
||||
merchants = list(db.execute(query, execution_options=exec_opts).scalars().unique().all())
|
||||
|
||||
return merchants, total
|
||||
|
||||
@@ -228,11 +240,19 @@ class MerchantService:
|
||||
Raises:
|
||||
MerchantNotFoundException: If merchant not found
|
||||
"""
|
||||
from app.core.soft_delete import soft_delete_cascade
|
||||
|
||||
merchant = self.get_merchant_by_id(db, merchant_id)
|
||||
|
||||
# Due to cascade="all, delete-orphan", associated stores will be deleted
|
||||
db.delete(merchant)
|
||||
db.flush()
|
||||
MERCHANT_CASCADE = [
|
||||
("stores", [
|
||||
("products", []),
|
||||
("customers", []),
|
||||
("orders", []),
|
||||
("store_users", []),
|
||||
]),
|
||||
]
|
||||
soft_delete_cascade(db, merchant, deleted_by_id=None, cascade_rels=MERCHANT_CASCADE)
|
||||
logger.info(f"Deleted merchant ID {merchant_id} and associated stores")
|
||||
|
||||
def toggle_verification(
|
||||
|
||||
@@ -79,12 +79,16 @@ def testing_session_local(engine):
|
||||
commits. This allows fixtures to remain usable after database operations
|
||||
without needing to refresh or re-query them.
|
||||
"""
|
||||
return sessionmaker(
|
||||
from app.core.database import register_soft_delete_filter
|
||||
|
||||
session_factory = sessionmaker(
|
||||
autocommit=False,
|
||||
autoflush=False,
|
||||
bind=engine,
|
||||
expire_on_commit=False, # Prevents lazy-load issues after commits
|
||||
)
|
||||
register_soft_delete_filter(session_factory)
|
||||
return session_factory
|
||||
|
||||
|
||||
@pytest.fixture(scope="session", autouse=True)
|
||||
|
||||
119
docs/backend/soft-delete.md
Normal file
119
docs/backend/soft-delete.md
Normal file
@@ -0,0 +1,119 @@
|
||||
# Soft Delete
|
||||
|
||||
## Overview
|
||||
|
||||
Business-critical records use soft delete instead of hard delete. When a record is "deleted", it gets a `deleted_at` timestamp instead of being removed from the database. This preserves data for investigation, auditing, and potential restoration.
|
||||
|
||||
## How It Works
|
||||
|
||||
### SoftDeleteMixin
|
||||
|
||||
Models opt into soft delete by inheriting `SoftDeleteMixin` (from `models/database/base.py`):
|
||||
|
||||
```python
|
||||
from models.database.base import SoftDeleteMixin, TimestampMixin
|
||||
|
||||
class MyModel(Base, TimestampMixin, SoftDeleteMixin):
|
||||
__tablename__ = "my_table"
|
||||
# ...
|
||||
```
|
||||
|
||||
This adds two columns:
|
||||
|
||||
| Column | Type | Description |
|
||||
|--------|------|-------------|
|
||||
| `deleted_at` | DateTime (nullable, indexed) | When the record was deleted. NULL = alive. |
|
||||
| `deleted_by_id` | Integer (FK to users.id, nullable) | Who performed the deletion. |
|
||||
|
||||
### Automatic Query Filtering
|
||||
|
||||
A `do_orm_execute` event on the session automatically appends `WHERE deleted_at IS NULL` to all SELECT queries for models with `SoftDeleteMixin`. This means:
|
||||
|
||||
- **Normal queries never see deleted records** — no code changes needed
|
||||
- **Relationship lazy loads are also filtered** — e.g., `store.products` won't include deleted products
|
||||
|
||||
### Bypassing the Filter
|
||||
|
||||
To see deleted records (admin views, restore operations):
|
||||
|
||||
```python
|
||||
# Legacy query style
|
||||
db.query(User).execution_options(include_deleted=True).all()
|
||||
|
||||
# Core select style
|
||||
from sqlalchemy import select
|
||||
db.execute(
|
||||
select(User).filter(User.id == 42),
|
||||
execution_options={"include_deleted": True}
|
||||
).scalar_one_or_none()
|
||||
```
|
||||
|
||||
## Models Using Soft Delete
|
||||
|
||||
| Model | Table | Module |
|
||||
|-------|-------|--------|
|
||||
| User | users | tenancy |
|
||||
| Merchant | merchants | tenancy |
|
||||
| Store | stores | tenancy |
|
||||
| StoreUser | store_users | tenancy |
|
||||
| Customer | customers | customers |
|
||||
| Order | orders | orders |
|
||||
| Product | products | catalog |
|
||||
| LoyaltyProgram | loyalty_programs | loyalty |
|
||||
| LoyaltyCard | loyalty_cards | loyalty |
|
||||
|
||||
## Utility Functions
|
||||
|
||||
Import from `app.core.soft_delete`:
|
||||
|
||||
### `soft_delete(db, entity, deleted_by_id)`
|
||||
|
||||
Marks a single record as deleted.
|
||||
|
||||
### `restore(db, model_class, entity_id, restored_by_id)`
|
||||
|
||||
Restores a soft-deleted record. Queries with `include_deleted=True` internally.
|
||||
|
||||
### `soft_delete_cascade(db, entity, deleted_by_id, cascade_rels)`
|
||||
|
||||
Soft-deletes a record and recursively soft-deletes its children:
|
||||
|
||||
```python
|
||||
soft_delete_cascade(db, merchant, deleted_by_id=admin.id, cascade_rels=[
|
||||
("stores", [
|
||||
("products", []),
|
||||
("customers", []),
|
||||
("orders", []),
|
||||
("store_users", []),
|
||||
]),
|
||||
])
|
||||
```
|
||||
|
||||
## Partial Unique Indexes
|
||||
|
||||
Tables with unique constraints (e.g., `users.email`, `stores.store_code`) use **partial unique indexes** that only enforce uniqueness among non-deleted rows:
|
||||
|
||||
```sql
|
||||
CREATE UNIQUE INDEX uq_users_email_active ON users (email) WHERE deleted_at IS NULL;
|
||||
```
|
||||
|
||||
This allows a soft-deleted user's email to be reused by a new registration.
|
||||
|
||||
## Adding Soft Delete to a New Model
|
||||
|
||||
1. Add `SoftDeleteMixin` to the model class
|
||||
2. Create an alembic migration adding `deleted_at` and `deleted_by_id` columns
|
||||
3. If the model has unique constraints, convert them to partial unique indexes
|
||||
4. If the model has relationships to users (ForeignKey to users.id), add `foreign_keys=` to those relationships to resolve ambiguity with `deleted_by_id`
|
||||
5. Register the test session factory with `register_soft_delete_filter()` if not already done
|
||||
|
||||
## What Stays as Hard Delete
|
||||
|
||||
Operational and config data that doesn't need investigation trail:
|
||||
|
||||
- Roles, themes, email settings, invoice settings
|
||||
- Cart items, application logs, notifications
|
||||
- Password/email verification tokens
|
||||
- Domains (store and merchant)
|
||||
- Content pages, media files
|
||||
- Import jobs, marketplace products
|
||||
143
docs/proposals/post-soft-delete-followups.md
Normal file
143
docs/proposals/post-soft-delete-followups.md
Normal file
@@ -0,0 +1,143 @@
|
||||
# Post Soft-Delete Follow-up Tasks
|
||||
|
||||
**Date:** 2026-03-28
|
||||
**Context:** During the soft-delete implementation session, several gaps were identified in the platform. This proposal outlines 6 follow-up tasks in priority order.
|
||||
|
||||
---
|
||||
|
||||
## 1. Admin Email Verification Gap (Quick Fix)
|
||||
|
||||
**Problem:** Admin users (super_admin, platform_admin) are created with `is_email_verified=False` (model default). Login checks `is_email_verified` and blocks unverified users. But there's no admin-facing email verification flow — no verification email is sent on admin creation, and `/resend-verification` is merchant-scoped.
|
||||
|
||||
**Impact:** Newly created admin accounts can't log in until somehow email-verified.
|
||||
|
||||
**Proposed Fix:** Auto-set `is_email_verified=True` when creating admin users via `admin_platform_service.create_super_admin()` and `create_platform_admin()`. Admins are created by super admins, so trust is implicit.
|
||||
|
||||
**Alternative:** Send a verification email on admin creation using the existing `EmailVerificationToken` model and `/verify-email` page endpoint.
|
||||
|
||||
**Files:**
|
||||
- `app/modules/tenancy/services/admin_platform_service.py` — `create_super_admin()`, `create_platform_admin()`
|
||||
|
||||
**Effort:** Small (< 30 min)
|
||||
|
||||
---
|
||||
|
||||
## 2. Customer Soft-Delete Endpoint (Compliance)
|
||||
|
||||
**Problem:** Customers have no delete endpoint at all — not soft delete, not hard delete. Only customer addresses can be deleted. This is a gap for GDPR/data-subject-deletion compliance.
|
||||
|
||||
**Proposed Fix:** Add soft-delete endpoints:
|
||||
- `DELETE /api/v1/store/customers/{customer_id}` — store owner/staff can soft-delete
|
||||
- `DELETE /api/v1/admin/customers/{customer_id}` — admin can soft-delete
|
||||
|
||||
Customer already has `SoftDeleteMixin`. Consider cascading to orders, addresses, and loyalty cards.
|
||||
|
||||
**Files:**
|
||||
- `app/modules/customers/routes/api/store.py` — new DELETE endpoint
|
||||
- `app/modules/customers/services/customer_service.py` — new `delete_customer()` method
|
||||
|
||||
**Effort:** Medium (1-2 hours)
|
||||
|
||||
---
|
||||
|
||||
## 3. Cascade Restore Utility
|
||||
|
||||
**Problem:** `restore()` only restores a single record. Restoring a merchant doesn't auto-restore its stores/products/customers/orders. Admin has to restore each entity one by one.
|
||||
|
||||
**Proposed Fix:** Add `restore_cascade()` to `app/core/soft_delete.py` mirroring `soft_delete_cascade()`. Walk the same relationship tree. Add optional `cascade=true` query param to existing restore endpoints:
|
||||
- `PUT /api/v1/admin/merchants/{id}/restore?cascade=true`
|
||||
- `PUT /api/v1/admin/stores/{id}/restore?cascade=true`
|
||||
|
||||
**Files:**
|
||||
- `app/core/soft_delete.py` — new `restore_cascade()` function
|
||||
- `app/modules/tenancy/routes/api/admin_stores.py` — update restore endpoint
|
||||
- `app/modules/tenancy/routes/api/admin_merchants.py` — update restore endpoint
|
||||
|
||||
**Effort:** Small-Medium (1 hour)
|
||||
|
||||
---
|
||||
|
||||
## 4. Admin Trash UI
|
||||
|
||||
**Problem:** The soft-delete API supports `?only_deleted=true` on admin list endpoints (stores, merchants, users) but there's no UI to browse or restore deleted records.
|
||||
|
||||
**Proposed Fix:** Add a "Trash" toggle/tab to admin list pages:
|
||||
- `admin/stores.html` — toggle between active stores and trash
|
||||
- `admin/merchants.html` — same
|
||||
- `admin/admin-users.html` — same (super admin only)
|
||||
|
||||
Each deleted row shows `deleted_at`, `deleted_by`, and a "Restore" button calling `PUT /api/v1/admin/{entity}/{id}/restore`.
|
||||
|
||||
**Implementation:** The Alpine.js components need a `showDeleted` toggle state that:
|
||||
- Adds `?only_deleted=true` to the list API call
|
||||
- Shows a different table header (with deleted_at column)
|
||||
- Replaces edit/delete actions with a Restore button
|
||||
|
||||
**Files:**
|
||||
- `app/modules/tenancy/templates/tenancy/admin/stores.html`
|
||||
- `app/modules/tenancy/templates/tenancy/admin/merchants.html`
|
||||
- `app/modules/tenancy/templates/tenancy/admin/admin-users.html`
|
||||
- Corresponding JS files in `app/modules/tenancy/static/admin/js/`
|
||||
|
||||
**Effort:** Medium (2-3 hours)
|
||||
|
||||
---
|
||||
|
||||
## 5. Admin Team Management Page
|
||||
|
||||
**Problem:** There is no admin-level page for managing store teams. The admin can see merchant users at `/admin/merchant-users`, but this is a user-centric view — not team-centric. Admin cannot:
|
||||
- View team members per store
|
||||
- Invite/remove team members on behalf of a store
|
||||
- See team composition across the platform
|
||||
|
||||
Store owners manage their teams at `/store/{code}/team`. Merchants manage across stores at `/merchants/account/team`. But admin has no equivalent.
|
||||
|
||||
**Proposed Fix:** Add `/admin/stores/{store_code}/team` page that reuses the existing store team API endpoints (`/api/v1/store/team/*`) with admin auth context. The admin store detail page should link to it.
|
||||
|
||||
**Components needed:**
|
||||
- Page route in `app/modules/tenancy/routes/pages/admin.py`
|
||||
- Template at `app/modules/tenancy/templates/tenancy/admin/store-team.html`
|
||||
- JS component (can largely reuse `store/js/team.js` patterns)
|
||||
- Menu item or link from store detail page
|
||||
|
||||
**Consideration:** Admin already has `/admin/store-roles` for role CRUD. The team page completes the picture.
|
||||
|
||||
**Effort:** Medium-Large (3-4 hours)
|
||||
|
||||
---
|
||||
|
||||
## 6. Merchant Team Roles Page
|
||||
|
||||
**Problem:** Store frontend has a full roles management page (`/store/{code}/team/roles`) with CRUD for custom roles and granular permissions. Merchant portal has no equivalent — merchants can only assign preset roles (manager, staff, support, viewer, marketing) during invite/edit, not create custom roles.
|
||||
|
||||
**Proposed Fix:** Add `/merchants/account/team/roles` page. Since roles are per-store in the data model, the page should:
|
||||
1. Let merchant pick a store from a dropdown
|
||||
2. Show roles for that store (reusing `GET /account/team/stores/{store_id}/roles`)
|
||||
3. Allow CRUD on custom roles (delegating to store team service)
|
||||
|
||||
**Files:**
|
||||
- New page route in `app/modules/tenancy/routes/pages/merchant.py`
|
||||
- New template at `app/modules/tenancy/templates/tenancy/merchant/team-roles.html`
|
||||
- New JS at `app/modules/tenancy/static/merchant/js/merchant-roles.js`
|
||||
- New API endpoints in `app/modules/tenancy/routes/api/merchant.py`
|
||||
- Menu item in `app/modules/tenancy/definition.py` (merchant menu)
|
||||
- i18n keys in 4 locale files
|
||||
|
||||
**Reference:** Store roles page at `templates/tenancy/store/roles.html` and `static/store/js/roles.js`
|
||||
|
||||
**Effort:** Large (4-5 hours)
|
||||
|
||||
---
|
||||
|
||||
## Priority & Sequencing
|
||||
|
||||
| # | Task | Priority | Effort | Dependency |
|
||||
|---|------|----------|--------|------------|
|
||||
| 1 | Admin email verification | Critical | Small | None |
|
||||
| 2 | Customer soft-delete | High (compliance) | Medium | None |
|
||||
| 3 | Cascade restore | Medium | Small | None |
|
||||
| 4 | Admin trash UI | Medium | Medium | None |
|
||||
| 5 | Admin team management | Medium | Medium-Large | None |
|
||||
| 6 | Merchant roles page | Low | Large | None |
|
||||
|
||||
Tasks 1-3 can be done in a single session. Tasks 4-6 are independent and can be tackled in any order.
|
||||
@@ -96,6 +96,7 @@ nav:
|
||||
- Store-in-Token Architecture: backend/store-in-token-architecture.md
|
||||
- Admin Integration Guide: backend/admin-integration-guide.md
|
||||
- Admin Feature Integration: backend/admin-feature-integration.md
|
||||
- Soft Delete: backend/soft-delete.md
|
||||
|
||||
# --- Frontend ---
|
||||
- Frontend:
|
||||
@@ -332,6 +333,7 @@ nav:
|
||||
- RBAC Cleanup Two-Phase Plan: proposals/rbac-cleanup-two-phase-plan.md
|
||||
- Store Login Platform Detection: proposals/store-login-platform-detection.md
|
||||
- Test API Deps Auth Dependencies: proposals/test-api-deps-auth-dependencies.md
|
||||
- Post Soft-Delete Follow-ups: proposals/post-soft-delete-followups.md
|
||||
|
||||
# --- Archive ---
|
||||
- Archive:
|
||||
|
||||
@@ -5,6 +5,7 @@ Database models package - Base classes and mixins only.
|
||||
This package provides the base infrastructure for SQLAlchemy models:
|
||||
- Base: SQLAlchemy declarative base
|
||||
- TimestampMixin: Mixin for created_at/updated_at timestamps
|
||||
- SoftDeleteMixin: Mixin for soft-deletable models (deleted_at/deleted_by_id)
|
||||
|
||||
IMPORTANT: Domain models have been migrated to their respective modules:
|
||||
- Tenancy models: app.modules.tenancy.models
|
||||
@@ -22,9 +23,10 @@ IMPORTANT: Domain models have been migrated to their respective modules:
|
||||
Import models from their canonical module locations instead of this package.
|
||||
"""
|
||||
|
||||
from .base import Base, TimestampMixin
|
||||
from .base import Base, SoftDeleteMixin, TimestampMixin
|
||||
|
||||
__all__ = [
|
||||
"Base",
|
||||
"SoftDeleteMixin",
|
||||
"TimestampMixin",
|
||||
]
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from sqlalchemy import Column, DateTime
|
||||
from sqlalchemy import Column, DateTime, ForeignKey, Integer
|
||||
|
||||
from app.core.database import Base
|
||||
|
||||
@@ -15,3 +15,20 @@ class TimestampMixin:
|
||||
onupdate=datetime.now(UTC),
|
||||
nullable=False,
|
||||
)
|
||||
|
||||
|
||||
class SoftDeleteMixin:
|
||||
"""Mixin for soft-deletable models.
|
||||
|
||||
Adds deleted_at and deleted_by_id columns. Records with deleted_at set
|
||||
are automatically excluded from queries via the do_orm_execute event
|
||||
in app.core.database. Use execution_options={"include_deleted": True}
|
||||
to bypass the filter.
|
||||
"""
|
||||
|
||||
deleted_at = Column(DateTime, nullable=True, index=True)
|
||||
deleted_by_id = Column(
|
||||
Integer,
|
||||
ForeignKey("users.id", ondelete="SET NULL"),
|
||||
nullable=True,
|
||||
)
|
||||
|
||||
296
tests/unit/core/test_soft_delete.py
Normal file
296
tests/unit/core/test_soft_delete.py
Normal file
@@ -0,0 +1,296 @@
|
||||
# tests/unit/core/test_soft_delete.py
|
||||
"""
|
||||
Unit tests for soft-delete infrastructure.
|
||||
|
||||
Tests the SoftDeleteMixin, automatic query filtering, and utility functions.
|
||||
"""
|
||||
|
||||
import uuid
|
||||
from datetime import UTC, datetime
|
||||
|
||||
import pytest
|
||||
|
||||
from app.core.soft_delete import restore, soft_delete, soft_delete_cascade
|
||||
from app.modules.tenancy.models import Merchant, Store, StoreUser, User
|
||||
|
||||
# ============================================================================
|
||||
# Fixtures
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def sd_owner(db):
|
||||
"""Create a user for soft-delete tests."""
|
||||
from middleware.auth import AuthManager
|
||||
|
||||
auth = AuthManager()
|
||||
uid = uuid.uuid4().hex[:8]
|
||||
user = User(
|
||||
email=f"sdowner_{uid}@test.com",
|
||||
username=f"sdowner_{uid}",
|
||||
hashed_password=auth.hash_password("pass123"),
|
||||
role="merchant_owner",
|
||||
is_active=True,
|
||||
)
|
||||
db.add(user)
|
||||
db.commit()
|
||||
db.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def sd_merchant(db, sd_owner):
|
||||
"""Create a merchant for soft-delete tests."""
|
||||
merchant = Merchant(
|
||||
name="SD Test Merchant",
|
||||
owner_user_id=sd_owner.id,
|
||||
contact_email=sd_owner.email,
|
||||
is_active=True,
|
||||
is_verified=True,
|
||||
)
|
||||
db.add(merchant)
|
||||
db.commit()
|
||||
db.refresh(merchant)
|
||||
return merchant
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def sd_store(db, sd_merchant):
|
||||
"""Create a store for soft-delete tests."""
|
||||
uid = uuid.uuid4().hex[:8]
|
||||
store = Store(
|
||||
merchant_id=sd_merchant.id,
|
||||
store_code=f"SDTEST_{uid.upper()}",
|
||||
subdomain=f"sdtest{uid}",
|
||||
name=f"SD Test Store {uid}",
|
||||
is_active=True,
|
||||
is_verified=True,
|
||||
)
|
||||
db.add(store)
|
||||
db.commit()
|
||||
db.refresh(store)
|
||||
return store
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def sd_member_user(db):
|
||||
"""Create another user to be a store member."""
|
||||
from middleware.auth import AuthManager
|
||||
|
||||
auth = AuthManager()
|
||||
uid = uuid.uuid4().hex[:8]
|
||||
user = User(
|
||||
email=f"sdmember_{uid}@test.com",
|
||||
username=f"sdmember_{uid}",
|
||||
hashed_password=auth.hash_password("pass123"),
|
||||
role="store_member",
|
||||
is_active=True,
|
||||
)
|
||||
db.add(user)
|
||||
db.commit()
|
||||
db.refresh(user)
|
||||
return user
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def sd_store_user(db, sd_store, sd_member_user):
|
||||
"""Create a StoreUser membership."""
|
||||
store_user = StoreUser(
|
||||
store_id=sd_store.id,
|
||||
user_id=sd_member_user.id,
|
||||
is_active=True,
|
||||
)
|
||||
db.add(store_user)
|
||||
db.commit()
|
||||
db.refresh(store_user)
|
||||
return store_user
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# SoftDeleteMixin basic behavior
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestSoftDeleteBasic:
|
||||
"""Test soft_delete() utility function."""
|
||||
|
||||
def test_soft_delete_sets_fields(self, db, sd_owner, sd_store):
|
||||
"""soft_delete() sets deleted_at and deleted_by_id."""
|
||||
soft_delete(db, sd_store, deleted_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
# Query with include_deleted to see the record
|
||||
store = (
|
||||
db.query(Store)
|
||||
.execution_options(include_deleted=True)
|
||||
.filter(Store.id == sd_store.id)
|
||||
.first()
|
||||
)
|
||||
assert store is not None
|
||||
assert store.deleted_at is not None
|
||||
assert store.deleted_by_id == sd_owner.id
|
||||
|
||||
def test_soft_deleted_excluded_from_queries(self, db, sd_owner, sd_store):
|
||||
"""Soft-deleted records are automatically excluded from normal queries."""
|
||||
store_id = sd_store.id
|
||||
soft_delete(db, sd_store, deleted_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
result = db.query(Store).filter(Store.id == store_id).first()
|
||||
assert result is None
|
||||
|
||||
def test_soft_deleted_visible_with_include_deleted(self, db, sd_owner, sd_store):
|
||||
"""Soft-deleted records are visible with include_deleted=True."""
|
||||
store_id = sd_store.id
|
||||
soft_delete(db, sd_store, deleted_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
result = (
|
||||
db.query(Store)
|
||||
.execution_options(include_deleted=True)
|
||||
.filter(Store.id == store_id)
|
||||
.first()
|
||||
)
|
||||
assert result is not None
|
||||
assert result.id == store_id
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Restore
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestRestore:
|
||||
"""Test restore() utility function."""
|
||||
|
||||
def test_restore_clears_deleted_fields(self, db, sd_owner, sd_store):
|
||||
"""restore() clears deleted_at and deleted_by_id."""
|
||||
soft_delete(db, sd_store, deleted_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
restored = restore(db, Store, sd_store.id, restored_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
assert restored.deleted_at is None
|
||||
assert restored.deleted_by_id is None
|
||||
|
||||
def test_restore_makes_record_visible(self, db, sd_owner, sd_store):
|
||||
"""After restore, record is visible in normal queries."""
|
||||
store_id = sd_store.id
|
||||
soft_delete(db, sd_store, deleted_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
restore(db, Store, store_id, restored_by_id=sd_owner.id)
|
||||
db.commit()
|
||||
|
||||
result = db.query(Store).filter(Store.id == store_id).first()
|
||||
assert result is not None
|
||||
|
||||
def test_restore_not_deleted_raises(self, db, sd_store):
|
||||
"""restore() raises ValueError if record is not deleted."""
|
||||
with pytest.raises(ValueError, match="is not deleted"):
|
||||
restore(db, Store, sd_store.id, restored_by_id=1)
|
||||
|
||||
def test_restore_not_found_raises(self, db):
|
||||
"""restore() raises ValueError if record doesn't exist."""
|
||||
with pytest.raises(ValueError, match="not found"):
|
||||
restore(db, Store, 99999, restored_by_id=1)
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Cascade soft delete
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestSoftDeleteCascade:
|
||||
"""Test soft_delete_cascade() utility function."""
|
||||
|
||||
def test_cascade_deletes_parent_and_children(
|
||||
self, db, sd_owner, sd_store, sd_store_user
|
||||
):
|
||||
"""soft_delete_cascade() deletes parent and its children."""
|
||||
count = soft_delete_cascade(
|
||||
db,
|
||||
sd_store,
|
||||
deleted_by_id=sd_owner.id,
|
||||
cascade_rels=[("store_users", [])],
|
||||
)
|
||||
db.commit()
|
||||
|
||||
assert count == 2 # store + store_user
|
||||
|
||||
# Both should be hidden from normal queries
|
||||
assert db.query(Store).filter(Store.id == sd_store.id).first() is None
|
||||
assert (
|
||||
db.query(StoreUser).filter(StoreUser.id == sd_store_user.id).first()
|
||||
is None
|
||||
)
|
||||
|
||||
# Both visible with include_deleted
|
||||
store = (
|
||||
db.query(Store)
|
||||
.execution_options(include_deleted=True)
|
||||
.filter(Store.id == sd_store.id)
|
||||
.first()
|
||||
)
|
||||
assert store is not None
|
||||
assert store.deleted_at is not None
|
||||
|
||||
su = (
|
||||
db.query(StoreUser)
|
||||
.execution_options(include_deleted=True)
|
||||
.filter(StoreUser.id == sd_store_user.id)
|
||||
.first()
|
||||
)
|
||||
assert su is not None
|
||||
assert su.deleted_at is not None
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Partial unique indexes
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestPartialUniqueIndexes:
|
||||
"""Test that unique constraints allow reuse after soft delete."""
|
||||
|
||||
def test_user_email_reusable_after_soft_delete(self, db):
|
||||
"""Soft-deleted user's email can be used by a new user."""
|
||||
from middleware.auth import AuthManager
|
||||
|
||||
auth = AuthManager()
|
||||
email = f"reuse_{uuid.uuid4().hex[:8]}@test.com"
|
||||
username = f"reuse_{uuid.uuid4().hex[:8]}"
|
||||
|
||||
user1 = User(
|
||||
email=email,
|
||||
username=username,
|
||||
hashed_password=auth.hash_password("pass123"),
|
||||
role="store_member",
|
||||
is_active=True,
|
||||
)
|
||||
db.add(user1)
|
||||
db.commit()
|
||||
|
||||
# Soft-delete user1
|
||||
soft_delete(db, user1, deleted_by_id=None)
|
||||
db.commit()
|
||||
|
||||
# Create user2 with same email — should succeed
|
||||
user2 = User(
|
||||
email=email,
|
||||
username=f"reuse2_{uuid.uuid4().hex[:8]}",
|
||||
hashed_password=auth.hash_password("pass123"),
|
||||
role="store_member",
|
||||
is_active=True,
|
||||
)
|
||||
db.add(user2)
|
||||
db.commit()
|
||||
db.refresh(user2)
|
||||
|
||||
assert user2.id is not None
|
||||
assert user2.email == email
|
||||
Reference in New Issue
Block a user