Skip to content

feat(auth): Support sms region config change on Tenant and Project level. - #1673

Merged
Xiaoshouzi-gh merged 10 commits into
masterfrom
sms-region
Apr 28, 2022
Merged

feat(auth): Support sms region config change on Tenant and Project level.#1673
Xiaoshouzi-gh merged 10 commits into
masterfrom
sms-region

Conversation

@Xiaoshouzi-gh

@Xiaoshouzi-ghXiaoshouzi-gh commented Apr 26, 2022

Copy link
Copy Markdown
Contributor

Support sms region config change on Tenant and Project level.
Added unit and integration tests.
Note the following changes were reviewed in recaptcha branch:

  • src/auth/auth-api-request.ts
  • src/auth/auth.ts
  • src/auth/project-config-manager.ts
  • Most of test/unit/auth/project-config-manager.spec.ts

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in an issue so that we
    can discuss it together.

renkelvin
renkelvin approved these changes Apr 27, 2022
View reviewed changes
Comment threadsrc/auth/project-config.ts Outdated
@lahirumarambalahirumaramba changed the title Support sms region config change on Tenant and Project level.feat(auth): Support sms region config change on Tenant and Project level.Apr 27, 2022
lahirumaramba
lahirumaramba approved these changes Apr 27, 2022
View reviewed changes

@lahirumarambalahirumaramba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks!
Left a few cleanup comments.

Comment threadsrc/auth/auth-config.ts Outdated
Comment threadsrc/auth/auth-config.ts
Comment threadsrc/auth/auth.ts Outdated
Comment threadsrc/auth/project-config-manager.ts Outdated
prameshj
prameshj reviewed Apr 27, 2022
View reviewed changes

@prameshjprameshj left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mostly nits, otherwise looks good to me.

Comment threadsrc/auth/auth-config.ts
Comment threadsrc/auth/auth-config.ts
Comment threadsrc/auth/auth-config.ts Outdated
Comment threadsrc/auth/auth-config.ts Outdated
Comment threadsrc/auth/auth-config.ts
Comment threadsrc/auth/auth-config.ts Outdated
Comment threadsrc/auth/project-config.ts Outdated
Comment threadsrc/auth/project-config.ts
kevinthecheung
kevinthecheung approved these changes Apr 28, 2022
View reviewed changes
Comment threadsrc/auth/tenant.ts
Comment threadsrc/auth/project-config.ts Outdated
Comment threadsrc/auth/project-config.ts Outdated
Comment threadsrc/auth/project-config.ts Outdated
Comment threadsrc/auth/project-config-manager.ts Outdated
Comment threadsrc/auth/auth-config.ts Outdated
Comment threadsrc/auth/auth-config.ts Outdated
@Xiaoshouzi-gh
Xiaoshouzi-gh merged commit 35df364 into masterApr 28, 2022
@Xiaoshouzi-gh
Xiaoshouzi-gh deleted the sms-region branch April 28, 2022 20:36
@Xiaoshouzi-gh
Xiaoshouzi-gh restored the sms-region branch May 2, 2022 17:38
Xiaoshouzi-gh added a commit that referenced this pull request May 2, 2022
@Xiaoshouzi-gh
Revert "feat(auth): Support sms region config change on Tenant and Pr…
0080a83
…oject level. (#1673)"
This reverts commit 35df364.
@Xiaoshouzi-ghXiaoshouzi-gh mentioned this pull request May 2, 2022
Merged
Xiaoshouzi-gh added a commit that referenced this pull request May 2, 2022
@Xiaoshouzi-gh
Revert "feat(auth): Support sms region config change on Tenant and Pr…
abdfb00
…oject level." (#1676)
* Revert "feat(auth): Support sms region config change on Tenant and Project level. (#1673)"
This reverts commit 35df364.
* Trigger CI
@renkelvinrenkelvin mentioned this pull request Oct 6, 2022
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lahirumarambalahirumarambalahirumaramba approved these changes
+3 more reviewers
@prameshjprameshjprameshj left review comments
@renkelvinrenkelvinrenkelvin approved these changes
@kevinthecheungkevinthecheungkevinthecheung approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Xiaoshouzi-gh@lahirumaramba@renkelvin@kevinthecheung@prameshj