Skip to content

[16.0][IMP] database_cleanup: purge orphaned attachments - #3566

Open
Aldeigja wants to merge 1 commit into
OCA:16.0from
cetmix:16.0-t5258-database_cleanup-orphaned-attachments-clean
Open

[16.0][IMP] database_cleanup: purge orphaned attachments#3566
Aldeigja wants to merge 1 commit into
OCA:16.0from
cetmix:16.0-t5258-database_cleanup-orphaned-attachments-clean

Conversation

@Aldeigja

Copy link
Copy Markdown

Add wizard to list ir.attachment records whose store_fname points to a missing file on disk when storage is "file", and allow purging those rows.

@Aldeigja
Aldeigja force-pushed the 16.0-t5258-database_cleanup-orphaned-attachments-clean branch from 699d8f0 to ab70051 Compare March 20, 2026 10:03

@StefanRijnhart StefanRijnhart left a comment

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.

Thanks! Judging from the name of the PR and the patterns from this module, I would have expected signalling attachments where the model no longer exists or the res_id is not present as a record. This makes sense as well, but to make it explicit what the issue is with each attachment (and to prepare for extension with other possible error conditions), would you mind adding a reason field to the wizard line model as is done in cleanup.purge.line, with for now a fixed reason like 'File missing in filestore'?

Comment thread database_cleanup/tests/test_purge_attachments.py Outdated
Comment thread database_cleanup/tests/test_purge_attachments.py
@Aldeigja
Aldeigja force-pushed the 16.0-t5258-database_cleanup-orphaned-attachments-clean branch from b372147 to 95fb498 Compare April 3, 2026 23:00
@Aldeigja
Aldeigja requested a review from StefanRijnhart April 3, 2026 23:12

@StefanRijnhart StefanRijnhart left a comment

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.

Thanks for the updates! Please squash commits.

Comment thread database_cleanup/models/purge_attachments.py Outdated
Comment thread database_cleanup/models/purge_attachments.py
Comment thread database_cleanup/models/purge_attachments.py Outdated
@Aldeigja
Aldeigja force-pushed the 16.0-t5258-database_cleanup-orphaned-attachments-clean branch from 95fb498 to e8b0061 Compare April 4, 2026 17:08
@Aldeigja
Aldeigja requested a review from StefanRijnhart April 4, 2026 17:11

@StefanRijnhart StefanRijnhart left a comment

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.

Thanks, the error message is stored correctly.

Image

Usability wise, I think as a user I would want to see some more information about the attachments, the res_model, res_id and res_name, with or without optional visibility. Is this not something that you want for your own purposes?

@Aldeigja

Aldeigja commented Apr 6, 2026

Copy link
Copy Markdown
Author

@StefanRijnhart

Usability wise, I think as a user I would want to see some more information about the attachments, the res_model, res_id and res_name, with or without optional visibility.
Is this not something that you want for your own purposes?

Our objective in creating this PR was to clean approximately 15,000 orphaned attachments simultaneously. We didn’t have to inspect every single attachment.
However, if users need to check details, they can do so by clicking on the line, opening the line pop-up, and then accessing the attachment form.
image

@StefanRijnhart StefanRijnhart left a comment

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.

OK, leaving my last comment as a nice to have. Nice work!

from .common import Common, environment


# Use post_install to get all models loaded more info: odoo/odoo#13458

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this comment belongs here


:var orphan: ir.attachment with backing file removed via os.unlink
:var valid: ir.attachment with file intact
:return: None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove redundant lines such as ":return: None" to avoid unnecessary noise

Comment thread database_cleanup/models/purge_attachments.py
@Aldeigja
Aldeigja force-pushed the 16.0-t5258-database_cleanup-orphaned-attachments-clean branch from e8b0061 to 831b3af Compare August 14, 2026 11:27
Add wizard to list ir.attachment records whose store_fname points to a
missing file on disk when storage is "file", and allow purging those
rows.

task 5258
@Aldeigja
Aldeigja force-pushed the 16.0-t5258-database_cleanup-orphaned-attachments-clean branch from 831b3af to 49ce4fe Compare August 14, 2026 11:41
@Aldeigja
Aldeigja requested a review from tendil August 14, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants