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

Don't show empty sections in result details #2791

Merged
merged 6 commits into from
Apr 7, 2021

Conversation

swaterkamp
Copy link
Member

@swaterkamp swaterkamp commented Mar 15, 2021

What:
Parse empty strings in NVT tags as undefined.

Why:
Instead of undefined information we got an empty string before. This fixes showing empty sections on the result details.

How:
Check data in dev-tools and visually inspect whether the sections are no longer visible if there is no information. Added an automated test for the empty string case.

Checklist:

@swaterkamp swaterkamp self-assigned this Mar 15, 2021
@swaterkamp swaterkamp requested review from a team, saberlynx and sarahd93 March 15, 2021 06:44
@swaterkamp swaterkamp changed the title Result empty sections Don't show empty sections in result details Mar 15, 2021
@codecov
Copy link

codecov bot commented Mar 15, 2021

Codecov Report

Merging #2791 (83265b7) into gsa-21.04 (74a1fd3) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 83265b7 differs from pull request most recent head 6a28c9d. Consider uploading reports for the commit 6a28c9d to get more accurate results
Impacted file tree graph

@@              Coverage Diff              @@
##           gsa-21.04    #2791      +/-   ##
=============================================
- Coverage      53.83%   53.82%   -0.01%     
=============================================
  Files           1075     1075              
  Lines          26226    26227       +1     
  Branches        7491     7492       +1     
=============================================
- Hits           14118    14116       -2     
- Misses         10993    10996       +3     
  Partials        1115     1115              
Impacted Files Coverage Δ
gsa/src/gmp/models/nvt.js 98.94% <100.00%> (+0.01%) ⬆️
gsa/src/web/entities/container.js 44.48% <0.00%> (-1.23%) ⬇️

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 c22a231...6a28c9d. Read the comment docs.

Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

I am fine with this change too but could you please also fix the models to set undefined if an empty string is used in these tags. Such things should to be fixed in the parsing part (model) to avoid having to implement this meaning of empty string at several places in the view.

@bjoernricks bjoernricks self-requested a review March 15, 2021 07:59
Copy link
Contributor

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

Clicked on the wrong item. Should have been a Request changes.

@swaterkamp swaterkamp force-pushed the resultEmptySections branch from c9264b2 to 104fb97 Compare March 25, 2021 13:15
@swaterkamp swaterkamp requested a review from bjoernricks March 25, 2021 16:36
gsa/src/gmp/models/nvt.js Outdated Show resolved Hide resolved
swaterkamp and others added 2 commits March 26, 2021 13:40
sarahd93
sarahd93 previously approved these changes Apr 6, 2021
@swaterkamp swaterkamp requested a review from bjoernricks April 7, 2021 07:30
@swaterkamp swaterkamp enabled auto-merge April 7, 2021 13:26
@sarahd93 sarahd93 dismissed bjoernricks’s stale review April 7, 2021 13:26

changes have been implemented

@swaterkamp swaterkamp merged commit 0e424d2 into greenbone:gsa-21.04 Apr 7, 2021
@swaterkamp swaterkamp deleted the resultEmptySections branch April 7, 2021 13:40
@swaterkamp swaterkamp added the port-to-main Use mergifiy to port PR to master label Apr 7, 2021
swaterkamp added a commit that referenced this pull request Apr 7, 2021
Don't show empty sections in result details (bp #2791)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port-to-main Use mergifiy to port PR to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants