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

fix: Add missing null to profile update #4179

Merged
merged 2 commits into from
Sep 4, 2020

Conversation

andreibarabas
Copy link
Contributor

You need to specify null when you want to remove the current user's photo

Description

Currently, if you want to remove a profile photoURL, you need to specify NULL, but typescript will complain about it.

Related issues

N/A

Release Summary

Fix: add missing type when removing profile photo

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • [x ] Yes
  • My change supports the following platforms;
    • [ x] Android
    • [ x] iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • [x ] I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • [x ] No

Test Plan

N/A


Think react-native-firebase is great? Please consider supporting the project with any of the below:

You need to specify null when you want to remove the current user's photo
@CLAassistant
Copy link

CLAassistant commented Aug 31, 2020

CLA assistant check
All committers have signed the CLA.

@vercel
Copy link

vercel bot commented Aug 31, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/izmg0z08l
✅ Preview: https://react-native-firebase-git-fork-andreibarabas-master.invertase.vercel.app

Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Hi there! Thanks for proposing this

It was very hard to find documentation related to it, the Java + Objective-C documents did not mention it but I found it in the Javascript SDK docs: https://firebase.google.com/docs/reference/js/firebase.User#updateprofile

So I see two issues:

  • could you also update the type for the user name? It shares this clearing style
  • could you update the documentation for the parameters? e.g. from "An optional photo URL for the user. Not passing it leaves it unchanged. Passing null clears the value"

@mikehardy mikehardy added Workflow: Waiting for User Response Blocked waiting for user response. plugin: authentication Firebase Authentication Type: Bug Fix labels Aug 31, 2020
@mikehardy
Copy link
Collaborator

@andreibarabas - a quick tweak of the PR in response to the feedback and we can merge this I think!

Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

I patched it up for you

@mikehardy mikehardy added Workflow: Pending Merge Waiting on CI or similar and removed Workflow: Waiting for User Response Blocked waiting for user response. labels Sep 4, 2020
@mikehardy
Copy link
Collaborator

Good to go pending CI queue clearing after #4209 is resolved

@mikehardy mikehardy merged commit b4436ea into invertase:master Sep 4, 2020
@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Sep 4, 2020
androidIsForVivek pushed a commit to androidIsForVivek/react-native-firebase that referenced this pull request Sep 15, 2021
…vertase#4179)

* fix: Add missing null to profile update

You need to specify null when you want to remove the current user's photo

* Update displayName to use same style, and docs to match

Co-authored-by: Mike Hardy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin: authentication Firebase Authentication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants