Show download link and icon based on contentUrl (#2868)

* Replace DownloadAttachment and UploadAttachment with FileAttachment * Add entry * Add PR number * Add optionals * Fix tests * Test for FileAttachment * Also check for <a> * Test relliability * Update CHANGELOG.md Co-Authored-By: TJ Durnford <[email protected]> * Apply PR comments Co-authored-by: TJ Durnford <[email protected]>
microsoft · Feb 6, 2020 · 17f4069 · 17f4069
1 parent 7efc568
commit 17f4069
Show file tree

Hide file tree

Showing 32 changed files with 393 additions and 204 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -56,6 +56,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 -  Fixes [#2822](https://github.com/microsoft/BotFramework-WebChat/issues/2822). Fixed `credentials` should return `authorizationToken` and `subscriptionKey` as string and allow empty LUIS reference grammar ID, by [@compulim](https://github.com/compulim) in PR [#2824](https://github.com/microsoft/BotFramework-WebChat/pull/2824)
 -  Fixes [#2751](https://github.com/microsoft/BotFramework-WebChat/issues/2751). Move documentation to docs folder, by [@corinagum](https://github.com/corinagum) in PR [#2832](https://github.com/microsoft/BotFramework-WebChat/pull/2832)
 -  Fixes [#2838](https://github.com/microsoft/BotFramework-WebChat/issues/2838). Fixed `concatMiddleware` should allow any middleware to call its downstream middleware twice, by [@compulim](https://github.com/compulim) in PR [#2839](https://github.com/microsoft/BotFramework-WebChat/pull/2839)
+-  Fixes [#2864](https://github.com/microsoft/BotFramework-WebChat/issues/2864). Replaced `DownloadAttachment` and `UploadAttachment` with `FileAttachment`, which shows the download link and icon if the attachment contains the `contentUrl`, by [@compulim](https://github.com/compulim) in PR [#2868](https://github.com/microsoft/BotFramework-WebChat/pull/2868)
 
 ### Changed
 

diff --git a/...s__/chrome-docker/file-attachment-js-show-zip-files-with-content-url-1-snap.png b/...s__/chrome-docker/file-attachment-js-show-zip-files-with-content-url-1-snap.png
diff --git a/.../chrome-docker/file-attachment-js-show-zip-files-without-content-url-1-snap.png b/.../chrome-docker/file-attachment-js-show-zip-files-without-content-url-1-snap.png
diff --git a/__tests__/attachmentMiddleware.js b/__tests__/attachmentMiddleware.js
@@ -45,7 +45,9 @@ test('file upload should show thumbnail and file name', async () => {
 
         return next({ activity, attachment });
       }
-    }
+    },
+    // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
+    useProductionBot: true
   });
 
   await driver.wait(uiConnected(), timeouts.directLine);

diff --git a/__tests__/fileAttachment.js b/__tests__/fileAttachment.js
@@ -0,0 +1,114 @@
+import { imageSnapshotOptions, timeouts } from './constants.json';
+
+import allImagesLoaded from './setup/conditions/allImagesLoaded';
+import minNumActivitiesShown from './setup/conditions/minNumActivitiesShown';
+import uiConnected from './setup/conditions/uiConnected';
+
+// selenium-webdriver API doc:
+// https://seleniumhq.github.io/selenium/docs/api/javascript/module/selenium-webdriver/index_exports_WebDriver.html
+
+jest.setTimeout(timeouts.test);
+
+test('show ZIP files with contentUrl', async () => {
+  const { driver, pageObjects } = await setupWebDriver({
+    createDirectLine: options => {
+      const directLine = window.WebChat.createDirectLine(options);
+      const patchedDirectLine = {
+        activity$: new Observable(observer => {
+          const subscription = directLine.activity$.subscribe({
+            next(activity) {
+              observer.next(
+                Object.assign({}, activity, {
+                  attachments: (activity.attachments || []).map(attachment =>
+                    Object.assign({}, attachment, {
+                      contentUrl: 'https://example.org/'
+                    })
+                  )
+                })
+              );
+            }
+          });
+
+          return () => subscription.unsubscribe();
+        }),
+
+        connectionStatus$: directLine.connectionStatus$,
+        postActivity: directLine.postActivity.bind(directLine),
+        token: directLine.token
+      };
+
+      return patchedDirectLine;
+    }
+  });
+
+  await driver.wait(uiConnected(), timeouts.directLine);
+
+  await pageObjects.sendFile('empty.zip');
+  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(allImagesLoaded(), timeouts.fetchImage);
+
+  const base64PNG = await driver.takeScreenshot();
+
+  expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);
+
+  await expect(
+    driver.executeScript(() =>
+      document.querySelector('[role="listitem"]:nth-child(1) a[target="_blank"]').getAttribute('href')
+    )
+  ).resolves.toEqual('https://example.org/');
+  await expect(
+    driver.executeScript(() =>
+      document.querySelector('[role="listitem"]:nth-child(2) a[target="_blank"]').getAttribute('href')
+    )
+  ).resolves.toEqual('https://example.org/');
+});
+
+test('show ZIP files without contentUrl', async () => {
+  const { driver, pageObjects } = await setupWebDriver({
+    createDirectLine: options => {
+      const directLine = window.WebChat.createDirectLine(options);
+      const patchedDirectLine = {
+        activity$: new Observable(observer => {
+          const subscription = directLine.activity$.subscribe({
+            next(activity) {
+              observer.next(
+                Object.assign({}, activity, {
+                  attachments: (activity.attachments || []).map(attachment =>
+                    Object.assign({}, attachment, {
+                      contentUrl: undefined
+                    })
+                  )
+                })
+              );
+            }
+          });
+
+          return () => subscription.unsubscribe();
+        }),
+
+        connectionStatus$: directLine.connectionStatus$,
+        postActivity: directLine.postActivity.bind(directLine),
+        token: directLine.token
+      };
+
+      return patchedDirectLine;
+    }
+  });
+
+  await driver.wait(uiConnected(), timeouts.directLine);
+
+  await pageObjects.sendFile('empty.zip');
+  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(allImagesLoaded(), timeouts.fetchImage);
+
+  const base64PNG = await driver.takeScreenshot();
+
+  expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);
+
+  await expect(
+    driver.executeScript(() => !!document.querySelector('[role="listitem"]:nth-child(1) a'))
+  ).resolves.toBeFalsy();
+  await expect(
+    driver.executeScript(() => !!document.querySelector('[role="listitem"]:nth-child(2) a'))
+  ).resolves.toBeFalsy();
+});
diff --git a/__tests__/hooks/useSendFiles.js b/__tests__/hooks/useSendFiles.js
@@ -1,5 +1,6 @@
 import { imageSnapshotOptions, timeouts } from '../constants.json';
 
