Fix #725, partial filename match for downloading fails

The failure is actually during the comparison - the file was being
saved using the partial name the author gave in the Step when
it should have been using the file's downloaded name.

Note that our current download method will, in ideal cases, let an
author re-name the downloaded file. Since it might not be
consistent, we might still avoid implementing this at this point.
Also, in ideal cases, the Step does already continue without
waiting for the whole file to download, which addresses #492. That
happened a while ago. It's worth considering closing that issue at
this point. We have to research under what circumstances a file
would fail to download with a `fetch` while still being able to
download with a click. Maybe with an encrypted interview.

Co-authored-by: ethanstrominger <ethanstrominger2@gmail.com>

.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, filename_matcher ) => {
       /* 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, "${ filename_matcher }")]`);
 
-      let msg = `"${ filename }" seems to be missing. Cannot find a link to that document.`;
+      let msg = `"${ filename_matcher }" 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: filename_matcher
+        });
+
         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 "${ filename_matcher }" using fetch (${ err_msg }). ALKiln will now fallback to the click download method.` });
+        scope.toDownload = filename_matcher;
         // 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": {