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

Job status tab bugs #4877

Closed
wants to merge 3 commits into from
Closed

Conversation

Adamits
Copy link
Contributor

@Adamits Adamits commented Jul 27, 2019

Description

This fixes a minor bug in the settings/job page wherein a user needs to re-upload a new resume every time they want to save an update.

Refers/Fixes

Fix Bug 2 of #4867

Testing

Tested locally in browser

Adamits added 2 commits July 27, 2019 18:31
and only return an error if there is no resume file AND
there is not already a resume for the profile in

Fixes: Bug 2 of gitcoinco#4867
@codecov
Copy link

codecov bot commented Jul 27, 2019

Codecov Report

Merging #4877 into master will decrease coverage by <.01%.
The diff coverage is 15.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4877      +/-   ##
==========================================
- Coverage   30.64%   30.63%   -0.01%     
==========================================
  Files         216      216              
  Lines       17411    17419       +8     
  Branches     2364     2366       +2     
==========================================
+ Hits         5336     5337       +1     
- Misses      11860    11867       +7     
  Partials      215      215
Impacted Files Coverage Δ
app/dashboard/views.py 14.29% <15.38%> (-0.02%) ⬇️

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 0d96b67...336c9ae. Read the comment docs.

@codecov
Copy link

codecov bot commented Jul 27, 2019

Codecov Report

Merging #4877 into master will increase coverage by 0.05%.
The diff coverage is 14.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4877      +/-   ##
==========================================
+ Coverage   30.64%   30.69%   +0.05%     
==========================================
  Files         216      216              
  Lines       17411    17438      +27     
  Branches     2364     2367       +3     
==========================================
+ Hits         5336     5353      +17     
- Misses      11860    11870      +10     
  Partials      215      215
Impacted Files Coverage Δ
app/dashboard/views.py 14.28% <14.28%> (-0.03%) ⬇️
app/grants/models.py 59.51% <0%> (+1.68%) ⬆️

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 0d96b67...5e74c67. Read the comment docs.

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

Codewise LGTM !
Could you add a working demo of the PR ? ^_^

Also kudos for the neat commit message

@Adamits
Copy link
Contributor Author

Adamits commented Jul 29, 2019

@thelostone-mc Thanks! This is my first PR, what would a working demo look like?

@octavioamu
Copy link
Contributor

Great @Adamits thanks for this. What @thelostone-mc is asking is for a video showing how is working with this changes, you can use this tool or any other.
Thanks!

@Adamits
Copy link
Contributor Author

Adamits commented Jul 29, 2019

Great - will do when i'm home from work tonight.

@octavioamu
Copy link
Contributor

Great - will do when i'm home from work tonight.

Thanks @Adamits

@Adamits
Copy link
Contributor Author

Adamits commented Jul 30, 2019

Hi, Upon fixing my mistake, I noticed that there is a comment showing the intended way for this to function originally was to simply catch the status code 400 or the no file upload error, and the bug is simply that they catch for a string, rather than int in the condition. Closing this PR and ill submit a new one with that simple fix...

The original issue has also been updated with steps to reproduce BUG 1, so I will address that as well in the new PR.

@Adamits Adamits closed this Jul 30, 2019
@Adamits Adamits deleted the job-status-tab-bugs branch July 30, 2019 02:05
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