LinkDesk/backend/docs/soft-deletion-commit-fix.md

Soft Deletion Commit Fix

Issue Description

The soft deletion functionality for shots and assets was not properly setting the deleted_by and deleted_at fields in the database. This was happening because the soft deletion services were using db.flush() to send SQL statements to the database but were not calling db.commit() to actually commit the transaction.

Root Cause

The issue was in the following services and their corresponding router endpoints:

1. ShotSoftDeletionService (backend/services/shot_soft_deletion.py)

   • Method: soft_delete_shot_cascade()
   • Used db.flush() but no db.commit()

2. AssetSoftDeletionService (backend/services/asset_soft_deletion.py)

   • Method: soft_delete_asset_cascade()
   • Used db.flush() but no db.commit()

3. RecoveryService (backend/services/recovery_service.py)

   • Methods: recover_shot(), recover_asset(), bulk_recover_shots(), bulk_recover_assets()
   • Used db.flush() but no db.commit()

4. BatchOperationsService (backend/services/batch_operations.py)

   • Methods: batch_delete_shots(), batch_delete_assets(), batch_recover_shots(), batch_recover_assets()
   • Used db.flush() but no db.commit()

The Difference Between flush() and commit()

• db.flush(): Sends pending SQL statements to the database within the current transaction, but doesn't commit the transaction
• db.commit(): Actually commits the transaction, making the changes permanent in the database

Without db.commit(), the changes were only visible within the current transaction and would be lost when the transaction ended.

Files Fixed

Router Endpoints (Added db.commit() calls):

1. backend/routers/shots.py

   • delete_shot() endpoint - Added commit after successful soft deletion

2. backend/routers/assets.py

   • delete_asset() endpoint - Added commit after successful soft deletion

3. backend/routers/admin.py

   • recover_shot() endpoint - Added commit after successful recovery
   • recover_asset() endpoint - Added commit after successful recovery
   • bulk_recover_shots() endpoint - Added commit after successful bulk recovery
   • bulk_recover_assets() endpoint - Added commit after successful bulk recovery
   • batch_delete_shots() endpoint - Added commit after successful batch deletion
   • batch_delete_assets() endpoint - Added commit after successful batch deletion
   • batch_recover_shots_enhanced() endpoint - Added commit after successful batch recovery
   • batch_recover_assets_enhanced() endpoint - Added commit after successful batch recovery

Pattern Applied:

# Before (incorrect):
result = service.soft_delete_shot_cascade(shot_id, db, current_user)

if not result.success:
    raise HTTPException(...)

return response_data

# After (correct):
result = service.soft_delete_shot_cascade(shot_id, db, current_user)

if not result.success:
    db.rollback()  # Also added rollback on failure
    raise HTTPException(...)

# Commit the transaction
db.commit()

return response_data

Testing

Created backend/test_shot_deletion_fix.py to verify the fix works correctly. The test confirms that both deleted_at and deleted_by fields are now properly set when performing soft deletion operations.

Impact

This fix ensures that:

1. Soft deletion operations properly set deleted_by and deleted_at fields
2. Recovery operations properly clear these fields
3. Batch operations work correctly
4. Database transactions are properly committed
5. Failed operations are properly rolled back

Prevention

To prevent similar issues in the future:

1. Always pair db.flush() with db.commit() in router endpoints
2. Add db.rollback() calls in error handling
3. Consider adding database transaction tests to verify commits work correctly
4. Review all service methods that modify database state to ensure proper transaction handling