Offers: saving an offer unchanged writes a revision, reopens a ticked-off reminder and undoes a postponement #442

New issue

Closed

opened 2026-10-01 10:46:44 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:44 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the offer views and services and checking the result with a unit test against the snapshot.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at applications/views.py:1355-1361 (OfferUpdateView.form_valid); applications/services.py has not moved, and the same unit test gives the same results.

Every save of the offer form is treated as a revision of the offer and a new answer-by date, whatever changed. Adding a line to Notes writes "Offer revised" on the timeline, sends an offer_recorded webhook with revised=true, and rebuilds the answer-by reminder: one the person ticked off is outstanding again, one they put off is moved back, and one they deleted is made afresh.

Where: applications/views.py:1315-1321 (OfferUpdateView.form_valid), applications/services.py:704-714 (revise_offer), applications/services.py:645-679 (_remind_to_answer)
What happens:
1. OfferUpdateView.form_valid runs offer = form.save() and then revise_offer(offer) with no look at form.has_changed() or form.changed_data.
2. revise_offer always appends an EventKind.OFFER entry, "Offer revised: …", and always calls _remind_to_answer.
3. _remind_to_answer knows nothing of the previous answer_by. For a date still ahead it rewrites the summary, sets done_at = None on a reminder that was done, and calls postpone_reminder(reminder, due), which puts due_at back to 09:00 the day before and clears notified_at. When Offer.reminder is null (the reminder was deleted; the field is SET_NULL), it creates a new one.
Checked: an offer with answer_by ten days out, reminder ticked off, then a notes-only edit posted through applications:offer_update. The timeline went from three entries to four ("Offer revised: 65,000 EUR per year") and the reminder's done_at was None again. A reminder put off to a later time moved from 2026-10-05 23:01 back to 2026-10-09 07:00 UTC after another notes-only edit, and after deleting the reminder a further edit made a new one.
Why it matters: the timeline is the record of what happened, and it now says the terms changed when they did not. A reminder the person dealt with or deliberately moved comes back and is announced through their notifiers, and a webhook consumer is told of a revision that never happened. change_status (applications/services.py:101) already refuses to "litter the timeline with entries saying nothing changed"; the offer path does not.
Fix: in OfferUpdateView.form_valid, return without calling revise_offer when not form.has_changed(). Have revise_offer take the fields that changed (or the previous row) and write the timeline entry and webhook only when the terms changed (base_amount, currency, period, variable_pay, equity, benefits, location, holidays, starts_on, answer_by), not for notes alone. Pass the previous answer_by to _remind_to_answer and touch the reminder only when the date moved; never reopen a ticked-off reminder or recreate a deleted one unless it did.

Done when

tests/test_offers.py has a test that edits only the notes of an offer whose reminder was ticked off and asserts no new EventKind.OFFER entry, done_at unchanged; a second that postpones the reminder, edits the notes and asserts due_at unchanged; and a third that moves answer_by and still sees the reminder follow it.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the offer views and services and checking the result with a unit test against the snapshot. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `applications/views.py:1355-1361` (`OfferUpdateView.form_valid`); `applications/services.py` has not moved, and the same unit test gives the same results. Every save of the offer form is treated as a revision of the offer and a new answer-by date, whatever changed. Adding a line to *Notes* writes "Offer revised" on the timeline, sends an `offer_recorded` webhook with `revised=true`, and rebuilds the answer-by reminder: one the person ticked off is outstanding again, one they put off is moved back, and one they deleted is made afresh. - **Where:** `applications/views.py:1315-1321` (`OfferUpdateView.form_valid`), `applications/services.py:704-714` (`revise_offer`), `applications/services.py:645-679` (`_remind_to_answer`) - **What happens:** 1. `OfferUpdateView.form_valid` runs `offer = form.save()` and then `revise_offer(offer)` with no look at `form.has_changed()` or `form.changed_data`. 2. `revise_offer` always appends an `EventKind.OFFER` entry, "Offer revised: …", and always calls `_remind_to_answer`. 3. `_remind_to_answer` knows nothing of the previous `answer_by`. For a date still ahead it rewrites the summary, sets `done_at = None` on a reminder that was done, and calls `postpone_reminder(reminder, due)`, which puts `due_at` back to 09:00 the day before and clears `notified_at`. When `Offer.reminder` is null (the reminder was deleted; the field is `SET_NULL`), it creates a new one. Checked: an offer with `answer_by` ten days out, reminder ticked off, then a notes-only edit posted through `applications:offer_update`. The timeline went from three entries to four ("Offer revised: 65,000 EUR per year") and the reminder's `done_at` was `None` again. A reminder put off to a later time moved from `2026-10-05 23:01` back to `2026-10-09 07:00` UTC after another notes-only edit, and after deleting the reminder a further edit made a new one. - **Why it matters:** the timeline is the record of what happened, and it now says the terms changed when they did not. A reminder the person dealt with or deliberately moved comes back and is announced through their notifiers, and a webhook consumer is told of a revision that never happened. `change_status` (`applications/services.py:101`) already refuses to "litter the timeline with entries saying nothing changed"; the offer path does not. - **Fix:** in `OfferUpdateView.form_valid`, return without calling `revise_offer` when `not form.has_changed()`. Have `revise_offer` take the fields that changed (or the previous row) and write the timeline entry and webhook only when the terms changed (`base_amount`, `currency`, `period`, `variable_pay`, `equity`, `benefits`, `location`, `holidays`, `starts_on`, `answer_by`), not for `notes` alone. Pass the previous `answer_by` to `_remind_to_answer` and touch the reminder only when the date moved; never reopen a ticked-off reminder or recreate a deleted one unless it did. ## Done when - `tests/test_offers.py` has a test that edits only the notes of an offer whose reminder was ticked off and asserts no new `EventKind.OFFER` entry, `done_at` unchanged; a second that postpones the reminder, edits the notes and asserts `due_at` unchanged; and a third that moves `answer_by` and still sees the reminder follow it.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:44 +00:00

tiagoagueda added the

tier

bug

labels

2026-10-01 10:46:44 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 10:05:00 +00:00

Treat an offer edit as a revision only when its terms change

tiagoagueda closed this issue

2026-10-04 10:05:00 +00:00