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

Showing closed reports #4244

Merged
merged 7 commits into from
May 29, 2024
Merged

Conversation

cservakt
Copy link
Collaborator

@cservakt cservakt commented May 16, 2024

[feat] Showing closed reports on Checker Coverage tab

It is an additional PR for Checker Coverage tab to list closed reports not just outstandings. The number of closed report can be clickable for the user if it is not zero.

Achieving this, it is necessary to add a report status filter to the report filter section that has two options: OUTSTANDING and CLOSED. A report is outstanding when its review status is unreviewed/confirmed and its detaction status is new/unresolved/reopened. When the new filter is set, the review and the detection status filters are not taken into account.

cservakt added 6 commits May 9, 2024 10:06
It is an additional PR for Checker Coverage tab to list closed reports not just outstandings. The number of closed report can be clickable for the user if it is not zero.

Achieving this, it is necessary to add a report status filter to the report filter section that has two options: OUTSTANDING and CLOSED. A report is outstanding when its review status is unreviewed/confirmed and its detaction status is new/unresolved/reopened. When the new filter is set, the review and the detection status filters are not taken into account.
It is an additional PR for Checker Coverage tab to list closed reports not just outstandings. The number of closed report can be clickable for the user if it is not zero.

Achieving this, it is necessary to add a report status filter to the report filter section that has two options: OUTSTANDING and CLOSED. A report is outstanding when its review status is unreviewed/confirmed and its detaction status is new/unresolved/reopened. When the new filter is set, the review and the detection status filters are not taken into account.
@cservakt cservakt added API change 📄 Content of patch changes API! GUI 🎨 new feature 👍 New feature request labels May 16, 2024
@cservakt cservakt added this to the release 6.24.0 milestone May 16, 2024
@cservakt cservakt requested review from bruntib and vodorok as code owners May 16, 2024 15:38
@bruntib
Copy link
Contributor

bruntib commented May 22, 2024

The review status and detection status filter options could be disabled in the GUI when report status is selected. Otherwise this patch looks good.

@dkrupp
Copy link
Member

dkrupp commented May 27, 2024

  1. The "Report Status" filter should behave like the other filters

When the new filter is set, the review and the detection status filters are not taken into account.
I think that the "Report Status" filter should behave the same way as the other filters.
A differnet beheviour for this specific filter would be very disturbing.

So the "Review Status", "Detection Status" should be an additional restricting filter. For example one might want to see the "closed" report whose review status is "false positive".

  1. The link from the coverage statistics should select teh "closed reports" only
    When the user clicks on the "closed reports" in the Checker coverage statistics tab, the filter should be set to show the "closed reports" only and the "Review Status" and "Detection Status" filters should be left empty.

  2. the new default filter should be the "oustanding reports"
    The new default filter should also be changed to show the "Outstanding" reports and the review status, detection status filters should be left empty.

We should orient the users towards using the Report Status filter instead of the "Review Status" and "Detection Status" filters. They should use these latter filters only if they want a detailed selection by the review/detection status.

Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

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

Setting the "report status" filter, should not cause the review/detection status filters to be ignored as it would be very unusal behaviour. See my detaild comment

@cservakt cservakt force-pushed the showing-closed-reports branch 2 times, most recently from 0dbfb8a to fae02ee Compare May 28, 2024 13:33
@cservakt cservakt requested a review from dkrupp May 28, 2024 13:34
@cservakt
Copy link
Collaborator Author

  1. The "Report Status" filter should behave like the other filters

When the new filter is set, the review and the detection status filters are not taken into account.
I think that the "Report Status" filter should behave the same way as the other filters.
A differnet beheviour for this specific filter would be very disturbing.

So the "Review Status", "Detection Status" should be an additional restricting filter. For example one might want to see the "closed" report whose review status is "false positive".

2. **The link from the coverage statistics should select teh "closed reports" only**
   When the user clicks on the "closed reports" in the Checker coverage statistics tab, the filter should be set to show the "closed reports" only and the "Review Status" and "Detection Status" filters should be left empty.

3. **the new default filter should be the "oustanding reports"**
   The new default filter should also be changed to show the "Outstanding" reports and the review status, detection status filters should be left empty.

We should orient the users towards using the Report Status filter instead of the "Review Status" and "Detection Status" filters. They should use these latter filters only if they want a detailed selection by the review/detection status.

Thanks for the suggestions! Now the report filter works as expected. The report status filter does not exclude the review and detection status filters.

Clicking to the number of the closed (or outstanding) reports, the url and the report filter are set correctly.

Changing the default filter for the reports and the statistics menu should be a different patch where we reconstruct the entire report filter.

@cservakt cservakt force-pushed the showing-closed-reports branch from fae02ee to 36722b4 Compare May 28, 2024 14:04
Copy link
Member

@dkrupp dkrupp left a comment

Choose a reason for hiding this comment

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

When testing I found the following minor faults:

  1. Please fix the help message of the report status field. Remove the text that the "review status and detection status is not prevalent"

  2. The Outstanding report number in the filter menu is not correctly counted. It always shows 0.

Otherwise looks good!

@cservakt cservakt force-pushed the showing-closed-reports branch from 36722b4 to 40b1000 Compare May 29, 2024 14:12
It is an additional PR for Checker Coverage tab to list closed reports not just outstandings. The number of closed report can be clickable for the user if it is not zero.

Achieving this, it is necessary to add a report status filter to the report filter section that has two options: OUTSTANDING and CLOSED. A report is outstanding when its review status is unreviewed/confirmed and its detaction status is new/unresolved/reopened. When the new filter is set, the review and the detection status filters are not taken into account.
@cservakt cservakt force-pushed the showing-closed-reports branch from 40b1000 to db33e7e Compare May 29, 2024 15:03
@cservakt
Copy link
Collaborator Author

When testing I found the following minor faults:

1. Please fix the help message of the report status field. Remove the text that the "review status and detection status is not prevalent"

2. The Outstanding report number in the filter menu is not correctly counted. It always shows 0.

Otherwise looks good!

I have fixed these minor bugs.

@cservakt cservakt requested a review from dkrupp May 29, 2024 15:10
@bruntib bruntib merged commit e2037fa into Ericsson:master May 29, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API change 📄 Content of patch changes API! GUI 🎨 new feature 👍 New feature request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants