851881 - Allow theme artists to upload new header/footer graphics

Status: RESOLVED WORKSFORME

(addons.mozilla.org Graveyard :: Developer Pages, defect, P4)

Description

[Christopher Van Wiemeersch [:cvan]]

Theme artists can currently update the header/footer graphics on getpersonas.com. I can't really follow the code path, but apparently it adds a new row to a "pending personas" table (maybe `ssc_personas`?) until the theme is approved, at which point the values from that row get moved over to the `personas` table.

I'd like to avoid versioning themes in the `versions` table (beyond the version 0 we create at submission). Maybe we can add two columns: `header_pending` and `footer_pending`.

On AMO, theme artists are free to edit any part of their listings. But if the header and/or footer graphics are changed, we need to have that theme change show up in a "Re-Review Queue."

Comment 1

[Christopher Van Wiemeersch [:cvan]]

Questions: What happens to a public theme when I try to change its header/footer graphic? Does it become pending? Does it remain public but with the older header/footer graphic? If it does remain public but I see my old header/footer graphic, then how do I know that my header change is in the "Re-review Queue"? Similarly, if I want to replace my header/footer graphic again while it's under review, I suppose we'll need to allow a "Cancel"/"Replace" option.

I welcome all ideas for simplification.

Comment 2

[Wil Clouser [:clouserw]]

Why do you want to avoid versions so badly?  Seems like it would fix all of this and be a flow users would already expect.

Comment 3

[Matthew N. [:MattN]]

(In reply to Chris Van Wiemeersch [:cvan] from comment #0)
> On AMO, theme artists are free to edit any part of their listings. But if
> the header and/or footer graphics are changed, we need to have that theme
> change show up in a "Re-Review Queue."

Are you saying that changing the name and description won't require re-review? If so, note that it's a change from getpersonas.com.  Hopefully the problem of spammy descriptions in getpersonas will go away with tagging on AMO. We'll just have tag spam instead :P

I don't think it's necessary to keep a copy of every version of the images. We don't currently and it's more disk space used for something only the author and reviewers would ever see.

Comment 4

[Wil Clouser [:clouserw]]

We don't require re-review for name changes for add-ons, I don't think we need them here either.  If we run into a problem we'll address it then.

Comment 5

[Matthew N. [:MattN]]

I agree but wanted to make sure that the change was known.

Comment 6

[Kevin Ngo [:ngoke]]

PR: https://github.com/mozilla/zamboni/pull/721

Comment 7

[Kevin Ngo [:ngoke]]

https://github.com/mozilla/zamboni/commit/bec27efe265b2bf63ab8ebaac4657e5e36374b5d

Comment 8

[Amy Tsay [:amyt]]

When I go to the edit listing page and upload a new header, I don't see the new image after I click Save. I do see it in the re-review queue, but when I approve it and click Submit, I get an error page.

Comment 9

[Kevin Ngo [:ngoke]]

amyt: On -dev or production? Also, can you try again and see what happens now? I don't have access to it right now.

Comment 10

[Amy Tsay [:amyt]]

This is on dev. Here is the theme I changed the header on last week: http://i.imgur.com/VExtFqv.png When I mouse over the image, the new image is what appears in the header. Also, when I approved it in the re-review queue, the images were blank: http://i.imgur.com/UIBTICe.png

Comment 11

[Wil Clouser [:clouserw]]

We've had to dump all the celery jobs on -dev twice in the past few days due to it backing up and stumbling over itself.  I hope this isn't using celery to resize images, but fyi in case there is something related in there.

Comment 12

[Christopher Van Wiemeersch [:cvan]]

(In reply to Wil Clouser [:clouserw] from comment #11)
> We've had to dump all the celery jobs on -dev twice in the past few days due
> to it backing up and stumbling over itself.  I hope this isn't using celery
> to resize images, but fyi in case there is something related in there.

We are using celery to resize and save thumbnails: https://github.com/mozilla/zamboni/blob/master/apps/addons/tasks.py#L136

Comment 13

[Kevin Ngo [:ngoke]]

Looks like the Celery queue is in a good state now, working fine for me now.

I can submit, approve, reupload, and approve the re-review with images visible.

Though, theme preview images are not visible on the bottom of the theme edit page.

Comment 14

[Amy Tsay [:amyt]]

Nothing happens when you click on Upload New Design on the Edit page.

Comment 15

[Christopher Van Wiemeersch [:cvan]]

Amy, works for me - http://cl.ly/image/3u2F420B0Q3l

Hey Kevin - we need to have the theme update_check return the new version:
https://github.com/mozilla/zamboni/blob/master/services/theme_update.py#L150

Comment 16

[Kevin Ngo [:ngoke]]

Should work now, I might have introduced a JS error yesterday. Try again!

Okay, will look/file bug for theme versions.