1217058 - Preserve grouping state when clicking links

Status: RESOLVED FIXED

(Tree Management :: Treeherder: Frontend, defect, P3)

Description

[Andreas Tolfsen ❲:ato❳]

The grouping state (group_state URL parameter) would be useful to preserve when navigating links.

If you’re in the list view with all groups expanded (&group_state=expanded) and want to see the jobs for an individual push, clicking the push link should preserve the expanded group state.

Comment 1

[Sabina Barbhuiya]

Hi I am an Outreachy Fall 2019 applicant. I would like to work on this issue. Can you please assign it to me?

Comment 2

[Albiona]

If the task will not be finished, I would like to continue working on it.

Comment 3

[sharonymun]

Hi, I'm currently an Outreachy applicant and I was hoping to work on this assignment if possible. Thank you

Comment 4

[Ejiro Ogidigbo]

Hi, I'm currently an Outreachy applicant. Could I work on this issue?

Comment 5

[Sarah Clements [:sclements]]

Hi Ejiro, yes I will assign it to you since everyone else who commented before you already has a bug assigned.

Comment 6

[Sarah Clements [:sclements]]

kevinejiro, were you still planning to work on this bug?

Comment 7

[Sarah Clements [:sclements]]

Since I haven't heard back from kevinejiro, I'm assigning this bug to someone else.

Comment 8

[Mellina Yonashiro]

Hi Cameron,

Could you help me with the STR?

1. On Treeherder, I clicked on the (+) to toggle group_state. This inserted the group_state=expanded URL parameter.
2. Next, I clicked on multiple links and buttons:

• "View only this push", "Collapse push data", "Pin Push", "Author filter". These keep the URL parameter.
• The links that doesn't keep the URL are: external links to Bugzilla and Mercurial and "View test".

Is it the right way to test it or I'm going to a different direction?

Thanks!

Comment 9

[Cameron Dawson [:camd]]

Melina: You're testing it correctly, yes. Do you mean you have a PR that fixes this? Because I can still reproduce this bug on Treeherder production.

To be clear, if you hover the link for "View only this push" if should reflect the state of expanded.

Comment 10

[Mellina Yonashiro]

Hi Cameron,

My STR were working, because the links are updated after some time or after a page refresh. Hehe

---

Could you give me some directions on this bug, please?

I have been studying and tried some approaches.

• Toggling Group State and Unclassified Failures/Filter Chicket behave differently. I started with that. Apparently refreshing the push link and URL params are a combination of changing the FilterModel.ulrParams and calling FilterModel.push(), that lives in the filter.js file.

• Toggling Group State doesn't trigger a Push link update, which seems to be made on PushHeader.jsx:267, on this.getLinkParams().

• I thought that maybe passing the groupCountsExpanded props or dispatching a redux action (which I didn't find any one related to the group_state) would trigger a component update. But it didn't :(

---

I know that you know everything I wrote here - I was just wondering if I was getting close or going into a totally different direction.

Thanks for your help.

Comment 11

[Cameron Dawson [:camd]]

Hi Melina-- Good eye focusing in on the FilterModel. That is, indeed, where the state of the filter chicklets are. But, as you said, groupCountsExpanded is actually independent of that. It's controlled by a state in App.jsx, and then passed as a prop to the other components down the list. If you follow that prop down, you'll see in Push.jsx that it is passed into PushJobs, but NOT passed into PushHeader. So you will need to pass that prop into PushHeader.jsx, and then add it to the shouldComponentUpdate check. I think that's all you'll need to do. If it doesn't show up, you may need to check the function getLinkParams so that it includes that query string param.

Comment 12

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/5533

Comment 13

[Cameron Dawson [:camd]]

This change fixes the issue. Nice work, Melina!