Replace `flutter_local_notifications` with pigeon bindings

[rajveermalviya]

Fixes: #351

[gnprice]

Exciting!

LMK if there are questions you have for me at this stage, or particular spots in the code you'd like an early high-level review of.

[rajveermalviya]

@gnprice, I have a question about whether this implementation is desirable. Currently it creates a pending intent using the ACTION_VIEW action with a URL payload for each notification. When the user opens the notification, the intent functions as a deeplink and is handled by the existing URL handling in Flutter. Then this url is parsed and handled here for incoming intents while the app is open and while the app is closed.

Also, when the intent is created we set explicity set the reciever component so that ensures that this instance of zulip://notificiation_open url intent is only handled by our app.

This way this implementation avoids creating a custom intent handler and reuses Flutter existing URL route handler.

[rajveermalviya]

Moved the discussion to chat thread here — https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20Drop.20.60flutter_local_notifications.60.20.23F856/near/1910352

[chrisbobbe]

It looks like there's been some further discussion on these changes: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/narrow.20links.20with.20userid.3F/near/1941174

Please post here when that's resolved and this is ready for another review. 🙂

[rajveermalviya]

Apologies for the delay in the revision. Thanks for the previous review @chrisbobbe, pushed a new revision, PTAL.

[chrisbobbe]

Hmm, looks like some tests are failing, could you take a look?

[rajveermalviya]

Weird, I can't repro locally. Error in CI is:

 Invalid argument(s): Failed to load dynamic library 'libsqlite3.so': libsqlite3.so: cannot open shared object file: No such file or directory
  dart:ffi                                                        new DynamicLibrary.open
  package:sqlite3/src/ffi/load_library.dart 52:27                 _defaultOpen
  package:sqlite3/src/ffi/load_library.dart 127:12                OpenDynamicLibrary.openSqlite
  package:sqlite3/src/ffi/api.dart 13:39                          sqlite3
  package:drift/native.dart 313:12                                _NativeDelegate.openDatabase
  package:drift/src/sqlite3/database.dart 79:19                   Sqlite3Delegate.open
  package:drift/src/runtime/executor/helpers/engines.dart 431:22  DelegatedDatabase.ensureOpen.<fn>

It seems unrelated to this PR, because I am getting the same failure on main in my fork. I'll investigate it later in the day, unless someone else does it first.

[chrisbobbe]

Rerunning the CI checks; this should fix it: https://github.com/zulip/zulip-flutter/pull/999#issuecomment-2417351945

[rajveermalviya]

Thanks for the review @chrisbobbe and @gnprice! @gnprice I've now rebased this PR on top of #1005 with suggested changes, PTAL.

[chrisbobbe]

I've unassigned myself since it's Greg's turn to review. 🙂

One thing was weird though. Here's that update as GitHub shows it here:

But the GitHub bot on CZO rendered it this way:

https://chat.zulip.org/#narrow/channel/494-mobile-github/topic/zulip-flutter/near/1964999

chrisbobbe unassigned gnprice to PR #856 Replace flutter_local_notifications with pigeon bindings.

The "Assignees" metadata in GitHub's right sidebar is showing correctly:

So, shrug? Odd, though.

[gnprice]

Seems like a bug in our GitHub bot. The #integrations channel would be a good place to report it; I think we have a contributor currently who'd be likely to pick up an issue like that.

[chrisbobbe]

Sure; reported there: https://chat.zulip.org/#narrow/channel/127-integrations/topic/GitHub.3A.20Wrong.20user.20identified.20as.20unassigned/near/1965136

[rajveermalviya]

Thanks for the review @gnprice! Pushed a new revision, PTAL.

[rajveermalviya]

Thanks for the review @gnprice! New revision pushed, PTAL.

[rajveermalviya]

Thanks for the review @gnprice. Pushed a new revision, PTAL.

[rajveermalviya]

Thanks for the review @gnprice! Pushed a new revision, PTAL.

Although I was unable to find the mentioned test protocol, I did some fresh manual testing of following scenarios:

• Recieved muliple messages from two different users in two different streams and thus different topics, notifications are correctly displayed and routed to correct conversation when opened.
• Recieved muliple messages from two different users in same stream but two different topics, notifications are correctly displayed and routed to correct conversation when opened.
• Recieved a message in one-one DM, notification is correctly displayed and routed to the DM page when opened.
• Recieved messages in group DM with two users (total 3 users), notification is correctly displayed and routed to the group DM page when opened.

(Tested the above cases with both foreground and background notifications)

• Recieved messages in a stream+topic conversation, notification is correctly displayed, marked as read the conversation from web client, notifications for that specific conversation were removed.
• Recieved messages in a one-one DM, notification is correctly displayed, marked as read the conversation from web client, notifications for that specific conversation were removed.

[rajveermalviya]

In addition to that, I also had the mobile notifications enabled for all channels in multiple (busy) servers to test the realm group summary notifications.

[rajveermalviya]

Thanks for the review @gnprice. Pushed a new revision, PTAL.

[gnprice]

Looks good — merging. Thanks for all your work getting this transition finished!