Enhance the filtering of the review queues to make finding add-ons easier. Editors need better review queue filtering to be able to search for add-ons that they’d like to review: * Add-on name with auto-complete for speeding up results * Add-on type * Operating system * Age of submission (e.g.: if add-on submitted > 30 days ago) * Mozilla application version

Additional requirements: - Add a column called "nomination age" with date since the add-on was nominated. This will allow us to determine how long it takes for an add-on to get reviewed. - Nomination age should be the default sort. Right now it's sorted by the age of the add-on which seems wrong - Make all columns sortable

I feel strongly that at least the sorting and nomination age column should go into 5.0.4. The current sorting by add-on age results in the appearance of new add-ons at the front of the line and doesn't really give us a sense of how long add-ons remain in the queue.

Let us know if we should break this out into two bugs or move some other bugs to 5.0.5 to accomodate this.

This is a dupe of bug 271031 but since this has more info I'll dupe the other one to this. (In reply to comment #2) > I feel strongly that at least the sorting and nomination age column should go > into 5.0.4. The current sorting by add-on age results in the appearance of new > add-ons at the front of the line and doesn't really give us a sense of how long > add-ons remain in the queue. (In reply to comment #3) > Let us know if we should break this out into two bugs or move some other bugs > to 5.0.5 to accomodate this. This has been broken for as long as AMO has had a queue (~5 years). Why the sudden need to bump other bugs from this milestone to fit this one in? I agree we should fix it soon but I don't think it's necessary to throw other bugs out for it.

Reason why it's a problem is because right now we have no sense of how long something is in the queue. For instance, I saw an add-on yesterday appear that was 1400 days old, that's when I realized that the queue is not sorted by length of time in queue but rather the age of the add-on. What this also means is there's no guarantee that if we work from the beginning of the queue to the end that all add-ons appear on the first page. It's deeply broken. If it means we separate out the "nomination age" column into a separate smaller bug, then we should do it.

I'm thinking that unless it's a monumental task to fix it, we should get that in ASAP. The fact that it's been broken so long, IMO, should force us to really look at a solution as this type of information can be very important to providing solid feedback to add-on authors. Right now, we can't give them any indication of how long they've been waiting or even an expected turn-around time.

I just want to point out the delicious irony of this new bug jumping to the top of the queue for 5.0.4 when other bugs have been around waiting their turn and getting feedback hoping to one day be given a target milestone and were finally assigned to 5.0.4, but are now behind this new bug that was just submitted and nominated today.

Shouldn't be bugs be positioned based on need and priority and not seniority?

All for the greater good.

Nick is going to split out elements of this bug into separate, smaller bugs and be really specific about what needs to happen so there won't be questions about it when someone starts implementing it. Those bugs will be assigned to 5.0.4, major severity, and assigned to no one. If/when someone gets finished with their other bugs those will be the next ones they work on.

Changing assignee to Nick as this is a tracking bug unless I am mistaken (cf. comment 11).

Attached is the spec & mockups.

Added search by add-on author's AMO email address

Moving this to 5.0.6 for now. Short update: I have some of the UI done for this, but the backend will need more work so this is unfortunately going to miss the 5.0.5 milestone.

(In reply to comment #17) > Moving this to 5.0.6 for now. Short update: I have some of the UI done for > this, but the backend will need more work so this is unfortunately going to > miss the 5.0.5 milestone. Is everything you have done for this checked in? If not, can you create a patch here?

This is the work I've done so far on this; the UI looks basically like the one suggested in the mockups. Note though that some of the input fields are still dummies.

Attached patch for review. I've gone over it pretty closely, but due to size it definitely needs more eyes. If there is a cleaner way to build queries several joins deep using Cake, I'd like to know!

Comment on attachment 376570 [details] [diff] [review] Patch, v1 Thanks Scott. I think the query speed is fine. In my testing the slowest, by far, was "select appversion.id..." and that's cacheable and doesn't change much. I'm going to r+ it because it's a lot better than what we have and doesn't have any giant problems (and also rey can give some feedback). Some things to change soon though (you can either make a patch against this patch, or commit and then patch against that if you want): - there are two queries that use OFFSET in them. If $_page is zero you take an offset with a negative value and the query fails. Reproduce by searching for an add-on name that doesn't exist in the queue. This actually happens if you search for any set of parameters that will return nothing (e.g. looking only for sunbird extensions on BSD or something). - "Age of submission" needs to be "Age of submission (days)". I think editors will be confused by this being "exactly 5 days" instead of "5 or less days" but I see that's what the spec has. Rey, are you sure that's what you want? - If you mouse over an add-on link you'll notice it has "num=_" appended to the URL. If you click on it you'll see in the top right a prev/next link. Editors use those to jump through the queue quickly and they should work with the current filter. If you need an idea of how to do this, bugzilla just stores a comma separated list of bug numbers in a cookie (with an arbitrary max number...maybe 100?). If the cookie exists and is parsable it's used. - I see "Add-on author AMO email" is in the spec but not on any of the mockups. How easy would it be to change the "Add-on" search box to search author email as well? If it's a big performance hit let's not but I think it would be possible. - (the last comment might trump this one but): the admin controller already has an ajaxy addonLookup() - can you reuse that instead of adding a new one? If not can we make an addonAndAuthorLookup()? - If you do end up adding new functions to the editor controller like addonLookup() that are called directly they need to have the SimpleAcl checks on them to make sure the user is an editor or admin. Just search for SimpleAcl in the editors_controller to see lots of examples.

What I would like is to be able to pull up add-ons that are "x" days old in the queue. It's to set a great sense of urgency for older add-ons so that they get priority attention over newer ones. Ideally, we would get to the point were the queue would eventually dwindle down to where the up numbers aren't used often, especially "10+". Do you see some bumps in this approach?

I think that sounds fine but I don't think it answered either of my questions..?

(In reply to comment #21) Thanks for the feedback, Wil. > I see "Add-on author AMO email" is in the spec but not on any of the mockups. > How easy would it be to change the "Add-on" search box to search author email > as well? If it's a big performance hit let's not but I think it would be > possible. Email search is certainly possible, though it doesn't look like the translation magic handles fields wrapped in an OR clause array (correct me if I'm wrong). If that's an easy enhancement, then adding email search is trivial. File a bug and make email search dependent? DB Performance wise, I think we'd be ok though a search for ".com" would return a huge result set. Client side does have a 4 character minimum before firing and we could always enforce that server-side as well if necessary. > (the last comment might trump this one but): the admin controller already has > an ajaxy addonLookup() - can you reuse that instead of adding a new one? If > not can we make an addonAndAuthorLookup()? I went with a dedicated lookup since the admin version preformats the ID with the status. I could've parsed the ID out easily enough, but a format change later on would break the editor side. I'll take care of the other issues real quick.

(In reply to comment #24) > (In reply to comment #21) > > Thanks for the feedback, Wil. > > > I see "Add-on author AMO email" is in the spec but not on any of the mockups. > > How easy would it be to change the "Add-on" search box to search author email > > as well? If it's a big performance hit let's not but I think it would be > > possible. > > Email search is certainly possible, though it doesn't look like the translation > magic handles fields wrapped in an OR clause array (correct me if I'm wrong). > If that's an easy enhancement, then adding email search is trivial. File a bug > and make email search dependent? We'll see if Rey wants it since it wasn't in the mockups. > DB Performance wise, I think we'd be ok though a search for ".com" would return > a huge result set. Client side does have a 4 character minimum before firing > and we could always enforce that server-side as well if necessary. Does this query happen before the "in queue" filter? If not the largest set returned would be the entire queue which is what we're doing now anyway. > > (the last comment might trump this one but): the admin controller already has > > an ajaxy addonLookup() - can you reuse that instead of adding a new one? If > > not can we make an addonAndAuthorLookup()? > > I went with a dedicated lookup since the admin version preformats the ID with > the status. I could've parsed the ID out easily enough, but a format change > later on would break the editor side. > I'd rather put an if() in the existing addonLookup and change the formatting then duplicate the entire function I think. Does that sound reasonable?

(In reply to comment #25) > Does this query happen before the "in queue" filter? If not the largest set > returned would be the entire queue which is what we're doing now anyway. The lookup query is not restricted by queue filter or addon status, so the largest result set would be all addons. > I'd rather put an if() in the existing addonLookup and change the formatting > then duplicate the entire function I think. Does that sound reasonable? That works for me.

(In reply to comment #23) > I think that sounds fine but I don't think it answered either of my > questions..? I may not have understood your questions if they were directed at me. Could you rephrase and I'll do my best to answer them.

(In reply to comment #25) > We'll see if Rey wants it since it wasn't in the mockups. Yes I would like a search by author email address.

(In reply to comment #27) > (In reply to comment #23) > > I think that sounds fine but I don't think it answered either of my > > questions..? > > I may not have understood your questions if they were directed at me. Could you > rephrase and I'll do my best to answer them. 1) Your mockup has a dropdown list of numbers next to "Age of Submission (days)." I'm wondering if that means "exactly this many days" or "this many days or less." I'm suggesting it mean "this many days or less." 2) comment 28 answered the question. Scott is planning on just changing the "Add-on" field to say "Add-on or Email" and have it search support addresses and user addresses and then append the results to the table.

I want it to be "exactly this many days" and if it becomes confusing, we'll change it in a new release. The 10+ should be anything older than or equal to 10 days.

Attached is v2 of my patch. It addresses issues discussed in <a href="#c21">comment #21</a> except for author email lookup. In addition, I made filters affect the counts displayed in the Editor Tools menu and queue tabs. This makes sense to me, but would be better with some sort of indicator when a filter is in affect. In the menu, there isn't much space so perhaps show a filter icon? Any thoughts?

Comment on attachment 377429 [details] [diff] [review] Patch, v2 - There is something weird with the add-on search filter. Most of the time it doesn't seem to apply, however, if I type in the name of an add-on and let it autocomplete it works. After that whatever I type (if I don't let it autocomplete) always turns into the old autocompleted one after the page loads. It's weird...if you can't reproduce I can give better steps. - The add-on search box still doesn't to author yet - I see what you mean about the numbers in the title. Maybe we could add a total? So it would say "Pending Updates (12/75)" if a filter was active? I also don't think it's particularly obvious a filter is active. What do you think of changing "Filter Queue" to "Filter Queue (ACTIVE)" or something? And putting ACTIVE in a bright color? Maybe I'm getting carried away. Rey - any ideas?

Wil, is this something that I can test as well so I can give some feedback? Is there a link to it?

Rey: once a patch lands, you can always test at https://preview.addons.mozilla.org/en-US/firefox/ (wait at least 15 minutes for the changes to show up).

(In reply to comment #34) > Rey: once a patch lands, you can always test at > https://preview.addons.mozilla.org/en-US/firefox/ (wait at least 15 minutes for > the changes to show up). Thanks Stephen.

Rey, it doesn't look like I have write access to the repository just yet. While I get that worked out, you can see the latest patch on my dev site: http://mccammos.khan.mozilla.org/remora/site/en-US/editors/queue/pending I won't be changing anything there until tomorrow morning.

Thanks Scott. I'm looking at it now.

I tested it out and it looks good. The only thing I couldn't verify was the Submission Days option since all entries are greater than 10 days. Can you put some bogus data in there to test?

You're an admin - you can push things in and out of the queue.

Rey, I bumped the nomination date on a handful of items in my dev db. In doing so I discovered that items less than 1 day old get excluded from any age filter at the moment. For completeness, I can add a "<1" option to catch those. Did you have any thoughts on the queue counts when filters are in affect?

Hi Scott, I think that the counts for each queue should remain their full count regardless of the filter and we should add a line that tells the editor how many results were pulled back. I've attached a screencap as reference.

Thanks, Rey. That seems like a clean approach. I'll keep the filtered count on the editors/review page in the text above the next/prev links: "# 1 of 12 in queue (filtered)" for example.

Great! Thanks Scott.

Attached patch (hopefully) resolves remaining issues. Changes include: - queue counts once again ignore filters - show filtered count inside filter box - filter age=1day now includes items <=1day - autocomplete search by name or author email - autocomplete results are now limited to items in queue

committed r25803 a few days ago

Comment on attachment 378418 [details] [diff] [review] Queue counts + name or author filter, v1 r+ but if I search for something that doesn't exist (like someones name + an application they don't develop for) I get: Notice: Array to string conversion in /site/app/controllers/components/editors.php on line 637 Could you fix that and then check this in? thanks

Thanks, Wil. I fixed the regression and committed r25919.

Rey -- know you're busy, but would you mind taking a look at this too? Thanks!

(In reply to comment #49) > Rey -- know you're busy, but would you mind taking a look at this too? Thanks! Never too busy for you guys. I'll check it right now.

Just some quick observations. The max version option only goes to 3.1b2pre. Is that because of the freshness of the dev DB? Other than that, this looks great.

(In reply to comment #51) > Just some quick observations. The max version option only goes to 3.1b2pre. Is > that because of the freshness of the dev DB? > > Other than that, this looks great. Yeah; 3.1b2pre is the latest version on preview: https://preview.addons.mozilla.org/en-US/firefox/pages/appversions Verified FIXED -- thanks for taking a look, Rey!

removing "push-needed" from 105 AMO 5.0.6 bugs; filter on "I hate stephend!"