Closed Bug 1022895 Opened 10 years ago Closed 10 years ago

Rejected themes are sometimes showing up in the Updates queue

Categories

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

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED INVALID
2015-01

People

(Reporter: amyt, Unassigned)

Details

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

Attachments

(3 files)

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.
Target Milestone: --- → 2014-07
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)
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
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)
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
Closed: 10 years ago
Resolution: --- → FIXED
Verified as fixed. The rejected themes are no longer displayed in the update queue.
Status: RESOLVED → VERIFIED
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)
Flags: needinfo?(atsay)
Whiteboard: [reviewteam:p2] → [ReviewTeam:P2][amy]
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Target Milestone: 2014-08 → 2015-01
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)
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
Closed: 10 years ago10 years ago
Resolution: --- → INVALID
Thanks for looking into it! The way it is currently isn't ideal, but will suffice.
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: