firebase-analytics icon indicating copy to clipboard operation
firebase-analytics copied to clipboard

Unable to set userId to null

Open ramoncarreras opened this issue 4 years ago • 17 comments

Describe the bug setUserId method does not accept null.

https://github.com/capacitor-community/firebase-analytics/blob/master/src/web.ts#L83

To Reproduce

   FirebaseAnalytics.setUserId({
       userId: null,
   });

Expected behavior When the user logs out, it must be possible to assign null value to it.

See here: https://firebase.google.com/docs/reference/android/com/google/firebase/analytics/FirebaseAnalytics#public-void-setuserid-string-id

ramoncarreras avatar Feb 15 '21 13:02 ramoncarreras

What is the exact Error message?

markxoe avatar Feb 15 '21 19:02 markxoe

The exact error message is: "userId property is missing":

Captura de Pantalla 2021-02-16 a les 8 20 51

ramoncarreras avatar Feb 16 '21 07:02 ramoncarreras

Could you try undefined? This should work, as it is the default value See https://github.com/capacitor-community/firebase-analytics/blob/master/src/web.ts#L80

markxoe avatar Feb 16 '21 08:02 markxoe

As long as the condition is !userId, the result will be the same using null or undefined.

See this test on Chrome's console:

Captura de Pantalla 2021-02-16 a les 10 22 12

IMHO the condition must be: if (userId === undefined)

ramoncarreras avatar Feb 16 '21 09:02 ramoncarreras

I could make a patch... I may be doing this later this day

markxoe avatar Feb 16 '21 09:02 markxoe

Thank you very much @markxoe

ramoncarreras avatar Feb 16 '21 09:02 ramoncarreras

I'm just thinking about whether that is possible with the web version; In the docs there is nothing of userId = undefined: https://firebase.google.com/docs/reference/js/firebase.analytics.Analytics#setuserid

And the type of the parameter is string, which is not assignable to undefined

markxoe avatar Feb 16 '21 10:02 markxoe

I have been doing some tests and, in web version, you can pass null to setUserId method and it works properly. I have tried it with undefined as well, and in this case it is not working.

So, IMHO, you must change L82 this way:

BEFORE if (!userId) {

AFTER if (userId === undefined) {

My test screenshot (using setUserId(null);):

Captura de Pantalla 2021-02-16 a les 11 47 39

ramoncarreras avatar Feb 16 '21 10:02 ramoncarreras

Isn't the user_id=null just the same as user_id="" ? This would be a more elegant solution IMHO

markxoe avatar Feb 16 '21 11:02 markxoe

I think userId="" may work... But:

my proposal was NULL because in the Android and iOS documentation says:

The user ID to ascribe to the user of this app on this device, which must be non-empty and no more than 256 characters long. Setting the ID to null removes the user ID.

See here and here.

ramoncarreras avatar Feb 16 '21 11:02 ramoncarreras

Okay, i think i have the perfect idea, i'm just gonna do it and propose my changes in a Pull-Request

markxoe avatar Feb 16 '21 11:02 markxoe

Okay, i think i have the perfect idea, i'm just gonna do it and propose my changes in a Pull-Request

Thank you.

ramoncarreras avatar Feb 16 '21 11:02 ramoncarreras

See #50

markxoe avatar Feb 16 '21 11:02 markxoe

Is this project dead?? 24/03/2022 and still isn't possible set null the userId?

JoseMancebo avatar Mar 24 '22 15:03 JoseMancebo

Looks like @capacitor-firebase/analytics supports this: setUserId(...).

Pynka avatar Apr 04 '22 17:04 Pynka

Looks like @capacitor-firebase/analytics supports this: setUserId(...).

No, it seems that the documentation is wrong :(

ryaa avatar Jul 01 '23 11:07 ryaa

It's not posible to change the user with this plugin. The documentation says: 'Setting the value to null removes the user property.' But with this plugin is not posible to set the user to null. In my iOS app when a someone logs out and then logs in with another user, the analytics persist the old user.

msoler95 avatar Jul 20 '23 09:07 msoler95