Closed Bug 793914 Opened 12 years ago Closed 11 years ago

Change "waiting time" to be time since last submission

Categories

(Marketplace Graveyard :: Reviewer Tools, defect, P2)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED
2013-03-14

People

(Reporter: adora, Assigned: robhudson)

References

Details

(Whiteboard: p=2)

In the reviewer queues, "waiting time" is currently how long it's been since the app was FIRST submitted, no matter how many times it's been submitted since then.  There's some value in knowing this so that trivial fixes get approved faster, but overall it's much more important for the review team to know how long an app has been waiting since last submission, since that's how long the ball has been in our court.
I swear to omg I filed this like six months ago. When I find it, I'll dupe it.
I believe this is simply changing our sort from using the created timestamp to using the modified timestamp.
Blocks: 741513
can we just update the nominated date on each submission? (for apps that arn't approved yet)
I take back what I said... if we sort by modified that would mean even an unrelated change to the app would throw it to the bottom of the queue.

Thanks Andrew for pointing out the nomination date which already exists for this purpose on the AMO side. Marketplace isn't using it at all currently but we could.
Here's my plan. Please point out the errors if you see any...

1) Pending queue: Sort by version.nominated
2) Update queue: Sort by version.nominated

By "version.nominated" I mean the "nominated" field on the version table (which is currently unused for apps). We will set "nominated" to the current date/time whenever the file status changes from any status to "PENDING".

This means the nomination should be updated when:
* New apps, both hosted and packaged, are submitted
* Hosted apps are resubmitted after rejection
* New versions of packaged apps are uploaded, either for new versions or after rejection

But not updated when:
* More info is requested or a comment is made


3) Rereview queue: Sort by rereview_queue.created
4) Escalation queue: Sort by escalation_queue.created

For the re-review and escalation queues, the rows in that table get created whenever the app enters that queue. I'm not sure how much back and forth there is once the app is here but my assumption is very little.
Assignee: nobody → robhudson.mozbugs
Target Milestone: --- → 2012-10-18
1, 2, and 3 sound good to me.  

What else could easily be done with submissions where the reviewer has requested more info?  I don't want waiting time to be reset in that case, but there should be some indicator in the queue that the submission isn't actionable until the developer replies.

For the escalation queue (4), this behavior sounds good for a start.  But I've only started moving apps to this queue a couple of days ago, so I reserve the right to discover more user needs as we go.  =]
I'm happy with the plan in comment 5
(In reply to Lisa Brewster [:adora] from comment #6)
> What else could easily be done with submissions where the reviewer has
> requested more info?  I don't want waiting time to be reset in that case,
> but there should be some indicator in the queue that the submission isn't
> actionable until the developer replies.

there is a flag that shows up in the queue... but it doesn't show how long its been waiting; there isn't a way for developers to reply to an info request on the site; and if they do make a change (e.g. editing the summary, removing device support) then we aren't informed.

Beyond the flag there could be some way of replying via marketplace that we would be able to record and respond to.  It sounds like a new bug if that's desirable.

Btw for some background, on AMO for info requests there used to be a link included in the email and form that showed the info request and a textarea field to reply to it.  This then emailed the reviewer who send the info request and added the response to the review history.  It was removed a while back (I think during the Zamboni rewrite) because it was difficult to re-implement.  It was kind of useful but it had two drawbacks - 1) developers would often ignore (shock!) the instructions and the link and reply to the email anyway. 2) the notification only went to the original reviewer so if that person went inactive (either lost interest in reviewing, or was just on vacation for 3 weeks) no-one else knew to do anything about it; and as everyone else then ignored add-ons in the queue with the info flag set some developers went weeks before anyone noticed they'd promptly replied.
found the AMO bug for reimplementing the reply box, for reference: https://bugzilla.mozilla.org/show_bug.cgi?id=749022
I made some progress this week. Bumping to finish next week.
Target Milestone: 2012-10-18 → 2012-10-25
Made a bit more progress but ran into trouble with either `uncached` or `raw` not including translations in my custom SQL. I need to spend some time to see why this is or try to add the translation query transform myself, or do it in the raw SQL. But bumping since there are more urgent bugs needing attention.
Target Milestone: 2012-10-25 → 2012-11-08
Target Milestone: 2012-11-08 → 2012-11-15
Target Milestone: 2012-11-15 → 2012-11-22
Target Milestone: 2012-11-22 → 2012-11-29
Target Milestone: 2012-11-29 → ---
Bump for :robhudson <3
Can we have a normalized column somewhere that keeps track of this? Eventually, this will/should all be in ES so it'd be trivial there.
Priority: -- → P2
Whiteboard: p=2
Target Milestone: --- → 2013-03-14
Blocks: 848445
https://github.com/mozilla/zamboni/commit/220aaec 

For QA:
* Submit a packaged app but don't approve it. Check the waiting time in the reviewer tools queue. Submit an update with a different version and verify the waiting time is the same on the new version.
* Submit a packaged app and approve it, taking note of the waiting time in the queue. Submit an update to the packaged app and verify that the waiting time is different and is the time when submitted.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
This addresses hosted apps too right? where they've been rejected and then resubmitted. 
this is the primary use case ATM.
(In reply to Andrew Williamson [:eviljeff] from comment #16)
> This addresses hosted apps too right? where they've been rejected and then
> resubmitted. 
> this is the primary use case ATM.

Yes, that'd be good to QA too... A hosted app being rejected and resubmitted gets a new waiting time.
Reverted due to problems of nomination not getting set correctly and 504s on the queue pages on -dev (too many db queries?)
https://github.com/mozilla/zamboni/commit/3ed2398
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://github.com/mozilla/zamboni/commit/d9853c03

This one cleaned up the nomination handling and also optimized the SQL queries so page loads are a tad faster. The last patch actually was causing 504s.

I verified the steps in comment #15 and #16.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.