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

feat: process push notifications received outside CIO SDK #38

Merged
merged 7 commits into from Apr 26, 2023

Conversation

mrehan27
Copy link
Contributor

Closes: https://github.com/customerio/issues/issues/9819

Changes

  • Added customer_io_messaging_push channel for push messaging method requests and platform implementation for push messaging module
  • Added CustomerIOPluginModule for introducing module concept in Flutter SDK
  • Added support to generate notification locally using RemoteMessage for FCM
  • Extensions functions on Map for type casting, on MethodCall for returning value provided by native method and new constants required for the changes

Sample Usages

For anyone receiving notifications outside Customer.io SDK, they can generate notifications locally using the following code snippet:

// For messages received when app is in foreground
CustomerIO.messagingPush().onMessageReceived(payload).then((handled) {
  // handled is true if notification was handled by Customer.io SDK; false otherwise
  return handled;
});

// For messages received when app is in background
CustomerIO.messagingPush().onBackgroundMessageReceived(payload).then((handled) {
  // handled is true if notification was handled by Customer.io SDK; false otherwise
  return handled;
});

where payload is map which contains notification payload received using FCM.

An example using FlutterFire would look like:

For listening and processing messages received when app is in background

Future<void> _firebaseMessagingBackgroundHandler(RemoteMessage message) async {
  await Firebase.initializeApp();
  CustomerIO.messagingPush().onBackgroundMessageReceived(message.toMap()).then((handled) {
    // handled is true if notification was handled by Customer.io SDK; false otherwise
    return handled;
  });
}

void main() async {
  // Initialize required SDKs
  FirebaseMessaging.onBackgroundMessage(_firebaseMessagingBackgroundHandler);
  // Run the app
}

For listening and processing messages received when app is in foreground

FirebaseMessaging.onMessage.listen((RemoteMessage message) {
  CustomerIO.messagingPush().onMessageReceived(message.toMap()).then((handled) {
    // handled is true if notification was handled by Customer.io SDK; false otherwise
    return handled;
  });
});

@mrehan27 mrehan27 requested a review from a team April 12, 2023 08:25
@mrehan27 mrehan27 self-assigned this Apr 12, 2023
@github-actions
Copy link

github-actions bot commented Apr 12, 2023

Pull request title looks good 👍!

If this pull request gets merged, it will cause a new release of the software. Example: If this project's latest release version is 1.0.0. If this pull request gets merged in, the next release of this project will be 1.1.0. This pull request is not a breaking change.

All merged pull requests will eventually get deployed. But some types of pull requests will trigger a deployment (such as features and bug fixes) while some pull requests will wait to get deployed until a later time.

This project uses a special format for pull requests titles. Expand this section to learn more (expand by clicking the ᐅ symbol on the left side of this sentence)...

This project uses a special format for pull requests titles. Don't worry, it's easy!

This pull request title should be in this format:

<type>: short description of change being made

If your pull request introduces breaking changes to the code, use this format:

<type>!: short description of breaking change

where <type> is one of the following:

  • feat: - A feature is being added or modified by this pull request. Use this if you made any changes to any of the features of the project.

  • fix: - A bug is being fixed by this pull request. Use this if you made any fixes to bugs in the project.

  • docs: - This pull request is making documentation changes, only.

  • refactor: - A change was made that doesn't fix a bug or add a feature.

  • test: - Adds missing tests or fixes broken tests.

  • style: - Changes that do not effect the code (whitespace, linting, formatting, semi-colons, etc)

  • perf: - Changes improve performance of the code.

  • build: - Changes to the build system (maven, npm, gulp, etc)

  • ci: - Changes to the CI build system (Travis, GitHub Actions, Circle, etc)

  • chore: - Other changes to project that don't modify source code or test files.

  • revert: - Reverts a previous commit that was made.

Examples:

feat: edit profile photo
refactor!: remove deprecated v1 endpoints
build: update npm dependencies
style: run formatter 

Need more examples? Want to learn more about this format? Check out the official docs.

Note: If your pull request does multiple things such as adding a feature and makes changes to the CI server and fixes some bugs then you might want to consider splitting this pull request up into multiple smaller pull requests.

@github-actions
Copy link

github-actions bot commented Apr 12, 2023

