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

Move scheme to webAuthentication() #150

Merged
merged 1 commit into from
Aug 23, 2022
Merged

Move scheme to webAuthentication() #150

merged 1 commit into from
Aug 23, 2022

Conversation

Widcket
Copy link
Contributor

@Widcket Widcket commented Aug 23, 2022

⚠️ This PR contains breaking changes

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

📋 Changes

Currently, Web Auth methods (login and logout) take a scheme parameter separately. Unlike the rest of the parameters, this one is not specific to each Web Auth method, but is something that applies to Web Auth in general (in Android). One would never specify a different value for login() than for logout().

Thus this parameter belongs to the Web Auth instance itself. webAuthentication() already takes a useCredentialsManager parameter for the same reasons. So having scheme in login() and logout() would be inconsistent, and become a potential source of confusion.

This PR moves scheme out of login() and logout(), up into webAuthentication().

Before/after (usage)

Screen Shot 2022-08-23 at 00 36 29

@Widcket Widcket added the review:tiny Tiny review label Aug 23, 2022
@Widcket Widcket requested a review from a team as a code owner August 23, 2022 03:58
@codecov
Copy link

codecov bot commented Aug 23, 2022

Codecov Report

Merging #150 (351a9a3) into ga (76af7b6) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##                 ga     #150   +/-   ##
=========================================
  Coverage     98.69%   98.69%           
  Complexity       75       75           
=========================================
  Files            75       75           
  Lines          1379     1380    +1     
  Branches        296      296           
=========================================
+ Hits           1361     1362    +1     
  Misses            6        6           
  Partials         12       12           
Flag Coverage Δ
auth0_flutter 100.00% <100.00%> (ø)
auth0_flutter_android 96.52% <ø> (ø)
auth0_flutter_ios 100.00% <ø> (ø)
auth0_flutter_platform_interface 99.26% <ø> (ø)

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

Impacted Files Coverage Δ
auth0_flutter/lib/auth0_flutter.dart 100.00% <100.00%> (ø)
auth0_flutter/lib/src/web_authentication.dart 100.00% <100.00%> (ø)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review:tiny Tiny review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants