1083736 - Filters dropdown panel doesn't update UI correctly

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect, P2)

Description

[James Graham [:jgraham]]

Using the filters dropdown an adding a field like "group symbol" "W" causes the URL to be updated to contain &field-job_group_symbol=w but hides all jobs. Reloading the page then causes the filter to apply, but the fact that the filter is applied doesn't get reflected in the dropdown UI. This makes the filtering functionality very painful to use.

Comment 1

[Kristján Oddsson [:koddsson]]

I'm not sure if I have to state this directly but I'd like to work on this bug if possible.

Comment 2

[Kristján Oddsson [:koddsson]]

In any case I made a PR here. https://github.com/mozilla/treeherder-ui/pull/240

Comment 3

[Ed Morley [:emorley]]

Cameron or Mauro, could you take a look at Kristján's PR? :-)

Comment 4

[Jonathan French (:jfrench)]

Attachment: treeherder-ui-PR#240

Thanks for helping out Kristján! I've added your same PR as a 'text' attachment (the path to the PR on Github), and flagged a review request for both Cameron and Mauro, whoever gets to it first.

Comment 5

[Mauro Doglio [:mdoglio]]

Just a quick update, I'm reviewing this patch

Comment 6

[Mauro Doglio [:mdoglio]]

@koddsson thanks for looking into this. I tried your PR locally and I still see the issue. Does it work for you locally?

Comment 7

[Kristján Oddsson [:koddsson]]

Let me double check that and get back to you :)

Comment 8

[Kristján Oddsson [:koddsson]]

Attachment: koddsson's PR functionality.

Comment 9

[Kristján Oddsson [:koddsson]]

Still works for me! It´s entierly possible that I misunderstood the issue though so I made a gif and attached it to this bug report.

Comment 10

[James Graham [:jgraham]]

I can't tell from that gif if it's working or not. It shows that some things are removed when the filter was applied, but in the original bug everything was removed, not just the things that don't match the filter.

Comment 11

[Kristján Oddsson [:koddsson]]

Oh man, I completely skipped trough that part when I was reading it and therefor thought that this was just about the missing tag from the UI. My PR does not address the issue of the jobs being all cleared out on filtering but I've got some time this weekend to sort that out if that's cool.

Comment 12

[Kristján Oddsson [:koddsson]]

I just pushed a change that should have this all cleaned up and fixed.

Comment 13

[Mauro Doglio [:mdoglio]]

I provided feedback on github

Comment 14

[James Graham [:jgraham]]

OK, I don't know if I can usefully help here without learning the whole treeherder frontend code. Which I would really prefer not to do ;)

Comment 15

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder-ui

https://github.com/mozilla/treeherder-ui/commit/2b384378c5e313b8b6100b4261678eabd6aa132e
[fix bug 1083736] Add emit with query on load.

https://github.com/mozilla/treeherder-ui/commit/5b2de77df4f085061c77035ddee32c3d131d9227
Merge pull request #240 from koddsson/fix-1083736

[fix bug 1083736] Add emit with query on load.

Comment 16

[Cameron Dawson [:camd]]

This is partially addressed in the merged PR, but needs more work.  That work will continue in this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1095184

Comment 17

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/51ad30dd1b2c26762147493850dd76428ecaedc8
[fix bug 1083736] Add emit with query on load.

https://github.com/mozilla/treeherder/commit/b06d041f9c22d6bfacecdf2ecc0ccada64b9ce73
Merge pull request #240 from koddsson/fix-1083736

[fix bug 1083736] Add emit with query on load.