Fix for issue #114
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]
changesfile included (see docs) -
[ ] Usage documentation added in case of new features
-
[x] Tests added*
-
tests
tox -e py3.9andinv lintrun locally
cc @astagi and @yakky
Also cc @erikw
Coverage increased (+0.002%) to 97.826% when pulling d015a5ca4bbeacf4ee411311e7e4108914e684db on uc-cjdavis:master into 71888ddec9d2ed5271e0c5ec524ebbcde898ed6d on nephila:master.
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.
@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
@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 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 Okay...I'll comment here once the PR is updated.
@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?
@yakky Two questions:
- Is there anything else needed for this PR before it can be merged?
- In working with
token_refreshin my branch, I realized that in order to use it properly theraw_requestclass attribute has to be somehow updated with the new auth and refresh tokens without callingclient.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 sorry, my bad. I have been away from the package for a while and this went unmerged.
Thanks a lot for your work