fix(dashboard): correct mime type is returned. Fixes: #2290 #2303

nitram509 · 2022-10-08T22:50:44Z

comments about this PR

I did refactor the code for static file serving into its own file.
Overall, I also did simplify the code and fixed former issues (see below).
Golang's mime package comes to the rescue when determining the correct mime type.

add: tests for static files serving
fix: when request /index.html, the base path was not modified
fix: wrong 'err' variable used when log.Errorf("Failed to stat file or dir %s: %v"...) was printed

relates to #2290

PS: I would like to participate the hacktoberfest and would appreciated the 'hacktoberfest-accepted' label, IF you're OK with it :)
PPS: any feedback is welcome.

Checklist:

Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
I've signed my commits with DCO
I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
My builds are green. Try syncing with master if they are not.
My organization is added to USERS.md.

fix: when request /index.html, base path was not modified fix: wrong 'err' variable used when log.Errorf("Failed to stat file or dir %s: %v"...) was printed change: when a file is not found, 404 is return (before: the index.html was returned) add: tests for static files serving Signed-off-by: nitram509 <[email protected]>

See https://sonarcloud.io/project/issues?resolved=false&types=BUG&pullRequest=2303&id=argoproj_argo-rollouts&open=AYO5y8lxtb83AIZrmShZ Signed-off-by: nitram509 <[email protected]>

github-actions · 2022-10-08T23:04:27Z

Go Published Test Results

1 791 tests 1 791 ✔️ 2m 30s ⏱️
  101 suites       0 💤
      1 files       0 ❌

Results for commit dca346b.

♻️ This comment has been updated with latest results.

codecov · 2022-10-08T23:05:44Z

Codecov Report

Base: 82.38% // Head: 81.54% // Decreases project coverage by -0.83% ⚠️

Coverage data is based on head (dca346b) compared to base (1394f4e).
Patch coverage: 76.27% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2303      +/-   ##
==========================================
- Coverage   82.38%   81.54%   -0.84%     
==========================================
  Files         121      123       +2     
  Lines       18476    18855     +379     
==========================================
+ Hits        15221    15375     +154     
- Misses       2468     2694     +226     
+ Partials      787      786       -1

Impacted Files	Coverage Δ
server/server.go	`0.00% <0.00%> (ø)`
server/server_static.go	`80.35% <80.35%> (ø)`
controller/metrics/metrics.go	`100.00% <0.00%> (ø)`
pkg/kubectl-argo-rollouts/cmd/lint/lint.go	`85.84% <0.00%> (+0.20%)`	⬆️
.../apis/rollouts/validation/validation_references.go	`90.65% <0.00%> (+0.21%)`	⬆️
controller/controller.go	`91.41% <0.00%> (+1.96%)`	⬆️
rollout/trafficrouting/istio/controller.go	`53.36% <0.00%> (+2.55%)`	⬆️
rollout/controller.go	`80.77% <0.00%> (+2.77%)`	⬆️
experiments/controller.go	`71.42% <0.00%> (+5.67%)`	⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

nitram509 · 2022-10-08T23:11:20Z

❓ I wonder 🤔 why the test-coverage went down?
The /server package did not contain any tests before, and I literally just created the first _test.go file in this package.
So I don't get, why the coverage is lower now since I wrote 4 additional test methods.

PS: I just measured the test coverage of the new file created, this is 87% (according to IntelliJ).

github-actions · 2022-10-08T23:22:29Z

E2E Tests Published Test Results

89 tests 86 ✔️ 45m 51s ⏱️
  1 suites   3 💤
  1 files   0 ❌

Results for commit dca346b.

♻️ This comment has been updated with latest results.

nitram509 · 2022-10-09T21:09:13Z

~~⚠️ while further testing, I realized, the UI puts the namespace in the path ... that clashes with this implementation and I'm about to work on a fix~~

done ✅

… router Signed-off-by: nitram509 <[email protected]>

server/server_static.go

zachaller · 2022-10-14T15:51:51Z

Can you fix DCO I think your merge commit missed it, then this will be good to merge thanks for contributing!

nitram509 · 2022-10-14T22:15:50Z

Sure, will take care of it.

Signed-off-by: nitram509 <[email protected]>

sonarcloud · 2022-10-15T07:48:31Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

nitram509 · 2022-10-15T07:49:29Z

fixed the DCO

…rgoproj#2303) * fix: correct mimetype is returned fix: when request /index.html, base path was not modified fix: wrong 'err' variable used when log.Errorf("Failed to stat file or dir %s: %v"...) was printed change: when a file is not found, 404 is return (before: the index.html was returned) add: tests for static files serving Signed-off-by: nitram509 <[email protected]> * fix sonar cloud complains about invalid HTML See https://sonarcloud.io/project/issues?resolved=false&types=BUG&pullRequest=2303&id=argoproj_argo-rollouts&open=AYO5y8lxtb83AIZrmShZ Signed-off-by: nitram509 <[email protected]> * fix send index.html when page not found, because client side React UI router Signed-off-by: nitram509 <[email protected]> * make variable private (feedback from review) Signed-off-by: nitram509 <[email protected]> Signed-off-by: nitram509 <[email protected]>

* fix: correct mimetype is returned fix: when request /index.html, base path was not modified fix: wrong 'err' variable used when log.Errorf("Failed to stat file or dir %s: %v"...) was printed change: when a file is not found, 404 is return (before: the index.html was returned) add: tests for static files serving Signed-off-by: nitram509 <[email protected]> * fix sonar cloud complains about invalid HTML See https://sonarcloud.io/project/issues?resolved=false&types=BUG&pullRequest=2303&id=argoproj_argo-rollouts&open=AYO5y8lxtb83AIZrmShZ Signed-off-by: nitram509 <[email protected]> * fix send index.html when page not found, because client side React UI router Signed-off-by: nitram509 <[email protected]> * make variable private (feedback from review) Signed-off-by: nitram509 <[email protected]> Signed-off-by: nitram509 <[email protected]>

nitram509 added 2 commits October 9, 2022 00:45

fix sonar cloud complains about invalid HTML

e86a72a

See https://sonarcloud.io/project/issues?resolved=false&types=BUG&pullRequest=2303&id=argoproj_argo-rollouts&open=AYO5y8lxtb83AIZrmShZ Signed-off-by: nitram509 <[email protected]>

nitram509 mentioned this pull request Oct 8, 2022

Feedback about the new pull request comment codecov/feedback#8

Open

fix send index.html when page not found, because client side React UI…

ede77cd

… router Signed-off-by: nitram509 <[email protected]>

zachaller reviewed Oct 11, 2022

View reviewed changes

server/server_static.go Outdated Show resolved Hide resolved

zachaller added hacktoberfest-accepted Accepted Hacktoberfest contribution ready-for-review Ready for final review labels Oct 11, 2022

zachaller added this to the v1.4 milestone Oct 11, 2022

zachaller approved these changes Oct 13, 2022

View reviewed changes

make variable private (feedback from review)

dca346b

Signed-off-by: nitram509 <[email protected]>

nitram509 force-pushed the issue2290-UI-not-setting-correct-mime-type branch from c563cef to dca346b Compare October 15, 2022 07:47

zachaller approved these changes Oct 19, 2022

View reviewed changes

zachaller approved these changes Nov 7, 2022

View reviewed changes

zachaller merged commit 32dea1a into argoproj:master Nov 7, 2022

nitram509 deleted the issue2290-UI-not-setting-correct-mime-type branch November 9, 2022 18:48

zachaller mentioned this pull request Nov 25, 2022

Extension UI "More" tab not appearing after opening another resource's details view argoproj-labs/rollout-extension#31

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dashboard): correct mime type is returned. Fixes: #2290 #2303

fix(dashboard): correct mime type is returned. Fixes: #2290 #2303

nitram509 commented Oct 8, 2022 •

edited

Loading

github-actions bot commented Oct 8, 2022 •

edited

Loading

codecov bot commented Oct 8, 2022 •

edited

Loading

nitram509 commented Oct 8, 2022

github-actions bot commented Oct 8, 2022 •

edited

Loading

nitram509 commented Oct 9, 2022 •

edited

Loading

zachaller commented Oct 14, 2022 •

edited

Loading

nitram509 commented Oct 14, 2022

sonarcloud bot commented Oct 15, 2022

nitram509 commented Oct 15, 2022

fix(dashboard): correct mime type is returned. Fixes: #2290 #2303

fix(dashboard): correct mime type is returned. Fixes: #2290 #2303

Conversation

nitram509 commented Oct 8, 2022 • edited Loading

comments about this PR

Checklist:

github-actions bot commented Oct 8, 2022 • edited Loading

Go Published Test Results

codecov bot commented Oct 8, 2022 • edited Loading

Codecov Report

nitram509 commented Oct 8, 2022

github-actions bot commented Oct 8, 2022 • edited Loading

E2E Tests Published Test Results

nitram509 commented Oct 9, 2022 • edited Loading

zachaller commented Oct 14, 2022 • edited Loading

nitram509 commented Oct 14, 2022

sonarcloud bot commented Oct 15, 2022

nitram509 commented Oct 15, 2022

nitram509 commented Oct 8, 2022 •

edited

Loading

github-actions bot commented Oct 8, 2022 •

edited

Loading

codecov bot commented Oct 8, 2022 •

edited

Loading

github-actions bot commented Oct 8, 2022 •

edited

Loading

nitram509 commented Oct 9, 2022 •

edited

Loading

zachaller commented Oct 14, 2022 •

edited

Loading