14.0 T3436 visible letters during suspension - #2150
Merged
Merged
Conversation
…Compassion correspondence.hold_letters()/create_commkit() move a sponsor's letter to state "Exception" whenever its FCP is suspended or (for Christmas letters) outside the Christmas period, so it is queued rather than sent. The portal's is_published compute (my_compassion) treated any "Exception" state as unpublished, so the letter vanished from MyCompassion entirely - sponsors got no confirmation their letter was even received, well before the 3-month suspension-notification threshold. Add on_hold, set whenever a letter is deliberately queued this way (and cleared on reactivate_letters()), so the portal can tell a benign hold apart from an actual failure (compose failure, quality check unsuccessful) that should stay hidden. Backfill on_hold for letters already stuck in Exception for a currently-held reason.
Adding on_hold as a plain column is cheap, but this DB's ~1.3M-row correspondence table made -u expensive regardless; pre-creating the column in a pre-migration (before _auto_init runs) matches Odoo's own create_column logic (odoo/fields.py update_db() only recomputes when the column didn't already exist), keeping this narrowly scoped instead of relying on the automatic full-table pass. is_published's backfill for it moved out of this module's migration: my_compassion loads after sbc_compassion in the dependency graph, so its field isn't registered here yet when this post-migration runs.
 Safe to merge. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
The backfill matched letters purely on the project's/template's current
state (currently suspended, currently outside the Christmas period), not
on why that specific letter is in "Exception". A letter already there for
a genuine failure (e.g. compose failure) could coincidentally belong to a
project that is currently suspended, wrongly marking it on_hold=True and
making a broken letter look like a normal in-progress one to the sponsor.
Both create_commkit() and hold_letters() always post a chatter message
with a fixed subject ("Project suspended"/"Christmas Hold") in the same
call that sets the state - durable, per-letter evidence of an actual hold,
now required in addition to the existing current-state check. Verified
against compassion_14: 4 of 122 previously-backfilled letters (all from
2020, no such message) lacked this evidence and are no longer matched.
Found by greptile-apps review on PR #2150.
danpa32
added a commit
that referenced
this pull request
Sep 7, 2026
The backfill matched letters purely on the project's/template's current
state (currently suspended, currently outside the Christmas period), not
on why that specific letter is in "Exception". A letter already there for
a genuine failure (e.g. compose failure) could coincidentally belong to a
project that is currently suspended, wrongly marking it on_hold=True and
making a broken letter look like a normal in-progress one to the sponsor.
Both create_commkit() and hold_letters() always post a chatter message
with a fixed subject ("Project suspended"/"Christmas Hold") in the same
call that sets the state - durable, per-letter evidence of an actual hold,
now required in addition to the existing current-state check. Verified
against stage18_neutralized: 4 of 126 previously-backfilled letters
lacked this evidence and are no longer matched.
Found by greptile-apps review on PR #2150 (compassion-modules, v14).
ecino
requested changes
Sep 9, 2026
Comment on lines
+1
to
+17
| from openupgradelib import openupgrade | ||
|
|
||
|
|
||
| @openupgrade.migrate() | ||
| def migrate(env, version): | ||
| if version: | ||
| # Pre-create the column so Odoo's _auto_init (running after this | ||
| # pre-migration, per odoo/modules/loading.py) finds it already | ||
| # exists and does not schedule a recompute for existing rows - | ||
| # matches odoo/tools/sql.py's own create_column for booleans. | ||
| openupgrade.logged_query( | ||
| env.cr, | ||
| """ | ||
| ALTER TABLE correspondence | ||
| ADD COLUMN IF NOT EXISTS on_hold boolean DEFAULT false | ||
| """, | ||
| ) |
Member
There was a problem hiding this comment.
This is no longer needed as your field is not a compute field anymore.
ecino
added a commit
that referenced
this pull request
Sep 9, 2026
…ailure (#2151) * [T3436] FIX: add on_hold to distinguish a benign letter hold from a failure correspondence.hold_letters()/create_commkit() move a sponsor's letter to state "Exception" whenever its FCP is suspended or (for Christmas letters) outside the Christmas period, queuing it rather than sending it - the same state used for an actual technical failure (_handle_compose_letter_failure equivalent) or a real quality-check failure. Nothing distinguished the two, so MyCompassion (separate commit) hid a benign hold from the sponsor with no confirmation their letter was ever received, well before the 3-month suspension-notification threshold. on_hold is set alongside state wherever a letter is put on hold this way, cleared in reactivate_letters(). Pre-creates the column in a pre-migration (before _auto_init runs) rather than letting it happen automatically: adding a field is cheap on its own, but this DB's correspondence table is large enough that letting -u discover the missing column itself pulled in a much broader recompute pass than needed. Backfills on_hold for letters already stuck in Exception for a currently-held reason. * [T3436] FIX: require durable per-letter evidence in the on_hold backfill The backfill matched letters purely on the project's/template's current state (currently suspended, currently outside the Christmas period), not on why that specific letter is in "Exception". A letter already there for a genuine failure (e.g. compose failure) could coincidentally belong to a project that is currently suspended, wrongly marking it on_hold=True and making a broken letter look like a normal in-progress one to the sponsor. Both create_commkit() and hold_letters() always post a chatter message with a fixed subject ("Project suspended"/"Christmas Hold") in the same call that sets the state - durable, per-letter evidence of an actual hold, now required in addition to the existing current-state check. Verified against stage18_neutralized: 4 of 126 previously-backfilled letters lacked this evidence and are no longer matched. Found by greptile-apps review on PR #2150 (compassion-modules, v14). * T3436 DEL unneeded migration script --------- Co-authored-by: Emanuel Cino <ecino@compassion.ch>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
T3436 — Sponsor's letter disappears from MyCompassion while the FCP is suspended
Reported: whenever an FCP is suspended, letters written by the sponsor automatically go into state
"Exception"and vanish from MyCompassion. Sponsors aren't informed about a suspension until it has lasted three months, so a letter disappearing immediately (with no confirmation it was ever received) is a problem.Flagged as related to ticket T3322, which found and deliberately deferred this exact gap as a follow-up.
Related PR
v14: CompassionCH/compassion-website#380
v18: #2151
What changed
Two repos, two versions each (v14 and v18 fixed in parallel, same root cause confirmed in both)
compassion-modules(sbc_compassion): addson_hold(plainBoolean, not computed), set toTruealongsidestate = "Exception"wherever a letter is put on hold for a benign reasoncompassion-website(my_compassion):is_published's compute now readson_hold— an S2B letter in"Exception"is published ifon_holdisTrue.on_holdis deliberately excluded from@api.dependsHow to test manually
state = "Exception"(or wait for/trigger a suspend→hold_letters_action()cycle on an active sponsorship with an existing letter).is_published = Falsefor that letter, and it's absent from/my2/children/lettersfor that sponsor.sbc_compassionthenmy_compassion— order matters, see migration notes above), or run the migrations via-u sbc_compassion,my_compassion.stateis still"Exception"(letter is still genuinely held, nothing sent to GMC), buton_hold = Trueandis_published = True.impersonate_login's "Switch Login" from Settings → Users) and confirm the letter now appears on/my2/children/letters, indistinguishable from a normally-processing letter (no badge/wording change — deliberate, matches the ticket's own framing of not revealing the suspension before 3 months)."Quality check unsuccessful", or manually seton_hold = Falseon an"Exception"letter) still stays hidden — no regression on the original behavior for real failures.