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)
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.
Comment 1•12 years ago
|
||
I swear to omg I filed this like six months ago. When I find it, I'll dupe it.
Assignee | ||
Comment 2•12 years ago
|
||
I believe this is simply changing our sort from using the created timestamp to using the modified timestamp.
Blocks: 741513
Comment 3•12 years ago
|
||
can we just update the nominated date on each submission? (for apps that arn't approved yet)
Assignee | ||
Comment 4•12 years ago
|
||
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.
Assignee | ||
Comment 5•12 years ago
|
||
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 | ||
Updated•12 years ago
|
Assignee: nobody → robhudson.mozbugs
Target Milestone: --- → 2012-10-18
Reporter | ||
Comment 6•12 years ago
|
||
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. =]
Comment 8•12 years ago
|
||
(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.
Comment 9•12 years ago
|
||
found the AMO bug for reimplementing the reply box, for reference: https://bugzilla.mozilla.org/show_bug.cgi?id=749022
Assignee | ||
Comment 10•12 years ago
|
||
I made some progress this week. Bumping to finish next week.
Target Milestone: 2012-10-18 → 2012-10-25
Assignee | ||
Comment 11•12 years ago
|
||
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
Updated•12 years ago
|
Target Milestone: 2012-11-08 → 2012-11-15
Assignee | ||
Updated•12 years ago
|
Target Milestone: 2012-11-15 → 2012-11-22
Assignee | ||
Updated•12 years ago
|
Target Milestone: 2012-11-22 → 2012-11-29
Assignee | ||
Updated•12 years ago
|
Target Milestone: 2012-11-29 → ---
Comment 12•11 years ago
|
||
3 and 4 have been worked on here https://github.com/mozilla/zamboni/commit/7ced0d4ccba43e024dc7c9fa0c066982ebb72f89
Reporter | ||
Comment 13•11 years ago
|
||
Bump for :robhudson <3
Comment 14•11 years ago
|
||
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.
Updated•11 years ago
|
Priority: -- → P2
Whiteboard: p=2
Target Milestone: --- → 2013-03-14
Assignee | ||
Comment 15•11 years ago
|
||
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
Comment 16•11 years ago
|
||
This addresses hosted apps too right? where they've been rejected and then resubmitted. this is the primary use case ATM.
Assignee | ||
Comment 17•11 years ago
|
||
(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.
Assignee | ||
Comment 18•11 years ago
|
||
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 → ---
Assignee | ||
Comment 19•11 years ago
|
||
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 ago → 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•