Chore/file manager and esigner bug fixes #729
Merged
+498
−138
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughParses list query for document listing; adds soft-delete marker and prevents deletion if signing containers exist; webhook updates now guard undefined fields; adds mime-type display helper and integrates it into UIs; File Manager syncs URL state and improves delete error messaging; DB watcher debounces reloads per table. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FileController
participant FileService
participant SignatureRepo
participant FileRepo
User->>FileController: DELETE /files/:id
FileController->>FileService: deleteFile(fileId, userId)
FileService->>SignatureRepo: countSignatures(fileId)
alt signatures > 0
SignatureRepo-->>FileService: count > 0
FileService-->>FileController: throw Error("signing containers ...")
FileController-->>User: HTTP 409 FILE_HAS_SIGNATURES
else no signatures
SignatureRepo-->>FileService: count = 0
FileService->>FileRepo: removeFileAccess(fileId)
FileRepo-->>FileService: access removed
FileService->>FileRepo: soft-delete file (set name/displayName = SOFT_DELETED_FILE_NAME)
FileRepo-->>FileService: file saved
FileService-->>FileController: success
FileController-->>User: HTTP 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…containers - FileService.deleteFile: check signature_containers count before delete; throw if file is part of any signing container - FileController.deleteFile: return 409 with code FILE_HAS_SIGNATURES when deletion is blocked so clients can show a clear message
…ed in signing) - On delete error, check for 409 and FILE_HAS_SIGNATURES or 'signing container' - Show specific toast: file cannot be deleted because it has been used in a signing container
…mes in File Manager appear in eSigner - Update existing file from repo so entity is mutable - Apply name, displayName, description, mimeType, size, md5Hash only when present in payload so renames and partial syncs propagate correctly
…rrent location
- Add updateUrlFromState() and call it from navigateToFolder and switchView
- Use goto(..., { replaceState: true }) with view and folderId query params
- Existing onMount already reads view and folderId from URL so refresh restores state
…no folderId - When updating existing file, set file.folderId only if local.data.folderId is present in the payload - Prevents eSigner webhooks (no folder concept) from overwriting folderId and moving deeply nested files to root when signed
…igner list, not all files from File Manager - esigner-api: getDocumentsWithStatus(userId, listMode). list=containers (default): only files with at least one signee; list=all: all owner/invited files - FileController.getFiles: read query param list=all and pass listMode - esigner new container page: load selectable files via GET /api/files?list=all into local state so picker shows any file; main list unchanged (containers only)
…CX, XLSX, PDF) instead of raw MIME - Add getMimeTypeDisplayLabel() in both apps: map long MIME types to short labels (XLSX, DOCX, PPTX, PDF) with fallback for unknown types - File Manager files/[id]: use for Type in subtitle and Details - eSigner files/[id]: use for Type in preview area and File Information
coodos
force-pushed
the
chore/file-manager-and-esigner-bug-fixes
branch
from
d982a49 to
39a0b13
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platforms/esigner-api/src/services/FileService.ts`:
- Around line 55-56: The code currently treats SOFT_DELETED_FILE_NAME
("[[deleted]]") as a sentinel and silently hides files named that, but there is
no validation preventing uploads or renames to that name; update FileService to
explicitly reject any incoming filename equal to SOFT_DELETED_FILE_NAME during
create/upload and during rename operations by validating the provided name early
(e.g., in the upload/create and renameFile methods) and returning or throwing a
validation error with a clear message; reference SOFT_DELETED_FILE_NAME and the
FileService methods (upload/create and renameFile) so the checks are added in
the entry points that persist or change file.name.
🧹 Nitpick comments (1)
platforms/esigner-api/src/services/FileService.ts (1)
7-9: Consider centralizing the soft-delete marker.This constant now exists in both file-manager-api and esigner-api. A shared constant (or config) would avoid drift if the marker changes later.
The afterUpdate subscriber was experiencing stale reads because findOne ran inside the transaction before commit. This caused webhooks to send the previous state instead of the current state, resulting in eSigner always being one update behind File Manager. Changes: - Refactored afterUpdate to pass metadata (entityId, relations) instead of immediately loading the entity - Created handleChangeWithReload that does the findOne INSIDE setTimeout after the transaction has committed - Set 50ms delay for files (ensures commit) vs 3s for other tables - This ensures the DB read happens post-commit with fresh data Root cause: TypeORM afterUpdate fires before transaction commit. When findOne uses a different connection from the pool, it sees the last committed state, not the pending uncommitted changes.
Same root cause as file-manager-api: afterUpdate called findOne inside the transaction before commit, causing stale reads. eReputation received the previous wishlist state instead of the current one. Changes: - Refactored afterUpdate to pass metadata (entityId, relations) instead of immediately loading the entity - Created handleChangeWithReload and executeReloadAndSend methods that do the findOne INSIDE setTimeout after transaction commit - Added Wishlist to getRelationsForEntity with ["user"] relation - Wishlists sync with 50ms delay (ensures commit), groups keep 3s debounce This ensures wishlist updates sync to eReputation immediately with fresh, post-commit data.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platforms/file-manager-api/src/web3adapter/watchers/subscriber.ts`:
- Around line 186-201: afterUpdate (the UpdateEvent handler) can't get the
entity id for repository.update() calls because UpdateEvent.entity and
UpdateEvent.databaseEntity are not populated; update flows that call
repository.update(id, data) will skip syncing. Fix by changing update use-sites
or payloads: either modify callers (e.g., GroupService::the update call at
GroupService.ts line ~218) to load the entity and use repository.save(entity) so
afterUpdate receives an entity, or ensure callers include the primary key in the
partial passed to repository.update(...) (so the subscriber can read the id from
event.databaseEntity or the update payload); update afterUpdate only if you add
explicit id extraction from event.queryRunner or event.payload, but preferred
approach is to change repository.update usages to save() or include the PK in
update data so afterUpdate can determine the id.
🧹 Nitpick comments (1)
platforms/file-manager-api/src/web3adapter/watchers/subscriber.ts (1)
248-336: Fixed delays (50ms/3s) create race condition risk under slow transactions.The current setTimeout-based reload may still encounter stale data if a transaction commits after the timeout. If entity modifications in this codebase use explicit transactions (via
DataSource.transaction()), consider using TypeORM'safterTransactionCommithook instead—it guarantees execution only after commit. However, this requires wrapping all relevant entity saves in explicit transactions; if most saves use auto-commit, this approach won't help. Verify transaction coverage in the codebase before refactoring.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of change
Issue Number
closes #633
closes #718
closes #719
closes #716
closes #714
closes #701
closes #700
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.