#1782 Add buttons for "All milestone tags", or "All active milestone tags"
Merged by pingou. Opened by mreynolds.
mreynolds/pagure issue1776  into  master

Download 1782.patch

I still want to improve the page behaviour when selecting milestones tags and issue tags...

1 new commit added

  • Improve how the milestone tags are dispalyed when filtering is applied

This is ready for review

No more briefcase?

This is looking quite far from what I had in mind, I'll need to test it

This is looking quite far from what I had in mind, I'll need to test it

Please do, I am quite happy with how it turned out. And yes I removed the briefcase on purpose. I think it makes the page cleaner - you'll see.

This is the best scenario to see the potential of this change:

  • Create two issue tags: "foo_tag", and "foo2_tag"
  • Create 3 milestones: milestone-1,..,milestone-3
  • Create two issues, one with milestone-1, the other with milestone-3
  • Add "foo_tag" to one of the issues

Then goto the roadmap page, and change between "active" and "all" milestones. See how the milestone tags change. Then select the foo_tag, and bounce between all the the page options: different milestones, status, "active/all milestones", the issue tags, etc.

rebased

@mreynolds I've tried a slightly different approach, would you mind testing it?
0001-Rework-a-little-the-active-milestone-all-milestone-f.patch

I also found that it speeds thing up a little with selecting a single milestone while 'All milestones' are shown

Looks like it missed my second path, so here it is: 0002-Rename-the-variable-all_ms_tags-to-all_stones.patch

That one is purely aesthetic :)

I didn't dare to push these changes directly to your branch as I feared it may break a little (though from my current tests it seems not to)

Sorry your patches break everything :(

Milestone tags get incorrectly reset when when clicking on issue tags or individual milestone tags. Also, clicking on "Active Milestones" does not reset the button (All Milestones is always active)

Also, clicking on "Active Milestones" does not reset the button

Ok fixed that one

Milestone tags get incorrectly reset when when clicking on issue tags or individual milestone tags

That one I don't quite follow :(

0001-Fix-toggling-the-All-Milestones-Active-Milestones-bu.patch
This is the patch fixing the toggling

Also, clicking on "Active Milestones" does not reset the button

Ok fixed that one

Milestone tags get incorrectly reset when when clicking on issue tags or individual milestone tags

That one I don't quite follow :(

Yeah this is the most important part. Once you select "active milestones" the milestone tag list must NOT change regardless of what milestone tag or issue tag you select. So with your fix I select active milestones, then if I click on the "milestone-1" tag then all the milestone tags appears (active and non active) - bad. Same when I click on an issue tag, it brings in all the milestone tags.

Getting this part to work correctly was the hardest part of my fix ;)

Ok, I found the error, it's something that was at first working, I changed it and introduced a bug, here is the fix:

diff --git a/ pagure/ui/issues.py b/ pagure/ui/issues.py
index 0d18fe9..f4fba3f 100644
--- a/ pagure/ui/issues.py      
+++ b/ pagure/ui/issues.py      
@@ -713,7 +713,7 @@ def view_roadmap(repo, username=None, namespace=None):
     status = flask.request.args.get('status', 'Open')
     milestones = flask.request.args.getlist('milestone', None)
     tags = flask.request.args.getlist('tag', None)
-    all_stones = flask.request.args.get('all_stones', False)
+    all_stones = flask.request.args.get('all_stones')
     active_milestones = flask.request.args.getlist('active_milestones', None)

EDIT: The reason behind the bug: this None is being converted into 'None' in the template and ends up making if all_stones return True because a string (not empty) returns True, so it ends up being as if we asked for all the milestones instead of just the active ones

Well this patch fixes the milestone and issue tags, but it caused another regression. Now there are no issues displayed regardless if I choose active or all milestones

Correction - it is working correctly :)

You scared me man! Watch out for these things, I almost die here :D

Ok, let's push the changes and review them here as well :)

4 new commits added

  • Fix the unit-tests
  • Fix toggling the All Milestones/Active Milestones buttons
  • Rename the variable all_ms_tags to all_stones
  • Rework a little the active milestone/all milestone filtering

I like this diff :)

Me too, ack

Let's merge then! :)

Hmm got merge conflicts trying to pull in your changes

Pull-Request has been merged by pingou

Thanks!

Metadata