Skip to content

[17.0][IMP] auto_backup_fs_file: GC-based cleanup, folder visibility, regression test#3683

Open
rrebollo wants to merge 5 commits into
OCA:17.0from
BinhexTeam:17.0-imp-auto_backup_fs_file
Open

[17.0][IMP] auto_backup_fs_file: GC-based cleanup, folder visibility, regression test#3683
rrebollo wants to merge 5 commits into
OCA:17.0from
BinhexTeam:17.0-imp-auto_backup_fs_file

Conversation

@rrebollo

@rrebollo rrebollo commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Port improvements from the v18 migration PR (#3619) back to v17.

Changes

F1+F2 — folder visibility and constrains

  • Override folder as required=False so fs_file records do not require a folder path
  • Add _check_folder_required_for_method constrains to re-enforce folder requirement only for local/sftp methods
  • Hide folder in form view when method == 'fs_file'
  • Show responsible_id only when method == 'fs_file'

F3 — explicit GC chain in db.backup.fs.file

  • Add @api.depends('db_backup_id.days_to_keep', 'create_date') to _compute_is_expired so recomputation is triggered correctly
  • Add explicit unlink() override to ensure fs_attachment GC stack (_fs_mark_for_gc) is triggered when a backup record is deleted
  • Remove dead get_fs_storage_filename() method (no longer called)

F4 — cleanup() via GC, not rm_file

  • Remove direct fs.rm_file() call in cleanup()
  • Physical file deletion now delegated to ir.attachment.unlink() → autovacuum GC

F5 — cleanup() reachable from action_backup()

  • Track successful_fs recordset inside action_backup()
  • Call successful_fs.cleanup() after super().action_backup()
  • Bug: previously cleanup() was never called for fs_file records via the production cron path

F6 — regression test

  • New test_action_backup_triggers_cleanup verifying the F5 fix end-to-end
  • GC chain assertion added to test_ordinary_flow
  • assertFalse(backup_config.folder) added to verify F1

Docs + typos

  • Updated ROADMAP, CONTEXT, USAGE with GC-based cleanup explanation
  • Fixed :wa typo in INSTALL.md

Related: #3619 (v18), #3621 (v16)
Migration issue: #2752
@BinhexTeam T22571

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:auto_backup_fs_file Module auto_backup_fs_file series:17.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants