Skip to content

[19.0][FIX] base_tier_validation: reviewer systray counter drifting (negative) - #52

Open
bosd wants to merge 1 commit into
OCA:19.0from
bosd:19.0-fix-systray-counter-drift
Open

[19.0][FIX] base_tier_validation: reviewer systray counter drifting (negative)#52
bosd wants to merge 1 commit into
OCA:19.0from
bosd:19.0-fix-systray-counter-drift

Conversation

@bosd

@bosd bosd commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Problem

The reviewer systray counter (tierReviewCounter) can show a wrong — even negative — value (e.g. a user seeing a negative number of reviews "to do").

Root cause

The counter is mutated by real-time bus deltas (review_created++, review_deleted-- in tier_review_service.esm.js). But TierValidation._update_counter sends those deltas to self.env.user.partner_id — the acting user — rather than the reviewers whose pending count actually changes:

self.env.user.partner_id._bus_send(channel, review_counter)

So whenever someone acts on a tier-validated record without being one of its reviewers (creating it, editing it, force-validating), the wrong user's counter is moved. An active approver accumulates a drift that eventually goes negative. The systray's own absolute count (res.users.review_user_countpending_count = len(records)) can never be negative — only the blind delta can.

Fix

  • JS (tier_review_service.esm.js): on the bus event, re-fetch the authoritative absolute count (review_user_count) instead of ++/--. This is drift-proof and can never go below zero, regardless of which user receives the event. (orm added to the service deps.)
  • Python (_update_counter): notify the reviewers' partners (the users whose pending count changes), falling back to the acting user only when the reviews are already gone (deletions), so reviewers get a prompt, correct live update.

Version bumped 19.0.1.0.3 → 19.0.1.0.4.

Also nticed this behaviour on V118

@OCA-git-bot OCA-git-bot added mod:base_tier_validation Module base_tier_validation series:19.0 labels Jun 23, 2026
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

@bosd
bosd force-pushed the 19.0-fix-systray-counter-drift branch from 7298322 to ea6f62a Compare June 23, 2026 10:07

@Dranyel-Bosd Dranyel-Bosd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM 💯

@LoisRForgeFlow

Copy link
Copy Markdown
Contributor

@bosd squash commits please

image

@bosd
bosd force-pushed the 19.0-fix-systray-counter-drift branch from dff0f9f to b877b4b Compare July 23, 2026 13:17
@bosd

bosd commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Done — squashed into a single commit and rebased onto the latest 19.0 (also dropped the manual version bump, since ocabot handles that on merge). Thanks!

The reviewer counter in the systray can display a wrong, even negative, value.

The counter was mutated by real-time bus deltas (``review_created`` -> ++,
``review_deleted`` -> --), but ``_update_counter`` sent those deltas to
``self.env.user.partner_id`` -- the *acting* user -- instead of the reviewers
whose pending count actually changes. So whenever a user acted on a
tier-validated record without being one of its reviewers (creating, editing,
force-validating), the wrong user's counter was moved, and an active approver
accumulated a drift that eventually went negative. The systray's own absolute
count is a ``len()`` and can never be negative; only the blind delta could.

Fixes:
- JS: the systray component now re-fetches the authoritative absolute count
  (``review_user_count``) on the bus event, instead of a separate service
  nudging a +/- delta. The dedicated ``tierReviewService`` (whose only job was
  that delta) is removed; the component already owns ``fetchSystrayReviewer``
  and the ORM via ``useService``, so the badge stays live without drift and can
  never go below zero.
- Python: ``_update_counter`` now notifies the reviewers' partners (the users
  whose pending count changes), falling back to the acting user only when the
  reviews are already gone (deletions).

The re-fetch is correct but it is not free, and the notification now reaches
every reviewer rather than one user. Approving a batch of documents therefore
fans out to (reviewers x open tabs x documents) recounts of
``review_user_count``, which is enough to occupy every HTTP worker. The
recounts are coalesced so that the fan-out cannot turn into a request storm:

- a 15 s leading+trailing debounce around the bus handler -- the leading edge
  keeps an isolated change instant, the trailing edge settles the final value;
- an in-flight guard, so a recount that outlasts the debounce window records
  that something changed and re-runs once instead of stacking concurrent calls
  on top of a slow one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:base_tier_validation Module base_tier_validation series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants