This repository has no description
0

Configure Feed

Select the types of activity you want to include in your feed.

working notes about card deletion cascades

+360
+360
.agent/logs/20260204_open_collection_card_removal.md
··· 1 + # Open Collection Card Removal Analysis 2 + 3 + **Date:** 2026-02-04 4 + **Issue:** Cards being removed from open collections when users remove them from their library 5 + 6 + --- 7 + 8 + ## Original Issue 9 + 10 + ### Observed Behavior 11 + 12 + When a user removed a card from their library, the card was also being removed from open collections, even when those collections were created and owned by other users. 13 + 14 + **Example Scenario:** 15 + 16 + - User A creates Open Collection X 17 + - User B creates Card Y and adds it to Collection X 18 + - User B removes Card Y from their library 19 + - **Result:** Card Y disappears from Collection X (unexpected) 20 + 21 + ### Expected Behavior 22 + 23 + When a user removes a card from their library: 24 + 25 + - The card should only be removed from collections owned by the card's author 26 + - Cards should remain in open collections created by other users 27 + 28 + --- 29 + 30 + ## Changes Made 31 + 32 + ### 1. CardLibraryService.ts 33 + 34 + **File:** `src/modules/cards/domain/services/CardLibraryService.ts` 35 + 36 + **Change:** Added conditional check to only remove from collections when the user removing the card is also the card author. 37 + 38 + ```typescript 39 + // Only remove from collections if the user removing is the card author 40 + // This ensures only card authors can clean up their own collections 41 + if (card.curatorId.equals(curatorId)) { 42 + // Get all collections owned by the card's author that contain this card 43 + const collectionsResult = 44 + await this.collectionRepository.findByCuratorIdContainingCard( 45 + card.curatorId, // Changed from curatorId to card.curatorId 46 + card.cardId, 47 + ); 48 + 49 + // ... remove from collections logic 50 + } 51 + ``` 52 + 53 + **Before:** `findByCuratorIdContainingCard(curatorId, card.cardId)` 54 + 55 + - Found collections owned by the person removing the card 56 + 57 + **After:** Wrapped in conditional + `findByCuratorIdContainingCard(card.curatorId, card.cardId)` 58 + 59 + - Only executes if person removing = card author 60 + - Finds collections owned by the card author 61 + 62 + ### 2. Test Coverage Added 63 + 64 + **File:** `src/modules/cards/tests/application/RemoveCardFromLibraryUseCase.test.ts` 65 + 66 + Added three new test cases: 67 + 68 + 1. **"should not remove from collections when non-author removes card from library"** 69 + - Alice creates Card A and Collection X 70 + - Bob adds Card A to his library 71 + - Bob removes Card A from his library 72 + - ✅ Card A remains in Alice's Collection X 73 + 74 + 2. **"should not remove card from open collections owned by others when card author removes from library"** 75 + - Alice creates Card A 76 + - Bob creates Open Collection Y 77 + - Alice adds Card A to Bob's Collection Y 78 + - Alice removes Card A from her library 79 + - ⚠️ Card A gets deleted (CASCADE removes from Collection Y) 80 + 81 + 3. **"should only remove from author-owned collections, not from open collections by others"** 82 + - Alice creates Card A 83 + - Alice creates Collection X, Bob creates Collection Y 84 + - Alice adds Card A to both collections 85 + - Bob adds Card A to his library (prevents deletion) 86 + - Alice removes Card A from her library 87 + - ✅ Card removed from Collection X, remains in Collection Y 88 + 89 + --- 90 + 91 + ## Analysis of `findByCuratorIdContainingCard` 92 + 93 + ### Method Purpose 94 + 95 + **File:** `src/modules/cards/infrastructure/repositories/DrizzleCollectionRepository.ts:298-391` 96 + 97 + ```typescript 98 + async findByCuratorIdContainingCard( 99 + authorId: CuratorId, 100 + cardId: CardId, 101 + ): Promise<Result<Collection[]>> 102 + ``` 103 + 104 + ### SQL Query Logic 105 + 106 + ```typescript 107 + .where( 108 + and( 109 + eq(collections.authorId, authorIdString), // Collection author = provided ID 110 + eq(collectionCards.cardId, cardIdString), // Collection contains the card 111 + ), 112 + ); 113 + ``` 114 + 115 + Finds collections where: 116 + 117 + - **Collection author** equals the provided `authorId` parameter 118 + - **Collection contains** the specified `cardId` 119 + 120 + ### Conclusion 121 + 122 + ✅ **Method is working correctly.** It properly filters collections by author ID and did NOT contribute to removing cards from other users' collections through the collection cleanup logic. 123 + 124 + --- 125 + 126 + ## Root Cause Discovery: Card Deletion CASCADE 127 + 128 + ### The Real Culprit 129 + 130 + The observed behavior of cards disappearing from other users' collections was NOT caused by the collection cleanup logic, but by **card deletion with database CASCADE**. 131 + 132 + **File:** `src/modules/cards/application/useCases/commands/RemoveCardFromLibraryUseCase.ts:112-150` 133 + 134 + ```typescript 135 + // Handle deletion with proper ordering for URL cards 136 + if (updatedCard.libraryCount === 0 && updatedCard.curatorId.equals(curatorId)) { 137 + // Delete the card from the database 138 + const deleteResult = await this.cardRepository.delete(updatedCard.cardId); 139 + } 140 + ``` 141 + 142 + ### Deletion Flow 143 + 144 + When a user removes a card from their library: 145 + 146 + 1. Card's `libraryCount` is decremented 147 + 2. If `libraryCount === 0` AND user is the card author: 148 + - Card is **deleted entirely** from the database 149 + 3. Database CASCADE (ON DELETE CASCADE) automatically removes the card from ALL collections 150 + 4. This includes collections owned by other users 151 + 152 + ### Example with CASCADE 153 + 154 + **Scenario:** 155 + 156 + - User B creates Card Y (User B is card author) 157 + - User B adds Card Y to their library (libraryCount = 1) 158 + - User A creates Open Collection X 159 + - User B adds Card Y to Collection X 160 + - User B removes Card Y from library 161 + 162 + **What happens:** 163 + 164 + 1. Card Y's libraryCount becomes 0 165 + 2. User B is card author ✓ 166 + 3. Card Y gets **DELETED** 167 + 4. Database CASCADE removes Card Y from Collection X 168 + 5. Card Y disappears from User A's collection 169 + 170 + ### Database Schema 171 + 172 + **File:** `src/modules/cards/tests/test-utils/createTestSchema.ts` 173 + 174 + ```sql 175 + CREATE TABLE IF NOT EXISTS collection_cards ( 176 + id UUID PRIMARY KEY DEFAULT uuid_generate_v4(), 177 + collection_id UUID NOT NULL REFERENCES collections(id) ON DELETE CASCADE, 178 + card_id UUID NOT NULL REFERENCES cards(id) ON DELETE CASCADE, -- CASCADE here! 179 + ... 180 + ) 181 + ``` 182 + 183 + When a card is deleted, `ON DELETE CASCADE` automatically removes all `collection_cards` entries. 184 + 185 + --- 186 + 187 + ## Current State 188 + 189 + ### What Our Changes Fixed ✅ 190 + 191 + 1. **Collection cleanup logic now correct:** 192 + - Only card authors can trigger collection cleanup 193 + - Only removes from collections owned by the card author 194 + - Non-authors removing cards from their library doesn't affect any collections 195 + 196 + 2. **Test coverage:** 197 + - Comprehensive tests verify the new behavior 198 + - Edge cases covered (non-author removal, mixed collections, etc.) 199 + 200 + ### What Our Changes DID NOT Fix ⚠️ 201 + 202 + **Card deletion CASCADE still affects other users' collections:** 203 + 204 + Even with our changes, cards are still removed from other users' collections when: 205 + 206 + 1. Card author removes the card from their library 207 + 2. Card's `libraryCount` drops to 0 208 + 3. Card gets deleted (because author is removing and libraryCount = 0) 209 + 4. CASCADE deletion removes from ALL collections, including those owned by others 210 + 211 + **Example:** 212 + 213 + - User B creates Card Y 214 + - User B adds to their library (libraryCount = 1) 215 + - User A adds Card Y to their Open Collection X 216 + - User B removes Card Y from library 217 + - Card Y deleted (libraryCount = 0, curator = owner) 218 + - CASCADE removes from Collection X ❌ 219 + 220 + --- 221 + 222 + ## Recommendations 223 + 224 + ### Option 1: Prevent Deletion if Card Exists in ANY Collections 225 + 226 + **Change:** Don't delete cards if they exist in any collection, regardless of ownership. 227 + 228 + ```typescript 229 + // Before deletion check: 230 + const collections = await this.collectionRepository.findByCardId(card.cardId); 231 + if (collections.length > 0) { 232 + // Don't delete - card is in use in collections 233 + return ok(card); 234 + } 235 + 236 + // Only delete if not in any collections 237 + if (updatedCard.libraryCount === 0 && updatedCard.curatorId.equals(curatorId)) { 238 + // Safe to delete 239 + } 240 + ``` 241 + 242 + **Pros:** 243 + 244 + - Cards remain accessible in collections even if removed from all libraries 245 + - Preserves curated collections 246 + 247 + **Cons:** 248 + 249 + - Cards may accumulate without being in any library 250 + - Need cleanup mechanism for orphaned cards 251 + 252 + ### Option 2: Only Prevent if in Card Author's Collections 253 + 254 + **Change:** Don't delete cards if they exist in collections owned by the card author. 255 + 256 + ```typescript 257 + const authorCollections = 258 + await this.collectionRepository.findByCuratorIdContainingCard( 259 + card.curatorId, 260 + card.cardId, 261 + ); 262 + 263 + if (authorCollections.length > 0) { 264 + // Don't delete - card is in author's collections 265 + return ok(card); 266 + } 267 + ``` 268 + 269 + **Pros:** 270 + 271 + - Card authors maintain control over their own collections 272 + - Still allows deletion if only in others' collections 273 + 274 + **Cons:** 275 + 276 + - Cards can still disappear from other users' collections 277 + - Doesn't solve the original issue 278 + 279 + ### Option 3: Soft Delete with Visibility Flag 280 + 281 + **Change:** Mark cards as "deleted" but keep them in database. 282 + 283 + ```typescript 284 + // Instead of deleting: 285 + card.markAsDeleted(); 286 + await this.cardRepository.save(card); 287 + 288 + // Collections can still reference the card 289 + // UI can choose to hide/show deleted cards 290 + ``` 291 + 292 + **Pros:** 293 + 294 + - No CASCADE issues 295 + - Collections maintain integrity 296 + - Can implement "undelete" functionality 297 + 298 + **Cons:** 299 + 300 + - More complex implementation 301 + - Need to handle deleted cards in queries 302 + 303 + ### Option 4: Collection-Based Ownership 304 + 305 + **Change:** Cards exist as long as they're in at least one collection OR library. 306 + 307 + ```typescript 308 + const totalReferences = 309 + updatedCard.libraryCount + 310 + (await this.collectionRepository.findByCardId(card.cardId)).length; 311 + 312 + if (totalReferences === 0 && updatedCard.curatorId.equals(curatorId)) { 313 + // Only delete if not in ANY library or collection 314 + await this.cardRepository.delete(updatedCard.cardId); 315 + } 316 + ``` 317 + 318 + **Pros:** 319 + 320 + - Natural model - collections are "collections of cards" 321 + - Solves the CASCADE issue 322 + - Cards exist as long as they're referenced anywhere 323 + 324 + **Cons:** 325 + 326 + - Changes the ownership model 327 + - Need to handle card updates when author removes from library 328 + 329 + --- 330 + 331 + ## Summary 332 + 333 + ### Key Findings 334 + 335 + 1. ✅ `findByCuratorIdContainingCard` is working correctly 336 + 2. ✅ Collection cleanup logic now only targets card author's collections 337 + 3. ⚠️ Card deletion CASCADE is the root cause of cards disappearing from other users' collections 338 + 4. ⚠️ Our changes fixed collection cleanup but NOT CASCADE deletion 339 + 340 + ### Next Decision Point 341 + 342 + **Should cards be deleted when they exist in collections owned by other users?** 343 + 344 + This is a product/design decision that affects: 345 + 346 + - Card ownership model 347 + - Collection integrity 348 + - User expectations 349 + - Database cleanup strategy 350 + 351 + Current implementation: Cards are deleted when `libraryCount = 0` and user is card author, regardless of collection membership. 352 + 353 + ### Files Modified 354 + 355 + - `src/modules/cards/domain/services/CardLibraryService.ts` 356 + - `src/modules/cards/tests/application/RemoveCardFromLibraryUseCase.test.ts` 357 + 358 + ### Type Check 359 + 360 + ✅ All type checks pass: `npm run build:check`