From aca8ea9c0b806b5352a04a742755d2cd1addd765 Mon Sep 17 00:00:00 2001
From: joostme <github@joost.io>
Date: Wed, 3 Oct 2018 16:02:14 +0200
Subject: [PATCH] refactor(service-worker): Format comments and add additional
 test (#25860)

- Format JSDoc for notificationClicks
- Add comment on why handleClick does not use hasOwnProperty
- Add additional test that uses handleClick without action

PR Close #25860
---
 packages/service-worker/src/push.ts           | 13 +++++------
 packages/service-worker/worker/src/driver.ts  |  2 ++
 .../service-worker/worker/test/happy_spec.ts  | 22 +++++++++++++++----
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/packages/service-worker/src/push.ts b/packages/service-worker/src/push.ts
index 3d073cee20..cc222ef948 100644
--- a/packages/service-worker/src/push.ts
+++ b/packages/service-worker/src/push.ts
@@ -27,15 +27,14 @@ export class SwPush {
 
   /**
    * Emits the payloads of the received push notification messages as well as the action the user
-   * interacted with.
-   * If no action was used the action property will be an empty string `''`.
+   * interacted with. If no action was used the action property will be an empty string `''`.
    *
-   * Note that the `notification` property is **not** a
-   * [Notification](https://developer.mozilla.org/en-US/docs/Web/API/Notification) object but rather
-   * a
+   * Note that the `notification` property is **not** a [Notification][Mozilla Notification] object
+   * but rather a
    * [NotificationOptions](https://notifications.spec.whatwg.org/#dictdef-notificationoptions)
-   * object that also includes the `title` of the
-   * [Notification](https://developer.mozilla.org/en-US/docs/Web/API/Notification) object.
+   * object that also includes the `title` of the [Notification][Mozilla Notification] object.
+   *
+   * [Mozilla Notification]: https://developer.mozilla.org/en-US/docs/Web/API/Notification
    */
   readonly notificationClicks: Observable < {
     action: string;
diff --git a/packages/service-worker/worker/src/driver.ts b/packages/service-worker/worker/src/driver.ts
index 053b431339..9ff50cfc75 100644
--- a/packages/service-worker/worker/src/driver.ts
+++ b/packages/service-worker/worker/src/driver.ts
@@ -309,6 +309,8 @@ export class Driver implements Debuggable, UpdateSource {
     notification.close();
 
     const options: {-readonly[K in keyof Notification]?: Notification[K]} = {};
+    // The filter uses `name in notification` because the properties are on the prototype so
+    // hasOwnProperty does not work here
     NOTIFICATION_OPTION_NAMES.filter(name => name in notification)
         .forEach(name => options[name] = notification[name]);
 
diff --git a/packages/service-worker/worker/test/happy_spec.ts b/packages/service-worker/worker/test/happy_spec.ts
index 1be7764863..d1278434bf 100644
--- a/packages/service-worker/worker/test/happy_spec.ts
+++ b/packages/service-worker/worker/test/happy_spec.ts
@@ -589,16 +589,30 @@ const manifestUpdateHash = sha1(JSON.stringify(manifestUpdate));
       }]);
     });
 
-    async_it('broadcasts notification click events', async() => {
+    async_it('broadcasts notification click events with action', async() => {
       expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
       await driver.initialized;
-      await scope.handleClick({title: 'This is a test', body: 'Test body'}, 'button');
+      await scope.handleClick(
+          {title: 'This is a test with action', body: 'Test body with action'}, 'button');
       const message: any = scope.clients.getMock('default') !.messages[0];
 
       expect(message.type).toEqual('NOTIFICATION_CLICK');
       expect(message.data.action).toEqual('button');
-      expect(message.data.notification.title).toEqual('This is a test');
-      expect(message.data.notification.body).toEqual('Test body');
+      expect(message.data.notification.title).toEqual('This is a test with action');
+      expect(message.data.notification.body).toEqual('Test body with action');
+    });
+
+    async_it('broadcasts notification click events without action', async() => {
+      expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
+      await driver.initialized;
+      await scope.handleClick(
+          {title: 'This is a test without action', body: 'Test body without action'});
+      const message: any = scope.clients.getMock('default') !.messages[0];
+
+      expect(message.type).toEqual('NOTIFICATION_CLICK');
+      expect(message.data.action).toBeUndefined();
+      expect(message.data.notification.title).toEqual('This is a test without action');
+      expect(message.data.notification.body).toEqual('Test body without action');
     });
 
     async_it('prefetches updates to lazy cache when set', async() => {