Three more places where the mock answered differently from the SAS service.
mp_abort (dcMockUtils): every mock refusal is now a sasjsAbort table
(MSG + MAC) written where the macro writes it, so the client shows the
abort modal and rejects the promise. A thrown error had become an HTTP
500 "Job execution failed" - the browser host discards the webout when a
program throws - which is unreadable to the user and hides the message in
a stack. mpAbort also stops the program, so nothing is written after a
refusal; each service that can abort catches that stop at its top level.
stagedata, loadfile, getauditfile and makedata now go through it, and the
hand-rolled sasjsAbort blocks are gone.
mp_lockanytable (dcMockUtils): a lock is a row in MPE_LOCKANYTABLE marked
LOCKED carrying the holder's job id, user and reference, exactly as the
macro records it, and a lock held by another session refuses the write
with the macro's message. postdata and rejection now lock MPE_REVIEW and
MPE_SUBMIT around each write, which is where postdata.sas and
rejection.sas take them. The macro's retry loop is collapsed to a single
attempt: the runtime is single threaded, so nothing can release a lock
while the request waits.
startupservice: saslibs is the distinct LIBREF of the permitted datasets,
which is what "create table saslibs as select distinct libref from
&syslast" produces - it was a four-column row describing the control
library, with fields the client does not read.
Tests: abort.test.js and locks.test.js join the harness suites; 25 pass,
and 9 of the 13 relevant cases fail against the base services.
Every screen reads public/startupservice, so a divergence there is a
divergence in the whole app. Three of its answers were fixed values:
xlmaps a hardcoded list of three maps, so editing MPE_XLMAP_RULES
or MPE_XLMAP_INFO changed nothing on the site
sasdatasets included the map target dataset, which the SAS service
deletes - the demo offered MPE_XLMAP_DATA as an editable
table while SAS refuses it
timezone TIMEZONE and SYSTIMEZONEIDENT pinned to Europe/London and
the offset to +0100, so a GMT session reported an hour of
DST it did not have
They now come from the same sources the SAS service reads: the current
MPE_XLMAP_RULES rows joined to MPE_XLMAP_INFO (coalescec to
<mpelib>.MPE_XLMAP_DATA), the target datasets removed from sasdatasets,
and the session zone and current offset taken from the runtime, with
TIMEZONE left blank as an unset option is.
The shared harness (sas/mocks/test/harness.js) makes the service tests
one-liners: it builds a temporary estate with every service on its drive
and runs a service file the way the runtime does, webout() included.
sas/mocks/test/startupservice.test.js covers the joins, the target
removal, closed rules and the offset.
Staging now opens on the deploy screen, and its folder picker asks
makedata for a listing. The mock ignored ACTION entirely: every request
seeded the control library, deleted makedata from the drive and replied
with the HTML success page. So the first BROWSE deployed the site and
closed the wizard the visitor was standing on, behind an "invalid Json
string" error from the adapter, and VALIDATE and SUBMIT then answered
500 "Program not found in drive".
The mock now dispatches on ACTION the way the SAS service does:
DIRLIST the folders below PARENT, one level at a time, with the
dirinfo/dirlist shape mpe_dirlist emits
WRITECHECK the checkwrite row mpe_writecheck emits, probed by creating
and removing a file
MAKEDATA the existing seeding path, unchanged
ACTION is required and refused when absent or unknown, and the two browse
actions are refused once the settings file exists, mirroring browse_gate.
Both input routes are accepted - the screen's fromjs table and a CLI
seed's SASControlTable - because %webout(FETCH) does not care which one
arrived; deploy/makedata.json now carries ACTION=MAKEDATA as the real
service requires.
sas/mocks/test/makedata-actions.test.js runs the service file itself
against a temporary estate and is wired into the build workflow.
Staging is the preview of the next release, and what it should preview first is
the deployment itself. DC routes to /deploy while makedata is still on the drive
(at <appLoc>/services/admin) and stops doing so once the wizard has run it and it
has removed itself - so a drive that carries the services and the licence but no
control library opens on the deploy screen, and the visitor deploys the control
library from the wizard into their own IndexedDB drive.
The seeding step is now gated on a tag: only the demo (which a visitor should be
able to use straight away) runs makedata before the drive is captured. The
staging path gets a guard of its own, because a seed that leaked into it would
only be visible as a site that opens on the home screen: makedata must be on the
drive and no mpe_tables.json may be there.
CONTRIBUTING updated: the two sites now ship different drives from the same
build.
The captures wrote values like OMSOBJ:SASLibrary\A59LNVZG.B500000K inside a
template literal, where \A is an escape, so the separator never reached the
client: it served OMSOBJ:SASLibraryA59LNVZG.B500000K. The same happened to
D:\MyDemo\Sankey and BUILTIN\Administrators. Converting the service to webout()
faithfully preserved the corrupted value - and the metadetails ASSOCURI, the one
field fixed by hand at conversion time, was left as the only member that kept its
separator, which is the inconsistency the review named.
Seventeen values corrected across five services:
lineage/fetchtablelineage info[0].LIBURI 1
lineage/getmetacols metacols[].COLURI 11
lineage/getmetatables PATH_SCHEMA, TABLEURI 2
public/refreshlibinfo libinfo[0].OWNERS 1
usernav/usermembersbyrole sasgroups[].URIMEM,
sasmembers[].URIMEM 2
Verified by deploying both trees on one server and comparing the parsed
responses: those seventeen fields are the only differences, and every other
member of all thirteen converted services is unchanged (metadetails' two
ASSOCURI, already carrying its separator, still differ from the capture for the
same reason).
Anything that splits a URI or a Windows path on the separator, or displays
OWNERS or PATH_SCHEMA, now sees the value the SAS metadata server returns.
3.14.0 ships @sasjs/lint 5.0.0, so the studio reports OS commands, and it is the
release the estate runs. It still declares @sasjs/utils ^4.7.0, so the envelope
formats from utils 4.8 (SYSCC as text, DATE7, TIME5, E8601DT26.6) are not yet
guaranteed by the floor - a fresh install resolves 4.8.0, a cached tree need not.
The comment says so, and the pin moves again when the floor lands.
Verified on a local 3.14.0 install (clean, resolving utils 4.8.0): the mock
estate's startupservice returns SYSDATE 04NOV26, SYSTIME 12:44, SYSCC "0" (text),
END_DTTM 2026-11-04T12:44:14.954000 (no Z). With that estate seeded, and against
a fresh app location, licensing.cy.ts passes 10/10 and metanav.cy.ts 1/1 on
Chromium.
The other nine services that answer with a captured response also assigned it to
_webout, envelope and all. They now emit their members through the runtime's
webout():
lineage/fetchtablelineage info, finalfinal, flatdata, idlookup
lineage/getmetacols metacols 11 rows
lineage/getmetatables metatables 1 row
usernav/usermembers users 2 rows
usernav/usermembersbygroup sasmembers 2 rows
usernav/usergroupsbymember groups 1 row
usernav/usermembersbyrole sasgroups 1 row, sasmembers 1 row
usernav/userroles roles 46 rows
public/refreshlibinfo libinfo 1 row
Each payload was taken from the response the old service serves, then both
trees were deployed on one server (/Public/app/dc and /Public/app/dcnew) and
compared as parsed JSON with the envelope keys excluded: all nine are equal row
for row, including the escaped values (the OMS URIs keep their backslash, and
refreshlibinfo keeps the quoted paths).
The envelope is the runtime's now, so SYSCC is numeric 0 rather than the string
"0", SYSDATE/SYSTIME are today's, SYSSITE is reported (these captures carried a
lower-cased syssite, which the client never read), and the SAS-only
_METAUSER/_METAPERSON are gone.
lineage/fetchlineage is deliberately left alone: its captured body is not valid
JSON today (bare quotes in the graphviz labels, raw newlines inside the STRING
values, and a lost backslash), so there is no payload to preserve and the
intended text needs a decision rather than a rewrite. Tracked separately.
The four metanav services assigned a captured 2022 response to _webout, envelope
and all, instead of using the runtime's webout() like every other service. The
members are unchanged:
metarepos outrepos 2 rows
metaobjects objects 1 row
metatypes types 71 rows
metadetails attributes 12 rows
associations 2 rows
Both trees were deployed on one server, at /Public/app/dc and /Public/app/dcnew,
and the responses compared as parsed JSON with the envelope keys excluded:
outrepos, objects and types are equal row for row.
metadetails changes in one place. The capture wrote the OMS URI as
OMSOBJ:Property\A59..., but \A is an escape in a JavaScript template literal,
so the backslash was consumed and the client received OMSOBJ:PropertyA59... -
a URI that cannot resolve. The member now carries the backslash the SAS
metadata server returns.
The envelope comes from the runtime now, which also drops three artefacts of
the recording: SYSCC "4" on metarepos (an error code from the captured day,
which the client reads as a SAS error), SYSDATE/SYSTIME frozen at 26SEP22, and
the SAS-only _METAUSER/_METAPERSON.
The mock services call webout(), as the server's runtime does. The browser mock
runtime that @sasjs/cli ships is what provides it there, and the 6.5.0 this repo
was pinned to does not: its runtime-core.js has no reference to the webout
prelude, and the build writes no webout-prelude.json for it to load. Every
service that calls webout() therefore throws "ReferenceError: webout is not
defined" in a published mock site while the build itself reports success.
6.9.0 is the first release that ships the prelude; this pins 6.10.0, the current
release. The transitive @sasjs/adapter and @sasjs/utils entries move with it.
The guard that keeps a runtime without webout()'s options argument from serving a
thinner response tested webout.length >= 4. Function.length stops counting at the
first default or rest parameter, so a runtime that does carry the option but
declares it `options = {}` or gathers the tail with `...args` reports three, and
every service that calls the guard refuses to start on a working runtime.
It now asserts the outcome instead: the member the option produces is looked for
in the response body the program is writing. `$sasdata`, `$viewdata`, `$vals` are
the members the client reads for column formats and labels, and their presence is
the thing that actually has to hold.
Where it runs matters too, and the first attempt got that wrong: called after
webout('CLOSE') the throw leaves a complete body behind, and the server serves it
- the caller sees a normal response with SYSCC 0 and no metadata, which is exactly
the silent failure this exists to prevent. It is called before the envelope is
closed, so an unhonoured option fails the run.
@sasjs/utils 4.7 makes webout() emit SYSSITE from a configured site id,
defaulting to 123, where %webout takes &syssite. Until 3.13.0 the prelude
wrote an empty string, and dc's client routes an expired licence key and a
wrong-organisation key only when startup_site_id is truthy - so on the mock
estate both pages were unreachable and licensing.cy.ts failed three cases.
Three values reached `run:` blocks through `${{ }}`, so the runner pasted them
into the script before the shell ever saw it - which makes the value part of the
code rather than an argument to it.
- `BASE="${{ github.base_ref }}"` (build.yaml, lighthouse.yaml) - a ref name in
the script.
- `echo ${{ secrets.SHEET_PWD }} | gpg --passphrase-fd 0` (all four workflows) -
a passphrase in the script. A quote in it ends the quoting, and a command
substitution in it runs.
- two `replace-in-files --replacement=...${{ secrets.CYPRESS_* }}...` lines
(build.yaml, release.yaml) - the same, inside a replacement string.
Each now arrives as a step `env:` value and is expanded as `"$VAR"`, so the shell
only ever sees data. Checked the replacement lines side by side with the old form:
identical for an ordinary value, and the new form keeps a value containing a space
intact where the old one word-splits it, and survives a value containing a single
quote where the old one is a shell syntax error.
The passphrase pipe also moves from `echo` to `printf '%s\n'`, so the byte the
decryption sees does not depend on how the shell's `echo` treats backslashes.
Review finding: when every body is refused by carriesClause, or the response
carries no FILTER_HASH, capture.hash stays empty - and storedClauseFor('')
then spends its 60 retries before reporting "no clause for <var> under",
which names neither the refused body nor the missing hash.
The hash is now asserted non-empty before the read, with the case's own
message, so a broken delivery fails where it happened.
The call count stays where it was - after the data load - because moving it
ahead of the read would check it before the app has issued the second POST
that the count exists to catch.
The duplicate check waited a fixed 250ms after reading the stored row, which is
wrong in both directions: on a slow runner the window can elapse before the first
response callback has incremented the counter, and a second POST landing after it
passes with two deliveries.
The app's sequence for an apply is validatefilter, then the data load for the
filtered route, so the load is the completion signal: the count is asserted after
it, and nothing depends on how fast the machine is. A delivery issued later than
that is a second apply, not a duplicate of this one.
Also corrects the ordering paragraph in cypress-specs.sh: it still said specs are
appended, in a file that now places va-ddc-host ahead of metanav. va-ddc-host does
not depend on the estate being unlicensed either - it passed 3/3 immediately after
metanav planted a key in the full CI run - so the bullet says what is true and
metanav stays the only spec with a fixed place.
Review finding: the matcher only accepted the variable name at the start of the
body or after a comma or newline, so a delivery wrapped any other way was not
counted - and the case then failed at the call-count assertion, reading as
"the app never called validatefilter". Reproduced against the previous
implementation: a urlencoded form value, a form value that starts with the
name, a JSON object keyed on the column and a JSON string value holding the
row all matched nothing.
The body is now expanded into the candidate strings it can carry - the raw
text, the percent-decoded text, each form field's value, and every string leaf
of a JSON body - and the name has to be a whole CSV field of one of them.
Field equality rather than delimiters means a quoted literal, a longer name,
an SQL fragment or another variable's name is still refused, and there is no
regular expression over user text to get wrong.
Also restores the ordering note in cypress-specs.sh: va-ddc-host asserts
nothing about licensing, so it runs before metanav, the one spec that plants a
licence key.
The pin moved to 3.12.3 with the webout() change but the comment above it still
named 3.12.2 as the current release - true when it was written, wrong under the
pin it now sits on. The comment also only carried the /ddc-host reason, which
does not explain this bump.
3.12.3 is the first server release declaring @sasjs/utils ^4.6.0, and that is
what gives webout() the options argument getdata, viewdata and getcolvals pass as
{ showmeta }. Say so where the constant is, so the next person to move it knows
the metadata members depend on it.
The services in this branch pass { showmeta } to webout(), which is
@sasjs/utils 4.6 and up. sasjs/server 3.12.3 is the first release whose
package.json asks for ^4.6.0 (a clean install resolves 4.6.1), so the pin
moves from 3.12.2 to 3.12.3 in every workflow that starts the mock estate.
Verified against the published package rather than the release notes:
`npm view @sasjs/server@3.12.3 dependencies.@sasjs/utils` answers ^4.6.0, and
`npm install @sasjs/server@3.12.3` in an empty directory installs
@sasjs/utils 4.6.1, whose webout prelude carries showmeta.
Review of the release diff: `npm install -g @sasjs/cli` and
`replace-in-files-cli` were unpinned, so CI ran whatever release was current
and a bad one would reach the pipeline unreviewed - next to a SASjs server
that is pinned to an exact version.
The CLI now comes from the version sas/package.json pins, which is the shape
docs.yaml and release-assets.yaml already use, so there is one source for it.
replace-in-files-cli, pm2 and wait-on are pinned to the versions in use
(4.0.0, 7.1.2, 9.5.1).
Both node expressions are checked from the directories they run in:
`require('./sas/package.json')` at the root and `require('../../package.json')`
in sas/mocks/sasjs both answer 6.5.0.
The three services that pass { showmeta } - getdata, viewdata, getcolvals - rely
on webout()'s fourth argument, which arrived in @sasjs/utils 4.6.0. @sasjs/server
3.12 declares ^4.4.0, so an install can resolve 4.5.x, and there the argument is
dropped silently: the response still looks complete but $sasdata, $viewdata and
$vals are absent, so the client's format and label paths stop being exercised
while the mock estate reports success.
requireWeboutOptions() checks that the generated webout() takes an options
argument (arity, because the option names live outside the callable function) and
throws naming the member, the cause and both fixes. A 4.6 runtime is unchanged.
The matcher tested the variable as a substring of the whole request body, so a
RAW_VALUE, an SQL fragment or another variable whose name contains it was counted
as another delivery of this clause and failed the count. It now requires the name
between its CSV delimiters, which is the filterquery VARIABLE_NM field; a quoted
literal cannot match, because its delimiters are quotes.
cy.wait('@validatefilter') used the 5s default, where the polling loop it replaced
tolerated 15s, and the message only arrives once the framed page has rendered.
It now waits for longerCommandTimeout, as the rest of this spec does.
The capture assertion also carries the tail of the host page's log in its message:
the failure reads "expected -1 to be above -1" otherwise, and the log is what says
whether the app never exposed its buffer or the frame never loaded.
Two review findings:
- The load handler re-posted pendingPayload on every load event of the frame,
so an in-app navigation or a self-reload posted a filter that belonged to
the mount which had already delivered it, and logged it as that mount's.
The handler now takes the payload (clearing pendingPayload and
pendingTiming) before posting, so a later load has nothing to send.
- The README illustrated report.json with one parameter while the committed
file carries three, and the prose below reads as though one parameter were
the shape. The example block is gone: the file is the example, and the keys
are documented as a table with the parameter rules kept underneath.
The last eleven services still built the response envelope through
dcMockUtils' webOutOpen/webOutObj/webOutClose. They now use the webout() the
SASjs Server emits into every JS program, so dcMockUtils no longer emulates
%webout at all - the helper existed only for these callers and is deleted
with them.
That needed the options object added to webout() in @sasjs/utils 4.6
(showmeta, maxobs), which is what a clean install of @sasjs/server@3.12.2
resolves to (^4.4.0 -> 4.6.1, checked). A server whose install still holds
4.4/4.5 refuses the option by name rather than ignoring it, so the failure is
loud rather than a response that is quietly missing its column metadata.
getdata, viewdata and getcolvals send their metadata as
`webout('OBJ', name, rows, { showmeta })`, and every other call keeps its
member name and rows unchanged.
One call was positional all along: startupservice's xlmaps is an array of
arrays, which the old helper serialised as-is and webout() refuses for OBJ.
It is now webout('ARR', 'xlmaps', xlmaps) - caught by the comparison run
below, not by the rewrite.
Two gaps in the guard that decides which validatefilter response belongs to the
case.
A body Cypress parses into an object stringified to "[object Object]", so no
call matched and the case failed on the count with nothing naming the body shape.
The body is now serialised the way the response already was - a string is used as
it is, anything else is JSON.stringify'd.
The count was asserted immediately after the first response, which is before a
second POST from the same apply could reach its own callback, so a duplicate read
as one call. The count is now asserted after the stored row has been read back,
with a settle that covers a second POST still in flight when the read returned.
Two review findings on host.js:
- postToFrame() used '*' as targetOrigin, so the filter clause went to
whatever document occupied the frame. The frame is the app on report.server,
so that origin is the only one that should receive it; the origin is now
resolved once from report.server (new URL(...).origin) and a report.server
that is not a URL logs a visible error instead of widening delivery. The
page also says so when it is not on the frame's origin, because the capture
probe cannot read across origins and that would otherwise look like a
harness failure.
- startEarlyReplay() accepted any __vaLastMessage once it had posted, and on
a remount to the same URL the old document is still on screen carrying the
buffer the previous mount captured. A capture now only counts once the
document has been replaced (mountDocument), both in the interval and in the
load-event check. Keying to the document rather than waiting for a load
event keeps the first post where the early window is - the interval starts
exactly as before.
Every push to a pull request queued another full run - lint, licence check, the
mocked estate and the twenty-six minute suite - and nothing cancelled the run for
the head it replaced. With two runners shared by every pull request in the repo,
those stale runs are picked oldest-first and delay the head that can actually be
merged: runs 3002, 3003 and 3005 were queued for heads that had already been
superseded while the current one waited behind them.
The group is the pull request, so a newer push cancels the run for the older
head. Pushes that are not pull requests are not affected - the release workflow
answers to its own group.
Three review findings on the harness:
- The usage line advertised `--pass`, which puts the secret in argv - readable
from `ps` by every account on the host, and kept in the shell history. The
flag is gone: SASJS_PASS stays the documented route, and `--pass-stdin`
reads one line from stdin for scripts that cannot use the environment.
- deploy() reported success on the deploy route's 2xx alone. That means the
tree was accepted, not that the app is served: a truncated `type: 'file'`
member still serves a 200 page, and a stale asset from a previous deploy is
a 200 by definition - both of which the README already lists as the sharp
edges of that route. The page and all three assets are now fetched back and
compared with the bytes that were sent, failing with the size and hash of
both copies; a 401 is reported as deployed-but-gated, since on a server-mode
estate that is the gate working. `--verify-only` runs just that half.
- Two harnesses stand in for a VA report (this page and
client/cypress/e2e/va-ddc-host.cy.ts) and nothing said which does what. The
README now names both roles and points at the contract of record - the
message shape and __vaLastMessage in va-messaging.service.ts, and the
pre-bootstrap capture rule in va-early.js - including why the probe differs
between them (Cypress proxies the framed document).
The intercept kept one hash from any validatefilter response in the case, so a
second call - another table, a revalidate on load - would have replaced it and
the rows read back would belong to a different clause. The response is now only
taken when the request body carries this case's variable, which is what tells one
clause from another, and a case that sees more than one call for that variable
fails on the count rather than silently keeping the last.
A hash that cannot be read is captured and rethrown in the command chain: thrown
inside the route handler, Cypress reports it as an uncaught error and the case
fails later as a bare "no clause under" timeout instead of naming the response.
The spec located its evidence as "the last stored row for this variable", which
cannot tell this run's row from one an earlier case or run left behind - so it
deleted the DC_JSLIB filter tables before each case instead, and that bought a
scheduling contract: it had to run after every other spec on the shared mock
estate, and it left the estate falsified for anything that ran after it.
The mock validatefilter already returns the FILTER_HASH it computed for the
clause, so the rows are now selected by the hash this case's own response names.
Nothing is cleared, and no spec has to run before or after it. A store that
already holds this clause holds the same rows; a clause that changed hashes
differently, so a regression cannot be satisfied by a stale row.
va-ddc-host moves back in front of metanav, which is the one spec with a real
ordering constraint (its licence key must not change another spec).
The SASjs Server emits a webout() block into every JS program it runs
(@sasjs/utils >= 4.4, server >= 3.12.0), mirroring %ms_webout in
@sasjs/core: OPEN, ARR, OBJ, CLOSE. Ten of the mock services are two-year
functional implementations of their SAS counterparts, and they were
hand-building that same envelope through dcMockUtils' webOutOpen/Obj/Close.
Move those ten onto webout(), so the response envelope is built by the
runtime that owns it - real SYSUSERID, SYSPROCESSNAME, host name, a numeric
SYSCC and a content-type header - instead of constants in a mock. The
services that send showmeta metadata stay on the dcMockUtils helpers,
which webout() has no argument for yet.
The old 3.6.2 server pin in release, lighthouse and sites predates the
block, so those three move to 3.12.2 with it.
The interval tested its deadline only after the contentWindow guard, so a frame
that never became postable (detached, mid-navigation, or a load that never
fires) returned on every tick and the timer ran for the life of the page.
The capture check and the deadline now both run before the guard, so the replay
always stops. Capture keeps precedence, so a message caught on the last tick is
still reported as a capture, and it only counts once something has been posted
in this replay - a buffer left by the previous load of the same frame cannot be
read as one that belongs to this mount.
attempts keeps counting posts rather than ticks, because that is what the lines
quote ("after N post(s)"); a tick skipped for want of a frame sent nothing. The
zero case is now stated separately, since "0 post(s)" otherwise reads as a
broken probe rather than a frame that never appeared.
3.12.2 is the current release and carries the packaged-executable fix for
/ddc-host (the page this job's va-ddc-host spec drives), so the version the
tests run against is the version that ships. 3.12.0 remains the floor for
`timing=early`.
Closes#449.
- SASJS_SERVER_VERSION 3.6.2 -> 3.12.1 in build.yaml. 3.12.0 is the floor:
/ddc-host with `timing=early` (the replay that lands before the app
bootstraps) arrived there, and it is absent from 3.11.3, which has no
/ddc-host at all.
- DDC_HOST_ENABLED=true in the job's .env. The server 404s /ddc-host unless it
is set - it is a testing tool and does not announce itself - so enabling the
spec without this would have failed on the first case.
- va-ddc-host moves from EXCLUDED to the END of SPECS in cypress-specs.sh: it
clears the DC_JSLIB filter tables before each case, so it must run last on the
shared mock estate. The ordering contract at the top of that file now records
that, and metanav's licence key, which no longer has to avoid being last
itself.
Review findings on the harness.
- timing=load posted nothing: mount() only started a replay for early/both and
the load handler only logged. Both and load now post on the frame's load as
well, which is the metadata path and the second copy VA sends, so the mode the
controls offer is a mode that works.
- a re-apply with the same prompts did not reload the frame: setting src to the
value it already has is a no-op in the browser, so the new load post (and the
refresh path generally) delivered nothing. The frame is reloaded explicitly
when the URL is unchanged.
- dataType was declared and never applied. A prompt configured as number now
arrives as 42 rather than "42", which is what DC's clause builder branches on;
a non-numeric value is left alone rather than becoming NaN.
- the prompt select is found by walking the selects instead of building a
[data-name="..."] selector, so a prompt name with a quote or bracket in it
cannot throw inside apply() and leave the page with no frame.
- the replay's give-up message now says when the app does not expose
window.__vaLastMessage, so a changed probe is not reported as a delivery
failure.
- deploy.mjs throws when the Node runtime cannot read Set-Cookie rather than
silently dropping every cookie and failing later as an unauthenticated 401.
report.json carries a numeric prompt so the numeric path is exercised by
default rather than only in theory.
Review findings on the CASE marker.
- when a column stops carrying the rule (a metadata reload, or the column
becomes formula or read-only) the badge was removed but the tooltip kept the
sentence. stripCaseHeaderTitle is exported and applied there too, so the
header says nothing it cannot back up.
- the strip now removes only the `; <hint>` form the composer produces, so a
title that happens to use the same words is left alone, and a title that is
only a hint collapses to empty rather than to a hint with a separator.
- composeCaseHeaderTitle takes `string | undefined`, which is what th.title is,
and the spec drops its `undefined as any` cast.
- resolveEnteredCaseTransform no longer takes an isDerived flag: the grid only
tints a derived column, it does not block entries, so excluding it would have
stopped converting values the editor converts today. The integration spec now
drives the real predicate, with read-only and formula columns alongside the
editable ones.
Review findings on the change-detection steps and the shared build artefact.
- A failed fetch or diff in "Detect client changes" used to fail the job: the
fail-safe only covered an output that was absent, not a step that errored.
Both workflows now exit 0 without writing client=false on any failure, so an
undecidable change set runs the suite instead of blocking the pull request.
- .gitea/scripts/ joins the change set in both workflows: it carries the spec
list (cypress-specs.sh) and the runner gate (check-runner-memory.sh), so a
change to what the suite runs, or to the gate that lets it run, must run it.
- A skipped job now says so in a named step and a notice annotation, rather than
reporting success from a list of greyed-out steps.
- The dist upload sets if-no-files-found: error, so an empty artefact fails the
producing job instead of surfacing later as "webSourcePath ... doesn't exist".
- lighthouserc.js records the cost of numberOfRuns: 1 - no median, so one slow
navigation is a re-run rather than a fix.
A parent page standing in for the VA report that embeds Data Controller as
data-driven content: the dropdowns are the report's prompts, and the iframe is
framed and fed the way VA frames and feeds it.
Deliberately outside the DC build. The host is not the child: shipping it in
DC's deployment would put a test harness on customer servers and let DC be
opened top-level as a page that frames itself.
- report.json defines the report: child AppStream name, table, sandbox set,
delivery mode and timing, and the prompts (label is the va_parmap key, since
DC matches on the label rather than the report's internal parameter name).
- two delivery modes, because they reach different code in DC: refresh (reload
the frame, replay from before load - the report-refresh path) and prompt (post
into the loaded frame - an in-place prompt change).
- deploy.mjs posts the folder to a SASjs Server with /SASjsApi/drive/deploy and
streamWebFolder, which is the same route sasjs cbd uses, so a static app is
published the usual way and served at /AppStream/<name>.
Addresses the retroactive review of #447.
- afterGetColHeader appended the case hint to th.title on every render, and
Handsontable reuses the th, so the tooltip grew a copy per render. The hint
is composed by composeCaseHeaderTitle, which strips any known hint first, so
a repeat call is a no-op and a rule change replaces the sentence.
- the marker is now gated on the same predicate the conversion uses,
resolveEnteredCaseTransform, so a formula, read-only or derived column no
longer advertises a conversion the hook refuses to make. The class is removed
when the predicate stops matching.
- resolvePkFormula and the unescapeFormula write (given an explicit source) join
CASE_TRANSFORM_SKIPPED_SOURCES: neither is an entry, and re-casing a computed
key or a live `=...` string corrupts it. force_cell_validation_value keeps
converting on purpose, stated in the docstring.
Review follow-up on the browser harness.
The exclusion of va-ddc-host now names the issue that tracks re-enabling it
(dc/dc#449), so the coverage gate cannot report a rotted exclusion as accounted
for. The comment also states the ordering constraint the spec carries: it clears
the DC_JSLIB filter tables per case, and every spec in that job shares one mock
estate, so it has to run last or be given its own.
The spec's comments and test name describe the behaviour and the harness, with
no ticket numbers, per AGENTS.md. The va-filter docstring says what the inference
is: a guess from the value's shape alone, which can disagree with the column in
either direction, and which costs the deferred call rather than the page because
the attempt suppresses the abort modal and falls back to the unfiltered load.
Measured over the last 150 workflow runs on this forge: a pull request costs
56 runner-minutes (Build-and-ng-test 5.8m, Build-and-test-development 31.2m,
lighthouse 19.3m), and when three pull requests are in flight the first job
queues 47-69 minutes before it starts.
- Build-and-ng-test publishes client/dist as an artifact and the Cypress job
downloads it instead of running a second cold `npm run build` (1.6m each).
Both jobs test the same bytes.
- lighthouse: numberOfRuns 1 (lhci's default of 3 tripled the 13.6m audit to
steady a median that only has to clear a fixed threshold), and the duplicate
/view/usernav/groups entry is removed (17 unique URLs, not 18).
- Cypress records no video in CI and takes screenshots on failure instead
(`--config video=false,screenshotOnRunFailure=true`, CLI only so local runs
and the demo recorder still record); the failure artifact carries
screenshots as well as any video.
- Both workflows detect whether client/ or sas/mocks/ changed and skip their
expensive steps when neither did - a sas-only or docs-only pull request no
longer pays 45 minutes for suites that cannot observe it. The gate is
`!= 'false'`, so a missing step output runs the suite rather than quietly
reporting green.
issue 444 was a month-name date from VA becoming a character literal, and it
only reproduced on a report refresh: the deferred initial load reads VA's
buffered message, which has no column metadata, so the SAS kind comes from the
value's shape. Posting a message into a running page exercises the other path -
it arrives after getdata, metadata in hand, so a date is formatted correctly
whatever the client does - which is why the unit spec could pin the fix but not
show the failure a report author sees.
The new spec frames the built app in sasjs/server's /ddc-host page, which mounts
it with VA's sandbox attribute set and, with timing=early, replays the message
from the moment the frame is created. That is the request and apply path a
refresh takes, with mock services instead of a VA estate.
Where the assertion sits: the mock public/validatefilter stores what it is sent,
so the clause is read back from the drive (mpe_filtersource.json) rather than
inferred from a request payload. The store is emptied per case because
mpFilterStore reuses an existing FILTER_RK for a clause it has already seen, and
a leftover row would answer for this run. The host page's own log is the second
witness: the message is captured before the frame finishes loading.
Three cases in one run - the WORDDATE shape from the issue, a DATE9 date that
always worked, and a character parameter. Against the unfixed service the first
stores 'August 02, 1971' and fails, while both controls pass; with the fix all
three store the expected literal. So the guard goes red for the defect and only
the defect.
The spec runs last, needs no SAS, and takes about a minute against the mock
estate the other specs already use.
Where a column carries a CASE=UPCASE or CASE=LOWCASE rule in
MPE_VALIDATIONS, the EDIT grid treated it as validation only: a value in
the other case was flagged after typing, so the user had to retype it or
accept a red cell and fix it later.
The pending value is now converted before it commits, driven by the rule
that already exists rather than a hard-coded column list:
- DcValidator.getCaseTransform(col) maps a CASE rule on the column to
'upper'/'lower' (the two rule values dq-validation.ts acts on).
- The existing beforeChange hook applies it alongside ROUND, so an edit, a
paste range and a fill-down all take the same path, the cursor and
selection stay put, and undo keeps its history.
- applyCaseTransform / isCaseTransformSkipped live in a small pure module
so the conversion and the source exclusions are testable directly.
Values are not converted when they are not user entry: loadData (the grid
re-sending its dataset on every updateSettings({data})), UndoRedo.undo /
redo, and the Revert menu item. Converting those would rewrite values the
service supplied, or hand back something other than the value the user
asked to restore, and would leave untouched rows looking modified.
A CASE-bearing column is marked in the header ('Aa' marker plus a tooltip)
so the user can tell the conversion came from the grid.
The CASE validation itself is unchanged: values from an API, staged data
or an Excel import are still validated as before.
The footer is a %let and two %if inside the PDF branch of the service, which
already runs inside a macro, so the mpe_pdffootnote wrapper added a file and a
call boundary for a value that is built once, in one place.
Behaviour is unchanged: the same filter wording, session user, timestamp and
the truncation notice when the cap bites, still driven by pdf_maxobs so the
printed row count and the printed cap cannot disagree. A blank obs count is
treated as no truncation, which was the one real edge the macro guarded.
getrawdata.test.sas loses test 5 with it - those assertions existed because
the text was composed by a macro the test could call. With the composition
inline there is no seam to assert on, because a PDF cannot be read back as
text in SAS. The service-level PDF assertion (signature present) still covers
the branch, and the footer is checked from the downloaded file: page 1 of the
artefact reads "No filter | sysadmin | 03NOV2025 13:48 Page 1 of 20" on SAS 9,
and the same request on Viya returns a 20 page report.
7.1.3 carries sasjs/core#510: mv_getjobresult now returns a binary webout
from a job result instead of writing its own "not registered" text, so the
PDF download can be asserted on Viya as well as on SAS 9.
The only other change in the range is 7.1.2's filter validation message
(sasjs/core#511), which this project does not call.
Two real findings from review.
ACTION: the default was assigned with %let before the value was read, so a
URL-supplied ACTION was overwritten and the parameter could never take effect.
The incoming value is now captured first with symget, guarded by symexist so a
missing name does not write to the log, and the default is assigned afterwards.
%global was tried first and is wrong here: declaring a name that already exists
as a local in the including scope does nothing, while the symputx that follows
writes the global - so &action kept resolving to the local and no action ran,
which is what broke the makedata test.
Separately: on the SASjs Server STP endpoint a query parameter is not delivered
to the service at all - a request with ?action=DIRLIST&parent=/tmp reaches the
program with no fromjs and no symexist('action'), and fails on WORK.FROMJS
before ACTION is considered. The configurator's link is therefore legacy on
that target; the adapter path (the Angular deploy screen) is the one that
works, and the capture above is what makes a parameter effective if it does
arrive.
Folder type: the picker compared FILE_OR_FOLDER to 'folder' case sensitively
while mpe_dirlist selects on upcase, so an uppercased value emptied the list.
The compare is now lowercase, and entries mp_dirlist listed but could not open
are surfaced - rendered without the drill-in and Use buttons, with the service
message beside them - instead of being filtered out after the macro's header
promised they would be visible. Two specs cover both, including one that
renders a locked row.
The service tests exercise the actions through a child session; these call the
macros in place, so the guard and the answers are covered without a request:
mpe_dirlist 7 assertions - iftrue=0 does nothing, one level, files
excluded, three levels, the depth cap, the maxobs cap with
TRUNCATED=Y, and a blank parent returning a root path
mpe_writecheck 5 assertions - iftrue=0 does nothing, a writable folder, a
file, a missing path with a reason, and the probe leaving an
existing file that already had its old name alone
They live in sasjs/macros beside the macros they test, which works: a test file
in a declared macro folder compiles to sasjsbuild/tests/macros/ and sasjs test
runs it from there.
Two traps, both hit before this passed:
- these macros write JSON to _webout, so the test has to point _webout at a
temp file while they run and put it back before the term program writes the
results - the first attempt redirected without restoring and the runner
reported "No webout was returned by job";
- descriptions in mp_assert must not contain a comma: an unquoted comma splits
the parameter list and the assertion fails to compile with "All positional
parameters must precede keyword parameters", which names the call rather than
the comma.
check_outfmt and check_keepme were macros around one data step each, so they are
now those data steps, in the body where they run. Nothing else about them
changes: the &inds trick replaces the `%if &exist` guard, so the step still
touches work.fromjs only when the table is there.
That inlining exposed a real bug, found by running the KEEPME path rather than
by reading it. keepme was initialised with %let inside the body, so it was
local to makedata_body, while the request set the global with symputx(...,'G').
self_destruct, called from the body, resolves the nearest scope first - the
local 0 - so a caller asking to keep the program had it deleted anyway. It is
now declared %global and initialised at file scope, before the body exists.
Verified on the estate: KEEPME=1 leaves the program in place (a follow-up
request still finds it), a normal run deletes it (the next request reports that
the program does not exist), the suite is 3/3 with 16/16 assertions, and the
seed returns STATUS SUCCESS.
Each action is now its own macro that takes `iftrue=` and returns immediately
when its action was not requested:
%mpe_dirlist(iftrue= (&action=DIRLIST))
%mpe_writecheck(iftrue= (&action=WRITECHECK))
%makedata_body(iftrue= (&action=MAKEDATA))
so the service reads as a list rather than a branch, and there is no open-code
%if left in it. mpe_adminsetup splits into mpe_dirlist and mpe_writecheck
along the action boundary it already had.
The four helper macros move back to file scope, above the body. With the
dispatch branch gone nothing is nested any more, so the noNestedMacros
exception comes off both files - the last reason for it was the branch I just
removed.
mp_abort takes its own condition, so the two refusals are two calls in open
code rather than branches inside a wrapper macro:
%mp_abort(iftrue= (&action=MISSING) ,mac=&_program ,msg=...)
%mp_abort(iftrue= (&action=BAD) ,mac=&_program ,msg=...)
check_action was never needed either - it wrapped a single data step whose only
condition is a data-step IF on &exist.
That leaves one branch in the file, the two-way dispatch between the browse
actions and the seeding body, which is what noMacroIfInOpenCode actually
requires to be inside a macro.
Replaces the risk-register framing with the assessment: there is no admin group
until makedata creates it, the caller is the administrator doing the deployment,
the configurator in the same window is more powerful than these two actions, and
a successful run deletes the service anyway.
Filed as deploy-window-authorization.md so the next review finds the reasoning
instead of re-raising the finding.
Five defects from review, all mine.
ACTION is no longer defaulted. `%let action=MAKEDATA;` meant a caller with no
input table at all ran the seeding body - so a URL that never mentioned ACTION
would create the control library. The default is MISSING, which is refused,
and a URL parameter is whitelisted through the same check with symget() so it
is never resolved as macro text.
The write probe no longer has a predictable name. fopen with 'O' truncates, so
a probe called .dcprobe_<pid> destroyed any pre-existing file of that name in
the folder being tested. It now carries the pid and a random suffix, checks
fileexist before use, and gives up rather than reuse a name.
DIRLIST returns folders and locked entries only. The old filter dropped 'file'
and passed every other value - blanks and unknown types - to the picker as a
folder. The filter is now an explicit whitelist, and the header describes what
is actually emitted.
A request with no input table is refused, covered by a test: that is the case
the default used to swallow.
ngOnDestroy is gone from the client component - it reset a flag on a component
being torn down.
The deploy-window exposure is recorded in .agents/docs/, with why the admin
group cannot be checked there: %mpeinit() reads &mpeadmins from the control
library that makedata creates, so before it runs there is no group to check -
which is also why the previous dirlist could never serve this screen.
Reverts the hoist. The helpers read better next to the body that calls them,
and the body runs once per deployment, so the nesting costs nothing at run
time - the only cost is the noNestedMacros exception in the two headers, which
is the narrower trade than four macros defined far from their only caller.
Wrapping the body in a macro had demoted four existing helpers - check_outfmt,
check_keepme, emit_response and self_destruct - to nested definitions, which
needed a noNestedMacros exception in the file header. They were at file scope
before, so hoisting them back is undoing that demotion rather than promoting
anything new, and the exception goes away.
No behaviour change: macro variable lookup is dynamic, so the helpers still
resolve the body's variables when the body calls them.
The deploy screens asked for a physical folder the deployment user can write
to, as a single text box, and found out whether it worked only by running the
whole deployment. They now get a picker and a Validate button, backed by the
makedata actions that already answer those two questions.
A new app-dc-location component renders the path as an editable input - the
picker is a convenience over it, never a replacement, because a folder the
deployment user cannot open is not listed and a customer may want a path that
does not exist yet. Browse lists one level at a time (ACTION=DIRLIST, DEPTH 1,
MAXOBS 500), with Up, a "Use this folder" for where you are standing, and a per
row Use. Validate calls ACTION=WRITECHECK and reports the verdict inline -
writeable, or the reason it is not - instead of leaving it to the deployment to
fail.
Both screens use it: the Viya setup screen (which runs makedata through the
adapter) and the Sasjs configurator (which opens makedata as a URL). The
mkdata calls now send ACTION=MAKEDATA, since the service requires it.
Details worth knowing:
- every value read from the service is trimmed: SAS has no zero length
character value, so a blank field arrives as a single space and reads as a
value to any `||` or `@if`.
- the listing's level 0 row is the folder being listed, not a folder to drill
into - except when the parent is blank, where the level 0 rows are the
platform roots.
- failures are shown inline rather than as abort modals: the two requests pass
suppressErrorAbortModal/suppressSuccessAbortModal, because a refused path is
an expected answer here, not a crash.
- parentOf handles a drive root (C:\) and a unix root without inventing an
empty path.
Frontend tests: 14 new specs (38 in the deploy folder, all passing), including
a template spec for the browser panel and one for the verdict line, both
mutation checked.
The deploy screen needs to find and test a writeable folder before the control
library exists, so these questions cannot be answered by an admin-gated
service: %mpeinit() reads &DC_LIBREF..mpe_config, which makedata itself
creates. That is why the previous dirlist could not run in the window it
existed for.
They are answered by the service that already owns that window instead.
makedata now requires an ACTION column:
MAKEDATA create the configuration tables (what it always did)
DIRLIST the folders below PARENT, DEPTH levels deep, MAXOBS capped
WRITECHECK does PATH exist, is it a folder, can we write in it
so a deployment has one service to run and the same one to remove - makedata
already self destructs after a successful run, and now nothing is left behind
that can enumerate the filesystem.
ACTION is not defaulted: a caller that does not say what it wants is refused
rather than having a control library seeded for it. The payloads under
deploy/ and the configurator's link are updated in the same change, so the
installer and the seeding scripts keep working.
The work is in mpe_browse, called from both server and viya makedata:
mp_dirlist does the listing (a blank parent is the platform roots), and
mp_validatecol(ISPATH) shape checks whatever is submitted. Folders are
returned a level at a time so a parent with tens of thousands of subfolders is
never transferred in full, and the finite depth is what makes a symlink loop
harmless.
The body of makedata is now a macro of its own, because the branch cannot be
open code - noMacroIfInOpenCode keeps the file loadable on every 9.4
maintenance level. Its four helper macros are unchanged.
Input is the adapter table as before, never macro text: a value is refused if
it carries a macro trigger, a quote or a statement separator, and only the
three known ACTION values are ever put into a macro variable.
sas9 is not included: its makedata is the metadata STP driven by the
configurator page through URL parameters, not by the adapter, and the sas9
configurator takes a typed path rather than a picker. That is a change of
input mechanism, not of this feature, so it is left for its own ticket.
A parameter named without a value arrives blank, and a blank reached the
footer's numeric comparisons as nothing to evaluate:
%if &filter eq 0 /* blank filter */
%if %eval(&nobs) gt %eval(&maxobs) /* blank obs count */
That raised %EVAL arithmetic errors, stopped mpe_pdffootnote and failed the
whole download instead of producing a report. Both are now normalised to
their defaults first, maxobs included.
The footer's user= is taken from mf_getuser() at the top of the service - it
is deliberately not an input, so a caller cannot assert who ran the report -
and it is passed through %superq because a display name may hold a comma,
which the macro call would otherwise read as a parameter separator.
Test 5 gains a case for the blank call.
mv_getgroupmembers declared the member name at $43, and a longer name was
truncated - or, on Viya, stopped the step with "Multiple lengths were
specified".
membername is declared at $256 in the same shape the macro now returns it: one
width for every group, so the tables the caller combines are never of differing
lengths, and a long active directory name is kept whole.
Adds PDF to getrawdata: the staged, filtered dataset is rendered by
mp_ds2pdf and streamed with contenttype=PDF, so the Download modal gains
a typeset report - landscape, SAS formats and labels applied, headers
repeated on every page, 'Page n of m', and the filter, user and timestamp
in the footer.
The report is capped at 2000 observations, about 75 landscape pages, and
the footer states the truncation when it bites. The cap is one macro
variable driving both maxobs= and the footer text, so the two cannot
disagree, and the footer itself is composed by mpe_pdffootnote so the
truncation wording can be asserted without reading text out of a PDF.
@sasjs/core goes 7.0.3 -> 7.1.0 for the macro; the bump also brings
mp_validatecol ISPATH, two mp_dirlist guards and an mx_execute test.
On Viya the report renders but the download does not arrive: mp_streamfile
fails ahead of the PDF branch inside mp_binarycopy (RECFM=N with lrecl=1
needs 256), which is upstream of this change and left alone here.
The stage that reports what the file is doing is meant to belong to the whole file, and it finished at "Extracting the data finished" whatever happened. A sheet whose parse rejected had already blanked its datasource and dropped itself from the submission in the per-sheet catch, so the user was told the file had extracted while a table was silently missing from the submission.
`allSettled` was already the right primitive, but its results said nothing: the catch swallowed the error, so every promise fulfilled. The catch now names the sheet on the component, the stage ends at a new `extractionFailed` state when anything was left out, and the pane lists what was left out and why.
The two reasons a sheet contributes nothing are kept apart, because they are not the same thing to the reader: a sheet the sheet utility could not read is a "could not extract" problem in red, while a sheet it parsed and found no rows in - which arrives as the rejection "No relevant data found" - is reported as "no rows found" without calling the file corrupt. Both leave the sheet out of the submission, and either one stops the stage reporting a clean finish.
The row derivation sits inside the try with the parse. Outside it, a throw on a row shape the sheet utility did not expect escaped as a rejection that `allSettled` swallows - the dataset stayed in the submission with `parsingTable` true and the stage reported a clean finish over a sheet that had been dropped.
The messages live in the no-table-selected pane, which is where the user is once extraction has finished; the file pane they were first put in unmounts as soon as the datasets are fetched, so they would never have been read there. The stage line keeps one id per state rather than two of the same: `file-loading-stage` in the file pane while the datasets are fetched, `file-extraction-stage` in the overlay while the sheets are extracted, and the Cypress spec asks for both.
The Cypress spec asks for both stage ids, because the element that carries the stage changes state, and asserts that a sheet left out of the submission is named in the pane - under whichever reason the sheet utility reported, since only the utility decides which of the two it is.
The spec builds the component through the test injector rather than with seven positional arguments, so a constructor that gains or reorders a dependency fails there (`NG0201: No provider found for ElementRef`) instead of binding a stub to the wrong collaborator and passing.
The comment claimed the sidebar animates open/closed. It does not: the
collapse is a plain margin-left on clr-vertical-nav.closed (styles.scss)
and margin is not in any transition there - Clarity's width transition on
.clr-vertical-nav does not apply, because the width never changes. What
does need the deferral is that Angular applies the class binding after the
event handler returns, so a synchronous measurement reads the pre-toggle
layout.
The spec now says what it covers: the scheduling contract, not whether a
measurement lands before or after a transition.
Unsubscribing stops future layout notifications but leaves a timer that
one already scheduled, so a sidebar toggle immediately followed by destroy
still ran calculateInputPosition() against a torn-down view. The timer id
is now held and cleared in ngOnDestroy, and the spec covers that window -
it fails if the clear is removed.
Also drops the EventService.dispatchEvent stub from the viewboxes spec,
which advertised a method this branch deletes.
The specs gated their assertions on a real setTimeout(..., 10) plus done,
which is wall-clock timing: a loaded runner can assert before the
component's own timer fires, and the negative case would then pass
vacuously. fakeAsync + tick order the assertion on the scheduler instead.
Also drops the stale half of the viewbox resize comment - the shared tail
no longer notifies listeners, it just persists and re-measures.
The synthetic resize dispatch was its only caller. Nothing else used it,
and a generic window-event dispatcher on a service invites exactly the
coupling this change removes.
The sidebar spec now asserts the behaviour directly - no resize event
reaches window listeners on a toggle - and fails when a dispatch is put
back, rather than asserting on the removed API.
A sidebar toggle and the viewbox data/resize paths dispatched a synthetic
window resize so other components would notice the layout moved. Two
thirds of that was unnecessary: Handsontable 18 attaches its own
ResizeObserver to the grid's parent, so a container change re-measures the
grid with no event at all (measured in Chromium: view.getLastSize().width
tracks a container resize 600 -> 380 -> 700 without any dispatch).
The one real consumer was the autocomplete dropdown, which follows its
input under the new layout. It now subscribes to
EventService.onSidebarToggle and re-runs calculateInputPosition(), and
unsubscribes on destroy; the window:resize listener stays for real
viewport changes.
Removed: sidebar.component.ts:53, viewboxes.component.ts:281, :705 and
:1571. The viewbox resize tail already re-measures each instance through
refreshTableAfterResize, and a data load renders the grid itself.
Scoping the pass to one grid left the page's other grids uncorrected: the
viewboxes each render their own Handsontable (shared/viewboxes), and the
multi-dataset preview renders two. Nothing called the pass for them, so
they kept role=treegrid and aria-rowcount=-1 - the failures this change
exists to remove. The old document-wide sweep happened to cover them.
observeGridAria(hot) runs the pass and keeps it running from the grid's
own afterRender, for grids whose component has no render hook of its own.
It defers by one microtask: Handsontable writes the row/column counts
after the render that follows a data load, so a pass inside the hook
still sees the previous frame.
Wired into getViewboxHotInstance (every viewbox that hands out an
instance) and into both multi-dataset instances.
The root `package.json` carries no install or postinstall script, so the
guidance to rely on "the composite install script ... which installs both
subprojects" sends the reader after something that does not exist, and
`client/.npmrc` told them the same thing in a comment.
The install section also contradicted itself: it opened by telling the reader
to write the SheetJS registry token into `client/.npmrc`, and closed by
explaining that `client/.npmrc` is a tracked policy file that must not carry a
token (the pre-commit hook rejects one). The token goes in the user config,
which is what the first block now says.
The install sequence itself is unchanged - root `npm ci`, then `client` and
`sas` explicitly - because that is what the repo actually needs.
Review follow-up. The table pass matched every table inside the grid root,
so a table a cell renderer owns picked up role=table and scope=col - which
contradicted the reason the wrapper loop avoids [role="treegrid"]. It now
matches table.htCore only (every HOT table carries that class, one per
view), and header cells are read with :scope > thead > tr > th so a
renderer's nested table is not touched. The separate aria-rowcount=-1 pass
is gone: the wrapper strip already removes that attribute from the grid's
own element, and matching it anywhere else was the same overreach.
The viewer also gets the editor's fallback chain, so a pass that runs
before hotInstance is assigned still finds the grid through
hotTableComponent.
Scoping the ARIA pass to the grid exposed a dependency the document-wide
sweep had been hiding: the sweep used to write aria-label from an
element's text, so cypress/e2e/full-table-search.cy.ts could select the
filter modal's submit button as aria-label="Ok" - a label only the sweep
ever wrote. Once the sweep stopped reaching the modal, that selector
matched nothing and the dev job failed on it.
An element's text already is its accessible name, so the synthesis is
gone from the pass, and the spec now selects the button by its text the
way a user would.
The helper-only spec passes even if the observer goes back to calling
updateSettings() - which is the regression the helper exists to prevent.
Move the observer into refreshGridAfterContainerResize.ts so a spec can
drive a real container resize through it, and assert that the resize
reaches refreshDimensions() without any settings call. Verified by
re-injecting the old wiring: the spec fails with 'Expected spy
updateSettings not to have been called', and passes once it is removed.
Also corrects the helper's claim about what refreshDimensions() measures:
it compares rootElement.getBoundingClientRect() with the last measured
size, and both grids take their height from a viewport-derived calc(), so
a resize that matters is a width or viewport change.
The sweep matched [role="treegrid"] inside the root, which would also
catch a treegrid a cell renderer put in a cell. The grid's own wrapper is
the root itself, so target that plus the overlay wrappers Handsontable
renders, and leave nested widgets alone.
Adds a live-grid spec: Handsontable writes role=treegrid/aria-rowcount
again on loadData, so the pass has to run after renders, not once.
The editor and viewer each ran fixAriaAccessibility() over the whole
document on every render, on a 3-second interval and from a
MutationObserver on document.body, and each call scheduled four further
copies of itself. The sweep removes the role/aria attributes that its own
observer watches, so it re-armed itself.
Measured with axe-core against Handsontable 18: the default markup fails
aria-required-children (critical) because the treegrid wrapper has no row
children, and disabling the tags with ariaTags:false only trades that for
empty-table-header while leaving the placeholder semantics missing. The
existing attribute sweep is therefore the right end state - it just has to
run against one grid's root, not the document.
Both components now call one shared helper with their own
hotInstance.rootElement, and the interval, the body observer and the four
deferred re-runs are gone; the afterRender/afterChange hooks and the init
calls are the only triggers.
The grid's container ResizeObserver (added in c7ba025 to replace the
afterRender window-resize dispatch of #246) refreshed by calling
updateSettingsSortSafe({ height }), and that wrapper's whole reason for
existing is that any updateSettings() while a multi-column sort is active
corrupts formula references into #REF!. It works around that by clearing the
sort, applying the settings and re-applying the sort - so every sidebar
toggle, modal open/close or window resize re-sorted the user's data, and an
open cell editor (anchored to visual coordinates) could end up writing to a
different row than the one being typed into.
refreshGridAfterContainerResize() replaces it with hot.refreshDimensions():
it compares the element's box with the last known size, re-measures the
overlays and redraws only when the size actually moved, touching no settings.
The decision lives in its own module so the spec can pin it - a real
Handsontable instance with an active sort and a formula column asserts that
the resize path calls refreshDimensions() and does not call updateSettings(),
clearSort() or sort(), that the sort config is unchanged, and that no cell
degrades to #REF!.
This is the narrow, independent piece of #246; the wider removal of the
remaining synthetic window resize dispatches (sidebar, viewboxes) is analysed
on the issue and is not part of this change.
Two findings from review.
downloadTablePdf() revoked the blob URL in the same task as the anchor click.
Firefox and Safari read the blob asynchronously, so that can cancel the
download and leave the user with no file; the revoke now runs on a later turn,
after the click has been handed to the download machinery.
getExportRange() clamped the selection's left edge up to skipLeadingCols but
then forced the right edge up to match, so a selection lying entirely inside
the leading housekeeping column(s) was shifted sideways into a column the user
never selected - the PDF contained column 1 when they had selected column 0.
The range is now clamped into the exportable columns and comes back empty
(right < left) when nothing is left, which the PDF path reports as "no table"
and the CSV/Excel path skips rather than exporting the wrong column.
The revoke change is only covered by the spec (it needs a browser to exercise);
the range change is covered by the spec and by the local harness, now 28 checks.
The modal decided what was wrong with two regular expressions of its own, and the answer disagreed with the log it was showing:
- an ODS HTML line was cut from the first capital `E` in it and lost its last character, so an error read `ERROR: File WORK.X.DATA does not exist.</font>`;
- a message SAS wrapped over several lines was reported once, truncated at the break, and the rest of it was not reported at all;
- the colon-less form SAS writes (`ERROR Libref X is not assigned.`) was missed on any line carrying markup;
- the notable notes - uninitialised variable, invalid data, a format too small - were not shown anywhere;
- clicking an issue searched the rendered text for its message, so a log that repeats a message (two `ERROR 22-322:` lines is ordinary) always jumped to the first of them.
`parseLog` from `@sasjs/utils` replaces all of it, the same function the VS Code extension and the SASjs server Studio use, so one log now reads identically in every host. Each request keeps the issues the parser reported, grouped by severity, and the tree gains a Notes list beside Errors and Warnings. The click scrolls to `issue.line` and flashes from that line to `issue.endLine`, and the lists no longer render at zero.
The modal is SAS Logs now, as are the menu entry, the deploy button and the abort modal's button; the request history is unchanged underneath.
`formatTimestamp` decided on `moment().format()`, which is 'Invalid date' - truthy - for anything it cannot parse, so its fallback could never fire. It asks `isValid()` now.
The warning flash was yellow where the warning text is ochre; both are ochre now, and the notes flash blue.
`@sasjs/utils` goes to 4.3.1, which is where the parser lives.
The new empty-grid spec built a grid from `data: []` alone, and Handsontable
reports zero columns when the data is empty and no `columns` setting defines
the schema - so buildGridPdfInput() correctly returned null and the spec failed
in CI while passing against the stub harness. The real grids declare their
columns, so the fixture does now too.
Two findings from the second review.
A row taller than the printable area was drawn straight through the footer and
off the page, losing the tail of the cell unannounced - MAX_COL_CHARS bounds a
column, not a value, so one long text field wraps to more lines than a page
holds. Rows are now clamped to the tallest row that fits the page, with ' ...'
marking the cut, and the height budget accounts for both margins so an
exactly-fitting row stays on the page it started on instead of leaving the
first page with headers only.
exportGridPdf() also returned silently for a grid with no rows, although the
writer renders an empty table as a valid header-only page. The input is now
built by buildGridPdfInput(), which returns null only when there are no
columns, so a rowless grid downloads a page saying "0 rows x N columns".
Tests: the tall-row spec asserts the marker, the single page and that no text
start falls outside the printable band; the empty-grid spec asserts the input
is built rather than skipped.
Two findings from review.
First, a regression in the CSV/Excel path: getExportRange() defaults `left`
to skipLeadingCols, so comparing it against skipLeadingCols made every
no-selection export look like a whole-grid export and the plugin put the
leading housekeeping column back in. The whole-grid test now compares the left
edge against visual column 0, so a skipped column stays out.
Second, the PDF row cap was applied after collectGridTable() had already read
every cell, so a very large grid still froze before the cap took effect.
collectGridTable() now takes a maxRows limit that clamps the row loop before
any cell is read, and exportGridPdf() still reports the true total in the
subtitle.
Both are covered: a spec asserts the CSV export receives range
[0, skipLeadingCols, lastRow, lastCol], and one asserts the cap stops cell
reads rather than collecting and slicing.
CI caught this on the real grid specs: getPlugin('hiddenColumns') returns the
plugin instance even when it is switched off, and isHidden() then dereferences
a map that enablePlugin() never created, so any grid without a hiddenColumns
setting threw instead of exporting.
Ask the plugin's enabled flag first and treat a failure as "visible" - an
export is better off with a column the user could not see than with an error
dialog. The spec adds a regression case for exactly the CI failure, and its
afterEach no longer destroys an already-destroyed instance when a test creates
no grid.
Adds a PDF entry to the right-click Export submenu, next to CSV and Excel,
so the grid as displayed can leave the browser as a printable document.
Implemented directly against PDF 1.4 rather than adding a PDF library: the
output is a text table, which needs no embedded fonts, no images and no
compression. The base-14 Courier font is used because its glyphs are exactly
600/1000 em wide, so column widths are arithmetic on character counts instead
of a font-metrics table - a 1200 x 12 grid is laid out in ~60 ms.
Layout: A4 landscape, bold header band repeated on every page with "Page n of
m" footers, numeric columns right-aligned, values word-wrapped inside their
column, and long tables paginated. Wide tables are fitted by shrinking the
body font first (7pt down to 5pt, so a 20-column table keeps full headers at
6pt) rather than squeezing columns; only at 5pt are columns scaled, and they
stop at 4 characters - past that trailing columns are dropped and the first
page says how many were kept, so nothing is omitted silently.
Encoding is WinAnsi, the widest set the base-14 fonts cover without embedding;
characters outside cp1252 print as '?'. A typeset, branded report is a
different feature and belongs on the server, where SAS formats and DC metadata
are available.
The export keeps the existing "what you see" contract: current selection (when
it spans more than one cell), otherwise the whole grid, hidden columns
omitted, leading housekeeping columns skipped. The range calculation that CSV
and Excel already used is now a shared getExportRange(), so the three formats
cannot drift apart. Rows are capped at 5000 and the subtitle says so.
Tests: pdf-export.util.spec.ts covers document structure (xref offsets, page
count, footer numbering), string escaping, WinAnsi mapping and fallback,
wrapping, alignment, multi-line values, pagination, font fitting and the
page-width invariant; hot-export.util.spec.ts covers grid extraction against a
real Handsontable instance.
A MIN_TOTAL_MB typo reached the integer test under set -e and aborted with "integer expression expected"; it now prints usage and exits 2. A kernel without MemAvailable no longer reports 0 MB available in the summary or the warning, so the oversubscription warning stays meaningful.
release.yaml (all three jobs), sites.yaml and release-assets.yaml run the same cold production build that the review jobs do, so they get the same pre-install check.
The header now says what the check rules out (an undersized box) and what it does not (two jobs sharing one box), so the MemAvailable warning is not read as a gate.
An undersized runner does not fail honestly: the kernel OOM killer takes
the build, so the job reports exitcode 137 at ng build, or Karma
disconnects mid-suite after twenty minutes. Both read as a flaky test
rather than as a box that cannot hold the job, and both cost a whole CI
slot.
The check runs first in the three client jobs (Build-and-ng-test,
Build-and-test-development, lighthouse), prints total and available RAM,
vCPU count and any cgroup limit, and exits 1 when the effective size is
under 8 GB - the supported minimum. A cold production build measures
3.8 GB peak on its own, so 8 GB leaves roughly 2x headroom.
An uploaded file's preview is validated in place, so the cells that fail
a rule carry Handsontable's invalid cell meta. Discarding the file
restores the loaded data, but cell meta is keyed by row/column and
survives that restore - without a re-validation the grid keeps painting
cells that are no longer invalid.
cancelEdit re-runs the validation (hot.validateCells()) after restoring
the data source, which is what clears them; this test drives the flow and
pins it: import a file that fails validation, discard it, then assert the
row count is back, no .htInvalid cell remains and the page is read-only
again. The manual-edit path (test 25) keeps the parsed values by design.
Refs #122
The multi-load page rendered the loading stage only inside a branch guarded by
`parsedDatasets.length`, and that collection is empty for the whole read and
sheet-scan phase - so two of the four stage labels ("Reading the file",
"Searching for the data in the file") were assigned but never reached the DOM,
and a large file sat on screen with no feedback at all.
The stage is now shown next to the file name and size from the moment a file is
attached, and again in the content area while the sheets are being parsed. The
labels cover every phase the user waits through: reading the file, searching
for the data in the file, file ready to load, fetching the dataset details (one
getdata call per dataset - the longest wait), extracting the data, and
extracting the data finished. A spinner accompanies the stages that have work
running; the idle stages and the finished state show the label alone.
Two corrections fall out of that: the stage only moves to `extracted` once
every sheet has been parsed rather than when the first one finishes, and the
read's continuation only advances the stage if the read is still the phase in
progress (the sheet scan runs in parallel and can finish first).
excel-multi-load.cy.ts gains test 4, which asserts the file size, the ready
stage, the fetching stage (the getdata calls are held open so it is on screen
long enough to assert) and the handover to the "select a dataset" hint.
Closes#121
Enables noMacroIfInOpenCode and promotes it to "error". Open-code %if is a SAS
9.4M5 feature, so the 19 sites it found in nine files were hiding a version
requirement the codebase does not document (the requirements page says 9.4M3).
Each is rewritten so the construct no longer appears in open code:
- Six blocks become a small macro called unconditionally, which is where the
conditional then lives: drop_cas_tables (demodata), add_derived_cols
(getdata), build_sasdata4 (getdata test 4), drop_derived (stagedata),
build_staged (getrawdata), and one per option in the 20261003 migration.
- Four blocks in the two makedata STPs get the same treatment: check_outfmt,
check_keepme, emit_response and self_destruct. Their `call symputx` writes
are now explicit about the global symbol table, because a %let or a plain
symput inside a wrapper would be local to it.
- Two guards in stagedata and getrawdata are removed rather than wrapped: the
SQL only selects into a list that is already %let blank, and with no derived
variables it returns no rows, so the guard was doing nothing. The trailing
drop keeps a conditional, now in a macro and tested with %trim, since a
zero-row select can leave a single blank in the variable.
- validatefilter's two %let branches become one data step with call symputx,
which writes to the global table the later %mp_filterstore reads.
Verified: `npm run lint:sas` clean with the rule as an error; `sasjs compile`
succeeds; on sas.4gl.io the full suite passes 64/64, including the derived
variable tests in getdata test 4, stagedata tests 3 and 4 and validatefilter,
which are the paths these wrappers touch.
The tree is clean under all of them, so the remaining warn-level rules can be
promoted without a single code change: hasDoxygenHeader, hasMacroNameInMend,
hasMacroParentheses, noNestedMacros, strictMacroDefinition, noSpacesInFileNames,
lowerCaseFileNames, noTabIndentation, noTrailingSpaces, noGremlins and
noNestedCommentsInHeader.
With that, every rule @sasjs/lint 4.9.0 can raise is an error - only
noEncodedPasswords was an error by default - so any new violation fails
`npm run lint:sas` and the CI step that runs it, instead of printing a warning
that leaves the exit code at 0.
Checked: a trailing space appended to a file reports
"Error Line contains trailing spaces" and a non-zero exit, where it used to be
a Warning.
sas/.sasjslint takes maxLineLength from 180 to 80 - the linter's own default -
and promotes it to "error".
53 of the 258 .sas files already hold a line longer than 80. Rewrapping them
would be a large, reviewable-only-in-theory diff, so each keeps its current
longest line as an exact ceiling via a per-file override, e.g.
@sasjslint {"maxLineLength": 95} in services/validations/tables_all.sas. A
file with no override is held to 80, and an overridden file cannot grow past
the length it has today: the ceiling is the file's longest line, not a round
number.
Both directions were checked: a 101 character line added to an
un-overridden file fails with "Line exceeds maximum length by 21 characters",
and a 114 character line added to macros/mpe_execute.sas (ceiling 101) fails
by 13. `npm run lint:sas` is clean with the setting in force.
Rung 6 of the #376 ratchet: `"lineEndings": "lf"` is enabled and promoted to
"error". Its 37 findings are cleared and nothing but line endings changed.
- 34 files ended without a newline; each now ends with exactly one LF.
- 3 files were stored with CRLF (services/public/getgroups.sas and the two
sas9 lineage services, getmetacols and getmetatables), against the repo's
own `* text=auto eol=lf` in .gitattributes. They are converted to LF.
Checked mechanically that this is whitespace only: for every changed file,
HEAD's content and the new content are identical once CRLF is folded to LF
and the trailing newline is stripped.
VERIFICATION
- `npm run lint:sas`: 0 warnings, 0 errors, exit 0.
- `sasjs compile`: succeeds.
- On sas.4gl.io, against a scratch appLoc seeded with makedata:
"Tests that pass: 64/64 (100%)".
dc