Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion base_tier_validation/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{
"name": "Base Tier Validation",
"summary": "Implement a validation process based on tiers.",
"version": "19.0.1.0.3",
"version": "19.0.1.0.4",
"development_status": "Mature",
"maintainers": ["LoisRForgeFlow"],
"category": "Tools",
Expand Down
21 changes: 19 additions & 2 deletions base_tier_validation/models/tier_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,28 @@ def _update_review_status(self):
reviews = self.filtered(lambda rev: rev.status in ["waiting", "pending"])
if not reviews:
return
next_seq = min(reviews.mapped("sequence"))
# Lowest still-open sequence *per record*, computed over the whole
# record's reviews -- NOT just this recordset. Computing it over
# ``self`` promotes a later tier prematurely whenever the method runs on
# a subset that excludes the lower-sequence predecessors, e.g.
# ``review_user_count`` calling ``user.review_ids._update_review_status()``
# for a second-tier reviewer (their own review is then the only -- and
# thus "minimum" -- sequence in the set, so it wrongly goes ``pending``).
min_seq_by_record = {}
for model, res_id in {(rev.model, rev.res_id) for rev in reviews}:
open_reviews = (
self.env[model]
.browse(res_id)
.review_ids.filtered(lambda r: r.status in ("waiting", "pending"))
)
min_seq_by_record[(model, res_id)] = min(open_reviews.mapped("sequence"))
for record in reviews:
if record.status != "waiting":
continue
if record.approve_sequence and record.sequence != next_seq:
if (
record.approve_sequence
and record.sequence != min_seq_by_record[(record.model, record.res_id)]
):
continue
record.status = "pending"
if record.definition_id.notify_on_pending:
Expand Down
61 changes: 61 additions & 0 deletions base_tier_validation/tests/test_tier_validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,67 @@ def test_19a_notify_on_pending_sequence_negative(self):
"Second-tier reviewer must be notified once promoted to pending.",
)

def test_19c_no_premature_promotion_on_subset(self):
"""Regression: ``_update_review_status`` run on a *subset* of a record's
reviews -- as ``review_user_count`` does per user, via
``user.review_ids._update_review_status()`` -- must gate the sequence
over the *whole* record, not the subset. Otherwise a later-tier review
is promoted to ``pending`` before its predecessor is approved, which
both hides it from the reviewer's systray (``can_review`` is False while
the lower tier is still pending) and fires a ``notify_on_pending``
message authored by -- and so never delivered to -- that reviewer.
"""
TierDefinition = self.env["tier.definition"]
test_record = self.test_model.create({"test_field": 2.5})
TierDefinition.create(
{
"model_id": self.tester_model.id,
"review_type": "individual",
"reviewer_id": self.test_user_1.id,
"definition_domain": "[('test_field', '=', 2.5)]",
"approve_sequence": True,
"notify_on_pending": True,
"sequence": 20,
"name": "First in sequence -- user 1",
}
)
TierDefinition.create(
{
"model_id": self.tester_model.id,
"review_type": "individual",
"reviewer_id": self.test_user_2.id,
"definition_domain": "[('test_field', '=', 2.5)]",
"approve_sequence": True,
"notify_on_pending": True,
"sequence": 10,
"name": "Second in sequence -- user 2",
}
)
reviews = test_record.request_validation()
review_second = reviews.filtered(lambda r: self.test_user_2 in r.reviewer_ids)
self.assertEqual(review_second.status, "waiting")
messages_before = test_record.message_ids

# Simulate the second-tier reviewer opening their systray: the promotion
# runs on *only* their own review -- the subset that excludes the
# first-tier predecessor.
review_second._update_review_status()
review_second.invalidate_recordset()

self.assertEqual(
review_second.status,
"waiting",
"A later-tier review must stay 'waiting' until its predecessor is "
"approved, even when _update_review_status runs on a subset that "
"excludes the predecessor.",
)
self.assertEqual(
test_record.message_ids,
messages_before,
"No notify_on_pending message must be posted from promoting a later "
"tier out of order.",
)

def test_19b_notify_review_available_no_op_when_no_users(self):
"""``_notify_review_available`` must short-circuit (no follower
added, no chatter message posted) when none of the passed reviews
Expand Down
Loading