search

blackjackkent / RPThreadTrackerV3.FrontEnd

GNU General Public License v3.0
4 stars 5 forks source link

Issue #144 | 1 test failed, but works as expected. #147

Closed Niaz-Ul-Haque closed 3 years ago

Niaz-Ul-Haque commented 3 years ago

Title

Issue #144 | 1 test failed but works as expected. Ran tests, 1 failed cause it was related to the old routing, not the latest one.

Description

I changed the path of routings to be redirected to the new issue page instead of emails, as asked.

Motivation and Context

It was asked as an issue and I was interested in working on this as apart of my coursework and self-practice. I thank the person for assigning me this task.

How Has This Been Tested?

The code has been tested. It came out with one testing error (ass expected). It is due to the fact that I changed the routing to a different site, the testing was for the old path (the email).

Screenshots (if appropriate):

Types of changes

Checklist:

Niaz-Ul-Haque commented 3 years ago

Reviewing why the eslint error occured.

Niaz-Ul-Haque commented 3 years ago

I am very sorry as I am not getting where is the error coming from. Please do let me know, and I am pretty positive that I can fix what is wrong. Please do let me know where it is, will be a very good practice for me. Thank you! I am getting this as a result. 

Test Suites: 286 passed, 286 total
Tests:       1299 passed, 1299 total
Snapshots:   1 obsolete, 171 passed, 171 total
Time:        117.711s
Ran all test suites.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! rpthreadtrackerv3.frontend@1.0.0 test:ci: `npm run lint && jest --config="./config/tests/jest.config.json" && codecov`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the rpthreadtrackerv3.frontend@1.0.0 test:ci script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/appveyor/.npm/_logs/2020-12-05T16_08_04_029Z-debug.log
Command exited with code 1
Running "on_failure" scripts
Invoke-RestMethod https://raw.githubusercontent.com/blackjackkent/appveyor-discord-webhook/master/send.ps1 -o send.ps1
./send.ps1 failure $env:WEBHOOK_URL
[Webhook]: Sending webhook to Discord...
[Webhook]: Successfully sent the webhook.
Build failed
blackjackkent commented 3 years ago

Hey! Thanks so much for working on this; looks like a good first pass! 🚀

I changed the path of routings to be redirected to the new issue page instead of emails, as asked.

This actually isn't quite what I had in mind for the update - which is not your fault; totally on me for not including more info in the issue, so I'll try to do better with that in the future. Here's a mockup of more what I had in mind:

image

So removing the contact form entirely in favor of a link that would take the user (in a new tab) to https://github.com/blackjackkent/RPThreadTrackerV3.FrontEnd/issues/new

I am very sorry as I am not getting where is the error coming from.

The testing issue is with a Jest snapshot that needs to be updated. Some of the tests in the Tracker codebase make use of Jest snapshot testing to help ensure that no components change their appearance in a way that we don't expect. You can learn more about it here:

https://jestjs.io/docs/en/snapshot-testing

I'm happy to help further explain if this is confusing but check out those docs first and see if it helps. :)

Niaz-Ul-Haque commented 3 years ago

Ahhhhh, It makes sense now haha. Sorry couldn't get what you meant in the beginning, I get it now! Thank you!. I'll dive back into it rn and try to do it the way you wanted it. Thank you for clearing up!

Niaz-Ul-Haque commented 3 years ago

Please do check now as I did it similar to the Mockup. Clicking on the button redirects you to the Github new issue page. But, sorry couldn't fix the snapshot thingy. Sorry about that. Other than that, did what was asked. Please do a review. And thank you for the opportunity!.

blackjackkent commented 3 years ago

Thanks again so much for contributing! :D I've left some comments about things that can be adjusted - mainly that this doens't need to be a form submission anymore and can just be a link.

If you want to dive deeper, you can also remove other references to the submitContactForm Redux action, since we won't be using it anymore, or I can address that in another PR.

Niaz-Ul-Haque commented 3 years ago

Fixed whats was asked after reviews. Please do take a look and let me know if there's more to change. Sorry for not understanding clearly earlier, and if there is more to change please do let me know. I'll try my best to resolve it. Thank you very much for pointing out the errors I made, will keep it in mind when doing something similar. Learned lots of things from this project, this PR in particular. Thank you for this opportunity!

Niaz-Ul-Haque commented 3 years ago

Feels good to see "All checks have passed". Ahh, Thank you again!

blackjackkent commented 3 years ago

Also, that debug.log file, can be deleted completely. :)

Niaz-Ul-Haque commented 3 years ago

Ahh thanks for pointing them out, will do that instantly!

Niaz-Ul-Haque commented 3 years ago

Done. Fixed after review. Please do let me know if there's anything left. Thank you!

blackjackkent commented 3 years ago

Looks like you need to fix the snapshot again since you updated the text in the component. :) Otherwise, this looks good!

Niaz-Ul-Haque commented 3 years ago

Looks like you need to fix the snapshot again since you updated the text in the component. :) Otherwise, this looks good!

Will do that rn haha mbmb

codecov-io commented 3 years ago

Codecov Report

Merging #147 (add16ca) into development (e5c3584) will increase coverage by 99.15%. The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff                @@
##           development     #147       +/-   ##
================================================
+ Coverage             0   99.15%   +99.15%     
================================================
  Files                0      287      +287     
  Lines                0     3205     +3205     
  Branches             0      588      +588     
================================================
+ Hits                 0     3178     +3178     
- Misses               0       27       +27
Impacted Files Coverage Δ
...c/display/views/help/components/ContactFormPane.js 100.00% <100.00%> (ø)
src/infrastructure/actions/ui/toggles.js 100.00% <0.00%> (ø)
...nfrastructure/sagas/public/fetchPublicViewsSaga.js 100.00% <0.00%> (ø)
...iews/threads/components/ThreadTableSubComponent.js 100.00% <0.00%> (ø)
src/display/views/public/PublicHeader.js 100.00% <0.00%> (ø)
...y/forms/upsert-public-view/UpsertPublicViewForm.js 100.00% <0.00%> (ø)
...astructure/actions/threads/generateRandomThread.js 100.00% <0.00%> (ø)
src/infrastructure/sagas/tags/bulkDeleteTagSaga.js 100.00% <0.00%> (ø)
src/infrastructure/sagas/ui/setSiteThemeSaga.js 100.00% <0.00%> (ø)
src/display/shared/breadcrumb/_routes.js 100.00% <0.00%> (ø)
... and 278 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 e5c3584...dae8f14. Read the comment docs.