test(download): check for the modal before using it, and click the menu item

Three review points.

attachExcelFile read '.clr-abort-modal .modal-title' unconditionally, so an
upload that raised no abort modal failed on the lookup, and the abort branch
then left the modal open for the next command to click through.  It asks the body
first: no abort modal means confirm the upload; an abort modal means assert its
title is not an Abort Message and stop there.

openDownloadModal clicked the deepest element holding the text.  The click
handler sits on the div[clrDropdownItem], so the item is selected by that
attribute and its text.

The spec list comment claimed download leaves nothing for later specs to trip
over.  It seeds the approval it needs and that approval stays, which the comment
now says.

.gitea/scripts/cypress-specs.sh

@@ -14,16 +14,19 @@
 # known green keeps its shape. metanav is deliberately absent - see
 # https://git.datacontroller.io/dc/dc/issues/395.
 #
-# download is held out for a different reason - its download button is covered by
-# a fixed-position element, see
-# https://git.datacontroller.io/dc/dc/issues/398.
+# download was held out while its selectors were stale - see
+# https://git.datacontroller.io/dc/dc/issues/398. Its first case uploads a fixture
+# to mpe_x_test and submits it, so it creates the approval it downloads instead of
+# depending on what earlier specs leave behind - but the approval stays behind.
+# It is appended because a spec added later goes on the end rather than reordering
+# the sequence that was known green.
 set -euo pipefail
 
-SPECS="csv-limited hook-programs liveness editor excel-multi-load excel csv filtering filter-panel licensing viewer-labels full-table-search viewbox stage browser-info ddl-export row-cell-limits derived-variables startupservice-launcher coltype-delete-record submitted-details embed-va"
+SPECS="csv-limited hook-programs liveness editor excel-multi-load excel csv filtering filter-panel licensing viewer-labels full-table-search viewbox stage browser-info ddl-export row-cell-limits derived-variables startupservice-launcher coltype-delete-record submitted-details embed-va download"
 
 # Specs that exist but are held out of CI, each with the reason. Listed here so
 # the coverage check below can tell a deliberate exclusion from a forgotten file.
-EXCLUDED="metanav download"
+EXCLUDED="metanav"
 
 case "${1:-}" in
   all)

client/cypress/e2e/download.cy.ts

@@ -40,18 +40,37 @@ context('download files test: ', function () {
   })
 
   it('1 | downloads audit file', (done) => {
-    visitPage('approve/toapprove')
+    /* The approve grid renders a download button per table that is waiting for
+      approval, and a freshly seeded estate has none, so create one: upload a
+      fixture to the test table and submit it from the stage screen.  The button
+      carries the table id, which the stage URL also carries, so the assertion
+      names the row this test created rather than whichever row comes first.
+      The screen is at review/approve; approve/toapprove is a route from before
+      the review module was namespaced and renders the 404 page. */
+    openTableFromTree(libraryToOpenIncludes, 'mpe_x_test')
 
-    cy.get('.app-loading', { timeout: longerCommandTimeout })
-      .should('not.exist')
-      .then(() => {
-        cy.get('.btn.btn-success')
-          .should('be.visible')
-          .then((buttons) => {
-            buttons[0].click()
-            const id = buttons[0].id
+    attachExcelFile('regular_excel.xlsx')
 
-            checkForFileDownloaded(id, 'zip', () => done())
+    submitExcel()
+
+    cy.url({ timeout: longerCommandTimeout })
+      .should('include', '/stage/')
+      .then((url) => {
+        const tableId = url.split('/stage/')[1].split(/[?#]/)[0]
+
+        visitPage('review/approve')
+
+        cy.get('.app-loading', { timeout: longerCommandTimeout })
+          .should('not.exist')
+          .then(() => {
+            cy.get(`[aria-label="Download audit file"][id="${tableId}"]`, {
+              timeout: longerCommandTimeout
+            })
+              .should('have.length', 1)
+              .first()
+              .click()
+
+            checkForFileDownloaded(tableId, 'zip', () => done())
           })
       })
   })
@@ -83,7 +102,7 @@ context('download files test: ', function () {
 
     openDownloadModal(() => {
       cy.get('select')
-        .select('Excel')
+        .select('EXCEL')
         .then(() => {
           cy.get('.btn.btn-sm.btn-success-outline').then((button) => {
             button.trigger('click')
@@ -103,7 +122,7 @@ context('download files test: ', function () {
 
     openDownloadModal(() => {
       cy.get('select')
-        .select('SAS Datalines')
+        .select('SAS')
         .then(() => {
           cy.get('.btn.btn-sm.btn-success-outline').then((button) => {
             button.trigger('click')
@@ -123,7 +142,7 @@ context('download files test: ', function () {
 
     openDownloadModal(() => {
       cy.get('select')
-        .select('SAS DDL')
+        .select('SAS_DDL')
         .then(() => {
           cy.get('.btn.btn-sm.btn-success-outline').then((button) => {
             button.trigger('click')
@@ -143,7 +162,7 @@ context('download files test: ', function () {
 
     openDownloadModal(() => {
       cy.get('select')
-        .select('TSQL DDL')
+        .select('TSQL_DDL')
         .then(() => {
           cy.get('.btn.btn-sm.btn-success-outline').then((button) => {
             button.trigger('click')
@@ -163,7 +182,7 @@ context('download files test: ', function () {
 
     openDownloadModal(() => {
       cy.get('select')
-        .select('PGSQL DDL')
+        .select('PGSQL_DDL')
         .then(() => {
           cy.get('.btn.btn-sm.btn-success-outline').then((button) => {
             button.trigger('click')
@@ -209,18 +228,22 @@ const checkForFileDownloaded = (
 }
 
 const openDownloadModal = (callback?: any) => {
-  cy.get('.btn.btn-sm.btn-outline.filterSide.dropdown-toggle')
+  /* filterSide is the viewer's options dropdown; it no longer carries a
+    dropdown-toggle class - see filter-panel.cy.ts and row-cell-limits.cy.ts.
+    Its entries are div[clrDropdownItem], not buttons, so select Download by its
+    text the way filter-panel.cy.ts selects Filter. */
+  cy.get('.btn.btn-sm.btn-outline.filterSide', {
+    timeout: longerCommandTimeout
+  })
     .click()
     .then(() => {
-      cy.get('clr-dropdown-menu button').then((buttons) => {
-        for (let button of buttons) {
-          if (button.innerText.toLowerCase().includes('download')) {
-            button.click()
+      /* the click handler is on the clrDropdownItem element, not on the span
+        holding the text */
+      cy.contains('clr-dropdown-menu [clrDropdownItem]', 'Download', {
+        timeout: longerCommandTimeout
+      }).click()
 
-            if (callback) callback()
-          }
-        }
-      })
+      if (callback) callback()
     })
 }
 
@@ -259,3 +282,39 @@ const openTableFromTree = (libNameIncludes: string, tablename: string) => {
       })
     })
 }
+
+const fixturePath = 'excels'
+
+/**
+ * Uploads a fixture to the open table.  attachFile drives the editor's file
+ * input directly, so no click on the hidden control is needed and no positional
+ * selector is involved.
+ */
+const attachExcelFile = (excelFilename: string) => {
+  cy.get('input[type="file"]#file-upload')
+    .should('have.length', 1)
+    .attachFile(`/${fixturePath}/${excelFilename}`)
+
+  /* an upload raises a confirmation modal; a failure raises the abort modal
+    instead.  Ask the body which one is there rather than waiting on a modal that
+    may never appear, and fail loudly on the abort rather than clicking through
+    the overlay. */
+  cy.get('body').then(($body) => {
+    const abortModal = $body.find('.clr-abort-modal').length > 0
+
+    if (abortModal) {
+      cy.get('.clr-abort-modal .modal-title')
+        .invoke('text')
+        .should('not.include', 'Abort Message')
+      return
+    }
+
+    cy.get('.modal-footer .btn.btn-primary').click()
+  })
+}
+
+const submitExcel = () => {
+  cy.get('.buttonBar button.preview-submit', {
+    timeout: longerCommandTimeout
+  }).click()
+}

client/src/app/deploy/sections/automatic/automatic.component.html

@@ -8,20 +8,6 @@
       @if (autodeployDone) {
         <p class="m-0 align-self-start">Done</p>
         <hr class="w-100" />
-        @if (autoDeployStatus.deployServicePack !== null) {
-          <div class="deploy-status-row">
-            @if (autoDeployStatus.deployServicePack === true) {
-              <clr-icon
-                class="deploy-success"
-                shape="success-standard"
-              ></clr-icon>
-            }
-            @if (!autoDeployStatus.deployServicePack === false) {
-              <clr-icon class="deploy-error" shape="times-circle"></clr-icon>
-            }
-            <p>Deploy SAS Jobs</p>
-          </div>
-        }
         @if (autoDeployStatus.runMakeData !== null) {
           <div class="deploy-status-row">
             @if (autoDeployStatus.runMakeData) {
@@ -33,16 +19,13 @@
             @if (autoDeployStatus.runMakeData === false) {
               <clr-icon class="deploy-error" shape="times-circle"></clr-icon>
             }
-            <p>Create database</p>
+            <p>Create control library</p>
           </div>
         }
         <hr class="w-100" />
         <div class="buttons">
           <button (click)="navigateToHome()" class="btn btn-primary mt-15 mr-0">
-            @if (
-              autoDeployStatus.deployServicePack === false ||
-              autoDeployStatus.runMakeData === false
-            ) {
+            @if (autoDeployStatus.runMakeData === false) {
               <clr-icon class="deploy-error" shape="times-circle"></clr-icon>
             }
             LAUNCH
@@ -78,9 +61,9 @@
 
 <h4 class="text-center my-15">Viya Deploy</h4>
 <p class="deploy-intro">
-  Configure and deploy Data Controller to your Viya environment. Deployment
-  creates the backend SAS services and the control library used by the
-  application.
+  Configure Data Controller for your Viya environment. The frontend and services
+  are already in place; this runs the one-off job that creates the control
+  library and its configuration data, then removes itself.
 </p>
 <hr />
 
@@ -248,19 +231,8 @@
   </div>
 }
 
-<!-- Keeping this for a reference in case future VIYA changes and starts allowing separate backend and frontend) -->
-
-<!-- <clr-checkbox-wrapper>
-  <input
-    clrCheckbox
-    [(ngModel)]="recreateDatabase"
-    (click)="recreateDatabaseClicked($event)"
-    type="checkbox"
-    checked
-    />
-  <label>Recreate database</label>
-</clr-checkbox-wrapper> -->
-
+<!-- Runs the one-off makedata job. This is the screen's only action: the
+     frontend and services are already deployed. -->
 <hr />
 
 <button
@@ -269,53 +241,3 @@
 >
   Deploy
 </button>
-
-<!-- Keeping this for a reference in case future VIYA changes and starts allowing separate backend and frontend) -->
-
-<!-- <button
-(click)="executeJson()"
-class="btn-autodeploy btn btn-primary d-inline-block mr-10"
-[disabled]="!jsonFile"
->
-Deploy {{ !jsonFile ? '(json file is not available)' : '' }}
-</button> -->
-
-<!-- <button
-(click)="uploadJsonAuto.click()"
-class="btn-autodeploy btn btn-primary d-inline-block mr-10"
->
-Upload different file to deploy
-</button>
-<input
-  #uploadJsonAuto
-  type="file"
-  hidden
-  (click)="clearUploadInput($event)"
-  (change)="onJsonFileChange($event)"
-  /> -->
-
-<clr-modal [(clrModalOpen)]="recreateDatabaseModal" [clrModalClosable]="false">
-  <h3 class="modal-title">Warning</h3>
-  <div class="modal-body">
-    This action will recreate the database (if it exists). For an initial
-    deployment, this is expected. If this is a re-deployment, you will lose any
-    existing tables in <strong>{{ dcPath }}</strong
-    >.
-  </div>
-  <div class="modal-footer">
-    <button
-      type="button"
-      class="btn btn-sm btn-primary"
-      (click)="recreateDatabaseModal = false; recreateDatabase = false"
-    >
-      Cancel
-    </button>
-    <button
-      type="button"
-      class="btn btn-sm btn-primary"
-      (click)="recreateDatabaseModal = false; recreateDatabase = true"
-    >
-      Ok
-    </button>
-  </div>
-</clr-modal>

client/src/app/deploy/sections/automatic/automatic.component.spec.ts

@@ -0,0 +1,153 @@
+import { SASjsConfig } from '@sasjs/adapter'
+import { AutomaticComponent } from './automatic.component'
+
+/**
+ * The Viya setup screen has one job: run `makedata` with the selections the
+ * form collected - the admin group, the DC path, and the compute context the
+ * job runs in. These tests pin that payload and the new-window URL, because a
+ * screen whose only action silently stopped calling the service would still
+ * compile, still render, and still look like it worked.
+ */
+describe('AutomaticComponent (Viya setup)', () => {
+  const build = () => {
+    const requests: any[][] = []
+
+    const sasJs: any = {
+      request: (...args: any[]) => {
+        requests.push(args)
+        return Promise.resolve({ result: [{}] })
+      }
+    }
+
+    const sasService: any = {
+      getSasjsConfig: () => new SASjsConfig(),
+      getExecutionPath: () => '/SASJobExecution/',
+      shouldLogin: { next: () => undefined },
+      viyaMakedataSuccessfull: () => Promise.resolve(true),
+      updateFileContent: () => Promise.resolve()
+    }
+
+    const component = new AutomaticComponent(
+      {
+        showAbortModal: () => undefined,
+        showInfoModal: () => undefined
+      } as any,
+      { downloadFile: () => undefined } as any,
+      sasService,
+      {} as any,
+      { error: () => undefined } as any,
+      {} as any
+    )
+
+    component.sasJs = sasJs
+    const config = {
+      contextName: 'Compute Reusable',
+      serverUrl: 'https://viya.example.com',
+      appLoc: '/Users/demo/dc'
+    } as any
+    component.sasJsConfig = config
+    // The new-window URL is built from the adapter config the component holds,
+    // while the request path reads the @Input one.
+    component.sasjsConfig = config
+    component.appLoc = '/Users/demo/dc'
+    component.dcPath = '/export/dc'
+    component.selectedAdminGroup = 'viyagroup18'
+
+    return { component, requests, sasService }
+  }
+
+  it('posts makedata with the admin group and the DC path', async () => {
+    const { component, requests } = build()
+    component.deployInNewWindow = false
+
+    await component.createDatabase()
+
+    expect(requests[0][0]).toBe('services/admin/makedata')
+    expect(requests[0][1]).toEqual({
+      fromjs: [{ ADMIN: 'viyagroup18', DCPATH: '/export/dc' }]
+    })
+  })
+
+  it('runs the job in the configured context, on the web method', async () => {
+    const { component, requests } = build()
+    component.deployInNewWindow = false
+
+    await component.createDatabase()
+
+    expect(requests[0][2]).toEqual({
+      useComputeApi: null,
+      contextName: 'Compute Reusable',
+      debug: true
+    })
+  })
+
+  it('prefers the context chosen in the form over the configured one', async () => {
+    const { component, requests } = build()
+    component.deployInNewWindow = false
+    component.computeContexts = [
+      { id: 'ctx-1', name: 'Selected Context' } as any
+    ]
+    component.selectedComputeContext = 'ctx-1'
+
+    await component.createDatabase()
+
+    expect(requests[0][2].contextName).toBe('Selected Context')
+  })
+
+  it('opens makedata in a new window with the selections as URL parameters', () => {
+    const { component } = build()
+    const opened: string[] = []
+    spyOn(window, 'open').and.callFake((url?: any) => {
+      opened.push(String(url))
+      return null
+    })
+    spyOn(component as any, 'pollMakedataCompletion')
+    component.deployInNewWindow = true
+
+    component.createDatabase()
+
+    expect(opened.length).toBe(1)
+    expect(opened[0]).toContain('/Users/demo/dc/services/admin/makedata')
+    expect(opened[0]).toContain('_contextname=Compute%20Reusable')
+    expect(opened[0]).toContain('admin=viyagroup18')
+    expect(opened[0]).toContain('dcpath=%2Fexport%2Fdc')
+    expect(opened[0]).toContain('_debug=131')
+  })
+
+  it('percent-encodes the values that need it in the new-window URL', () => {
+    // A context name or admin group can hold characters that would otherwise
+    // split the query string - an ampersand would start a new parameter and a
+    // space would arrive truncated.
+    const { component } = build()
+    const opened: string[] = []
+    spyOn(window, 'open').and.callFake((url?: any) => {
+      opened.push(String(url))
+      return null
+    })
+
+    component.runMakedataInNewWindow({
+      contextName: 'SAS Job Execution & co',
+      admin: 'group 1+2',
+      dcPath: '/export/dc data'
+    })
+
+    expect(opened[0]).toContain('_contextname=SAS%20Job%20Execution%20%26%20co')
+    expect(opened[0]).toContain('admin=group%201%2B2')
+    expect(opened[0]).toContain('dcpath=%2Fexport%2Fdc%20data')
+    // the parameter names themselves are not encoded
+    expect(opened[0]).toContain('&admin=group%201%2B2')
+  })
+
+  it('starts the job when deploy is pressed', () => {
+    // The screen's only button calls this; a version where it stopped doing so
+    // would leave the setup page unable to do anything at all.
+    const { component } = build()
+    const spy = spyOn(component, 'createDatabase').and.returnValue(
+      Promise.resolve() as any
+    )
+
+    component.runAutoDeploy()
+
+    expect(spy).toHaveBeenCalled()
+  })
+})

client/src/app/deploy/sections/automatic/automatic.component.ts

@@ -51,17 +51,8 @@ export class AutomaticComponent implements OnInit {
 
   public selectedComputeContext: string = ''
   public makeDataResponse: string = ''
-  public jsonFile: any = null
   public autodeploying: boolean = false
   public autodeployDone: boolean = false
-  public recreateDatabaseModal: boolean = false
-  public isSubmittingJson: boolean = false
-  public isJsonSubmitted: boolean = false
-  /**
-   * Default was `false` when deploy was done with frontend and backend separately.
-   * Now we are using only streaming app, so we always want to recreate database (makedata)
-   */
-  public recreateDatabase: boolean = true
   public createDatabaseLoading: boolean = false
   public adminGroupsLoading: boolean = false
   public currentUserInfoLoading: boolean = false
@@ -86,18 +77,14 @@ export class AutomaticComponent implements OnInit {
   public currentUserInfo: ViyaApiCurrentUser | null = null
   public computeContexts: ComputeContextItem[] = []
 
-  /** autoDeployStatus
-   * This object presents the status for two steps that we have for deploy.
-   * `deployServicePack` - Creating services based on `viya.json`
-   * `runMakeData` - Running `makedata` service
-   * If any of them is `null` or `false` that means step failed
-   * and will be shown to user on deploy done modal.
+  /**
+   * Status of the one step this screen runs: `makedata`, which creates the
+   * control library and its configuration data and then deletes itself.
+   * `null` means it has not run yet, `false` that it failed.
    */
   public autoDeployStatus: {
-    deployServicePack: any
     runMakeData: any
   } = {
-    deployServicePack: null,
     runMakeData: null
   }
 
@@ -420,61 +407,15 @@ export class AutomaticComponent implements OnInit {
   }
 
   /**
-   * Executes sas.json file to deploy the backend
-   * Method will first try to run the `auto deploy`
-   * If that fails the rest of the code is ignored.
-   * If request is successfull, method will continue to try
-   * to create database if checkbox is toggled on
+   * Runs the deployment. The control library and its configuration data are
+   * created by `makedata`, which self-deletes once it has run; the frontend and
+   * services arrived with the sasjs deploy, so there is nothing else to deploy
+   * from here.
    */
-  public async executeJson() {
-    this.isSubmittingJson = true
-
-    try {
-      let uploadJsonFile = await this.sasJs.deployServicePack(
-        this.jsonFile,
-        this.dcAdapterSettings?.appLoc,
-        undefined,
-        undefined,
-        true
-      )
-
-      this.autoDeployStatus.deployServicePack = true
-      this.isJsonSubmitted = true
-    } catch (ex: any) {
-      let textEx = ''
-
-      if (typeof ex.message !== 'string') {
-        textEx = JSON.stringify(ex).replace(/\\/gm, '')
-      } else {
-        textEx = ex.message
-      }
-
-      this.autoDeployStatus.deployServicePack = false
-      this.eventService.showInfoModal(
-        'Deploy error',
-        `Exception: \n ${textEx !== '' ? textEx : ex}`
-      )
-      this.autodeploying = false
-      this.autodeployDone = false
-
-      return
-    }
-
-    this.isSubmittingJson = false
-  }
-
-  public async runAutoDeploy(executeJson: boolean = false) {
+  public runAutoDeploy() {
     if (!this.deployInNewWindow) this.autodeploying = true
 
-    if (executeJson) {
-      this.executeJson()
-    }
-
-    if (this.recreateDatabase) {
-      this.createDatabase()
-    } else {
-      if (!this.deployInNewWindow) this.autodeployDone = true
-    }
+    this.createDatabase()
   }
 
   /**
@@ -578,9 +519,9 @@ export class AutomaticComponent implements OnInit {
     let serverUrl = this.sasjsConfig.serverUrl
     let appLoc = this.sasjsConfig.appLoc
     const execPath = this.sasService.getExecutionPath()
-    let contextname = `&_contextname=${params.contextName}`
-    let admin = `&admin=${params.admin}`
-    let dcPath = `&dcpath=${params.dcPath}`
+    let contextname = `&_contextname=${encodeURIComponent(params.contextName)}`
+    let admin = `&admin=${encodeURIComponent(params.admin)}`
+    let dcPath = `&dcpath=${encodeURIComponent(params.dcPath)}`
     // Debug is ALWAYS enabled for makedata — it runs only once during
     // deployment and the full log is needed to diagnose any issues.
     let debug = '&_debug=131'
@@ -708,22 +649,6 @@ export class AutomaticComponent implements OnInit {
     this.deployService.downloadFile(content, filename, extension)
   }
 
-  public async onJsonFileChange(event: any) {
-    let file = event.target.files[0]
-
-    this.jsonFile = await this.deployService.readFile(file)
-  }
-
-  public recreateDatabaseClicked(event: Event) {
-    ;(<HTMLInputElement>event.target).checked === true
-      ? (this.recreateDatabaseModal = true)
-      : ''
-  }
-
-  public clearUploadInput(event: Event) {
-    this.deployService.clearUploadInput(event)
-  }
-
   public openSasRequestsModal() {
     this.eventService.openRequestsModal()
   }

client/src/app/deploy/sections/sasjs-configurator/sasjs-configurator.component.spec.ts

@@ -0,0 +1,88 @@
+import { ServerType } from '@sasjs/utils/types/serverType'
+import { SasjsConfiguratorComponent } from './sasjs-configurator.component'
+
+/**
+ * The SAS9 / SASjs Server setup screen cannot deploy anything: it builds the
+ * one-off `makedata` URL and opens it, with the admin group and the directory
+ * the control library will live in. These tests pin the URL shape, because a
+ * wrong parameter name or a lost encode still produces a page that opens and
+ * merely fails inside SAS, where nobody is looking.
+ */
+describe('SasjsConfiguratorComponent (SAS9 / SASjs setup)', () => {
+  const build = (serverType: ServerType) => {
+    const sasService: any = {
+      getExecutionPath: () => '/SASStoredProcess/do',
+      sasjsMakedataChecking: () => Promise.resolve(false),
+      logout: () => undefined
+    }
+
+    const component = new SasjsConfiguratorComponent(
+      sasService,
+      { getServerInfo: () => ({ subscribe: () => undefined }) } as any,
+      { replaceState: () => undefined } as any
+    )
+
+    component.sasJsConfig = {
+      serverType,
+      serverUrl: 'https://sas9.example.com',
+      appLoc: '/User Folders/dc'
+    } as any
+    component.dcDirectory = '/tmp/DataController'
+    component.dcAdminGroup = 'DCADMINS'
+
+    const opened: string[] = []
+    spyOn(window, 'open').and.callFake((url?: any) => {
+      opened.push(String(url))
+      return null
+    })
+
+    return { component, opened, sasService }
+  }
+
+  it('opens makedata on SAS9 with the admin group and the directory', () => {
+    const { component, opened } = build(ServerType.Sas9)
+
+    component.makeData()
+
+    expect(opened.length).toBe(1)
+    expect(opened[0]).toContain(
+      '_program=/User Folders/dc/services/admin/makedata'
+    )
+    expect(opened[0]).toContain('admin=DCADMINS')
+    expect(opened[0]).toContain('path=%2Ftmp%2FDataController')
+    expect(opened[0]).toContain('_debug=131')
+  })
+
+  it('stays on the page origin for SAS9 rather than the configured server', () => {
+    const { component, opened } = build(ServerType.Sas9)
+
+    component.makeData()
+
+    // A SAS9 deployment is reached through the URL the user is already on; the
+    // configured serverUrl would point the browser at a host it may not reach.
+    expect(opened[0].startsWith(window.location.origin)).toBe(true)
+    expect(opened[0]).not.toContain('sas9.example.com')
+  })
+
+  it('keeps the directory readable for SASjs Server', () => {
+    const { component, opened } = build(ServerType.Sasjs)
+
+    component.makeData()
+
+    // The SASjs branch does not encode the path, so a plain directory stays as
+    // written; encoding it there would make makedata look for a folder with %2F
+    // in its name.
+    expect(opened[0]).toContain('path=/tmp/DataController')
+  })
+
+  it('checks the SASjs Server deployment before reloading the app', () => {
+    const { component, sasService } = build(ServerType.Sasjs)
+    const spy = spyOn(sasService, 'sasjsMakedataChecking').and.returnValue(
+      Promise.resolve(false)
+    )
+
+    component.makeData()
+
+    expect(spy).toHaveBeenCalled()
+  })
+})

client/src/app/editor/editor.component.ts

@@ -3647,7 +3647,16 @@ export class EditorComponent
     if (clauses.length === 0) return false
 
     try {
-      const res: any = await this.sasStoreService.saveQuery(libds, clauses)
+      // A `va_parmap` entry is a column the report author typed, so this
+      // request can legitimately fail: the table may not have that column, and
+      // validatefilter aborts on an unknown variable (dc/dc#404). Attempting it
+      // is still worth it - when the mapping is right the very first getdata is
+      // already filtered - but the failure must not stop the page. An abort
+      // modal here covers an editor that is about to load fine; the mapping
+      // banner names the bad entry once the real metadata arrives.
+      const res: any = await this.sasStoreService.saveQuery(libds, clauses, {
+        suppressErrorAbortModal: true
+      })
       const id = res?.result?.[0]?.FILTER_RK
       const table = res?.result?.[0]?.FILTER_TABLE
       if (id === undefined || table === undefined) return false

client/src/app/services/deploy.service.ts

@@ -26,22 +26,4 @@ export class DeployService {
       document.body.removeChild(downloadLink)
     }
   }
-
-  public readFile(file: File): Promise<any> {
-    return new Promise((resolve, reject) => {
-      let JsonFileReader = new FileReader()
-
-      JsonFileReader.onload = () => {
-        if (JsonFileReader.result) {
-          resolve(JSON.parse(JsonFileReader.result.toString()))
-        }
-      }
-
-      JsonFileReader.readAsText(file)
-    })
-  }
-
-  public clearUploadInput(event: Event) {
-    ;(<HTMLInputElement>event.target).value = ''
-  }
 }

client/src/app/services/sas-store.service.spec.ts

@@ -116,6 +116,53 @@ describe('SasStoreService', () => {
       expect(seen.length).toBe(0)
     })
   })
+
+  describe('saveQuery', () => {
+    const capture = (adapterResponse: any = { result: [] }) => {
+      const calls: any[][] = []
+      const store = new SasStoreService(
+        {
+          request: (...args: any[]) => {
+            calls.push(args)
+            return Promise.resolve({ adapterResponse })
+          }
+        } as any,
+        {} as any,
+        {} as any
+      )
+      return { store, calls }
+    }
+
+    it('posts the filter to public/validatefilter', async () => {
+      const { store, calls } = capture()
+      await store.saveQuery('DC.MY_TABLE', [
+        { VARIABLE_NM: 'A', OPERATOR_NM: '=', RAW_VALUE: "'x'" } as any
+      ])
+
+      expect(calls[0][0]).toBe('public/validatefilter')
+      expect(calls[0][1].iwant[0].filter_table).toBe('DC.MY_TABLE')
+      expect(calls[0][1].filterquery.length).toBe(1)
+    })
+
+    it('passes wrapper options through, so a rejected filter can stay quiet', async () => {
+      // The VA deferred load attempts a filter built from a va_parmap entry the
+      // table may not have, and must not cover a page that loads fine with an
+      // abort modal (dc/dc#404).
+      const { store, calls } = capture()
+      await store.saveQuery('DC.MY_TABLE', [], {
+        suppressErrorAbortModal: true
+      })
+
+      expect(calls[0][3]).toEqual({ suppressErrorAbortModal: true })
+    })
+
+    it('sends no wrapper options when the caller gives none', async () => {
+      const { store, calls } = capture()
+      await store.saveQuery('DC.MY_TABLE', [])
+
+      expect(calls[0][3]).toBeUndefined()
+    })
+  })
 })
 
 /**

client/src/app/services/sas-store.service.ts

@@ -8,6 +8,7 @@ import { HelperService } from './helper.service'
 import { QueryClause } from '../models/TableData'
 import { globals } from '../_globals'
 import { FilterClause, FilterGroup, FilterQuery } from '../models/FilterQuery'
+import { RequestWrapperOptions } from '../models/request-wrapper/RequestWrapperOptions'
 import {
   $DataFormats,
   EditorsGetDataSASResponse,
@@ -395,13 +396,30 @@ export class SasStoreService {
     ).adapterResponse
   }
 
-  public async saveQuery(libds: string, filterQuery: QueryClause[]) {
+  /**
+   * Persists a filter and returns its FILTER_RK.
+   *
+   * `wrapperOptions` reaches the request wrapper, so a caller that can live with
+   * a rejected filter (the VA deferred load, whose `va_parmap` entry may name a
+   * column the table does not have) can suppress the abort modal that would
+   * otherwise cover a page which is about to load perfectly well.
+   */
+  public async saveQuery(
+    libds: string,
+    filterQuery: QueryClause[],
+    wrapperOptions?: RequestWrapperOptions
+  ) {
     let tables = {
       iwant: [{ filter_table: libds }],
       filterquery: filterQuery
     }
 
-    const res = await this.sasService.request('public/validatefilter', tables)
+    const res = await this.sasService.request(
+      'public/validatefilter',
+      tables,
+      undefined,
+      wrapperOptions
+    )
 
     this.filter.next(res)
 

sas/mocks/sasjs/services/public/getddl.js

@@ -1,7 +1,8 @@
 let mimeType = `application/octet-stream`
 let filename = `dc-download`
 
-if (libref) filename = `${libref}.ddl`
+if (libref && ds) filename = `${libref}_${ds}.ddl`
+else if (libref) filename = `${libref}.ddl`
 
 fs.writeFile(_SASJS_WEBOUT_HEADERS, `Content-type: ${mimeType} \n Content-Disposition: attachment;filename=\"${filename}\"`, function (err) {
   if (err) throw err;

sas/package-lock.json

@@ -7,7 +7,7 @@
       "name": "dc-sas",
       "dependencies": {
         "@sasjs/cli": "6.2.0",
-        "@sasjs/core": "6.1.4"
+        "@sasjs/core": "7.0.1"
       }
     },
     "node_modules/@asamuzakjp/css-color": {
@@ -199,6 +199,12 @@
         "@sasjs/lint": "^4.3.0"
       }
     },
+    "node_modules/@sasjs/cli/node_modules/@sasjs/core": {
+      "version": "6.1.4",
+      "resolved": "https://registry.npmjs.org/@sasjs/core/-/core-6.1.4.tgz",
+      "integrity": "sha512-vIF2LIftxPWg4CvO3i5raEfrHFHY/6S+MqVGpjRR71dgjLRJEYNyPJgGqD6BCZwHv5V6/2nDnBi/BIIMiLXfzA==",
+      "license": "MIT"
+    },
     "node_modules/@sasjs/cli/node_modules/@sasjs/utils": {
       "version": "4.1.0",
       "resolved": "https://registry.npmjs.org/@sasjs/utils/-/utils-4.1.0.tgz",
@@ -235,9 +241,9 @@
       }
     },
     "node_modules/@sasjs/core": {
-      "version": "6.1.4",
-      "resolved": "https://registry.npmjs.org/@sasjs/core/-/core-6.1.4.tgz",
-      "integrity": "sha512-vIF2LIftxPWg4CvO3i5raEfrHFHY/6S+MqVGpjRR71dgjLRJEYNyPJgGqD6BCZwHv5V6/2nDnBi/BIIMiLXfzA==",
+      "version": "7.0.1",
+      "resolved": "https://registry.npmjs.org/@sasjs/core/-/core-7.0.1.tgz",
+      "integrity": "sha512-M75475GuOMTmbmrB8V/Hgcwm2j3PdIHlgmkbjtT88ALSQuzRPBAjEK2IDDYIUn6rsVJG9aZWvUpRV7fzrOjK/w==",
       "license": "MIT"
     },
     "node_modules/@sasjs/lint": {

sas/package.json

@@ -34,7 +34,7 @@
   "private": true,
   "dependencies": {
     "@sasjs/cli": "6.2.0",
-    "@sasjs/core": "6.1.4"
+    "@sasjs/core": "7.0.1"
   },
   "overrides": {
     "nanoid": "3.3.18"

sas/sasjs/macros/mpe_execute.sas

@@ -152,7 +152,10 @@
     %mp_abort(
       iftrue=(1=1)
       ,mac=&sysmacroname
-      ,msg=%str(&program did not complete: state=&state &msg)
+      /* %bquote, not %str: the child's message is interpolated here, and a
+        comma in it would otherwise split this call into extra positional
+        parameters before mp_abort ever sees it */
+      ,msg=%bquote(&program did not complete: state=&state &msg)
     )
   %end;
 %end;

sas/sasjs/services/editors/restore.test.2.sas

@@ -0,0 +1,172 @@
+/**
+  @file
+  @brief testing restore of a change to a special missing value
+
+  @details A cell holding a special missing (._ .A-.Z) and changed to an
+    ordinary number must be reverted to that special missing. mp_stripdiffs
+    writes the old value into a generated UPDATE statement, and PUT drops the
+    leading period of a special missing, so the statement used to be
+    "set SOME_BESTNUM =_" - which PROC SQL reads as a column name.
+
+  <h4> SAS Macros </h4>
+  @li mp_assert.sas
+  @li mx_execute.sas
+
+**/
+
+%let _program=&appLoc/services/editors/restore;
+
+/* seed the base table with a special missing - the audit stores whatever the
+  base holds, so this is the value the revert has to write back */
+proc sql noprint;
+update &dclib..mpe_x_test set some_bestnum=._
+  where primary_key_field=3;
+quit;
+
+/**
+  * load 1 - change that cell to an ordinary number
+  */
+data work.sascontroltable;
+  action='LOAD';
+  message="special missing prep";
+  libds="&dclib..MPE_X_TEST";
+  output;
+  stop;
+run;
+
+data work.jsdata;
+  set &dclib..mpe_x_test(where=(primary_key_field=3)
+    rename=(some_date=dt2 SOME_DATETIME=dttm2 SOME_TIME=tm2)
+  );
+  /* the adapter sends these as strings */
+  some_date=put(dt2,date9.);
+  SOME_DATETIME=put(dttm2,datetime19.);
+  some_time=put(tm2,time.);
+  drop dt2 dttm2 tm2;
+  _____DELETE__THIS__RECORD_____='No';
+  some_bestnum=42;
+run;
+
+%mx_execute(&appLoc/services/editors/stagedata,
+  viyacontext=&defaultcontext,
+  inputdatasets=work.jsdata work.sascontroltable,
+  outlib=web1,
+  mdebug=&sasjs_mdebug
+)
+
+%let status=0;
+data _null_;
+  set web1.sasparams;
+  if status='SUCCESS' then call symputx('status',1);
+  call symputx('dsid',dsid);
+run;
+%mp_assert(
+  iftrue=(&status=1 and &syscc=0),
+  desc=Checking the special missing change was staged,
+  outds=work.test_results
+)
+
+data work.sascontroltable;
+  ACTION='APPROVE_TABLE';
+  TABLE="&dsid";
+  DIFFTIME="%sysfunc(datetime())";
+  output;
+  stop;
+run;
+%mx_execute(&appLoc/services/auditors/postdata,
+  viyacontext=&defaultcontext,
+  inputdatasets=work.sascontroltable,
+  outlib=web3,
+  outref=wb3,
+  mdebug=&sasjs_mdebug
+)
+
+%let status=0;
+data _null_;
+  set web3.apparams;
+  if response='SUCCESS!' then call symputx('status',1);
+run;
+%mp_assert(
+  iftrue=(&status=1 and &syscc=0),
+  desc=Checking the change was approved,
+  outds=work.test_results
+)
+
+/* the load reference to revert */
+%let loadref=0;
+data _null_;
+  set &dclib..mpe_submit end=last;
+  if last then call symputx('loadref',table_id);
+run;
+
+/**
+  * revert it - the old value in the diff is the special missing
+  */
+data work.restore_in;
+  load_ref="&loadref";
+  output;
+  stop;
+run;
+%mx_execute(&_program,
+  viyacontext=&defaultcontext,
+  inputdatasets=work.restore_in,
+  outlib=web1,
+  mdebug=&sasjs_mdebug
+)
+
+%let newref=0;
+data _null_;
+  set web1.restore_out;
+  call symputx('newref',loadref);
+run;
+%mp_assert(
+  iftrue=(&newref ne 0 and &syscc=0),
+  desc=Checking the revert was submitted,
+  outds=work.test_results
+)
+
+/* approve the reversion */
+data work.sascontroltable;
+  ACTION='APPROVE_TABLE';
+  TABLE="&newref";
+  DIFFTIME="%sysfunc(datetime())";
+  output;
+  stop;
+run;
+%mx_execute(&appLoc/services/auditors/postdata,
+  viyacontext=&defaultcontext,
+  inputdatasets=work.sascontroltable,
+  outlib=web3,
+  outref=wb3,
+  mdebug=&sasjs_mdebug
+)
+
+%let status=0;
+data _null_;
+  set web3.apparams;
+  if response='SUCCESS!' then call symputx('status',1);
+run;
+%mp_assert(
+  iftrue=(&status=1 and &syscc=0),
+  desc=Checking the reversion was approved,
+  outds=work.test_results
+)
+
+/* and the cell is back to the special missing, not a number and not blank */
+%let spmcheck=0;
+data _null_;
+  set &dclib..mpe_x_test(where=(primary_key_field=3));
+  if some_bestnum=._ then call symputx('spmcheck',1);
+  putlog some_bestnum=;
+run;
+%mp_assert(
+  iftrue=(&spmcheck=1),
+  desc=Checking the special missing was restored,
+  outds=work.test_results
+)
+
+/* put the fixture back as makedata left it */
+proc sql noprint;
+update &dclib..mpe_x_test set some_bestnum=44
+  where primary_key_field=3;
+quit;

sas/sasjs/services/editors/stagedata.test.4.sas

@@ -594,6 +594,9 @@ run;
   inputdatasets=work.sascontroltable work.jsdata,
   outlib=web8,
   outref=wb8,
+  /* the child is expected to abort here, so the failure must be asserted
+    rather than aborting this program */
+  failfast=0,
   mdebug=&sasjs_mdebug
 )
 

sas/sasjs/services/hooks/mpe_security_postedit.sas

@@ -35,5 +35,7 @@ run;
 
 %mp_abort(iftrue=(&errval=1)
   ,mac=mpe_security_postedit.sas
-  ,msg=%str(&errmsg)
+  /* %bquote: the submitted value is interpolated into this message, and a
+    comma in it would split this call into extra positional parameters */
+  ,msg=%bquote(&errmsg)
 )