Merge pull request #959 from SuffolkLITLab/725_download_match

Fix #725, partial filename match for downloading fails

.github/workflows/github_server.yml

@@ -52,8 +52,8 @@ jobs:
       EMPTY_STRING: ""
       NORMAL_USER_EMAIL: alkiln@example.com
       NORMAL_USER_PASSWORD: User123^
-      WRONG_EMAIL=wrong_email@example.com
-      WRONG_PASSWORD=wrong_password
+      WRONG_EMAIL: wrong_email@example.com
+      WRONG_PASSWORD: wrong_password
 
     steps:
       # Place the root directory in this branch to access
@@ -160,7 +160,7 @@ jobs:
           #### Developer note: You can probably leave the rest out
           #### To learn more, see https://assemblyline.suffolklitlab.org/docs/alkiln/writing/#optional-inputs
           ALKILN_TAG_EXPRESSION: "${{ env.ALKILN_TAG_EXPRESSION }}"
-          ALKILN_VERSION: delete
+          ALKILN_VERSION: download
 
       #### Developer note: Example of making an issue when tests fail
       #### that includes the text of the failure output file

.github/workflows/playground.yml

@@ -47,8 +47,8 @@ jobs:
       NORMAL_USER_PASSWORD: ${{ secrets.USER_NO_PERMISSIONS_PASSWORD }}
       NORMAL_USER_API_KEY: ${{ secrets.USER_NO_PERMISSIONS_API_KEY }}
       INVALID_API_KEY: invalidAPIkey
-      WRONG_EMAIL=wrong_email@example.com
-      WRONG_PASSWORD=wrong_password
+      WRONG_EMAIL: wrong_email@example.com
+      WRONG_PASSWORD: wrong_password
       EMPTY_STRING: ""
       SECRET_VAR1: secret-var1-value
       SECRET_VAR2: secret-var2-value
@@ -81,4 +81,4 @@ jobs:
           # want to check up on this.
           ALKILN_TAG_EXPRESSION: "${{ env.ALKILN_TAG_EXPRESSION }}"
           #### Developer note: You can probably leave this out
-          ALKILN_VERSION: delete
+          ALKILN_VERSION: download

CHANGELOG.md

@@ -45,14 +45,20 @@ Format:
 
 ## [Unreleased]
 
+<!-- ## [5.13.2] - 2024-11-21 -->
+
+### Fixed
+- Fixes download Step unable to use a partial filename match. The Step needed the whole name, including the extension. See [#725](https://github.com/SuffolkLITLab/ALKiln/issues/725).
+
+## [5.13.1] - 2024-10-23
+
 ### Changed
 
 - Moves "expected" status error to only be visible to internal test errors. Closes [#933](https://github.com/SuffolkLITLab/ALKiln/issues/933).
 
 ### Fixed
 
 - Detects failed sign in. Closes [#918](https://github.com/SuffolkLITLab/ALKiln/issues/918).
-- Updates pdfjs-dist for node v20 and v22. See [#952](https://github.com/SuffolkLITLab/ALKiln/issues/952).
 
 ### Internal
 
@@ -63,6 +69,7 @@ Format:
 - Adds decision docs
 - Updated CONTRIBUTING.md
 - Added example.env, closes [#374](https://github.com/SuffolkLITLab/ALKiln/issues/374)
+- Updates pdfjs-dist for node v20 and v22. See [#952](https://github.com/SuffolkLITLab/ALKiln/issues/952).
 
 ## [5.13.0] - 2024-07-11
 

docassemble/ALKilnTests/data/sources/observation_steps.feature

@@ -239,19 +239,19 @@ Scenario: I compare different PDFs
   Given the final Scenario status should be "failed"
   And the Scenario report should include:
   """
-  Could not find the existing PDF at DOES_NOT_EXIST.pdf
+  ALK0156
   """
   And the Scenario report should include:
   """
-  The PDFs were not the same.
+  ALK0157
   """
-  And the Scenario report should include: 
+  And the Scenario report should include:
   """
-  The new PDF added:
+  -  diff
   """
   And the Scenario report should include:
   """
-  -  diff
+  ALK0093
   """
   Given I start the interview at "test_pdf"
   Then the question id should be "proxy vars"
@@ -268,6 +268,7 @@ Scenario: I compare different PDFs
   And I tap to continue
   # Next page
   Then the question id should be "2_signature download"
-  When I download "2_signature.pdf"
+  # Match a partial name
+  When I download "2_signatu"
   And I expect the baseline PDF "DOES_NOT_EXIST.pdf" and the new PDF "2_signature.pdf" to be the same
   And I expect the baseline PDF "linear_2_signature-Baseline.pdf" and the new PDF "2_signature.pdf" to be the same

lib/scope.js

@@ -2781,7 +2781,7 @@ module.exports = {
       await scope.afterStep(scope);
     },  // Ends scope.steps.sign()
 
-    download: async ( scope, filename ) => {
+    download: async ( scope, full_or_partial_file_href ) => {
       /* Taps the link that leads to the given filename to trigger downloading.
       *    and waits till the file has been downloaded before allowing the tests to continue.
       *    WARNING: Cannot download the same file twice in a single scenario.
@@ -2791,47 +2791,86 @@ module.exports = {
       * TODO: Properly wait for download to complete. See notes in
       * scope.js scope.detectDownloadComplete()
       */
-      let [elem] = await scope.page.$$(`xpath/.//a[contains(@href, "${ filename }")]`);
+      let [elem] = await scope.page.$$(`xpath/.//a[contains(@href, "${ full_or_partial_file_href }")]`);
 
-      let msg = `"${ filename }" seems to be missing. Cannot find a link to that document.`;
+      let msg = `"${ full_or_partial_file_href }" seems to be missing on the page. Cannot find a link to that document.`;
       if ( !elem ) { reports.addToReport(scope, { type: `error`, code: `ALK0152`, value: msg }); }
       expect( elem, msg ).to.exist;
 
       let failed_to_download = false;
       let err_msg = "";
       try {
-        const binaryStr = await scope.page.evaluate(el => {
+
+        const { disposition,  binaryStr } = await scope.page.evaluate(async function ( el ) {
           const url = el.getAttribute("href");
           return new Promise(async (resolve, reject) => {
             const response = await fetch(url, {method: "GET"});
             const reader = new FileReader();
             reader.readAsBinaryString(await response.blob());
-            reader.onload = () => resolve(reader.result);
+            // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition
+            reader.onload = () => resolve({
+              disposition: response.headers.get(`Content-Disposition`),
+              binaryStr: reader.result
+            });
             reader.onerror = () => reject(`🤕 ALK0153 ERROR: Error occurred on page when downloading ${ url }: ${ reader.error }`);
           });
         }, elem);
+
+        err_msg = `could not get the actual name of the file from the response headers.`;
+        let actual_filename = await scope.steps.get_response_filename({
+          disposition, default_name: full_or_partial_file_href
+        });
+
         if (binaryStr !== '') {
+          err_msg = `binary data for download was empty`;
           const fileData = Buffer.from(binaryStr, 'binary');
-          fs.writeFileSync(`${ scope.paths.scenario }/${ filename }`, fileData);
-          reports.addToReport(scope, { type: `row info`, code: `ALK0154`, value: `Downloaded ${ filename } (${ fileData.length } bytes) to ${ scope.paths.scenario }`});
+          fs.writeFileSync(`${ scope.paths.scenario }/${ actual_filename }`, fileData);
+          reports.addToReport(scope, { type: `row info`, code: `ALK0154`, value: `Downloaded "${ actual_filename }" (${ fileData.length } bytes) to ${ scope.paths.scenario }`});
         } else {
           failed_to_download = true;
-          err_msg = `Couldn't download ${ filename }, binary data for download was empty`;
         }
+
       } catch (error) {
         failed_to_download = true;
         err_msg = error;
       }
 
       if (failed_to_download) {
-        reports.addToReport(scope, { type: `warning`, code: `ALK0155`, value: `Could not download file using fetch (${ err_msg }). ALKiln will now fallback to the click download method.` });
-        scope.toDownload = filename;
+        reports.addToReport(scope, { type: `warning`, code: `ALK0155`, value: `Could not download a file matching the name "${ full_or_partial_file_href }" using fetch (${ err_msg }). ALKiln will now fallback to the click download method.` });
+        scope.toDownload = full_or_partial_file_href;
         // Should this be using `scope.tapElement`?
         await elem.evaluate( elem => { return elem.click(); });
         await scope.detectDownloadComplete( scope );
       }
     },  // Ends scope.steps.download()
 
+    get_response_filename: async function({ disposition, default_name=`found_no_file_name.pdf` }) {
+      /** Given a fetch response headers' Content-Disposition str, return the
+       *  filename of the response's file.
+       *
+       * @return { string | null } - Name of fetched file
+       * */
+      let filename = default_name;
+      if ( disposition ) {
+        filename = disposition.split(`;`)[1].split(`=`)[1];
+      }
+
+      // Handle potential UTF-8 encoded filenames
+      if ( filename.toLowerCase().startsWith( `utf-8''` )) {
+        filename = decodeURIComponent( filename.replace( /utf-8''/i, `` ));
+      } else {
+        // Replace starting and ending quotes if they exist
+        filename = filename.replace( /^['"]/, `` ).replace( /['"]$/, `` );
+      }
+
+      // TODO: Add debug log here
+      // console.log(`filename:`, filename);
+
+      // TODO: Add to the report if we had to use the default name
+
+      return filename;
+    },  // Ends scope.steps.get_response_filename()
+
     compare_pdfs: async function (scope, {existing_pdf_path, new_pdf_path}) {
       let existing_paths = await scope.findFiles(scope, {to_find_names: [existing_pdf_path]});
       if (existing_paths.length == 0) {
@@ -2852,7 +2891,7 @@ module.exports = {
         let removed_str = diffs.filter(part => part.removed).reduce((err_str, part) => {
           return err_str + `- ${ part.value }\n`
         }, 'The new PDF removed: \n');
-        let msg = `The PDFs were not the same.\n${ added_str }\n${removed_str}\n\n You can see the full PDFs at ${ full_existing_path } and ${ full_new_pdf_path}`;
+        let msg = `The PDFs were different.\nAdded:\n${ added_str }\nRemoved:\n${removed_str}\n\nThere might be more information if you actually look at the files. You can see the full PDFs at ${ full_existing_path } and ${ full_new_pdf_path}`;
         reports.addToReport(scope, { type: `error`, code: `ALK0157`, value: msg });
         scope.failed_pdf_compares.push(msg);
       }

lib/steps.js

@@ -1285,10 +1285,11 @@ After(async function(scenario) {
   if ( scope.failed_pdf_compares.length > 0) {
     let msg = scope.failed_pdf_compares.reduce((str, new_msg) => `${ str }\n―――\n${ new_msg }`)
     changeable_test_status = `FAILED`;
+    // TODO: This may be redundant and therefore confusing
     reports.addToReport(scope, {
       type: `error`,
       code: `ALK0093`,
-      value: `PDF comparison failed ${ scope.failed_pdf_compares.length } time(s)\n―――\n${ msg }\n―――\n`
+      value: `ALKiln ran into an error when it tried to compare ${ scope.failed_pdf_compares.length } PDF(s)\n―――\n${ msg }\n―――\n`
     });
   }
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@suffolklitlab/alkiln",
-  "version": "5.13.1",
+  "version": "5.13.1-fix-download-name",
   "description": "Integrated automated end-to-end testing with docassemble, puppeteer, and cucumber.",
   "main": "lib/index.js",
   "scripts": {