Rejected themes are sometimes showing up in the Updates queue

RESOLVED INVALID

Status

P2
normal
RESOLVED INVALID
4 years ago
3 years ago

People

(Reporter: amyt, Unassigned)

Tracking

unspecified
2015-01
x86
Mac OS X

Details

(Whiteboard: [ReviewTeam:P2][amy])

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
Sometimes, rejected themes show up in the Updates queue. Here is an example: https://addons.mozilla.org/editors/themes/queue/single/verde-hay-azul. Not sure what is causing this. If you go to the main Updates queue: https://addons.mozilla.org/editors/themes/queue/updates, you don't see verde-hay-azul in the list.

cc'ing candelora, who reported the problem and may have details to add.
(Reporter)

Comment 1

4 years ago
Created attachment 8437199 [details]
Screenshot 2014-06-09 13.45.08.png
(Reporter)

Updated

4 years ago
Target Milestone: --- → 2014-07
(Reporter)

Comment 2

4 years ago
Created attachment 8444735 [details]
Another example - 6/23/14
I need some more info on your process to fix that bug.

First, which URL do you hit that contains the rejected theme? From your second screenshot it looks to be /editors/themes/queue/updates but your first comment states that you don't see the addon at that URL so I'm confused.

Second, to better understand your workflow which URLs should contain rejected themes and which ones shoudln't?
Assignee: nobody → david
Flags: needinfo?(atsay)
(Reporter)

Comment 4

4 years ago
You could see rejected themes in the log: https://addons.mozilla.org/editors/themes/logs. You should be able to filter by "reject", but that doesn't seem to be working. For this bug, we could tell https://addons.mozilla.org/editors/themes/queue/single/verde-hay-azul was rejected by the Review History on the bottom of the page.

There are 3 queues: Pending, Flagged, and Updates. None of those queues should include rejected themes.

Does this help?
Flags: needinfo?(atsay)
Target Milestone: 2014-07 → 2014-08
(Reporter)

Updated

4 years ago
Priority: -- → P2
Whiteboard: [reviewteam:p2]
Amy, thanks for your answer. I still don't get if the display of the Reject line in the theme log at the bottom of your first attachment is valid though https://bug1022895.bugzilla.mozilla.org/attachment.cgi?id=8437199

Should we keep it or remove it from that page too?

The case with Pending/Flagged/Updates tabs of the second attachment should be fixed in this pull-request: https://github.com/mozilla/olympia/pull/169
Flags: needinfo?(atsay)
(Reporter)

Comment 6

4 years ago
Hi David, the "reject" message in the log should remain, because it was rejected. The theme just shouldn't be listed in the Update queue. Does this help?
Flags: needinfo?(atsay)
Thanks Amy, merged in https://github.com/mozilla/olympia/commit/cdd535fa695409d99886c2e7ecbc5ff052269b5c

You should soon be able to test it on -dev.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Comment 8

4 years ago
Verified as fixed.
The rejected themes are no longer displayed in the update queue.
Status: RESOLVED → VERIFIED
(Reporter)

Comment 9

4 years ago
This is happening again: http://i.imgur.com/960ebcs.png
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Amy: do you confirm that this theme is no longer in that state in production? Do you have another example in order to check his rereview/locks status?

I have hard time reproducing it locally and I don't want to fix half of the issue again.
Flags: needinfo?(atsay)
Amy, can you give me the URL of your screenshot? I guess that's the rereview one but I prefer to be sure.
Flags: needinfo?(atsay)
(Reporter)

Comment 13

4 years ago
Do you mean the Updates queue? https://addons.mozilla.org/editors/themes/updates
Flags: needinfo?(atsay)

Updated

4 years ago
Whiteboard: [reviewteam:p2] → [ReviewTeam:P2][amy]
https://github.com/mozilla/olympia/commit/d94f2c93cadbab4f6bc92f5dac32eae5435f3437
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: 2014-08 → 2015-01
(Reporter)

Comment 15

4 years ago
Was this fix pushed? If so, does it apply to future instances? I ask bc there are still rejected themes in the Updates queue.
Yes, that fix was pushed. It's hard to fix because I wasn't able to reproduce it, I hope my successor will :)
Assignee: david → nobody
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ok, I believe that I managed to reproduce the issue:
1/ upload a new theme
2/ reject it
3/ modify the theme by clicking on the "upload new design" link above the header image
4/ the theme will now appear in the "updates" queue

Amy: can you confirm that this is the issue you're seeing? I'll attach a screenshot.

If that is the issue you're seeing, then I'm pretty sure this is by design, and expected: the theme is updated (with a new design), so it should be re-reviewed, right? And thus, should be displayed in the "updates queue"?
Flags: needinfo?(atsay)
Created attachment 8560389 [details]
Rejected theme displayed in the updates queue once a new design is uploaded
(Reporter)

Comment 19

4 years ago
Ah, I see. Yes that would make sense. The Rejected label is misleading, but it tells us that the theme was previously rejected and has now been updated?
Flags: needinfo?(atsay)
I believe that's the point yes.

On the other hand, I believe :mat (Mathieu Pillard) told me that on zamboni, if a theme is rejected, then it shouldn't go through the "updates" queue if it's modified, but rather go through the "pending" queue again, as if it was new.
The rationale is that the updates queue should be reserved to themes that have already been approved: it's therefore easier to approve it again, on the grounds that it should be trustworthy. On the contrary, if it's been rejected already, then maybe it's not trustworthy, and should go through the full scrutiny again.

I don't know what's the correct way, nor if the current behavior is expected.

If you feel it should be otherwise, please open a new bug so we can dig into it.
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → INVALID
(Reporter)

Comment 21

4 years ago
Thanks for looking into it! The way it is currently isn't ideal, but will suffice.
(Assignee)

Updated

3 years ago
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.