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

Report results loading #1863

Merged
merged 27 commits into from
Dec 17, 2019
Merged

Report results loading #1863

merged 27 commits into from
Dec 17, 2019

Conversation

bjoernricks
Copy link
Contributor

@bjoernricks bjoernricks commented Dec 13, 2019

Missing:

  • Report ID filter
  • Reloading
  • fix isUpdating indicator while loading full report
  • fix changing filter while loading report -> was broken before and will be fixed in another PR
  • fix setting sort=foo when sort-reverse=bar is applied
  • Error handling

Checklist:

@codecov
Copy link

codecov bot commented Dec 13, 2019

Codecov Report

Merging #1863 into gsa-8.0 will decrease coverage by 0.12%.
The diff coverage is 9.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           gsa-8.0   #1863      +/-   ##
==========================================
- Coverage    40.03%   39.9%   -0.13%     
==========================================
  Files          977     977              
  Lines        22475   22557      +82     
  Branches      6331    6324       -7     
==========================================
+ Hits          8997    9001       +4     
- Misses       12192   12259      +67     
- Partials      1286    1297      +11
Impacted Files Coverage Δ
gsa/src/web/pages/reports/thresholdmessage.js 66.66% <ø> (ø) ⬆️
gsa/src/web/store/entities/reports.js 73.68% <0%> (ø) ⬆️
gsa/src/web/pages/reports/detailscontent.js 6.45% <0%> (ø) ⬆️
gsa/src/web/pages/reports/deltadetailspage.js 5.33% <100%> (ø) ⬆️
gsa/src/web/entities/withDefaultFilter.js 42.85% <25%> (-17.15%) ⬇️
gsa/src/web/pages/reports/details/resultstab.js 6.38% <4.65%> (-10.29%) ⬇️
gsa/src/web/pages/reports/detailspage.js 5.75% <50%> (+0.34%) ⬆️

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 2ada7a4...58a4168. Read the comment docs.

@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #1863 into gsa-8.0 will decrease coverage by 0.22%.
The diff coverage is 14.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           gsa-8.0    #1863      +/-   ##
===========================================
- Coverage    40.04%   39.82%   -0.23%     
===========================================
  Files          978      979       +1     
  Lines        22480    22636     +156     
  Branches      6310     6368      +58     
===========================================
+ Hits          9003     9015      +12     
- Misses       12191    12311     +120     
- Partials      1286     1310      +24
Impacted Files Coverage Δ
gsa/src/web/pages/reports/details/alertactions.js 6.55% <ø> (ø) ⬆️
gsa/src/web/pages/reports/thresholdmessage.js 66.66% <ø> (ø) ⬆️
gsa/src/web/pages/reports/deltadetailscontent.js 10% <ø> (ø) ⬆️
gsa/src/web/store/entities/reports.js 73.68% <0%> (ø) ⬆️
gsa/src/web/pages/reports/details/toolbaricons.js 15.38% <0%> (ø) ⬆️
gsa/src/web/pages/reports/detailscontent.js 3.75% <0%> (-1.25%) ⬇️
gsa/src/web/pages/reports/deltadetailspage.js 5.33% <100%> (ø) ⬆️
gsa/src/web/pages/reports/details/summary.js 6.25% <11.11%> (-0.77%) ⬇️
...a/src/web/pages/reports/details/deltaresultstab.js 16.66% <16.66%> (ø)
gsa/src/web/entities/withDefaultFilter.js 42.85% <25%> (-17.15%) ⬇️
... and 4 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 f1643e0...70b473e. Read the comment docs.

Allow to specify the prop name used by withDefaultFilter and use
defaultFilter by default instead of filter.
Update ResultsTab component to load report results via <get_results
filter="report_id=foo"/> command.

Implement sorting and pagination via filter changes.
Always add passed name property to all debug messages.
Automatically reload results data at the ResultsTab of a report.
This same functionality is already provided by the Reload component.
Don't show sort, sort-reverse, first and rows filter params at the
bottom of the ResultsTab. Due to the different filter handling in the
components the full filter may be very confusing.
@bjoernricks bjoernricks marked this pull request as ready for review December 17, 2019 10:46
@bjoernricks bjoernricks requested a review from a team December 17, 2019 10:46
gsad/src/gsad_gmp.c Outdated Show resolved Hide resolved
gsad/src/gsad_gmp.c Outdated Show resolved Hide resolved
gsad/src/gsad_gmp.c Show resolved Hide resolved
This filter param is only for internal usage and must not be used
outside of gsa. It can vanish everytime.
If the report filter is changed when the ResultsTab isn't mounted (measn
that another tab is displayed currently) it needs to load the results
with this changed filter instead of the stored filter.
Fallback to current report filter is the results filter isn't available.
This allows for easier debugging issues with the current filter used for
getting the results.
This avoids displaying and using wrong filter terms like duplicated sort
terms.
If an error occurs during loading the results display an ErrorPanel with
details about the error.
If loading a report has an error and we are already displaying a report
show the error at the summary.
Also change displayed message if no report could be loaded at all.
Only pass simple required data to components. If the component only
requires the report id the report id shouls be passed as a prop directly
instead of passing the whole report model.
Update report ToolBarIcons and DetailsContent to use reportId prop
directly.
Boolean props should be named isSomething or hasSomething. This is a
leftover from early implementations.
Update the props of ToolBarIcons to accept only a object shape for
report and don't require it. The ToolBarIcons render just find if the
report isn't provided.
Don't display empty lists at the different tabs which state that no
entities are available. Show a loading indicator if the full report is
still be loaded.
Be more precise about where this filter comes from.
By default it seems that resetting the report filter creates a filter
with sort=name. The name column doesn't exist for results. Therefore we
should reset the filter explicitly to a specific sorting.
Copy link
Member

@timopollmeier timopollmeier left a comment

Choose a reason for hiding this comment

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

The gsad part looks okay to me now.

@bjoernricks
Copy link
Contributor Author

Thanks for the reviews 👍

@bjoernricks bjoernricks merged commit a7bb94b into greenbone:gsa-8.0 Dec 17, 2019
@bjoernricks bjoernricks deleted the report-results-loading branch December 17, 2019 13:40
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.

3 participants