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

Filter history #253

Merged
merged 9 commits into from
Oct 20, 2021
Merged

Filter history #253

merged 9 commits into from
Oct 20, 2021

Conversation

hannaeko
Copy link
Member

@hannaeko hannaeko commented Jul 28, 2021

Purpose

Add filter usage in history component.

Context

Changes

Add "buttons" to filter the tests list. This uses the "filter" parameter of the API, now that it is correctly working.

Screenshot 2021-08-04 at 10-57-11 Zonemaster

Screenshot 2021-08-04 at 10-57-19 Zonemaster

Screenshot 2021-08-04 at 10-57-26 Zonemaster

How to test this PR

Load a result and verify if you can filter the test list correctly.

@hannaeko hannaeko requested review from matsduf and a user July 28, 2021 15:38
@hannaeko hannaeko mentioned this pull request Jul 29, 2021
@matsduf matsduf added this to the v2021.2 milestone Jul 29, 2021
@matsduf
Copy link
Contributor

matsduf commented Jul 29, 2021

The solution proposed here requires requering. Issue zonemaster/zonemaster-backend#830 suggests a solution that lets GUI get information on which test is delegated and which is undelegated when receiving the full list. That could make it possible to tag the full list to give more information to the users.

@blacksponge, what do you think?

@matsduf
Copy link
Contributor

matsduf commented Jul 29, 2021

I think it would be more reasonable to show delegated only by default. By running a undelegated test you can always create both total failure and total success. Secondly, I think it would be valuable to see which are delegated and which are not when all are listed (requires zonemaster/zonemaster-backend#830 to be resolved). Thirdly, even though the three-way selection (delegated, undelegated, all) it would give a simple interface to only have default (delegated) and "also show undelegated tests".

Else, it works as expected.

@hannaeko
Copy link
Member Author

hannaeko commented Jul 29, 2021

The solution proposed here requires requering.

I don't think that is much of an issue but I can wait for the other issue to be resolved. (I can even make that happen)

I think it would be valuable to see which are delegated and which are not when all are listed

I fully agree with you. I was also thinking that maybe it can be valuable to also show that information on the result page (maybe related to #232)

I think it would be more reasonable to show delegated only by default.

I can do that, sure.

Thirdly, even though the three-way selection (delegated, undelegated, all) it would give a simple interface to only have default (delegated) and "also show undelegated tests".

I am not so sure, I do find it valuable to only list undelegated tests, when we want to compare with a previous similar test.

@hannaeko hannaeko linked an issue Aug 4, 2021 that may be closed by this pull request
Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

This works fine! Please just add support for singular and plural, respectively.

src/app/components/history/history.component.html Outdated Show resolved Hide resolved
src/app/components/history/history.component.html Outdated Show resolved Hide resolved
src/app/components/history/history.component.html Outdated Show resolved Hide resolved
src/app/components/history/history.component.html Outdated Show resolved Hide resolved
src/assets/i18n/en.json Outdated Show resolved Hide resolved
src/assets/i18n/da.json Outdated Show resolved Hide resolved
src/assets/i18n/fi.json Outdated Show resolved Hide resolved
src/assets/i18n/fr.json Outdated Show resolved Hide resolved
src/assets/i18n/nb.json Outdated Show resolved Hide resolved
src/assets/i18n/sv.json Outdated Show resolved Hide resolved
Copy link
Contributor

@matsduf matsduf left a comment

Choose a reason for hiding this comment

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

I have 8 tests on iis.se in the database, but the GUI only shows the four oldest tests.

(I had not restarted the GUI enough.)

@hannaeko hannaeko merged commit d5ea519 into zonemaster:develop Oct 20, 2021
@ghost
Copy link

ghost commented Nov 25, 2021

Testing v2021.2

lgtm

@ghost ghost added the S-ReleaseTested Status: The PR has been successfully tested in release testing label Nov 25, 2021
@ghost ghost mentioned this pull request Nov 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-ReleaseTested Status: The PR has been successfully tested in release testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to filter tests in history
2 participants