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.

Sign up for GitHub

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

Jump to bottom

feat(auth): Add returnOobLink to the ActionCodeSettings #502

Open
wants to merge 33 commits into
base: dev
Choose a base branch
from
Open

feat(auth): Add returnOobLink to the ActionCodeSettings #502

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
54b8114
Merge dev into master
google-oss-bot May 21, 2020
cef91ac
Merge dev into master
google-oss-bot Jun 16, 2020
77177c7
Merge dev into master
google-oss-bot Oct 22, 2020
a957589
Merge dev into master
google-oss-bot Jan 28, 2021
eb0d2a0
Merge dev into master
google-oss-bot Mar 24, 2021
05378ef
Merge dev into master
google-oss-bot Mar 29, 2021
4121c50
Merge dev into master
google-oss-bot Apr 14, 2021
928b104
Merge dev into master
google-oss-bot Jun 2, 2021
02cde4f
Merge dev into master
google-oss-bot Nov 4, 2021
6b40682
Merge dev into master
google-oss-bot Dec 15, 2021
e60757f
Merge dev into master
google-oss-bot Jan 20, 2022
bb055ed
Merge dev into master
google-oss-bot Apr 6, 2022
e8dfc32
Add returnOobLink to the ActionCodeSettings
Dal-Papa Jun 27, 2022
23a1f17
Merge dev into master
google-oss-bot Oct 6, 2022
1d24577
Merge dev into master
google-oss-bot Nov 10, 2022
f9dc53b
Merge remote-tracking branch 'upstream/dev'
Dal-Papa Nov 14, 2022
12a4788
Fix all tests with both returnOobLink cases
Dal-Papa Nov 15, 2022
4cf413a
Change `ReturnOobLink` into `SendEmailLink`
Dal-Papa Nov 23, 2022
54af579
Avoid name conflict with url package
Dal-Papa Nov 23, 2022
981442d
Add an unset test case and a comment
Dal-Papa Mar 8, 2023
61c6c04
Merge dev into master
google-oss-bot Apr 6, 2023
32af2b8
[chore] Release 4.12.0 (#561)
lahirumaramba Jun 22, 2023
02300a8
Revert "[chore] Release 4.12.0 (#561)" (#565)
lahirumaramba Jul 11, 2023
74c9bd5
Merge dev into master
google-oss-bot Jul 12, 2023
37c7936
Merge dev into master
google-oss-bot Sep 25, 2023
bef15cf
Merge remote-tracking branch 'fruitz-fork/master'
Dal-Papa Oct 13, 2023
b28ccfb
Merge remote-tracking branch 'upstream/dev'
Dal-Papa Oct 13, 2023
b04387e
Merge dev into master
google-oss-bot Nov 23, 2023
c348341
Merge remote-tracking branch 'upstream/master'
Dal-Papa Mar 4, 2024
da18d13
Fix comment
Dal-Papa Mar 4, 2024
77f51b8
Merge branch 'dev'
Dal-Papa Mar 6, 2024
147a277
Merge branch 'dev'
Dal-Papa Mar 29, 2024
e5a5134
Resolve conflicts on tests
Dal-Papa Mar 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions auth/email_action_links.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type ActionCodeSettings struct {
AndroidMinimumVersion string `json:"androidMinimumVersion,omitempty"`
AndroidInstallApp bool `json:"androidInstallApp,omitempty"`
DynamicLinkDomain string `json:"dynamicLinkDomain,omitempty"`
ReturnOobLink bool `json:"returnOobLink"`
Copy link
Contributor

@pragatimodi pragatimodi Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

API Review feedback was to rename this variable to SendEmailLink in order to maintain the default value false for boolean

Copy link
Contributor

Choose a reason for hiding this comment

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

@Dal-Papa Would you be able to make this change, please?

Copy link
Author

Choose a reason for hiding this comment

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

As you want but that will break the override that was happening on the key name due to the toMap function...

}

func (settings *ActionCodeSettings) toMap() (map[string]interface{}, error) {
Expand Down
31 changes: 31 additions & 0 deletions auth/email_action_links_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ var testActionCodeSettings = &ActionCodeSettings{
AndroidPackageName: "com.example.android",
AndroidInstallApp: true,
AndroidMinimumVersion: "6",
ReturnOobLink: true,
Copy link

Choose a reason for hiding this comment

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

Can we remove this in the base testActionCodeSettings struct? We need a test case to verify that if this field is not specified, returnOobLink is still set to true, due to sdk implementation.

I suggest adding a new test case in the newly added test, see my other comment. Thanks!

}
var testActionCodeSettingsMap = map[string]interface{}{
"continueUrl": "https://example.dynamic.link",
Expand All @@ -49,6 +50,7 @@ var testActionCodeSettingsMap = map[string]interface{}{
"androidPackageName": "com.example.android",
"androidInstallApp": true,
"androidMinimumVersion": "6",
"returnOobLink": true,
}
var invalidActionCodeSettings = []struct {
name string
Expand Down Expand Up @@ -309,6 +311,35 @@ func TestEmailVerificationLinkError(t *testing.T) {
}
}

func TestEmailVerificationSendEmail(t *testing.T) {
s := echoServer(testActionLinkResponse, t)
defer s.Close()

Copy link

Choose a reason for hiding this comment

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

Can we call this "TestEmailVerificationCustomOobLinkSettings" or similar?

Then, we can add 2 cases:

cases := []bool{true, false}

testActionCodeSettingsCustom := map[string]interface{}{}
// make a copy to avoid modifying the base maps.
for k, v := range testActionCodeSettings {
   testActionCodeSettingsCustom[k] = v 
}
testActionCodeSettingsMapCustom :=  map[string]interface{}{}
for k, v := range testActionCodeSettingsMapCustom {
   testActionCodeSettingsMapCustom[k] = v 
}

for _, returnOobLink := range cases {
  testActionCodeSettingsCustom.ReturnOobLink = returnOobLink
  testActionCodeSettingsCustomMap["returnOobLink"] = returnOobLink
 link, err := s.Client.EmailVerificationLink(..., testActionCodeSettingsCustom)
...
...
}

It would be great if you can add a similar test for EmailSignInLink and PasswordReset too. Thanks!

testActionCodeSettingsNoReturn := testActionCodeSettings
testActionCodeSettingsNoReturn.ReturnOobLink = false
testActionCodeSettingsMapNoReturn := testActionCodeSettingsMap
testActionCodeSettingsMapNoReturn["returnOobLink"] = false
link, err := s.Client.EmailSignInLink(context.Background(), testEmail, testActionCodeSettingsNoReturn)
if err != nil {
t.Fatal(err)
}
if link != testActionLink {
t.Errorf("EmailSignInLink() = %q; want = %q", link, testActionLink)
}

want := map[string]interface{}{
"requestType": "EMAIL_SIGNIN",
"email": testEmail,
"returnOobLink": false,
}
for k, v := range testActionCodeSettingsMapNoReturn {
want[k] = v
}
if err := checkActionLinkRequest(want, s); err != nil {
t.Fatalf("EmailSignInLink() %v", err)
}
}

func checkActionLinkRequest(want map[string]interface{}, s *mockAuthServer) error {
wantURL := "/projects/mock-project-id/accounts:sendOobCode"
return checkActionLinkRequestWithURL(want, wantURL, s)
Expand Down