Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add link to test results for multiple job grops #1860

Merged

Conversation

OleksandrOrlov
Copy link

@OleksandrOrlov OleksandrOrlov commented Nov 14, 2018

Add link to test results for multiple job grops

There were no link to Test Results Page for parent job groups on
Dashboard Page.

The PR adds the link to the parent job group item. It allows to open
Test Results Page in new browser tab by performing middle mouse click or
by calling context menu and selecting 'Open link in new tab'.

Old behavior remains the same while doing left button mouse click.

Related ticket: 42851

Copy link
Contributor

@coolo coolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the icon a little random - and pretty non-obvious where it links to.

What is the actual problem? That the link you expect to lead to test results just collapses?

t/ui/14-dashboard-parents.t Outdated Show resolved Hide resolved
@OleksandrOrlov
Copy link
Author

@coolo,

What is the actual problem?

The actual problem is that there is no way to proceed to test results for the whole job group from Dashboard Page. Only child job groups have the links while parent group title is just an anchor for expanding/collapsing the list of its child jobs.

I find the icon a little random - and pretty non-obvious where it links to

It is general link icon. From my point of view it is obvious, that it is a link for the item in the same row. In the actual case, for the 'BuildXXX'. Also, it has 'title' attribute, that states: "View Test Results".

But, sure, this could be discussed. If you think that there might be better design, I'm open for the ideas and discussion.

@okurz
Copy link
Member

okurz commented Nov 14, 2018

Have you discussed with @Martchus who did the original implementation of the folding if that should be changed together with including the new link? Maybe have the title of the parent job group link to the complete result from all subgroups in the parent job group and have a button/link for folding on a different place, e.g. the triangle icon with increased size?

@coolo
Copy link
Contributor

coolo commented Nov 14, 2018

I rather have the build number linking to where you expect it to - and have an extra icon to collapse

@OleksandrOrlov
Copy link
Author

@coolo,

I rather have the build number linking to where you expect it to - and have an extra icon to collapse

It might be done for sure, but this affects current user experience. I didn't want to change the existing behavior and make users learn new behavior for already known item.

@OleksandrOrlov
Copy link
Author

@okurz, I discussed with him. He didn't have any restrictions or strict guides on how it must be implemented. So, I've chosen the way by myself.

@coolo
Copy link
Contributor

coolo commented Nov 14, 2018

Happens :)

I still don't agree with the proposal

@Martchus
Copy link
Contributor

Maybe have the title of the parent job group link to the complete result from all subgroups

The PR currently doesn't provide such a link. The link added here is not restricted to any group. I suppose that is a bug which definitely should be fixed before merging (regardless how we display the link in the end).

e.g. the triangle icon with increased size?

Would make sense.


Note that this change - although the screenshot only shows the parent group overview - also affects the main dashboard. On the main dashboard the default is collapsed and I suppose users are used to click on the build number to expand.

So not sure how to add this link in the best way.

Another idea would be a drop-down icon to open a context/drop-down menu for further links like this one.

assets/stylesheets/dashboard.scss Outdated Show resolved Hide resolved
assets/stylesheets/dashboard.scss Outdated Show resolved Hide resolved
t/ui/14-dashboard-parents.t Outdated Show resolved Hide resolved
@OleksandrOrlov OleksandrOrlov force-pushed the 42851_link_for_parent_job_groups branch 2 times, most recently from 615c569 to b4c66d9 Compare November 14, 2018 15:12
@OleksandrOrlov
Copy link
Author

OleksandrOrlov commented Nov 14, 2018

@coolo, @okurz, @Martchus I've implemented it in a different way.
Bootstrap out-of-the-box allows to specify 'href' for <a> alongside with the 'data-target' attribute. That allows to save the collapse/expand behavior, but at the same time provides the link for the element.

Now, by doing middle-click or right-click->Open in new tab, the results for the whole Build could be viewed.

Do you like this behavior more?

@coolo
Copy link
Contributor

coolo commented Nov 14, 2018

Add a title to make this behavior discoverable and we're settled

@OleksandrOrlov
Copy link
Author

@coolo, ok. What kind of message do you expect to have here?

Is "Open in new tab to view Test Results" enough? Or "Right click and open in new tab to view Test Results"?

@Martchus
Copy link
Contributor

I would make the title "Open in new tab to view entire build".

I've noticed that the link is still not limited to the groups contained by the parent group. But wouldn't that make sense?

@OleksandrOrlov OleksandrOrlov force-pushed the 42851_link_for_parent_job_groups branch 2 times, most recently from 2c2eaa7 to 99a21bb Compare November 20, 2018 22:18
@codecov
Copy link

codecov bot commented Nov 20, 2018

Codecov Report

Merging #1860 into master will increase coverage by 22.89%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #1860       +/-   ##
==========================================
+ Coverage    67.6%   90.5%   +22.89%     
==========================================
  Files         127     148       +21     
  Lines        9434   10249      +815     
==========================================
+ Hits         6378    9276     +2898     
+ Misses       3056     973     -2083
Impacted Files Coverage Δ
lib/OpenQA.pm 69.23% <0%> (ø)
lib/OpenQA/WebAPI/Controller/API/V1/Command.pm 30.76% <0%> (ø)
lib/OpenQA/Script.pm 90% <0%> (ø)
lib/OpenQA/Parser/Format/Base.pm 100% <0%> (ø)
lib/OpenQA/WebAPI/Plugin/Fedmsg.pm 96.87% <0%> (ø)
lib/OpenQA/Worker/Cache/Task.pm 100% <0%> (ø)
lib/OpenQA/WebAPI/Controller/API/V1/Locks.pm 96.77% <0%> (ø)
lib/OpenQA/WebAPI/Controller/Admin/Influxdb.pm 98.03% <0%> (ø)
lib/OpenQA/Parser/Format/LTP.pm 100% <0%> (ø)
lib/OpenQA/Parser/Format/IPA.pm 100% <0%> (ø)
... and 77 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d4420d...2a42bd4. Read the comment docs.

@OleksandrOrlov OleksandrOrlov force-pushed the 42851_link_for_parent_job_groups branch 3 times, most recently from 6df44b1 to 212caf7 Compare November 21, 2018 08:07
Oleksandr Orlov added 2 commits November 21, 2018 10:18
There were no link to Test Results Page for parent job groups on
Dashboard Page.

The commit adds the link to the parent job group item. It allows to open
Test Results Page in new browser tab by performing middle mouse click or
by calling context menu and selecting 'Open link in new tab'.

[42851](https://progress.opensuse.org/issues/42851)
The test verifies if the parent job group contains the valid url for the
entire build.

Related ticket: [42851](https://progress.opensuse.org/issues/42851)
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it locally with OSD data.

@coolo coolo merged commit 121b6ee into os-autoinst:master Nov 21, 2018
coolo pushed a commit that referenced this pull request Nov 21, 2018
commit 121b6ee
Merge: 1d4420d 2a42bd4
Author:     Stephan Kulow <stephan@kulow.org>
AuthorDate: Wed Nov 21 11:02:30 2018 +0100
Commit:     GitHub <noreply@github.com>
CommitDate: Wed Nov 21 11:02:30 2018 +0100

    Merge pull request #1860 from OleksandrOrlov/42851_link_for_parent_job_groups

    Add link to test results for multiple job grops
@okurz
Copy link
Member

okurz commented Nov 21, 2018

missed the typo in the commit message :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants