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

fix(reportsService): issues/52 name report package download #54

Merged
merged 1 commit into from
May 31, 2019

Conversation

cdcabrera
Copy link
Member

@cdcabrera cdcabrera commented May 29, 2019

What's included

  • fix(reportsService): issues/52 name report package download
    • reportsService, move to axios, add responseType
    • helpers, remove downloadPackage in favor of downloadData

Notes

  • This corrects the naming issue around the report by adding in the tar.gz. The current package download generates the name based on the browser, which can be confusing... Chrome labels it download.gz and Firefox labels it [some hash].gz. To round it out the file doesn't open on RHEL without the tar in there, this fix corrects all of that.

How to test

Coverage and basic unit test check

  1. update the NPM packages with $ yarn
  2. $ yarn test

Run the Dev UI

The Dev UI can be used for a quick confirmation on just the file name coming through. Trying to unzip it will create an infinite loop of annoyance since the tar.gz isn't legit.

  1. update the NPM packages with $ yarn
  2. make sure Docker is running, then
  3. $ yarn start
  4. Next, navigate to the scans page, you should see what appears to be randomized data. Go ahead and hit the download button, if you don't see a download button on scans, refresh the page until you do. Then check your download directory to confirm the file name appears as report_id_[HUGE RANDOM ID]_[DATE]_[TIME].tar.gz

Run the Review UI

To fully test the package name is successfully you'll need to run

  1. update the NPM packages with $ yarn
  2. make sure Docker is running, then
  3. $ yarn start:review ... this should compile the image, then run the container with the latest changes
  4. Next you'll need to generate a successful report. That means setting up a source, running a scan. Then navigate to the scans view, download the package. Within your download directory you should see report_id_[ID]_[DATE]_[TIME].tar.gz. The file should expand accordingly and provide you with a directory containing
    • report_id_[ID]
      • deployments.csv
      • deployments.json
      • details.csv
      • details.json

Example

...

Updates issue/story

fixes #52

* reportsService, move to axios and add blob responseType
* helpers, remove downloadPackage in favor of downloadData
@codecov-io
Copy link

codecov-io commented May 31, 2019

Codecov Report

Merging #54 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #54      +/-   ##
==========================================
+ Coverage   69.12%   69.13%   +0.01%     
==========================================
  Files          91       91              
  Lines        2131     2132       +1     
  Branches      338      338              
==========================================
+ Hits         1473     1474       +1     
  Misses        567      567              
  Partials       91       91
Impacted Files Coverage Δ
src/services/reportsService.js 100% <100%> (ø) ⬆️

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 6bf20ec...7634a9b. Read the comment docs.

@bclarhk
Copy link

bclarhk commented May 31, 2019

It would be nice (future enhancement) to change the download names to match the name of the scan(s) and appended 'merged' to merged files too.

@cdcabrera cdcabrera requested a review from bclarhk May 31, 2019 18:53
Copy link

@bclarhk bclarhk left a comment

Choose a reason for hiding this comment

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

👍

@cdcabrera cdcabrera merged commit d79bdd5 into quipucords:master May 31, 2019
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.

RHEL report package downloads have to be renamed in order to open
4 participants