Skip to content

Conversation

Shiranuit
Copy link
Contributor

What does this PR do ?

This PR enables the automatic resubscription of the SDK when logged in using the JWT property

@Shiranuit Shiranuit self-assigned this Aug 9, 2021
@Shiranuit Shiranuit marked this pull request as draft August 9, 2021 07:40
@Shiranuit Shiranuit changed the base branch from master to 7-dev August 9, 2021 07:52
@Shiranuit Shiranuit marked this pull request as ready for review August 9, 2021 08:23
@codecov
Copy link

codecov bot commented Aug 9, 2021

Codecov Report

Merging #657 (19b2523) into 7-dev (1f04b94) will increase coverage by 0.03%.
The diff coverage is 80.00%.

❗ Current head 19b2523 differs from pull request most recent head df9c5b4. Consider uploading reports for the commit df9c5b4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            7-dev     #657      +/-   ##
==========================================
+ Coverage   85.65%   85.68%   +0.03%     
==========================================
  Files          36       36              
  Lines        1708     1712       +4     
  Branches      309      311       +2     
==========================================
+ Hits         1463     1467       +4     
  Misses        183      183              
  Partials       62       62              
Impacted Files Coverage Δ
src/Kuzzle.ts 85.75% <80.00%> (+0.17%) ⬆️

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 1f04b94...df9c5b4. Read the comment docs.

@Shiranuit Shiranuit requested a review from Aschen August 9, 2021 08:26
@@ -429,6 +429,8 @@ export class Kuzzle extends KuzzleEventEmitter {

set jwt (encodedJwt) {
this.auth.authenticationToken = encodedJwt;

this._loggedIn = encodedJwt ? true : false;
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be a getter checking the existence of this.auth.authenticationToken instead of a static member synced by another setter?

Copy link
Contributor Author

@Shiranuit Shiranuit Aug 10, 2021

Choose a reason for hiding this comment

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

I can't because in cookieAuth mode there is no JWT property set but that doesn't mean the SDK is not logged in, that's why I cannot always check if there is a JWT but I can set a property when people do want to put a JWT by hand.

@Shiranuit Shiranuit merged commit 40607d2 into 7-dev Aug 10, 2021
@Shiranuit Shiranuit deleted the reauth-using-jwt-property branch August 10, 2021 12:39
@Shiranuit Shiranuit mentioned this pull request Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants