818145 - Can't remove devices in a re-review

Status: RESOLVED FIXED

(Marketplace Graveyard :: Reviewer Tools, defect)

Description

[Andrew Williamson [:eviljeff]]

Current functionality is to flag an app that adds devices for re-review.
However, we can't remove supported devices in a re-review, only reject entirely or clear the re-review (where the developer receives no notification).

I can't think of a clean solution for this right now but some possible ones are:
a) have an approval option, with the device checkboxes, on the re-review form, but only used for this scenario.
b) treat these kind of reviews as normal (non- re-review) app reviews.  

The unreviewed devices should probably not be installable until we review them either (though that may be another bug)

Comment 1

[Wil Clouser [:clouserw]]

Lisa - what do you want to do here?  CC UX if you want their input.

Comment 2

[Andrew Williamson [:eviljeff]]

?

Comment 3

[Lisa Brewster [:adora]]

Apps should not be available on devices we've not approved.  Bram, any ideas on how to make sense of this?

Clarifying the current flow:
1. App submitted for platforms A, B, and C
2. App approved only for platform A
3. Developer resubmits to add support for platforms B and C

Desired result: app is not available for end users on platforms B and C until approved by the app review team.  Approval controls should allow the reviewer to change supported platforms at any stage of publication.  (for example, if the issue on platform B is fixed but platform C still doesn't work, or if fixing the issues on platform B and C caused a regression that breaks platform A, etc)

Actual result: app is immediately available on platforms B and C.  App lands in the re-review queue after submission, and there are no controls to approve per-device.

Comment 4

[Andrew Williamson [:eviljeff]]

What I have to do with this scenario at the moment is:
* remove the devices via the developer Manage page
* send an info request to the developer telling them
* then reload the review page to clear the re-review flag

Its messy and removing the devices via Manage requires Staff/Admin permissions which aren't made available to standard reviewers.

Comment 5

[Bram Pitoyo [:bram]]

I am going to work on this tomorrow.

Somewhat related question for Lisa: where should I post my mockups for the App History attachment feature?

Comment 6

[Bram Pitoyo [:bram]]

Attachment: app review states - explanation - 01

I had a long chat with Lisa and a bit with Cvan today, and found out that the problem seemed easy to fix.

But this problem is also about informing developers of their app status and giving developers power to publish on any platform that’s been approved, anytime they want.

For that, we will need a better app status indicator, and this is what this mockup visualizes.

The user flow is as follows:

1. First, developer goes through the DevHub app submission process and selects a platform. Let’s say that he selects platform a, b and c.

2. After submission, a reviewer logs into the Reviewer Tools and determine whether the app truly is compatible with all the platforms. Let’s rename the tab from “push to public”, which is the developer’s job, to “Approve”, which is the reviewer’s job. Then, under the “Device override” field, only platform a, b and c can be approved, even though we have many other platforms. This is to prevent reviewers from accidentally approving platforms that the developer did not submit for.

3. After approval, any of these two scenarios can happen:
  a) The platform is approved; now it’s up to the developer to make the app on that platform public
  b) The platform is not approved; the developer should make correction and resubmit for that platform

In order to inform developers of their app progress, we’ll design a simple status table that will indicate the status of their app on each platform.

Currently, this table exist at the “Manage My Submissions” page, although maybe it’s more appropriate to have this be located under the “Manage Status” page.

The table contains each platform on a column, and each status on a row. The statuses are “submitted”, “approved” and “published”.

Inside the “submitted” row:
* Green means that a developer has submitted for that platform
* Exclamation mark means that the submission is incomplete
* Blank means that the platform is not selected, so the submission did not happen

Inside the “approved” row:
* Green means that the reviewer has approved an app for that platform
* Exclamation mark means that the platform is not approved for some reason
* Blank means that the app on that platform is currently undergoing approval

Inside the “published” row:
* Unchecked box means that the app is approved for that platform, but not yet published. Sometimes, this is called the “disabled” state.
* Developer can check the box and hit “Save” to publish the app for that platform
* Exclamation mark means that Mozilla has chosen to take the app down and prevent its installs (“moz-disabled” state)

