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

[notifications] Check if icon file exists before removing #13539

Merged
merged 4 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/expo-notifications/CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@

- Fixed Android notifications not respecting the `shouldPlaySound` property in `setNotificationHandler`. ([#13411](https://github.com/expo/expo/pull/13411) by [@cruzach](https://github.com/cruzach))
- Force device ID to lowercase before sending to Expo's servers. (Only applicable if you're using `ExpoPushToken`s). ([#13409](https://github.com/expo/expo/pull/13409) by [@cruzach](https://github.com/cruzach))
- Fixed plugin to not throw if the notification icon isn't set, and there's no notification icon present in the Android project.

### 💡 Others

Expand Down Expand Up @@ -125,9 +126,11 @@ _This version does not introduce any user-facing changes._
- Changed class responsible for handling Firebase events from `FirebaseMessagingService` to `.service.NotificationsService` on Android. ([#10558](https://github.com/expo/expo/pull/10558) by [@sjchmiela](https://github.com/sjchmiela))

> Note that this change most probably will not affect you — it only affects projects that override `FirebaseMessagingService` to implement some custom handling logic.

- Changed how you can override ways in which a notification is reinterpreted from a [`StatusBarNotification`](https://developer.android.com/reference/android/service/notification/StatusBarNotification) and in which a [`Notification`](https://developer.android.com/reference/android/app/Notification.html?hl=en) is built from defining an `expo.modules.notifications#NotificationsScoper` meta-data value in `AndroidManifest.xml` to implementing a `BroadcastReceiver` subclassing `NotificationsService` delegating those responsibilities to your custom `PresentationDelegate` instance. ([#10558](https://github.com/expo/expo/pull/10558) by [@sjchmiela](https://github.com/sjchmiela))

> Note that this change most probably will not affect you — it only affects projects that override those methods to implement some custom handling logic.

- Removed `removeAllNotificationListeners` method. You can (and should) still remove listeners using `remove` method on `Subscription` objects returned by `addNotification…Listener`. ([#10883](https://github.com/expo/expo/pull/10883) by [@sjchmiela](https://github.com/sjchmiela))
- Fixed device identifier being used to fetch Expo push token being backed up on Android which resulted in multiple devices having the same `deviceId` (and eventually, Expo push token). ([#11005](https://github.com/expo/expo/pull/11005) by [@sjchmiela](https://github.com/sjchmiela))
- Fixed device identifier used when fetching Expo push token being different than `Constants.installationId` in managed workflow apps which resulted in different Expo push tokens returned for the same experience across old and new Expo API and the device push token not being automatically updated on Expo push servers which lead to Expo push tokens corresponding to outdated Firebase tokens. ([#11005](https://github.com/expo/expo/pull/11005) by [@sjchmiela](https://github.com/sjchmiela))
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Expand Up @@ -52,7 +52,7 @@ const soundPath = path.resolve(__dirname, './fixtures/cat.wav');
const projectRoot = '/app';

describe('Android notifications configuration', () => {
beforeAll(async () => {
beforeEach(async () => {
const icon = fsReal.readFileSync(iconPath);
const sound = fsReal.readFileSync(soundPath);
vol.fromJSON(
Expand All @@ -65,10 +65,13 @@ describe('Android notifications configuration', () => {
vol.writeFileSync('/app/assets/notificationSound.wav', sound);
});

afterEach(() => {
vol.reset();
});

afterAll(() => {
jest.unmock('@expo/image-utils');
jest.unmock('fs');
vol.reset();
});

it(`returns null if no config provided`, () => {
Expand All @@ -92,6 +95,24 @@ describe('Android notifications configuration', () => {
const after = getDirFromFS(vol.toJSON(), projectRoot);
expect(Object.keys(after).sort()).toEqual(LIST_OF_GENERATED_NOTIFICATION_FILES.sort());
});

it('Safely remove icon if it exists, and ignore if it doesnt', async () => {
const before = getDirFromFS(vol.toJSON(), projectRoot);
// first set the icon
await setNotificationIconAsync(projectRoot, '/app/assets/notificationIcon.png');

// now remove
await setNotificationIconAsync(projectRoot, null);

const after = getDirFromFS(vol.toJSON(), projectRoot);
expect(before).toMatchObject(after);

// now remove again to make sure we don't throw in that case
await setNotificationIconAsync(projectRoot, null);

const final = getDirFromFS(vol.toJSON(), projectRoot);
expect(before).toMatchObject(final);
});
});

function setUpDrawableDirectories() {
Expand Down
Expand Up @@ -181,7 +181,10 @@ function removeNotificationIconImageFiles(projectRoot: string) {
Object.values(dpiValues).forEach(async ({ folderName }) => {
const drawableFolderName = folderName.replace('mipmap', 'drawable');
const dpiFolderPath = resolve(projectRoot, ANDROID_RES_PATH, drawableFolderName);
unlinkSync(resolve(dpiFolderPath, NOTIFICATION_ICON + '.png'));
const iconFile = resolve(dpiFolderPath, NOTIFICATION_ICON + '.png');
if (existsSync(iconFile)) {
unlinkSync(iconFile);
}
});
}

Expand Down