617679 - Adjust "retain in sandbox" button

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Admin/Editor Tools, defect, P1)

Description

[Wil Clouser [:clouserw]]

On the review queue pages we have a button that says "retain in sandbox".  That should say "Reject" on all pages where it is used and should flip the add-on status to STATUS_NULL.

Comment 1

[Barry Chen]

Attachment: patch for changing sandbox to reject for reviews

Comment 2

[Barry Chen]

Patch includes text changes and setting the status to STATUS_NULL.  It does not change any of the 'sandbox' string literals in the PHP/js/html (e.g. `$data['Approval']['ActionField'] == 'sandbox'`).  Let me know if that's not cool.

Comment 3

[Barry Chen]

Attachment: patch for changing sandbox to reject for reviews

Comment 4

[Wil Clouser [:clouserw]]

Comment on attachment 496447 [details] [diff] [review]
patch for changing sandbox to reject for reviews

Can you change "queues" to "queue" in the last string?

As for the bigger picture, this does what I asked, I hope it's the right thing. :)  Once it lands on PAMO, Jorge can check it out.  Making them STATUS_NULL makes then "incomplete" (not disabled) but that has the effect of making their detail pages 404s and such.  So, I think it accomplishes what we want.  Let's find out! :)

Comment 5

[Barry Chen]

It's in r79072 now.

Comment 6

[Justin Scott [:fligtar]]

If an add-on is rejected for preliminary review, it should be set to Disabled.

Comment 7

[Jeff Balogh (:jbalogh)]

(In reply to comment #6)
> If an add-on is rejected for preliminary review, it should be set to Disabled.

Is that Disabled like admin-disabled?  They won't be able to change their status anymore.  If we drop them to NULL they'll be able to upload and re-request review.

Comment 8

[Justin Scott [:fligtar]]

Sorry, I meant that the file reviewed should be set to Admin disabled.

Comment 9

[Barry Chen]

Reopening because the language in the e-mail templates need to be updated.

@clouserw and @fligtar: so the add-on and file should have STATUS_DISABLED when the add-on's rejected for a preliminary review?  What about rejection on a nominated for public review?

Comment 10

[Barry Chen]

Also, do we need a new addonlog type for this change?  Pending update rejections are logged with RETAIN_VERSION right now.

Comment 11

[Justin Scott [:fligtar]]

Only the add-on file can be disabled on prelim review. Outcomes of a full review can either be disabled, prelim reviewed, or fully reviewed.

I don't know what the copy for the various addonlog messages are but I'd be happy to review them.

Comment 12

[Barry Chen]

It's likely because I don't have any background on the editor review process and came in during a transitional period, but man I'm just getting more and more confused as I work on these review related bugs. :(

If there's a comprehensive document somewhere that describes how the statuses of addons and files change throughout the review process, then link me to it and stop reading.

Otherwise, here is what I know.  Please educate me. :)

Preliminary Reviews
--
Addon and files have the unreviewed status (formerly sandbox).  Once approved, the addon and file(s) have the lite status.  If rejected, the addon and file(s) gets the null status(???).  Files for an update to a preliminarily reviewed addon have the unreviewed status(???).  When an update is approved, the files receives the same status as the addon, lite or lite_and_nominated.

Full Reviews
--
A preliminarily reviewed addon can be nominated for a full review, making it lite_and_nominated.  If an editor rejects a lite_and_nominated addon, then it goes back to lite.  Upon approval, the addon and files get the public status.

Addons with the nominated status are in the full reviews queue without being preliminarily reviewed.  A rejection results in a null status for the addon(???).  An approval results in a public status.

Files for an update to a fully reviewed addon have a pending status.  When the update is approved, the files get the public status.  If an editor rejects the pending update, then the files receive the null status.


@fligtar I did not see a way for an editor to disable an addon.  Are you referring to the admin tools?  We have not updated the statuses there as far as I know.

Thanks!

Comment 13

[Barry Chen]

Attachment: handle full reviews

Comment 14

[Barry Chen]

(In reply to comment #13)
> Created attachment 498299 [details] [diff] [review]
> handle full reviews

This patch handles the changes to full reviews.  The push to public and grant prelim review actions currently share icon images.

Comment 15

[Barry Chen]

Attachment: handle lite_and_nominated reviews

This patch follows the patch in attachment 498299 [details] [diff] [review].  Please apply and review them in order.

Comment 16

[Barry Chen]

Attachment: handle pending updates

Review changes for pending updates to public add-ons

Comment 17

[Barry Chen]

(In reply to comment #16)
> Created attachment 498663 [details] [diff] [review]
> handle pending updates
> 
> Review changes for pending updates to public add-ons

Forgot to mention that this patch follows attachment 498572 [details] [diff] [review].

@clouserw Let me know if you haven't looked at any of these patches and want me to squash them.

Comment 18

[Barry Chen]

Attachment: review changes for full reviews and pending updates

Had to remove a line from one of the previous patches, so might as well squash them.  This consolidated patch has all the changes for the reviews in the full reviews and pending updates queues.

Comment 19

[Barry Chen]

All the changes for editors are in svn trunk @ r79711.