python-taiga icon indicating copy to clipboard operation
python-taiga copied to clipboard

Fix for issue #114

Open uc-cjdavis opened this issue 4 years ago • 10 comments

Description

Describe:

Updates tests/resources/auth_user_success.json file to add support for the refresh token Adds support in client.py for the returned refresh token.

References

Fixes bug #114

Checklist

  • [x] I have read the contribution guide

  • [x] Code lint checked via inv lint

  • [x] changes file included (see docs)

  • [ ] Usage documentation added in case of new features

  • [x] Tests added*

  • tests tox -e py3.9 and inv lint run locally

uc-cjdavis avatar Oct 12 '21 21:10 uc-cjdavis

cc @astagi and @yakky

uc-cjdavis avatar Oct 12 '21 22:10 uc-cjdavis

Also cc @erikw

uc-cjdavis avatar Oct 20 '21 16:10 uc-cjdavis

Coverage Status

Coverage increased (+0.002%) to 97.826% when pulling d015a5ca4bbeacf4ee411311e7e4108914e684db on uc-cjdavis:master into 71888ddec9d2ed5271e0c5ec524ebbcde898ed6d on nephila:master.

coveralls avatar Oct 23 '21 08:10 coveralls

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (284f825) 96.38% compared to head (b2bd5cb) 96.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #115   +/-   ##
=======================================
  Coverage   96.38%   96.38%           
=======================================
  Files           8        8           
  Lines         884      885    +1     
  Branches       66       66           
=======================================
+ Hits          852      853    +1     
  Misses         19       19           
  Partials       13       13           
Flag Coverage Δ
unittests 96.38% <100.00%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
taiga/client.py 90.47% <100.00%> (+0.09%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Oct 23 '21 08:10 codecov[bot]

@uc-cjdavis thanks for this submission, I see it only includes the json change, not the client.py change to actually support the refresh_token Are you going to add it in this PR? I will take care of fixing the unrelated linting issues

yakky avatar Oct 23 '21 08:10 yakky

@uc-cjdavis thanks for this submission, I see it only includes the json change, not the client.py change to actually support the refresh_token Are you going to add it in this PR? I will take care of fixing the unrelated linting issues

@yakky I'd planned to add refresh_token support in a separate PR since it relies on a successful test with this change in place; it's ready to go once this is approved/merged.

uc-cjdavis avatar Oct 25 '21 17:10 uc-cjdavis

@uc-cjdavis I think it's easier if you wrap everything in a single pull request: the test will be fixed anyway and it will make review easier

yakky avatar Oct 25 '21 21:10 yakky

@yakky Okay...I'll comment here once the PR is updated.

uc-cjdavis avatar Oct 25 '21 21:10 uc-cjdavis

@yakky PR updated.
How should I handle the changes/114.bugfix file, since I'm technically adding a feature? Or could this be considered a bugfix?

uc-cjdavis avatar Oct 26 '21 13:10 uc-cjdavis

@yakky Two questions:

  1. Is there anything else needed for this PR before it can be merged?
  2. In working with token_refresh in my branch, I realized that in order to use it properly the raw_request class attribute has to be somehow updated with the new auth and refresh tokens without calling client.auth() again. I can cobble together a function but it may take some time...should this PR be updated or a new one created?

uc-cjdavis avatar Dec 08 '21 16:12 uc-cjdavis

@uc-cjdavis sorry, my bad. I have been away from the package for a while and this went unmerged.

Thanks a lot for your work

yakky avatar Apr 23 '23 20:04 yakky