fix: image not shown when image url in notification payload #172

Shahroz16 · 2023-02-17T11:55:11Z

In the case of custom payload,

{
  "message":{
    "notification": {
      "body" : "Meet scientific Yogi & Empathy Expert Thiemo️",
      "title" : "Yoga + Meditation + Science = :bulb:️️",
      "image": "https://userimg-assets.customeriomail.com/images/client-env-101262/1676545327104_thiemo_01GSCY703G786FJE0F5FT34H0N.jpeg"
    },
    "data": {
      "screen": "/users/9b906b6c-3152-4083-87d4-455d9d6e4e13?utm_source=cio&utm_medium=push&utm_campaign=thiemo_push&utm_content=9b906b6c-3152-4083-87d4-455d9d6e4e13&utm_term={{delivery_id}}"
    }
  }
}

If the app is in the foreground, Service would receive payload, and we need to check for notification.image.

More information : https://customerio.slack.com/archives/C02580R06/p1676625992450419

Complete each step to get your pull request merged in. Learn more about the workflow this project uses.

Assign members of your team to review the pull request.
Wait for pull request status checks to complete. If there are problems, fix them until you see that all status checks are passing.
Wait until the pull request has been reviewed and approved by a teammate
After pull request is approved, and you determine it's ready add the label "Ready to merge" to the pull request and it will automatically be merged.

image when in notification payload, doesn't show in foreground

codecov · 2023-02-17T11:55:21Z

Codecov Report

Merging #172 (6ec31dd) into beta (bab311b) will not change coverage.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               beta     #172   +/-   ##
=========================================
  Coverage     62.12%   62.12%           
  Complexity      210      210           
=========================================
  Files            91       91           
  Lines          2041     2041           
  Branches        257      258    +1     
=========================================
  Hits           1268     1268           
  Misses          676      676           
  Partials         97       97

Impacted Files	Coverage Δ
...messagingpush/CustomerIOPushNotificationHandler.kt	`0.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2023-02-17T11:55:26Z

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.0.1. 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 · 2023-02-17T11:59:42Z

Build available to test
Version: shahroz-fix-image-foreground-SNAPSHOT
Repository: https://s01.oss.sonatype.org/content/repositories/snapshots/

levibostian · 2023-02-17T18:32:28Z

messagingpush/src/main/java/io/customer/messagingpush/CustomerIOPushNotificationHandler.kt

-        // This is custom logic to add images to a push notification. Some image URLs have not
-        // been able to display an image in a push. https://github.com/customerio/issues/issues/7293
-        // Instead, we recommend customers use `notification.image` in the FCM payload to bypass
-        // our logic and use the logic from FCM's SDK for image handling instead.


This comment was added to help out customers in the past where they would not see an image for some image URLs (the ticket has some example URLs that failed).

That ticket was made a while ago so there is a chance that the customer issue has been resolved and our SDK is able to show images with all URLs now. If that is true, I am OK with removing this comment.

If our SDK still encounters problems with some image URLs, I am still OK with removing this comment as long as we create a new ticket for us to address this problem of increasing image URL compatibility in the SDK in the future.

Your concern is very valid Levi,

From LJ's response in the ticket

It ended up not being an issue with the link, but instead an issue with the payload they were using to test. @levibostian any reason to keep this image open?

and my testing, I tested the very same URL you mentioned in the ticket that didn't work,

It works:

And for the reasons, that we want to push people to use Rich Push which is data only, hence I decided to remove the comment.

we create a new ticket for us to address this problem of increasing image URL compatibility in the SDK in the future.
Would you advise creating it now or when an issue occurs? because I noticed the PR that adds it just got merged and we didn't get this issue reported for months after we reworked the image handling in SDK and sorted all the edge cases. Happy to create a ticket, if its serves a purpose.

levibostian · 2023-02-17T18:33:15Z

messagingpush/src/main/java/io/customer/messagingpush/CustomerIOPushNotificationHandler.kt

        try {
-            // check for image
-            val notificationImage = bundle.getString(IMAGE_KEY)
+            // check for image in data and notification payload


Could you add a comment explaining why we are checking for an image in data and in notification payload? So future readers of this code understand the decision behind doing this.

levibostian · 2023-02-17T18:38:52Z

Changed PR title to be more descriptive. I thought this would be helpful for when this commit gets added to the changelog. Anyone else can change the title if they have a better suggestion 👍

## [3.3.0-beta.4](3.3.0-beta.3...3.3.0-beta.4) (2023-02-17) ### Bug Fixes * image not shown when image url in notification payload ([#172](#172)) ([0abdc85](0abdc85))

github-actions · 2023-02-17T21:13:30Z

🎉 This PR is included in version 3.3.0-beta.4 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

## [3.3.0](3.2.0...3.3.0) (2023-02-22) ### Features * add setting a in-app event listener ([#147](#147)) ([5fd9559](5fd9559)) * in-app feature no longer requires orgId ([#163](#163)) ([fc2a08e](fc2a08e)) ### Bug Fixes * image not shown when image url in notification payload ([#172](#172)) ([0abdc85](0abdc85)) * moved shared wrapper code ([#158](#158)) ([51af98f](51af98f)) * remove currentRoute parameter in in-app event listener ([#159](#159)) ([688e4a5](688e4a5)) * rename in app listener keys ([#164](#164)) ([f540eaf](f540eaf)) * set gist dependency to use latest 3.X.Y version ([#170](#170)) ([a019c36](a019c36)) * set gist user token incase identifier exists ([#162](#162)) ([44cc4d1](44cc4d1)) * update CustomerIOFirebaseMessagingService to open ([#174](#174)) ([edce7f5](edce7f5)) * upgrade dependencies ([#146](#146)) ([6da8b8d](6da8b8d)) * use maven style dependency range syntax ([#171](#171)) ([ba83214](ba83214))

github-actions · 2023-02-22T20:18:33Z

🎉 This PR is included in version 3.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

fix image not shown

7022b5c

image when in notification payload, doesn't show in foreground

Shahroz16 requested a review from a team February 17, 2023 11:55

mrehan27 approved these changes Feb 17, 2023

View reviewed changes

levibostian approved these changes Feb 17, 2023

View reviewed changes

levibostian changed the title ~~fix: image not shown~~ fix: image not shown when image url in notification payload Feb 17, 2023

updated comment

6ec31dd

Shahroz16 merged commit 0abdc85 into beta Feb 17, 2023

Shahroz16 deleted the shahroz/fix-image-foreground branch February 17, 2023 21:12

github-actions bot added the released on @beta label Feb 17, 2023

github-actions bot added the released label Feb 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: image not shown when image url in notification payload #172

fix: image not shown when image url in notification payload #172

Shahroz16 commented Feb 17, 2023

codecov bot commented Feb 17, 2023 •

edited

github-actions bot commented Feb 17, 2023 •

edited

Examples:

github-actions bot commented Feb 17, 2023

levibostian Feb 17, 2023

Shahroz16 Feb 17, 2023

levibostian Feb 17, 2023

levibostian commented Feb 17, 2023

github-actions bot commented Feb 17, 2023

github-actions bot commented Feb 22, 2023

fix: image not shown when image url in notification payload #172

fix: image not shown when image url in notification payload #172

Conversation

Shahroz16 commented Feb 17, 2023

codecov bot commented Feb 17, 2023 • edited

Codecov Report

github-actions bot commented Feb 17, 2023 • edited

Examples:

github-actions bot commented Feb 17, 2023

levibostian Feb 17, 2023

Choose a reason for hiding this comment

Shahroz16 Feb 17, 2023

Choose a reason for hiding this comment

levibostian Feb 17, 2023

Choose a reason for hiding this comment

levibostian commented Feb 17, 2023

github-actions bot commented Feb 17, 2023

github-actions bot commented Feb 22, 2023

codecov bot commented Feb 17, 2023 •

edited

github-actions bot commented Feb 17, 2023 •

edited