Hey, there @mrehan27 👋🤖. I'm a bot here to help you.

⚠️ Pull requests into the branch main typically only allows changes with the types: fix. From the pull request title, the type of change this pull request is trying to complete is: feat. ⚠️

This pull request might still be allowed to be merged. However, you might want to consider make this pull request merge into a different branch other then main.

This project uses a special format for pull requests titles. Expand this section to learn more (expand by clicking the ᐅ symbol on the left side of this sentence)...

This project uses a special format for pull requests titles. Don't worry, it's easy!

This pull request title should be in this format:

<type>: short description of change being made

If your pull request introduces breaking changes to the code, use this format:

<type>!: short description of breaking change

where <type> is one of the following:

  • feat: - A feature is being added or modified by this pull request. Use this if you made any changes to any of the features of the project.

  • fix: - A bug is being fixed by this pull request. Use this if you made any fixes to bugs in the project.

  • docs: - This pull request is making documentation changes, only.

  • refactor: - A change was made that doesn't fix a bug or add a feature.

  • test: - Adds missing tests or fixes broken tests.

  • style: - Changes that do not effect the code (whitespace, linting, formatting, semi-colons, etc)

  • perf: - Changes improve performance of the code.

  • build: - Changes to the build system (maven, npm, gulp, etc)

  • ci: - Changes to the CI build system (Travis, GitHub Actions, Circle, etc)

  • chore: - Other changes to project that don't modify source code or test files.

  • revert: - Reverts a previous commit that was made.

Examples:

feat: edit profile photo
refactor!: remove deprecated v1 endpoints
build: update npm dependencies
style: run formatter 

Need more examples? Want to learn more about this format? Check out the official docs.

Note: If your pull request does multiple things such as adding a feature and makes changes to the CI server and fixes some bugs then you might want to consider splitting this pull request up into multiple smaller pull requests.

Copy link
Member

@levibostian levibostian left a comment

Choose a reason for hiding this comment

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

I like the API designed (thanks for that in the PR description) and the implementation seems fine to me.

The only part of the PR I hesitate on is the idea of CustomerIOPluginModule and this idea of having Flutter SDK modules map to a native Android module.

I like the idea of keeping Flutter SDK push code encapsulated in it's own files and classes as you have done. But I am not understanding the value that we get from trying to map Flutter SDK to Android modules?

@mrehan27
Copy link
Contributor Author

Thanks @levibostian. I just wanted to make it easier for us to decide the correct file/class for adding new features. And mapping Flutter modules to native modules would probably be the easiest criteria. Also, if at any point, we make our wrapper SDKs modular like native and make adding in-app and push module optional, having them mapped to separate files will make the transition easier for us I think.

I'm still okay for defining any other criteria for splitting functionality into multiple files to keep the code readable and easier to maintain, but mapping them to native modules is probably the one I would still vote for.

Copy link
Collaborator

@Shahroz16 Shahroz16 left a comment

Choose a reason for hiding this comment

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

Looks good, but need through testing, would appreciate if we can add test cases for the method channel as well

lib/messaging_push/method_channel.dart Outdated Show resolved Hide resolved
@levibostian
Copy link
Member

@mrehan27 in response to your message here I would prefer that we remove the plugin/module functionality from this PR. I suggest this for 2 reasons.

  1. I appreciate you thinking ahead and considering the possibility of breaking apart the Flutter SDK into modules like we do with our native SDKs. However, I prefer not to maintain this module code until we decide to implement that feature.

  2. Also, I don't want to make decisions today for this possible future feature that could limit our flexibility. By us deciding in this PR today how Flutter modules might be split up, that limits our decisions in the future for this feature. Maybe in the future we decide not to make push a separate module? Maybe we do? I see cons with making that decision today for our future selves.

With all that being said, I appreciate you breaking it apart into files/classes. I think the API looks great as well.

@mrehan27
Copy link
Contributor Author

Thanks for sharing the concerns @levibostian. This still remains my secondary concern 😅 My main goal was to break it apart into files/classes based on some criteria so it is easier to take decisions for correct files in future as well. We don't have any other criteria for breaking down the functionality right now. And if we remove plugin/module functionality, this may cause most of the work in this PR to be re-done, which isn't ideal.

Since we all agree on the api design, we can remove plugin concept in future PRs if needed without changing public apis. For now, I would prefer sticking with the solution and not introduce any major changes in this PR.

@FDuhen
Copy link

FDuhen commented Apr 20, 2023

Any updates about this promising PR ?
We're having issues using CIO alongside with Flutter Local Notifications, and from what I've understood, this update would help us a lot

@mrehan27
Copy link
Contributor Author

Hey @FDuhen. Thank you for reaching out. This is currently being tested and will be released once testing is complete.

@Shahroz16 Shahroz16 merged commit 7b5cb7e into main Apr 26, 2023
5 checks passed
@Shahroz16 Shahroz16 deleted the feat/process-push-notification branch April 26, 2023 13:04
github-actions bot pushed a commit that referenced this pull request Apr 26, 2023
## [1.1.0](1.0.0...1.1.0) (2023-04-26)

### Features

* process push notifications received outside CIO SDK ([#38](#38)) ([7b5cb7e](7b5cb7e))
github-actions bot pushed a commit to nagyist/customerio-flutter that referenced this pull request Oct 18, 2023
## 1.0.0 (2023-10-18)

### Features

* added missing methods ([customerio#17](https://github.com/nagyist/customerio-flutter/issues/17)) ([73f29e6](73f29e6))
* added SDK config  ([#1](#1)) ([e8ed7dd](e8ed7dd))
* in-app dismiss support ([customerio#51](https://github.com/nagyist/customerio-flutter/issues/51)) ([c4d21f2](c4d21f2))
* process push notifications received outside CIO SDK ([customerio#38](https://github.com/nagyist/customerio-flutter/issues/38)) ([7b5cb7e](7b5cb7e))
* tracking and in-app added ([#2](#2)) ([c23f2d9](c23f2d9))

### Bug Fixes

* add test coverage and refactored scripts ([customerio#34](https://github.com/nagyist/customerio-flutter/issues/34)) ([f7f2387](f7f2387))
* autoupdate to latest major version of iOS SDK ([customerio#40](https://github.com/nagyist/customerio-flutter/issues/40)) ([974a342](974a342))
* formatting issues ([d67fa7e](d67fa7e))
* hardcode android native SDK version ([customerio#61](https://github.com/nagyist/customerio-flutter/issues/61)) ([587f559](587f559))
* in-app concurrency issue android ([customerio#73](https://github.com/nagyist/customerio-flutter/issues/73)) ([93332a4](93332a4))
* in-app crash for no browser ([customerio#94](https://github.com/nagyist/customerio-flutter/issues/94)) ([8b859ed](8b859ed))
* in-app messages not displaying for release builds on Android ([customerio#65](https://github.com/nagyist/customerio-flutter/issues/65)) ([1d742c2](1d742c2))
* in-app remove gist org id ([customerio#19](https://github.com/nagyist/customerio-flutter/issues/19)) ([ce4cc9e](ce4cc9e))
* iOS crash on forced unwrapping  ([customerio#59](https://github.com/nagyist/customerio-flutter/issues/59)) ([f514174](f514174))
* missing methods and extra dependency ([2c5deca](2c5deca))
* missing opened metric on android 12 and above ([customerio#43](https://github.com/nagyist/customerio-flutter/issues/43)) ([1a61e0e](1a61e0e))
* obj-c bindings issue ([0dbe4ef](0dbe4ef))
* plugin version in user-agent ([a10e482](a10e482))
* release script typo ([2a8b7ae](2a8b7ae))
* stack-overflow caused by BQ recursion ([customerio#90](https://github.com/nagyist/customerio-flutter/issues/90)) ([ebc7511](ebc7511))
* typo fixed ([customerio#9](https://github.com/nagyist/customerio-flutter/issues/9)) ([a5107df](a5107df)), closes [#7](#7) [#8](#8)
* updated android dependency to auto update ([customerio#24](https://github.com/nagyist/customerio-flutter/issues/24)) ([040cef2](040cef2))
* updated icon and typo ([57c6eef](57c6eef))
* updated module name from common to CioInternalCommon ([customerio#55](https://github.com/nagyist/customerio-flutter/issues/55)) ([d81f8df](d81f8df))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants