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)
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.
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Target Milestone: --- → 2014-07
Reporter | ||
Comment 2•10 years ago
|
||
Comment 3•10 years ago
|
||
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•10 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)
Updated•10 years ago
|
Target Milestone: 2014-07 → 2014-08
Reporter | ||
Updated•10 years ago
|
Priority: -- → P2
Whiteboard: [reviewteam:p2]
Comment 5•10 years ago
|
||
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•10 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)
Comment 7•10 years ago
|
||
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
Comment 8•10 years ago
|
||
Verified as fixed.
The rejected themes are no longer displayed in the update queue.
Status: RESOLVED → VERIFIED
Reporter | ||
Comment 9•10 years ago
|
||
This is happening again: http://i.imgur.com/960ebcs.png
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Comment 10•10 years ago
|
||
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)
Reporter | ||
Comment 11•10 years ago
|
||
I still see them in the update queue, yes: http://i.imgur.com/lw4zL0k.png. Do you need the links to them? Here are some:
https://addons.mozilla.org/editors/themes/queue/single/tann3
https://addons.mozilla.org/editors/themes/queue/single/verde-hay-azul
https://addons.mozilla.org/editors/themes/queue/single/zedbaziii
Does this help?
Flags: needinfo?(atsay)
Comment 12•10 years ago
|
||
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•10 years ago
|
||
Do you mean the Updates queue? https://addons.mozilla.org/editors/themes/updates
Flags: needinfo?(atsay)
Updated•10 years ago
|
Whiteboard: [reviewteam:p2] → [ReviewTeam:P2][amy]
Comment 14•10 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Target Milestone: 2014-08 → 2015-01
Reporter | ||
Comment 15•10 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.
Comment 16•10 years ago
|
||
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 → ---
Comment 17•10 years ago
|
||
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)
Comment 18•10 years ago
|
||
Reporter | ||
Comment 19•10 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)
Comment 20•10 years ago
|
||
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 ago → 10 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 21•10 years ago
|
||
Thanks for looking into it! The way it is currently isn't ideal, but will suffice.
Assignee | ||
Updated•9 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.
Description
•