Skip to content

fix: cover update fails#302

Open
7eliassen wants to merge 1 commit into
mainfrom
fix/nonexistent-file-deletion-error
Open

fix: cover update fails#302
7eliassen wants to merge 1 commit into
mainfrom
fix/nonexistent-file-deletion-error

Conversation

@7eliassen

Copy link
Copy Markdown
Contributor

Problem

When updating a note cover, the API deletes metadata about the previous cover stored in the files table. If no such record exists, the program throws an error (File not found) and the cover field in note settings is not updated.

What was done

Added error handling for FileUploader.deleteFile call in patchNoteSettingsByNoteId. Deletion errors are logged as warnings and no longer interrupt the flow.

@github-actions

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 85.77% (🎯 80%)
⬇️ -0.03%
9281 / 10820
🔵 Statements 85.77% (🎯 80%)
⬇️ -0.03%
9281 / 10820
🔵 Functions 79.55% (🎯 80%)
🟰 ±0%
284 / 357
🔵 Branches 84.78% (🎯 80%)
🟰 ±0%
468 / 552
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/domain/service/noteSettings.ts 94.32%
⬇️ -1.36%
88%
🟰 ±0%
100%
🟰 ±0%
94.32%
⬇️ -1.36%
139-140, 146-157, 254-255
Generated in workflow #923 for commit 618ec87 by the Vitest Coverage Report Action

try {
await this.shared.fileUploader.deleteFile(noteSettings.cover);
} catch (error) {
this.logger.warn('Previous cover not deleted', { err: error });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to take a look at logs after the release and find out the reason why previous file could not be deleted

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the note settings patch flow to prevent cover updates from failing when deletion of the previous cover file record errors (e.g., missing files table entry), improving robustness of cover replacement.

Changes:

  • Wraps fileUploader.deleteFile for the previous cover in a try/catch so deletion failures don’t abort patchNoteSettingsByNoteId.
  • Logs a warning when previous cover deletion fails, then proceeds with updating the cover setting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +146 to +150
try {
await this.shared.fileUploader.deleteFile(noteSettings.cover);
} catch (error) {
this.logger.warn('Previous cover not deleted', { err: error });
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants