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

[firebase_messaging] adding support for deleteInstanceId and setAutoInitEnabled #875

Merged
merged 3 commits into from
Nov 14, 2018

Conversation

ffeu
Copy link
Contributor

@ffeu ffeu commented Oct 30, 2018

fix flutter/flutter#20627

If an app has a basic flow with user login and logout AND receives notifications based on the logged user, the logout method needs to delete the current instanceID connected to Firebase Cloud Messaging. Otherwise, the unauthenticated app (after the user logs out) will still receive notifications from the previous logged user.

As deleteInstanceId might automatically recreate a new one, autoInitEnabled and setAutoInitEnabled must be provided as well.

Note: Firebase Java plugin uses deleteInstanceId ("Id"), in this Dart implementation deleteInstanceID ("ID") is being used.

@ffeu
Copy link
Contributor Author

ffeu commented Oct 30, 2018

Hi @kroikie , can you please take a look at this one?

Note: the only way I could manage the Java deleteInstanceId to work was using a Thread, as described here.

@kroikie kroikie self-assigned this Oct 30, 2018
test('setAutoInitEnabled', () {
bool enabled = true;
firebaseMessaging.setAutoInitEnabled(enabled);
verify(mockChannel.invokeMethod('setAutoInitEnabled', enabled));
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you verify explicitly here that true and false are called here?

verify(mockChannel.invokeMethod('setAutoInitEnabled', true));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that what you mean?

    // assert that we havent called the method yet
    verifyNever(firebaseMessaging.setAutoInitEnabled(true));

    firebaseMessaging.setAutoInitEnabled(true);

    // assert we called the method with enabled = true
    verify(mockChannel.invokeMethod('setAutoInitEnabled', true));

    // assert that enabled = false was not yet called
    verifyNever(firebaseMessaging.setAutoInitEnabled(false));

    firebaseMessaging.setAutoInitEnabled(false);

    // assert call with enabled = false was properly done
    verify(mockChannel.invokeMethod('setAutoInitEnabled', false));

What I really would like to do is to call autoInitEnabled() to check its value after calling setAutoInitEnabled, but as both method implementations lie on the plugin, we don't have access on testing time - I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kroikie do you think that's ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that look good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, it's done! 👍

@kroikie
Copy link
Contributor

kroikie commented Nov 14, 2018

Much appreciated! Thank you.

@kroikie kroikie merged commit 48c04b2 into flutter:master Nov 14, 2018
@ffeu ffeu deleted the deleteinstanceid_setautoinit branch November 16, 2018 13:59
andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
…nitEnabled (flutter#875)

* adding support for deleteInstanceId and setAutoInitEnabled
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
julianscheel pushed a commit to jusst-engineering/plugins that referenced this pull request Mar 11, 2020
…nitEnabled (flutter#875)

* adding support for deleteInstanceId and setAutoInitEnabled
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
3 participants