+import allOutgoingActivitiesSent from '../setup/conditions/allOutgoingActivitiesSent';
 import minNumActivitiesShown from '../setup/conditions/minNumActivitiesShown';
 import uiConnected from '../setup/conditions/uiConnected';
 
@@ -9,7 +10,10 @@ import uiConnected from '../setup/conditions/uiConnected';
 jest.setTimeout(timeouts.test);
 
 test('calling sendFile should send files', async () => {
-  const { driver, pageObjects } = await setupWebDriver();
+  const { driver, pageObjects } = await setupWebDriver({
+    // TODO: [P3] Offline bot did not reply with a downloadable attachment, we need to use production bot
+    useProductionBot: true
+  });
 
   await driver.wait(uiConnected(), timeouts.directLine);
 
@@ -24,6 +28,7 @@ test('calling sendFile should send files', async () => {
   });
 
   await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine);
 
   const base64PNG = await driver.takeScreenshot();
 

diff --git a/__tests__/hooks/useSendPostBack.js b/__tests__/hooks/useSendPostBack.js
@@ -15,7 +15,7 @@ test('calling sendPostBack should send a post back activity', async () => {
 
   await pageObjects.runHook('useSendPostBack', [], sendPostBack => sendPostBack({ hello: 'World!' }));
 
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(1), timeouts.directLine);
 
   const base64PNG = await driver.takeScreenshot();
 

diff --git a/__tests__/language.js b/__tests__/language.js
@@ -14,7 +14,7 @@ test('setting language to empty string should not crash', async () => {
   await driver.wait(uiConnected(), timeouts.directLine);
   await pageObjects.sendMessageViaSendBox('echo Hello', { waitForSend: true });
 
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   const base64PNG = await driver.takeScreenshot();
 

diff --git a/__tests__/markdown.js b/__tests__/markdown.js
@@ -29,7 +29,7 @@ test('null renderMarkdown function', async () => {
   await pageObjects.sendMessageViaSendBox('echo **This text should be plain text**', { waitForSend: true });
 
   await driver.wait(allImagesLoaded(), timeouts.fetchImage);
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   const base64PNG = await driver.takeScreenshot();
 

diff --git a/__tests__/sendBox.js b/__tests__/sendBox.js
@@ -26,7 +26,7 @@ test('should focus send box when message is being sent', async () => {
 
   await driver.wait(uiConnected(), timeouts.directLine);
   await pageObjects.sendMessageViaSendBox('Hello, World!', { waitForSend: true });
-  await driver.wait(minNumActivitiesShown(1), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
 
   const base64PNG = await driver.takeScreenshot();
 

diff --git a/__tests__/sendTypingIndicator.js b/__tests__/sendTypingIndicator.js
@@ -67,7 +67,7 @@ test('typing indicator should not display after second activity', async () => {
   });
 
   await pageObjects.sendMessageViaSendBox('typing', { waitForSend: true });
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   const base64PNG = await driver.takeScreenshot();
   expect(base64PNG).toMatchImageSnapshot(imageSnapshotOptions);

diff --git a/__tests__/setup/conditions/minNumActivitiesShown.js b/__tests__/setup/conditions/minNumActivitiesShown.js
@@ -1,5 +1,19 @@
-import { By, until } from 'selenium-webdriver';
+import { Condition } from 'selenium-webdriver';
 
 export default function minNumActivitiesShown(numActivities) {
-  return until.elementLocated(By.css(`[role="listitem"]:nth-child(${numActivities})`));
+  return new Condition(`${numActivities} activities is shown`, async driver => {
+    // To run hooks (WebChatTest.runHook), the code internally create an activity.
+    // Inside the activity renderer, it call hooks, but return empty visually.
+    // This activity is invisible and should not count towards "minNumActivitiesShown".
+
+    const numActivitiesShown = await driver.executeScript(() =>
+      [].reduce.call(
+        document.querySelectorAll('[role="listitem"]'),
+        (numActivitiesShown, child) => numActivitiesShown + (child.children.length ? 1 : 0),
+        0
+      )
+    );
+
+    return numActivitiesShown >= numActivities;
+  });
 }
diff --git a/__tests__/speech.selectVoice.js b/__tests__/speech.selectVoice.js
@@ -20,7 +20,7 @@ describe('selecting voice based on language', () => {
 
       await pageObjects.sendMessageViaMicrophone('echo 123');
 
-      await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+      await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
       await driver.wait(speechSynthesisUtterancePended(), timeouts.ui);
 
       await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty('voice', {
@@ -42,7 +42,7 @@ describe('selecting voice based on language', () => {
 
       await pageObjects.sendMessageViaMicrophone('echo 123');
 
-      await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+      await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
       await driver.wait(speechSynthesisUtterancePended(), timeouts.ui);
 
       await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty('voice', {
@@ -66,7 +66,7 @@ describe('selecting voice based on language', () => {
 
     await pageObjects.sendMessageViaMicrophone('echo 123');
 
-    await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+    await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
     await driver.wait(speechSynthesisUtterancePended(), timeouts.ui);
 
     await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty('voice', {

diff --git a/__tests__/speech.synthesis.js b/__tests__/speech.synthesis.js
@@ -110,7 +110,7 @@ describe('speech synthesis', () => {
     await pageObjects.sendMessageViaMicrophone('echo Hello, World!');
 
     await expect(speechRecognitionStartCalled().fn(driver)).resolves.toBeFalsy();
-    await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+    await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
     await expect(pageObjects.startSpeechSynthesize()).resolves.toHaveProperty(
       'text',
@@ -140,7 +140,7 @@ describe('speech synthesis', () => {
         }
       });
 
-      await pageObjects.sendMessageViaMicrophone('input hint expected');
+      await pageObjects.sendMessageViaMicrophone('hint expected');
 
       await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
 

diff --git a/__tests__/styleOptions.js b/__tests__/styleOptions.js
@@ -35,7 +35,7 @@ describe('style options', () => {
       waitForSend: true
     });
 
-    await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+    await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
     expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
   });

diff --git a/__tests__/suggestedActions.js b/__tests__/suggestedActions.js
@@ -57,7 +57,7 @@ describe('suggested-actions command', () => {
     const imBackButton = buttons[1];
 
     await imBackButton.click();
-    await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
+    await driver.wait(minNumActivitiesShown(4), timeouts.directLine);
     await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine);
 
     const base64PNG = await driver.takeScreenshot();
@@ -120,7 +120,7 @@ describe('suggested-actions command', () => {
     const postBackStringButton = buttons[4];
 
     await postBackStringButton.click();
-    await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
+    await driver.wait(minNumActivitiesShown(4), timeouts.directLine);
     await driver.wait(allOutgoingActivitiesSent(), timeouts.directLine);
 
     const base64PNG = await driver.takeScreenshot();

diff --git a/__tests__/timestamp.js b/__tests__/timestamp.js
@@ -24,7 +24,7 @@ test('update timestamp language on-the-fly', async () => {
   await driver.wait(uiConnected(), timeouts.directLine);
   await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true });
 
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 
@@ -83,7 +83,7 @@ test('prepend text', async () => {
   await driver.wait(uiConnected(), timeouts.directLine);
   await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true });
 
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 });
@@ -94,7 +94,7 @@ test('change timestamp grouping on-the-fly', async () => {
   await driver.wait(uiConnected(), timeouts.directLine);
   await pageObjects.sendMessageViaSendBox('echo Hello, World!', { waitForSend: true });
 
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 

diff --git a/__tests__/updateActivity.js b/__tests__/updateActivity.js
@@ -78,12 +78,12 @@ test('should not replace activity without activity ID', async () => {
   await pageObjects.sendMessageViaSendBox('echo This message will not be replaced by a carousel.', {
     waitForSend: true
   });
-  await driver.wait(minNumActivitiesShown(2), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
 
   await expect(driver.takeScreenshot()).resolves.toMatchImageSnapshot(imageSnapshotOptions);
 
   await pageObjects.sendMessageViaSendBox('carousel', { waitForSend: true });
-  await driver.wait(minNumActivitiesShown(3), timeouts.directLine);
+  await driver.wait(minNumActivitiesShown(5), timeouts.directLine);
   await driver.wait(allImagesLoaded(), timeouts.fetchImage);
   await driver.wait(scrollToBottomCompleted(), timeouts.scrollToBottom);