Fix/473 quarantine noupdate

[LunarCapsule127]

Why is this change needed?

data/quarantine_cron.xml declared two crons and two config params in a plain <odoo> block with no noupdate, so every module upgrade rewrote all four to the shipped defaults. An admin who tuned a retention window, changed a cron interval, or deliberately disabled a cron got silently reset on the next upgrade.

Split out of the #470 review, which fixed the same defect in scan_sweep_cron.xml.

Fixes #473.

How was the change implemented?

1. Wrapped quarantine_cron.xml in <odoo noupdate="1">.
2. Added migrations/19.0.2.2.0/post-migrate.py, since the loader only honors noupdate at record-creation time; on databases that already installed the module, the four ir.model.data rows exist with noupdate = False and the XML change alone would not flip them. The migration sets noupdate = TRUE on those rows and leaves the stored values untouched, so tuned values survive and untouched defaults stay as shipped.
3. Bumped the manifest to 19.0.2.2.0 so the migration runs.

New unit tests

None added; extended the existing regression test.

Unit tests executed by the author

No local environment; relying on repository CI to run spp_attachment_av_scan tests on this PR.

How to test manually

1. Install the module, then change one of the quarantine params (e.g. set spp_attachment_av_scan.quarantine_retention_days to 30) or disable one of the crons.
2. Upgrade the module.
3. Confirm your change survives instead of snapping back to the shipped default.

Related links

• Extends feat(spp_attachment_av_scan): sweep attachments stranded at pending scan #470
• Closes spp_attachment_av_scan: quarantine_cron.xml crons/params are not noupdate — upgrades silently reset admin-tuned values #473

[kneckinator]

The version goes to 19.0.2.2.0 but readme/HISTORY.md still ends at ### 19.0.2.1.0, and no fragment is added. Every other released version of this module has a matching ### <version> section (19.0.2.0.1, 19.0.2.0.2, 19.0.2.1.0), and README.rst/README.md render their changelog from that file, so the shipped module will advertise 19.0.2.2.0 while its published changelog stops at the previous version -- an admin reading the README has no record that the quarantine crons/params became noupdate and that a migration touched ir_model_data. oca-gen-addon-readme runs --if-source-changed, so pre-commit stays green and the gap ships silently.

[LunarCapsule127]

Added a ### 19.0.2.2.0 fragment to readme/HISTORY.md covering both halves the file becoming noupdate="1" and the post-migration reconciling ir_model_data and regenerated README.rst / static/description/index.html so the published changelog no longer stops a version short of what the manifest advertises.

[kneckinator]

The stated rationale is wrong for Odoo 19, and it is wrong in a direction that matters. In odoo/tools/convert.py, xml_import._tag_record short-circuits on the file-level attribute before it ever reaches _load_records:

# in update mode, the record won't be updated if the data node explicitly
# opt-out using @noupdate="1". A second check will be performed in
# model._load_records() using the record's ir.model.data `noupdate` field.
if self.noupdate and self.mode != 'init':
    ...
    if record := env['ir.model.data']._load_xmlid(xid):
        self.idref[xid] = record.id
        return None

self.noupdate comes from <odoo noupdate="1"> via _tag_root, not from the ir_model_data row, so on an already-installed database the XML change alone already stops the reset -- the flag is not "only honored when a record is first created." What the migration actually does is reconcile the ir_model_data.noupdate column, which is what ir.model.data._process_end and the new test read. That is still worth doing, but the comment should say so.

Why it is worth fixing rather than leaving: as written, this reasoning implies (a) a pre-migrate would be required for correctness here, and (b) the XML-only fix merged for scan_sweep_cron.xml in #470 is broken. Neither is true, and the next person to copy this docstring will act on both.

[LunarCapsule127]

You're right, and thanks for pointing at _tag_record rather than just saying it was wrong🙏 The docstring now says the file-level noupdate from <odoo noupdate="1"> is by itself enough to stop the reset on an already-installed database, since _tag_root reads it off the element and _tag_record returns before _load_records whenever the mode isn't init. It states explicitly that no pre-migrate is needed and that the XML-only fix in #470
is correct as it stands, so nobody copying this acts on either wrong inference. What the migration is actually for is now stated as the point: _build_update_xmlids_query upserts (model, res_id, write_date) and never writes noupdate, so databases carrying these xml_ids keep noupdate = false which is the column _process_end and the regression test read.

[kneckinator]

Minor: these four records belong to the quarantine/forensic feature, not the pending-scan sweep, and the test's docstring ("The comments on these records invite the admin to tune them") does not hold for them -- data/quarantine_cron.xml carries no tuning comments at all, unlike scan_sweep_cron.xml. Someone auditing quarantine behaviour will look in tests/test_ir_attachment.py, where _cron_purge_old_quarantined_files and _cron_cleanup_forensic_downloads are already covered, and will not find this guard. Either move the four names into a sibling test there, or generalize this test's name/docstring to cover the module's data records as a whole.

[LunarCapsule127]

Moved rather than generalized the four quarantine records now live in test_ir_attachment.py as test_the_quarantine_crons_and_params_are_not_reset_by_a_module_upgrade, alongside the existing _cron_purge_old_quarantined_files and _cron_cleanup_forensic_downloads coverage, so someone auditing quarantine behaviour finds it where they'd look. test_pending_scan_sweep.py keeps only its own sweep records, and its docstring about tuning comments is true again.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.88%. Comparing base (df808ef) to head (6302026).

Additional details and impacted files

@@           Coverage Diff           @@
##             19.0     #479   +/-   ##
=======================================
  Coverage   76.88%   76.88%           
=======================================
  Files         703      703           
  Lines       45732    45739    +7     
=======================================
+ Hits        35160    35166    +6     
- Misses      10572    10573    +1     

Flag	Coverage Δ
spp_attachment_av_scan	86.72% <ø> (ø)
spp_base_common	91.07% <ø> (ø)
spp_programs	67.58% <ø> (ø)
spp_registry	88.94% <ø> (-0.03%)	⬇️
spp_security	69.56% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[LunarCapsule127]

Thanks so much for the thorough review, @kneckinator, and for the same on #397. The level of detail here is greatly appreciated!

I'm going to work through all the feedback across #397, #478 and #479 over the weekend and push updates that meet the requirements you and @gonzalesedwin1123 have laid out. I'll comment on each PR once they're updated. Thanks again for taking the time!🙏