Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Add updatePassword functionality to firebase_auth plugin. #678

Merged
merged 1 commit into from
Sep 27, 2018
Merged

Add updatePassword functionality to firebase_auth plugin. #678

merged 1 commit into from
Sep 27, 2018

Conversation

slightfoot
Copy link
Member

Required functionality in recent project. Also moved updateProfile in dart code from FirebaseAuth to FirebaseUser I believe this is the correctly place. So release changelog should be updated to reflect this breaking change.

@kroikie kroikie self-assigned this Jul 20, 2018
@kroikie
Copy link
Contributor

kroikie commented Jul 20, 2018

@slightfoot Thanks for the PR. This change looks good. Would you mind rebasing on master, I'd be happy to merge then.

@slightfoot
Copy link
Member Author

@kroikie rebased on master.

ffeu added a commit to ffeu/plugins that referenced this pull request Jul 23, 2018
@ffeu
Removing updatePassword as it is already included in flutter#678
b3a2518
@ffeu
Copy link
Contributor

ffeu commented Jul 23, 2018

Just linking to issues.

FirebaseUser.updatePassword() - flutter/flutter#16775 flutter/flutter#19525

@kroikie
Copy link
Contributor

kroikie commented Jul 25, 2018

@slightfoot There is already a PR that updates email and it should land soon. Would you mind making this PR focus on just the password update?

@slightfoot
Copy link
Member Author

@kroikie I think you need to look at how these differ. Mine has the updateEmail/updatePassword/updateProfile in the FirebaseUser. Along with the Android code using the supplied TaskVoidCompleteListener. If I was to remove the updateEmail it would spread the functionality across classes.

@slightfoot
Copy link
Member Author

@kroikie rebased and fixed the merge conflicts.

@kroikie kroikie changed the title Add updateEmail and updatePassword functionality to firebase_auth plugin. Add updatePassword functionality to firebase_auth plugin. Jul 26, 2018
@kroikie kroikie changed the title Add updatePassword functionality to firebase_auth plugin. Add updateEmail and updatePassword functionality to firebase_auth plugin. Jul 26, 2018
@kroikie
Copy link
Contributor

kroikie commented Jul 26, 2018

@slightfoot updateEmail has already been added, if you could remove the updateEmail parts of the PR I'd be happy to merge, thanks.

@slightfoot
Copy link
Member Author

slightfoot commented Jul 26, 2018 •
edited
Loading

@kroikie I'm happy to make changes. I need direction, because as I said in the original PR description and later on, that I moved updateProfile/updateEmail/updatePassword from FirebaseAuth to FirebaseUser. So if I remove updateEmail then updateProfile and updatePassword will be in FirebaseUser and updateEmail will be in FirebaseAuth. This is not what we want.. so where do you want these. I feel where I have them at the moment in FirebaseUser is the correct location.
Let me know what you want.

@ffeu
Copy link
Contributor

ffeu commented Jul 26, 2018 •
edited
Loading

If you guys allow me a suggestion, I would keep updateEmail() inside FirebaseUser (as it is already on the platforms and as it is already coded in this PR), and would publish this PR with a new major or minor version for the plugin.

The good thing of keeping the method in the "same place" as the Android and iOS SDKs is that currently there's no documentation on firebase for flutter. So I see this similarity as an advantage.

Actually, thinking in this way right now this PR contains two fixes:

  • corrects misplacement of updateEmail from FirebaseAuth to FirebaseUser
  • corrects misplacement of updateProfile from FirebaseAuth to FirebaseUser

Think about it:

Android:

FirebaseAuth.getInstance().getCurrentUser().updateEmail("user@example.com")

iOS:

Auth.auth().currentUser?.updateEmail(to: email)

Flutter:

_auth.currentUser().updateEmail() // it could be this
_auth.updateEmail() // but it's currently this

@slightfoot
Copy link
Member Author

@ffeu great comments, I agree.
@kroikie let me know if you want me to update anything regarding the above.

@ffeu ffeu mentioned this pull request Aug 15, 2018
Closed
@alardizabal
Copy link

Hi, any updates? Updating a user's password would be great.

@slightfoot
Copy link
Member Author

Actually sorting this out now along with fixing flutter/flutter#18312

@slightfoot slightfoot force-pushed the firebase_auth_update branch 2 times, most recently from 2a7dd25 to 92156b3 Compare September 7, 2018 00:06
@slightfoot slightfoot changed the title Add updateEmail and updatePassword functionality to firebase_auth plugin. Add updatePassword functionality to firebase_auth plugin. Sep 7, 2018
@slightfoot
Copy link
Member Author

@kroikie I've rebased to the latest master. Hold off on any merge. I have another PR coming up which you might want to review, and bring into the same release. I'll bring the other branch on top of this one so as to resolve any conflicts upfront.

@joshkendrick joshkendrick mentioned this pull request Sep 7, 2018
Closed
@slightfoot slightfoot force-pushed the firebase_auth_update branch 2 times, most recently from ce4e3b4 to cdc9efc Compare September 9, 2018 07:41
@slightfoot slightfoot mentioned this pull request Sep 9, 2018
Merged
@slightfoot
Copy link
Member Author

@kroikie I've added @Deprecated notices to the old updateEmail and updateProfile in FirebaseAuth to address moving them to FirebaseUser. This is now ready to be merged. Also see #775

kroikie
kroikie reviewed Sep 10, 2018
View reviewed changes
packages/firebase_auth/lib/firebase_auth.dart Outdated Show resolved Hide resolved
packages/firebase_auth/CHANGELOG.md Outdated Show resolved Hide resolved
@slightfoot
Added updatePassword functionality to firebase_auth plugin.
2d18942 
Move `updateEmail` and `updateProfile` from FirebaseAuth to FirebaseUser. Minor cleani-up in Java.

Updated tests.

Fix dartfmt issue.

Fix merge conflicts.

Deprecated old methods.

Updated Changelog.

Review changes.
@slightfoot
Copy link
Member Author

@kroikie review changes made.

@slightfoot
Copy link
Member Author

nudge @kroikie 😉

@zanecidan
Copy link

zanecidan commented Sep 25, 2018 •
edited
Loading

Hi, any update on this?

@kroikie
Copy link
Contributor

kroikie commented Sep 27, 2018

@slightfoot Thanks a lot for this, sorry for the delay.

@kroikie kroikie merged commit aa694d4 into flutter:master Sep 27, 2018
@slightfoot
Copy link
Member Author

@kroikie no problem. #775 can be merged in on top. They both have the same version.

andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
@slightfoot @andreidiaconu
Added updatePassword functionality to firebase_auth plugin. (flutte…
8e1a4e5 
…r#678)

Add updatePassword functionality to firebase_auth plugin.
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
@andreidiaconu
Revert "Added updatePassword functionality to firebase_auth plugin. (…
bb15df6 
…flutter#678)"

This reverts commit 8e1a4e5.
@topex-psy
Copy link

any update on this?

@slightfoot
Copy link
Member Author

@topex-psy this was merged a long time ago. You can find the functionality here.
https://pub.dev/documentation/firebase_auth/latest/firebase_auth/FirebaseUser/updatePassword.html

julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
@slightfoot @kroikie
Added updatePassword functionality to firebase_auth plugin. (flutte…
2adb20c 
…r#678)

Add updatePassword functionality to firebase_auth plugin.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kroikie kroikie kroikie left review comments

Assignees

@kroikie kroikie

Labels
cla: yes flutterfire
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@slightfoot @kroikie @ffeu @alardizabal @zanecidan @topex-psy @googlebot