Four specs failed on release/7.20 once the list went from 19 to 24. Three of the
four are addressed here; the fourth is a spec that cannot pass and is now
excluded with an issue.
download.cy.ts - it clears the downloads folder in an afterEach through
cy.task('deleteFolder'), and that task was registered only in
cypress/plugins/index.js, which Cypress 10+ never loads unless it is required
from cypress.config.ts. The task is now registered in setupNodeEvents, where
Cypress looks, and the login in its beforeEach only fills the form when the form
is actually on screen - the unconditional version failed with 'the center of this
element is hidden from view' whenever the session was already live.
csv-limited.cy.ts and hook-programs.cy.ts - neither is broken; both depend on
where they sit in the run. csv-limited asserts the free-tier 'Locked Feature'
modal, which an earlier spec removes by planting a valid licence, and
hook-programs asserts an approval row against its target table before other specs
have added rows to the approval tables. The list is now the sequence that was
green for months, with csv-limited first and hook-programs second, and the newer
specs appended rather than inserted. The script says why order matters, because
the next person to alphabetise it will break the same two specs.
metanav.cy.ts - cannot pass, and says so in its own source: '// BLOCKER ...
.clr-accordion-header-button always null although it is present on the page'.
With the duplicate login in its beforeEach removed it now reaches that assertion
and fails there, so it is excluded from the list with #395 as the reason, and the
login fix is kept for whoever picks it up. The coverage check now distinguishes a
deliberate exclusion from a forgotten file, so nothing can go missing silently.
Not verified locally: the specs need an estate seeded by 'sasjs cbd -t server-ci'
plus makedata, which a scratch mock does not reproduce - the app never leaves
app-loading. The CI run is the verification.
Brings in the diagnostics fixes from the base branch; conflicts were the
paramMap type widening to VaParmapTarget and the new paramsStable argument,
both of which the transform branch already carries.
Review findings, all about a warning that misleads:
- The banner heading said the mapping was ignored, but two of the three
lists it renders are mappings that are applied (the AND-ed pair) or that
were never applicable (a parameter VA does not send). Neutral heading.
- unmatchedMappings reported a key as unsent as soon as any parameter had
been seen. VA sends the parameters a report object's query uses, so a
name that has not arrived yet looked like a typo. A mapping is now only
judged once a message introduces no new parameter name, i.e. once the
object's parameter set has settled.
- sameColumnMappings counted a column the table does not have, so one
mistake produced the unresolved-mapping line and the AND line. A column
that is not in the table is now left to unresolvedMappings.
- The Cypress cases asserted exact row counts, which encode the demo
table's contents. They now read the unfiltered count from the app and
assert the filtered grid is smaller (or empty), so a fixture change
cannot fail a behaviour test.
A VA parameter can now carry a transform on the column side of a mapping:
va_parmap=SELECTED_DATE-D_DATE:ADD(QUARTER_START,-1)
which filters D_DATE on the report value AND the same point one quarter
earlier, as a single IN clause - so the AND rule, the clause signature,
saveQuery and the reload loop are untouched.
Transforms resolve to one clause with these operations:
- ADD(<ANCHOR>,<offset>) the value plus one anchor beside it -> IN
- RANGE(<PERIOD>,<offset>) a closed period -> BETWEEN
Anchors: QUARTER_START/END, MONTH_START/END, YEAR_START/END.
Periods: QUARTER, MONTH, YEAR. Offset is a whole number of periods,
defaulting to 0.
The names describe the result rather than mirroring SAS intnx: the
arithmetic runs in the browser, so mirroring intnx would mean
reimplementing its alignment letters and their corners (week alignment,
's' and 'm') with no guarantee of matching SAS. A report author can read
ADD(QUARTER_START,-1) without knowing intnx.
Resolution is anchored on the parameter value, never on today, so a saved
report reproduces the same rows. A transform that cannot run where it is
used - a non-temporal column, a value that is not a date - is skipped and
reported in the existing warning banner rather than silently filtering on
the raw value.
The parse-time problems were logged at app init and then again by the
editor's warning loop, which spreads them into the banner's text. The loop
now logs only what the instance discovers for itself (unresolved column,
unmatched key, AND-ed pair); the parse problems stay in the banner and keep
their single init-time console line.
E2e case 11 pins it: one unparseable entry plus one valid-but-missing
mapping, so the loop certainly runs and logs its own line, and exactly one
line mentions the parse problem.
Two review findings, both paths where a filter could look applied while
producing nothing.
- `unresolvedMappings` judged a mapping against an empty column list while the
grid was still loading, so every entry was briefly reported as naming a
column the table does not have. An empty or unset `cols` now means "not known
yet" and returns nothing, so the banner appears only once metadata exists.
- Two parameters mapped to one column are AND-ed, so different values yield no
rows at all, and nothing said so. `sameColumnMappings()` now reports the
contended column and both parameters. The clauses are deliberately left as an
intersection: turning that AND into an OR is a filter-semantics decision, not
a diagnostic, so it is tracked separately.
Covered by unit tests for both (unloaded column list, one line per contended
column) and e2e case 10, which drives two parameters onto one column and
asserts the banner plus the empty grid.
Three follow-ups from review, all the same class of defect: an author error
that leaves an unfiltered grid looking like a working filter.
- A duplicate key in `va_parmap` silently kept whichever entry came last, so
`va_parmap=DEPT-REGION&va_parmap=DEPT-OTHER` filtered OTHER with nothing to
show for it. One VA parameter now maps one column: the first entry wins and
a conflicting duplicate is reported. The same mapping written twice is not a
conflict, and two different parameters mapping to one column stay valid -
that is how two report objects filter the same column from their own
parameters.
- A parameter-side typo (mapping `DROPDOWN_TAB1` when the report parameter is
`DROPDOWN_TAB_1`) produced no clause, no banner and no console line.
VaMessagingService now records every parameter key it has seen, across
editor reloads, and the editor reports a mapping key that VA has never sent.
Judged only once parameters are actually arriving, because VA sends just the
parameters its query uses - an absent key is not on its own an error, and a
key that turns up later stops being reported.
- The deferred first load cannot check a mapped column against metadata. On
SAS `mp_filtercheck` aborts the save, so the load falls back to unfiltered;
the banner now covers that path too, and an e2e case seeds the pre-bootstrap
message buffer to prove it (and to prove a valid mapping filters the very
first getdata).
A `va_parmap` entry that cannot be used was dropped in silence, in both
places it can fail: at parse time (no `-` separator, a missing side, or a
column that is not a plain SAS name) and at apply time (the mapped column is
not in the table being opened). Either way the parameter is skipped, which
degrades to "no filter" - a typo in the report's URL then looks exactly like
a working filter over the whole table.
Both are now reported in terms the report author can act on:
- a warning banner in the editor (VA mode) naming the parameter, the column
and the table, and pointing at the `va_parmap` entries in the URL
- console.warn for each distinct problem, so it is findable from the report's
console too
The parser returns `{ map, problems }` rather than a bare Map, VaFilterService
gains `unresolvedMappings()` for the entries the loaded table cannot satisfy,
and the editor merges both sets for the banner (logging once per distinct set
rather than once per VA message).
The banner is covered by two e2e cases, one asserting its painted box inside
the viewport (VA hides the app header, so it has to stand on its own) and one
covering a parse-time failure before any VA message arrives. The alert markup
keeps the message inside the alert's text flow so it wraps, instead of being
laid out beside a title and pushed off the right edge.
Two DC instances over the same table in one VA report could not filter the
same column from different parameters: parameter -> column matching is by
name, and two VA parameters cannot share a label.
The `va_parmap` URL parameter overrides that match per instance, one entry
per mapping (`va_parmap=<VA parameter>-<DC column>`, repeated for more):
...?embed=va&va_parmap=REGION_TAB1-REGION
The mapping is additive (unmapped parameters keep the name match), the
column is validated as a plain SAS name at parse time, and an entry only
applies where the named column exists in the table being opened - a miss
skips that parameter rather than falling back to the same-name match the
mapping exists to override. Entries split at the last hyphen, so a label
may itself contain hyphens, and the key matches the parameter label first,
then its internal name (pr###).
Parsed once at app init onto globals (like embed / vaApplyMode) so the
editor reloads a filter apply performs keep the mapping, and both filter
paths (the metadata build and the deferred initial load) honour it.
Also repairs embed-va.cy.ts: a cy.visit differing from the current URL
only in the hash is a same-document navigation, so ?embed=va never took
effect and both existing cases failed; their layout assertions also
described an earlier design (VA now opens read-only with an Edit button
and hides Filter). Cases 3 and 4 drive two mappings of the same column
through a real postMessage and assert the applied filter and row count.
derived-variables.cy.ts:97 expected dc-derived-first on the REGION header,
because the comment assumed REGION opens the computed block. The grid follows
MPE_DERIVED order, and that order is LINE_VALUE, REGION, REGION_LABEL - so the
marker was on LINE_VALUE and correct. The spec asserted the wrong column, and
the development pipeline has been red on it since.
Dumping the header row on a local mock estate (sasjs/server desktop mode,
RUN_TIMES=js) shows:
6 | LINE_VALUE | dc-derived-header dc-derived-first
7 | REGION | dc-derived-header
8 | REGION_LABEL | dc-derived-header beforeHiddenColumn htLastVisibleHeader
and the same dump against the unmodified release/7.19 client, so this was never
a client defect.
The assertion now states the invariant instead: at least one derived header, the
first of them opens the block, and no other one carries the marker. That holds
whatever order the control table returns the variables in.
The hover text on a derived column said only that the column cannot be edited,
which tells the user nothing about what it holds. It now carries the column
label - the configured VAR_LABEL, or the generated name=expression that
mp_derivedvars writes when the administrator left the label blank - and falls
back to the plain rule when the label merely repeats the column name.
The cypress case that marks derived columns apart now also checks the tooltip
for both shapes: REGION_LABEL carries a configured label, REGION falls back to
the generated one.
Derived variables are read-only, but they were rendered exactly like any other
read-only column: dimmed via readonlyCell, with no indication that the column is
computed at read time rather than locked by a rule or by column level security.
The grid could not tell the difference either - getdata holds a derived column
read-only with a READONLY DQ rule, which is also a rule type a table can be
configured with.
getdata now names the derived columns in sasparams.DERIVEDVARS (space separated,
upper case, from MPE_DERIVED's outvars). The EDIT screen holds that list, and
marks those columns with their own classes instead of the read-only dimming:
a tinted header with an 'fx' marker and a tooltip, tinted italic cells, and a
left edge on the first derived column so the computed block reads as a group.
Blue is the info family, leaving the primary key green and the warning ochre to
their existing meanings. Light and dark values are both set.
The SASjs mock getdata.js returns the same field, so the Cypress suite exercises
the styling; the existing derived-variables spec asserted readonlyCell on the
derived header, which is now the fallback for a plain read-only column, so that
assertion moves to dc-derived-header and a physical column stays the control.
Lint: prettier --check clean on the touched client files; sasjs lint reports no
new warnings for getdata.sas.
#359 has landed, so the CLI bump this branch was waiting on is now in the base
and the merge brings it in. One conflict, in
client/cypress/support/commands.ts: both sides reworked loginAndUpdateValidKey.
The base version decides from the body captured before login, which is where
the original bug was - on an estate that shows a login form that capture has no
nav tree, so the command early-returned and never applied a key, and the upload
specs then found no file input. This branch re-reads the loaded page and treats
the licence banner as the signal. Kept this side; prettier re-indented the
result. Every other file merged automatically, and the hookErrmsg veto in
stagedata.js and loadfile.js survives the merge.
The case rested on an editor not appearing after a fixed wait, which passes for
any reason the editor did not open. The editor marks a read-only column with
readonlyCell on the header and on its cells, and STATE in the same grid is the
control, so the state is asserted directly and the sleep goes.
Mutating the mock to drop the READONLY rule fails the case on exactly that
assertion (expected '<th.darkTH>' to have class 'readonlyCell'); restoring it
passes 5/5 in about 50s.
should('not.exist') passes the moment the element is missing, so a timeout on
it does not widen the window: an editor that opens after the assertion has
already passed goes unnoticed. The wait gives it that window, which is what the
case needs to rule the cell's editability out.
The screen opens read-only until the edit button is pressed, so the case
asserted nothing about the column: no cell opened an editor. It now enters edit
mode first, asserts the editor's absence directly rather than typing at
whatever happens to be focused, and a sibling case proves a physical column in
the same grid does open one - so the silence is about the column, not the
screen.
Mutating the mock to drop the READONLY rule fails the derived case and passes
the physical one; restoring it passes both (5/5, ~48s).
loginAndUpdateValidKey decided whether a key was already working by looking
for the nav tree, and looked for it in the body captured before login. A free
tier estate renders the tree as well, so on an unlicensed estate the command
took the 'already licensed' branch every time and never applied a key. Every
upload spec then lost its feature set: onShowUploadModal shows the locked
feature dialog instead of the panel, so input#file-upload never renders.
The loaded page is read again after login, the licence banner decides whether
a key is needed, and an estate with no key is left untouched unless the caller
asked for one - the free tier specs are written against that state.
Measured on an unlicensed mock estate: excel.cy.ts 24 failing in 14m 1s before,
24 passing in 5m 44s after; csv.cy.ts 1 failing before, 1 passing after.
COLS.CLS_RULE is the only thing that marks a column read-only in the grid,
and the client applies it only when CLS_FLAG is set - which column level
security drives. A table with no CLS rules therefore rendered its derived
columns as ordinary editable cells, although the values are computed at read
time and dropped from the submission.
getdata now unions a READONLY dq rule per derived variable, which the client
honours unconditionally, and the mock emits the same rule. The SAS test
asserts the rule; the cypress spec double-clicks a derived cell and asserts the
value is unchanged.
The estate now ships no licence key at all - the placeholder word 'invalid' made
startupservice return an unusable key pair and produced the decrypt-failure page
on every HTTPS site. An unlicensed estate simply runs on the free tier.
loginAndUpdateValidKey therefore no longer infers 'already licensed' from the
nav tree: it detects the free-tier banner (#demo-banner) and uploads a key
through the licensing screen, which posts to admin/registerkey - the API the
licence page's upload button uses.
- zip entries carry flag bit 11 when the name is non-ASCII, so an extractor
reads `BKP_` plus an accented filename back by name instead of CP437
- loadfile and stagedata name the staged CSV after the libds the request
arrived with, before a hook can re-point the submission
- stagedata writes that CSV at all (%mp_ds2csv does), so a grid submission's
audit pack carries the same artefact as an upload's
- loadfile writes no BKP_ file: loadfile.sas copies one only for a second,
non-CSV fileref, which the mock does not accept
- getauditfile says access is unenforced rather than that there is no
permission store (MPE_SECURITY is seeded)
- the special-missings clip no longer claims the estate carries a valid key
The globvars check in licence.service reports a failed startupservice run as
well, so its modal needs the service name for the new-window button - that was
the fourth startup failure path and the only one still a dead end.
Also guard the decodeURIComponent call in the Cypress drive/folder intercept,
so a lone '%' in a URL cannot throw inside the handler and abort the request.
When the startupservice fails, the modal is a dead end: the adapter returned
no log, so there is nothing to act on. The modal now offers a button that runs
the same service through the JES / STP web app in a new browser window, where
the SAS log is returned as the page body.
`_debug=131` is always added (the log is the point), and `_executionTasks` is
never included, so on Viya the run is not submitted as a Compute task and
cannot hit the execution-task timeout. The URL is built by `buildServiceUrl`,
which works for SAS 9, Viya and SASjs Server, and is the same shape the deploy
configurator already uses to open makedata.
The startup failure modals now carry the service name, which is what shows the
button: all three failure branches (malformed response, missing properties,
request rejected) pass `public/startupservice`.
Confirming the "source file will be discarded" warning coerced every
numeric column of the preview with parseInt, which truncated the
fraction: 0.901045951946194 became 0, and 1234.56 became 1234.
parseFloat converts the same values without dropping precision, and a
cell the parser could not read as a number is left as text so the
editor's numeric validation flags it instead of showing NaN.
Adds an Excel spec case that reads the first data row before and after
the switch to edit mode and asserts the decimal value survives.
A selection over DC_MAXOBS_WEBEDIT or DC_MAXCELLS_WEBEDIT was refused with
"Table is too big", leaving the user with no grid and no way to see what the
filter had matched. getdata now serves the first rows that fit and flags it:
- the row cap is min(DC_MAXOBS_WEBEDIT, DC_MAXCELLS_WEBEDIT / columns), so a
wide table is still bounded by the cell budget
- sasparams carries TRUNCATED (0/1) and TRUNC_MSG, naming the limit reached
- the EDIT screen shows a warning triangle next to the table name, with the
message in a tooltip and the advice to filter
The mock mirrors the new behaviour, the Cypress spec asserts the truncated
grid and the tooltip (real hover, opacity checked so a hidden tooltip cannot
pass), and the SAS limit test now asserts the served row counts and message
rather than a cancelled child job.
A derived variable may need to reference one defined before it (a label that
concatenates a derived region, for example), so the order has to be
configured rather than assumed.
- MPE_DERIVED: new VARIABLE_ORDER column (ddl, makedatamodel, migration).
- mpe_derived: statements are generated in VARIABLE_ORDER then name sequence.
VARIABLE_ORDER is required and must be a positive integer. A reference to a
derived variable with an equal or higher order is rejected, because SAS
would otherwise treat it as uninitialised and return a missing value with no
error; references to lower-order variables are allowed.
- mock: same ordering, each variable compiled against the physical columns
plus those defined earlier, so the frontend can exercise a chained variable.
- makedata: VARIABLE_ORDER on both demo rows plus a chained REGION_LABEL.
- cypress: asserts the chained value in the grid.
- tests: the getdata test seeds a second variable built on the first.
- docs: VARIABLE_ORDER documented, with the ordered rule replacing the
self-contained one.
So the EDIT screen can be exercised before a SAS server is available.
- dcMockUtils: mpeDerived() reads MPE_DERIVED for a table and mpeDeriveRows()
adds the computed columns to the rows and to the schema, before the filter
runs - the same order as the real service.
- compileSasExpression() covers the subset of SAS expression syntax the mock
configs use (ifc, cats, catx, upcase, lowcase, strip, trim, length, substr,
index, comparisons, AND/OR and IN). Anything else leaves the column missing,
as a failing SAS expression would. The real evaluation is done by SAS.
- getdata / getcolvals apply the derivation before filtering, so a filter on a
derived column works and dropdown values come from the derived data.
- makedata: MPE_DERIVED schema and rows, a new TESTDATA.DERIVED_DEMO table
(REGION, LINE_VALUE) and its MPE_TABLES registration.
- cypress: derived-variables.cy.ts covers display and filtering on a character
and a numeric derived column.
Also bumps @sasjs/core to 5.6.0, which provides the mp_filterstore targetds=
parameter the validatefilter change depends on.
Two findings from the review at 72a7963.
whereClauseCreator's guard could not match a BETWEEN: checkOperator composes
it as `<from> AND <to>` and the letters of AND are outside the character class,
so the temporal rewrite was skipped and the filter-panel preview kept the
internal day counts (`BETWEEN 42 AND 43`). The separator is removed before the
test, so a BETWEEN pair is shown as literals like a single value or an IN list.
Binding the range dropdown to column.unformatted made the field render the
internal value: setOptionValue assigns the clicked option's value to the input's
ngModel, so after choosing 12FEB1960 the field read 42. The option value is the
formatted label again, as the single-value list and the IN modal already are,
and createFilterQueryTable's BETWEEN branch maps each end back to its
unformatted value before building the literal - the same lookup
whereClauseCreator applies for the preview.
Three new unit cases pin the BETWEEN preview and the BETWEEN RAW_VALUE in both
directions. 576/576 unit tests pass.
With the pickers off, a temporal BETWEEN takes both of its values from a
soft-select over the column's values, so the clause is composed from whatever
that list binds. The test chooses 12FEB1960 for both ends of SOME_DATE and
asserts the applied clause carries the literals and still selects the 5 rows
the internal value selected.
Typing into the range fields - which is what the existing helper does - does
not exercise the binding, so both values are picked from the datalist instead.
Also accepts the evaluation agreement in beforeAll when it is presented. It
is per-origin localStorage, so a fresh browser profile lands on it and the nav
tree never renders; the accept is guarded, so it is a no-op once an earlier
spec in the run has accepted it.
The clause shown above the grid (sasparams.FILTER_TEXT) is the resolved WHERE
clause, so a date filter read SOME_DATE = 34322 rather than a date. The
client now sends a SAS literal for a DATE, DATETIME or TIME column -
'20APR2042'd, '01JAN1960:00:00:42'dt, '00:00:42't - which SAS accepts in the
same place, so the applied filter is unchanged and the clause is readable.
HelperService.sasTemporalLiteral builds the literal arithmetically in UTC, so
the day cannot shift with the browser timezone, and fractional seconds are
preserved. It returns null for anything that is not a temporal value, and the
callers then send the raw value exactly as before.
Used by createFilterQueryTable (the applied filter), whereClauseCreator (the
clause preview in the filter panel), dynamicWhereClauseCreator (the filter
getcolvals applies to the value list) and the VA filter service.
Two fixes fall out of the same code paths:
- the dynamic where clause read the second BETWEEN value from
queryObj[groupInd][clauseInd], which is undefined, so a BETWEEN clause threw;
- an IN over a temporal column sent the *formatted* value, which the backend
refuses for a numeric column - the entries are now mapped back to the
unformatted value before the literal is built.
The mock filter parser reads the literals too. A time literal carries no date
part, so it needs its own pattern - the date pattern alone silently matched
nothing and a time filter returned no rows.
filtering.cy.ts asserts the new clause text and, for every temporal case, the
row count, so a clause that reads well but filters differently cannot pass.
getdata.sas synthesised _____DELETE__THIS__RECORD_____ itself - it added
the column to work.out, hoisted it in front of the PKs, set it to "No" on
every row, gave it a No/Yes dropdown in the metadata and forced MAXLEN=3
in maxvarlengths. In the editor that column is a constant: it is only ever
"No" on load, and its Yes/No rule is already injected client-side
(deleteRecordColumnRule.ts, since #253's vertical-array fix). So the
backend was sending a column the client already owns.
The editor now creates the column, the same way EDIT_STATUS / dc.row_status
is client-only:
- getdata.sas no longer emits it (data steps, the outdata hoist, the
ctrlOptions dropdown, the maxvarlengths override).
- deleteRecordColumn.ts asserts the response does NOT carry the name, then
prepends it to COLHEADERS. Everything downstream (DcValidator's rules,
columnHeader, headerArray) is keyed off COLHEADERS order, so the delete
column is still index 0 and the grid is unchanged.
- Every row is seeded to 'No' before the validator or HyperFormula sees it.
- A response that does carry the name now throws instead of rendering a
table whose delete column would shadow a real column of that name. The
throw lands in the existing getdata .catch, which sets getdataError and
shows the error state. The name is reserved; the docs note is in the
sibling docs PR.
The multi-dataset loader (multi-dataset.component.ts) builds its own
DcValidator from the same responses, so it goes through the same helper.
Mocks updated to the new contract (server + sas9 getdata); postdata,
getstagetable and makedata are untouched - they consume or seed the flag
legitimately, and the upload contract in excel.md / files.md is unchanged.
Verified: client unit tests 570/570, npm run build clean, sasjs lint clean
on the changed SAS, sasjs compile -t server succeeds, and against the
mocked estate coltype-delete-record.cy.ts 2/2, editor.cy.ts 55/55,
excel.cy.ts 23/23 and row-cell-limits.cy.ts 5/5.
Review feedback on this PR.
- The fallback list bypassed mergeColsRules, so col.TYPE was never set and every
column reached the picker as char: char operators for numerics, and unquoted
character values, which %mp_filtercheck rejects on the value list and on the
submit alike. getcols.sas and the mock now emit TYPE in the client's
vocabulary - char for C, num for N/DATE/DATETIME/TIME - and the cypress case
no longer stops at selecting the variable: it sets a value, submits, and
asserts the applied clause reads SITE_NAME = 'Bristol', so the quoting is the
assertion.
- The temporal exclusion is dropped, matching getdata.sas and viewdata.sas,
whose cols payloads include those columns - the drop is on the data, not on
the cols. The mock's getdata.js cols had the same divergence and is aligned;
it keeps the exclusion for the rows. That also removes the SAS/mock
disagreement on the excluded set, since there is no set.
- client/.npmrc added with ignore-scripts and save-exact (plus legacy-peer-deps
and fund, matching the root), so a local `cd client && npm i` is guarded the
way the CI's is.
Verified against the running mock: for MPE_CONFIG (TXTEMPORAL) getcols returns
TX_FROM/TX_TO with TYPE=num and DDTYPE=DATETIME, the char columns as TYPE=char,
and editors/getdata returns cols including both temporal columns while the rows
still exclude them. row-cell-limits.cy.ts is 5/5.
Review feedback on this PR, plus the conflict with the base.
Conflict: row-cell-limits.cy.ts, where both branches added a case. Both are
kept - the config-driven limits case is 4, the filter-picker-after-abort case
is 5 - with the helpers from each side.
- %local in open code is a macro error ("not valid in open code"), logged on
every call, and the variables are never created - so a table with no
mpe_tables row left the INTO variables unresolved. It is %global now, the
way getdata.sas declares the same three variables.
- The hand-rolled format-prefix test typed only DATETIME/DATE/TIME, so YYMMDD,
MMDDYY, DDMMYY, MONYY, E8601DA, B8601DA, E8601DT and NLDATM columns reached
the picker as numeric. The proc contents step is replaced with
%mp_getcols(&ds, outds=cols1) - the call getdata.sas already makes - so the
fallback list is typed exactly as the normal path types it, with the
temporal-column exclusion applied on top.
Review feedback on this PR.
- getdata.js read MPE_CONFIG from the requested table's own library, so the
read failed for every non-control libref and both limits silently fell back
to the shipped defaults. It now reads mpeDataDir, as viewdata.js already
did. Reproduced first: with DC_MAXOBS_WEBEDIT=5, DC_JSLIB.MPE_X_TEST
aborted while TESTDATA.DEMO_01 loaded untouched.
- exportdb.js mapped DATETIME to TIMESTAMP for TSQL as well, and T-SQL's
TIMESTAMP is a deprecated synonym for rowversion, so datetime columns were
declared as rowversions. TSQL now gets DATETIME2; PGSQL still gets
TIMESTAMP. DOUBLE PRECISION is left alone - it is a T-SQL synonym for
float, not an invalid type.
- loadDcConfig read the config row as direct properties, but a row that has
been through the app's own edit-and-approve flow comes back from the adapter
with upper-case keys, so the approved option was skipped and fell back to
its default - the same silent-default failure by another route. It now
reads every field through colVal, like the rest of the mocks.
- row-cell-limits.cy.ts gains a case that moves DC_MAXCELLS_WEBEDIT to a
non-default value through the editor, asserts that a non-control-libref
table is then refused, and puts the option back. It fails if the read is
dropped: verified by reinstating the getdata.js bug and watching it fail.
A limit aborting the table request left the filter picker with no variables,
so the "please filter and try again" advice could not be acted on: the
picker's list came from the columns that arrived with the table, and there
was no table.
The list now falls back to public/getcols, which reads the column metadata
without the rows. The normal path is unchanged - the service is only called
when the caller has no columns to hand - so nothing extra is requested for a
table that loaded.
- SasStoreService.getCols() calls public/getcols and returns cols.
- SasStoreService.setQueryVariablesFromTable() takes the columns the caller
already has and reaches for the service only when they are empty.
- editor openQb() and viewboxes openFilter() use it (the viewer's filter is
reachable after a VIEW abort, the editor's after an EDIT abort).
- public/getcols.sas keeps ddtype (already computed, dropped by the keep) and
excludes the temporal columns exactly as the editor does, so the fallback
list matches the table's own.
- New mock public/getcols.js mirrors the service.
Verified: the mock's list matches editors/getdata for every fixture except the
aborting one (MPE_X_SEARCH: getdata 0 cols, getcols 9), and a new Cypress case
in row-cell-limits.cy.ts aborts a too-big EDIT, opens the filter and asserts
all nine columns are offered. 89 Cypress tests green across the six affected
specs.
The mocked estate could not exercise what 7.17 changed.
- getdata.js and viewdata.js hard-coded their limits, so editing
DC_MAXOBS_WEBEDIT / DC_MAXCELLS_WEBEDIT / DC_MAXOBS_WEBVIEW in DC made
no difference. They now read them from MPE_CONFIG via a new
loadDcConfig/dcConfigNumber pair in dcMockUtils, which mirrors the loop
in the generated settings.sas: DC-scope rows only, var_active=1 only,
current rows only (now < tx_to), latest tx_from wins. An option that is
absent or switched off falls back to the service's own default, which is
what %symexist does in the real service.
- There was no mock for services/admin/exportdb, so the System screen's
Export DC Library DDL action hit a service that did not exist. The new
mock renders every table in the DC libref as basic DDL - SAS, PGSQL and
TSQL flavours, schema defaulting to the libref, a unique index from the
MPE_TABLES buskey, and the download headers mp_streamfile produces. DDL
only: the real service appends inserts for SAS and PGSQL, but
serialising the library's rows in a mock buys no test value.
ddl-export.cy.ts gains two cases that fetch the URL the action opens and
assert the DDL, so the spec covers the service and not just the URL.
Review follow-ups on the row/cell limits:
- getdata.sas checked the limits twice - once before the sort that caps
work.out, and again after PRE_EDIT_HOOK. It is now a single check,
after both, so the counts cover everything that could reach the
browser: the rows the selection matched, plus anything a hook has
added. The payload cap moves to where work.outdata is built, which is
the only place that needs it. Each mp_abort also names its option once
- the limit was repeated in the message text as well as the condition.
- The migration closed only var_active=1 rows but inserted active rows
unconditionally, so an option a site had switched off was silently
switched back on. It now leaves those alone and logs a note.
- row-cell-limits.cy.ts test 1 matched a regex against the grid's text,
which the fixture's own columns satisfy (DEPTH_M reaches 4293,
SAMPLE_COUNT 4210), so it did not pin the raised VIEW cap. It now
reads the PK column - the first td of a .ht_master row - and requires
values in the PK range 1001-2000 with a maximum above 1500, which the
old 500 row cap could never render.
VIEW default 500 -> 2000. EDIT default 100 -> 250, plus a new
DC_MAXCELLS_WEBEDIT (200000) so a wide selection is refused on cells as
well as rows, whichever is reached first. Measured end-to-end: 2000 rows
of a 9 column table is ~1s in VIEW / ~6s in EDIT, but 100 rows of a 1001
column table is already ~17s in EDIT - a row cap cannot bound that.
The EDIT guard also moves: it now checks the filtered row count before
the sort that caps work.out, and again after PRE_EDIT_HOOK, which can
replace work.out with a larger table.
Adds a fourth admin action to the System information screen - Export DC
Library DDL - with a secondary flavour chooser (SAS, PGSQL, TSQL) and an
optional schema box for the DB flavours. The selection is passed to the
existing services/admin/exportdb service in a new window as URL params.
Also bumps @sasjs/adapter to ^4.19.1, adds a Cypress spec for the three
flavour states, and adds a TSQL case to the exportdb SAS test.
Closes#179
hook-programs.cy.ts was in no CI spec list, so it never ran - the only
end-to-end coverage of the new mock hook execution (mockHookSource plus
the PRE/POST_EDIT_HOOK blocks) and of the loader's target re-registration
check was dead weight. Add it to the development job's --spec list.
It goes second, not last: csv-limited.cy.ts asserts the free tier and so
must keep the first slot (before any spec applies a licence key), and test
2 reads the approval queue, which is paged oldest first - a changeset
submitted by an earlier spec pushes its own row off the first page. The
spec header records the constraint. Full list verified locally: 15 specs,
132 tests, all passing.
The post-edit-hook spec asserted on the last row of the approval queue.
The queue also holds changesets from earlier specs in the same run, so the
position is not stable - it failed on an estate with rows left over from a
previous run while passing on a fresh one. Search every row instead.
Both startupservice call sites pass null, and the attach guard skipped a
null payload, so the service whose log a support ticket is read from
never received browser_info / browser_url_vars.
The e2e test named for the startup service only asserted on getdata, so
the gap was invisible. The name is corrected to what it asserts, its
sibling sweep now excludes the diagnostics services (it must, once the
startup service really does carry them), and the guard itself is covered
by a unit spec - red before the fix, green after.
The editor screenshot was taken during the transition from the viewer, so it
captured the viewer's panel - byte-identical to the viewer shot. Wait for the
editor-only chrome (.editor-title, .btnCtrl) before asserting or shooting.
dc
allan
hermes