Closed Bug 867752 Opened 11 years ago Closed 11 years ago

[shipping] don't cut off pushes in signoff view for just pending sign-offs

Categories

(Webtools Graveyard :: Elmo, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Pike, Assigned: peterbe)

References

Details

Attachments

(1 file)

In bug 668761, we changed the queries to only show the sign-offs that come up after the latest action, or 10 pushes.

In practice, this isn't great.

If there's a pending sign-off and a fallback, in particular, I miss a bit of history of how things go towards that sign-off.

I propose that we change this to:

max((accepted or 10), pending, rejected) pushes.

Which would be minimal push_date.

One way to achieve that is to get the earliest push_date that's at most 10 pushes back (so that you get the 8th push if there are only 8), and add that to the list.

Putting this into the P1 bucket, because it really makes reviewing sign-offs awkward right now.
I've taken the code we talked about and implemented it. 
https://github.com/peterbe/elmo/compare/feature;bug867752-dont-cut-off-pushes-in-signoff-view-for-just-pending-sign-offs

(I hope that makes sense). 

The suite fails on one error. Here's the traceback::

======================================================================
FAIL: test_5_pushes_no_fallback (elmo.apps.shipping.tests.test_signoff.SignOffAnnotatedPushesTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/peterbe/dev/MOZILLA/ELMO/elmo/apps/shipping/tests/test_signoff.py", line 444, in test_5_pushes_no_fallback
    eq_(len(pushes), 1)
  File "/Users/peterbe/dev/MOZILLA/ELMO/elmo/vendor/lib/python/nose/tools.py", line 31, in eq_
    assert a == b, msg or "%r != %r" % (a, b)
AssertionError: 2 != 1

----------------------------------------------------------------------
Ran 6 tests in 1.499s


So, what happens there is that we go further back in time even if the "cutoff" was not a rejected or pending. Changing `if cutoff_dates:` to `if len(cutoff_dates) > 1:` fixes the first part of the remaining breaking test but naturally breaks the others. 

Perhaps I can leave this here and we can talk about the rest on the phone.
By the way, my branch implements this https://etherpad.mozilla.org/Z5cQgogqgY
after our phone discussion.
Attachment #748911 - Flags: review?(l10n)
Comment on attachment 748911 [details] [diff] [review]
More pushes if not accepted

Review of attachment 748911 [details] [diff] [review]:
-----------------------------------------------------------------

r=djangocon
Attachment #748911 - Flags: review?(l10n) → review+
Commit pushed to develop at https://github.com/mozilla/elmo

https://github.com/mozilla/elmo/commit/ae035a0b06de2b4f3984a1532ad287618b97ba4b
fixes bug 867752 - don't cut off pushes in signoff view for just pending sign-offs, r=Pike
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee: nobody → peterbe
Target Milestone: --- → 3.3
Target Milestone: 3.3 → 3.2
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: