Skip to content

Add Notification Channels configuration for Android O #713

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

Merged
merged 12 commits into from
Aug 22, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ android:
- tools
- platform-tools
- build-tools-25.0.3
- android-25
- doc-25
- android-26
- doc-26

before_install:
- pip install --user codecov
Expand Down
13 changes: 13 additions & 0 deletions Parse/src/main/java/com/parse/NotificationCompat.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import android.annotation.TargetApi;
import android.app.Notification;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.app.PendingIntent;
import android.content.Context;
Expand Down Expand Up @@ -95,6 +96,9 @@ public Notification build(Builder b) {
}
}
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
postJellyBeanBuilder.setChannelId(b.mNotificationChannel.getId());
}
return postJellyBeanBuilder.build();
}
}
Expand Down Expand Up @@ -124,6 +128,7 @@ public static class Builder {
Bitmap mLargeIcon;
int mPriority;
Style mStyle;
NotificationChannel mNotificationChannel;
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry man, I just noticed this... I am not sure if this might throw at runtime on older APIs, even if not used. Could we just store String mNotificationChannelId here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right again, this will crash. Sending just the Id is better.


Notification mNotification = new Notification();

Expand Down Expand Up @@ -188,6 +193,14 @@ public Builder setContentTitle(CharSequence title) {
return this;
}

/**
* Set the notification channel of the notification, in a standard notification.
*/
public Builder setNotificationChannel(NotificationChannel channel) {
mNotificationChannel = channel;
return this;
}

/**
* Set the text (second row) of the notification, in a standard notification.
*/
Expand Down
2 changes: 2 additions & 0 deletions Parse/src/main/java/com/parse/ParseNotificationManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import android.app.Activity;
import android.app.Notification;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.app.PendingIntent;
import android.content.ComponentName;
Expand All @@ -20,6 +21,7 @@
import android.content.res.Resources;
import android.content.res.Resources.NotFoundException;
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.os.Bundle;
import android.util.SparseIntArray;

Expand Down
32 changes: 32 additions & 0 deletions Parse/src/main/java/com/parse/ParsePushBroadcastReceiver.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
*/
package com.parse;

import android.annotation.TargetApi;
import android.app.Activity;
import android.app.Notification;
import android.app.NotificationChannel;
import android.app.NotificationManager;
import android.app.PendingIntent;
import android.content.BroadcastReceiver;
import android.content.Context;
Expand Down Expand Up @@ -260,6 +263,27 @@ protected Class<? extends Activity> getActivity(Context context, Intent intent)
return cls;
}

/**
* Retrieves the channel to be used in a {@link Notification} if API >= 26, if not null. The default returns a new channel
* with id "parse_push", name "Push notifications" and default importance.
*
* @param context
* The {@code Context} in which the receiver is running.
* @param intent
* An {@code Intent} containing the channel and data of the current push notification.
* @return
* The notification channel
*/
@TargetApi(Build.VERSION_CODES.O)
protected NotificationChannel getNotificationChannel(Context context, Intent intent) {
NotificationManager nm = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);
NotificationChannel notificationChannel = new NotificationChannel("parse_push", "Push notifications", NotificationManager.IMPORTANCE_DEFAULT);

// Android doesn't create a new channel if the properties of the channel hasn't changed
nm.createNotificationChannel(notificationChannel);
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 move the channel creation outside, maybe in getNotification(), right after we call getNotificationChannel() ? If someone overrides this method, the creation won't happen

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return notificationChannel;
}

/**
* Retrieves the small icon to be used in a {@link Notification}. The default implementation uses
* the icon specified by {@code com.parse.push.notification_icon} {@code meta-data} in your
Expand Down Expand Up @@ -364,6 +388,8 @@ protected Notification getNotification(Context context, Intent intent) {
PendingIntent pDeleteIntent = PendingIntent.getBroadcast(context, deleteIntentRequestCode,
deleteIntent, PendingIntent.FLAG_UPDATE_CURRENT);



// The purpose of setDefaults(Notification.DEFAULT_ALL) is to inherit notification properties
// from system defaults
NotificationCompat.Builder parseBuilder = new NotificationCompat.Builder(context);
Expand All @@ -376,6 +402,12 @@ protected Notification getNotification(Context context, Intent intent) {
.setDeleteIntent(pDeleteIntent)
.setAutoCancel(true)
.setDefaults(Notification.DEFAULT_ALL);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
NotificationChannel notificationChannel = getNotificationChannel(context, intent);
parseBuilder.setNotificationChannel(notificationChannel);
}

if (alert != null
&& alert.length() > ParsePushBroadcastReceiver.SMALL_NOTIFICATION_MAX_CHARACTER_LIMIT) {
parseBuilder.setStyle(new NotificationCompat.Builder.BigTextStyle().bigText(alert));
Expand Down
6 changes: 3 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ buildscript {
jcenter()
}
dependencies {
classpath 'com.android.tools.build:gradle:2.3.2'
classpath 'com.android.tools.build:gradle:2.3.3'
}
}

Expand All @@ -22,11 +22,11 @@ allprojects {
}

ext {
compileSdkVersion = 25
compileSdkVersion = 26
buildToolsVersion = "25.0.3"
Copy link
Contributor

@natario1 natario1 Aug 19, 2017

Choose a reason for hiding this comment

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

Should this be changed? Also in travis. I'm not sure, just asking.

Copy link
Contributor

Choose a reason for hiding this comment

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

As before, shouldn't we build with something starting with 26? Here and in travis file... No?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to change the build tools since I don't know what the routines are here on that, but I can change to 26.0.1

I could also upgrade the support library to 26.0.1, but then I'd have to raise the minSDK to 14, since they removed support for <14


supportLibVersion = '25.3.1'

minSdkVersion = 9
targetSdkVersion = 25
targetSdkVersion = 26
}