785979 - Add delete button in devhub app version list

Status: VERIFIED FIXED

(Marketplace Graveyard :: Developer Pages, defect, P2)

Description

[Rob Hudson [:robhudson]]

This will give the developer the ability to delete or disable versions of their packaged app.

What rules, if any, do we need for this? For example, I think AMO allows the developer to delete versions that have files that haven't yet been reviewed. Otherwise, the developer can only disable versions.

Comment 1

[Christopher Van Wiemeersch [:cvan]]

(In reply to Rob Hudson [:robhudson] from comment #0)
> What rules, if any, do we need for this? For example, I think AMO allows the
> developer to delete versions that have files that haven't yet been reviewed.
> Otherwise, the developer can only disable versions.

I think that's totally reasonable.

Comment 2

[Andrew Williamson [:eviljeff]]

on AMO, for addons:
1) a developer can delete any version, reviewed or not.
2) a *file* can be deleted before a review, but not after (developers are instructed to delete the entire version instead).
3) versions and files can't be disabled at all by developers (the disabled state is used for editor rejections).

btw, 1) causes no end of issues as the review notes for that version doesn't show up on the review page and re-using the version number (a common developer idea) causes a mirroring bug where the wrong xpi is served.  Plus it means anyone who installs before approval doesn't get the fix.  

I wanted what you thought happens (versions can't be deleted after review) from a security standpoint but the principle that developers have full control over their own code won out.

.....

For packaged apps I'd be happy with the same arrangement, i.e. any version can be deleted, whenever they want.  Assuming the issues above with add-ons aren't replicated with Apps (!!!)  

Developer disabling I have no problem with but its not that necessary really and complicates the code (i.e. what status do you use?  You can't use disabled as its already going to be used.  If you create an extra status (like app level developer disable) then its got to be checked in many different places.)

Comment 3

[Rob Hudson [:robhudson]]

(In reply to Andrew Williamson [:eviljeff] from comment #2)
> For packaged apps I'd be happy with the same arrangement, i.e. any version
> can be deleted, whenever they want.  Assuming the issues above with add-ons
> aren't replicated with Apps (!!!)

Perhaps we can soft-delete them and modify our queries to ignore soft-deleted versions everywhere so the records still exist but don't cause issues.

I'm curious how this affects updates. If the developer deletes the latest version, version-check would actually revert back to a prior version as the "update". Which might be ok, but something to consider when we build the version-check for packaged apps.

Comment 4

[Andrew Williamson [:eviljeff]]

(In reply to Rob Hudson [:robhudson] from comment #3)
> (In reply to Andrew Williamson [:eviljeff] from comment #2)
> > For packaged apps I'd be happy with the same arrangement, i.e. any version
> > can be deleted, whenever they want.  Assuming the issues above with add-ons
> > aren't replicated with Apps (!!!)
> 
> Perhaps we can soft-delete them and modify our queries to ignore
> soft-deleted versions everywhere so the records still exist but don't cause
> issues.

the review notes exist somewhere for the deleted add-on versions its just the code that builds the review history doesn't include them.  Actually I think Kris may have fixed that in a recent push.

> I'm curious how this affects updates. If the developer deletes the latest
> version, version-check would actually revert back to a prior version as the
> "update". Which might be ok, but something to consider when we build the
> version-check for packaged apps.

For add-ons Firefox refuses to downgrade to an earlier version number which works around this issue - if the runtime does too then its okay.  Random suggestion: I wonder if some sort of version timestamp (set on upload) would be a good idea so the actual version number is ignored and a newer version is required.

Comment 5

[Wil Clouser [:clouserw]]

Note that UX might be suggesting changes to these rules a bit in an attempt to simplify things.

Comment 6

[Rob Hudson [:robhudson]]

(In reply to Rob Hudson [:robhudson] from comment #0)
> This will give the developer the ability to delete or disable versions of
> their packaged app.
> 
> What rules, if any, do we need for this? For example, I think AMO allows the
> developer to delete versions that have files that haven't yet been reviewed.
> Otherwise, the developer can only disable versions.

FWIW, I was originally confused. On AMO we only allow the dev to disable the add-on itself, not the version. Also on AMO, the dev can delete the version at any time, regardless of current status.

If we want to do the same for Marketplace, I'd like to first verify that we handle this case properly throughout the codebase, and also with regards to updates and Firefox OS. For example, when a dev deletes a version does the update service then return the prior approved version? And what does Firefox OS do when that happens?

Comment 7

[Rob Hudson [:robhudson]]

For now, the developer can delete any version no matter of status:
https://github.com/mozilla/zamboni/commit/972da04

Comment 8

[Cristian Boldan]

Verified as fixed in https://marketplace-dev.allizom.org/developers/ on FF19 (Win 7)
Postfix screencast http://screencast.com/t/9sRs7Hgx8qlv
Closing bug.