With these check boxes, we give developer control over app publishing — provided that the app is approved, of course. Want to release the app in every platform? You can do that. Want to release the app in one platform while working on the other platform? You can do that. Giving developer control makes for a good differentiating factor between our marketplace and competitor’s.

Lastly, inside every exclamation mark is a link that the developer can use to fix the problem. Maybe these links will lead to the latest comment from the reviewer, describing the problem?

Comment 7

[Andrew Williamson [:eviljeff]]

(In reply to Bram Pitoyo [:bram] from comment #6)
> Created attachment 703209 [details]
> app review states - explanation - 01

Yes, this is much closer to the partial approval scenario we were looking for last year with the reviewer tools. 

> 3. After approval, any of these two scenarios can happen:
>   a) The platform is approved; now it’s up to the developer to make the app
> on that platform public
>   b) The platform is not approved; the developer should make correction and
> resubmit for that platform

Currently, if all requested platforms are approved, in most cases a) leads to the app being public immediately. There is a checkbox in the upload flow ' Publish my app in the Firefox Marketplace as soon as it's reviewed. ' which defaults to true.  My feeling is this is what most developers want/expect (though I have no evidence to back that up)

> Inside the “submitted” row:
> * Green means that a developer has submitted for that platform
> * Exclamation mark means that the submission is incomplete
> * Blank means that the platform is not selected, so the submission did not
> happen

I don't believe there is a way currently for a submission to be complete for some platforms but not for others (all details are app, rather than platform specific; any platform specific choices, such as payment, force the app to be limited to supported platforms).  The exclamation marks still work and make sense, but the mockup scenario couldn't happen.

> Inside the “published” row:
> * Unchecked box means that the app is approved for that platform, but not
> yet published. Sometimes, this is called the “disabled” state.

'approved but waiting' (STATUS_PUBLIC_WAITING) is what could be easily used instead.  (Currently 'disabled' is an app level setting.)

> * Exclamation mark means that Mozilla has chosen to take the app down and
> prevent its installs (“moz-disabled” state)

We wouldn't take down a single platform, we'd take down the whole app.  Otherwise it would look the same as the scenario where platforms are marked as incompatible.  I'm not sure if that makes a difference to your flow.

Comment 8

[Bram Pitoyo [:bram]]

(In reply to Andrew Williamson [:eviljeff] from comment #7)
> There is a checkbox in the upload flow
> ' Publish my app in the Firefox Marketplace as soon as it's reviewed. '
> which defaults to true.  My feeling is this is what most developers
> want/expect (though I have no evidence to back that up)

I have no evidence to the contrary, but feel like the “review table” (let’s call it that for now) is going to complement this checkbox nicely.


> I don't believe there is a way currently for a submission to be complete for
> some platforms but not for others […]

You are right. Submission is an all-or-nothing deal. A developer either completes the submission process for *all* the platforms he selected, or didn’t complete at all.

In this case, then, the exclamation mark should be displayed for all the selected platforms, rather than just one. By clicking on any of the links, he could continue the submission process.


> We wouldn't take down a single platform, we'd take down the whole app.

Then the same thing should happen as the scenario above: we put exclamation mark on all platforms when we take an app down. You’re right in assuming that it doesn’t make any difference to my flow.


A much simpler way to indicate incomplete submission and taken down app is by posting a big note to the side of the app name and not bother with using the table at all. But then it eliminates the table’s two main strength, which are: 1) Indicating which platform you’re supporting at the moment, and 2) Allow you to publish on each platform independently.

Comment 9

[Christopher Van Wiemeersch [:cvan]]

This is really confusing - can someone tell me what the verdict was here?

Comment 10

[Bram Pitoyo [:bram]]

Attachment: app review states - explanation - 02

(In reply to Chris Van Wiemeersch [:cvan] from comment #9)
> This is really confusing - can someone tell me what the verdict was here?

This set of user flow should clarify the verdict here.

Basically, we’re going to have two “review tables” or “status tables” that will tell user what the submission, approval and publishing status for each platform is.

One of the tables will be located on the “Manage My Submissions page”, and the other one’s on “Manage Status”.

Comment 11

[Rob Hudson [:robhudson]]

Closing in favor of 846438 to open up development and smaller implementation bugs.