feat: resource view registries for table and details views - #95
Merged
Merged
Conversation
The test suite could not run at all: the dummy app failed to boot because @ember/string was missing (required by ember-data 4.12), and CI only ran lint and build, so nothing exercised the addon. Test harness: - Add @ember/string so the dummy app boots. - Declare ember-cli-string-helpers, an undeclared runtime dependency used by the crud service, humanize and get-model-name. - Add packages to pnpm-workspace.yaml, required by pnpm 11. Coverage: - Wire ember-cli-code-coverage, which needs three pieces that were absent: config at the addon's configPath (tests/dummy/config/coverage.js), the istanbul babel plugin on both the dummy app and this addon's own tree, and a QUnit.done hook that ships the report. Instrumenting the addon tree is what makes coverage describe addon/ rather than only the dummy app. - Force-load addon modules after the suite so files without tests stay in the denominator instead of silently dropping out. - Fail loudly rather than hang if the coverage upload stalls. - Add scripts/check-coverage.mjs, a per-file 100% gate that also fails when an eligible addon file is missing from the report, with its own node:test suite covering both the passing and failing paths. Tests: - Replace 54 generated "TODO: Replace this with your real tests" stubs with behavioral tests covering nullish, empty, boundary and invalid input. - Pin the actual contract of four utils that ignore their arguments and always return true (ison, reverse-point, is-function, hason-structure) and of get-mime-type, which returns an extension rather than a mime type. CI: - Run the full suite with coverage and enforce the gate; previously no tests ran. - Upgrade to checkout@v4, setup-node@v4 with pnpm cache, pnpm/action-setup@v4, and install with --frozen-lockfile. - Upload lcov to Codecov with fail_ci_if_error so a broken upload is visible. - Add least-privilege permissions, concurrency cancellation, and gate the publish jobs on the test job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two problems kept the suite from ever producing a coverage report. The socket service builds a SocketCluster client in its constructor, and socketcluster-client retries a failed connection forever. Under test that meant an endless stream of failed WebSocket connections to the testem server, so the page never went idle. Tests now plant a marker script node that satisfies the load-socketcluster-client initializer's own idempotency guard, and replace the global with an inert fake. No production code changes. Four utils imported config from `@fleetbase/console/config/environment`, hard-coupling the addon to one consuming app and making the modules unloadable anywhere else, including the dummy app. They now use `ember-get-config`, which is already a dependency and is what the rest of the addon uses. In the console app this resolves to the same config. Also gate the coverage hook on a config flag so normal test runs do not pay for collecting and shipping coverage, and repeat the plugin's default node_modules and mirage excludes, which a project-level `excludes` replaces rather than extends. Without them istanbul instruments every dependency. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lean index.js is published, so requiring the ember-cli-code-coverage devDependency at module scope tripped n/no-unpublished-require and failed CI lint. The plugin is only needed while running this repository's own suite, so it is now resolved behind the same COVERAGE env var it keys off. Consumers of the published addon never load it. Also replace the upstream forceModulesToBeLoaded with a scoped version. The upstream helper walks every module in the build and a module whose import cannot be resolved wedges the end of the run. Failures are collected instead of thrown, which is safe because an unevaluated module is simply absent from the report and scripts/check-coverage.mjs fails the build when that happens. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Node 22.23 resolves a bare directory argument to node --test as a module path rather than a directory, so the step failed in CI while passing on the local 22.22. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The dummy app disables prototype extensions, which is the Octane default, and that exposed three real defects: - group-by called arr.objectAt() and pushObject() on plain arrays, so it threw for every caller not passing an Ember array. - get-mime-type called objectAt() on the result of Object.keys(). - array-utils re-exported `default` from stable-by-ids, which only has a named export, so `arrayUtils.stableByIds` was undefined. The app re-export had the same mistake. Also fixed: - extract-coordinates reassigned `latitude` in the missing-longitude branch, so a coordinate pair with no longitude returned [0, null] instead of [0, 0]. Covered by a regression test. - is-waypoint-record imported ../models/waypoint, which does not exist anywhere in this addon, so importing the module threw for every consumer. Removed as dead code along with its app re-export and stub test. Flagged for review in the pull request: this is an API removal, but the export could not be used. - Removing that unloadable module also unwedged the end of the test run, so coverage is now posted and written without intervention. Corrected assertions in four of my own tests that encoded the wrong contract (Ember treats an empty Map as blank; isFinite(null) is true; the app re-export only forwards default exports). Coverage now reports 162 of 168 eligible addon files, up from 140 of 169. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Writing real tests for the utils surfaced three genuine bugs: - app/utils/array-utils.js re-exported `default`, but the addon module only has named exports, so importing sameIds/stableByIds/arrayUniqueBy from the app path yielded undefined. Same mistake as stable-by-ids. - context-component-callback treated `options: null` as an object, because `typeof null` is 'object', and threw while reading the callback off it. - copy-to-clipboard and lazy-load-script stubs were raising unhandled global failures that QUnit attributed to whichever test happened to be running, so unrelated tests failed. Both now stub their boundary (the navigator clipboard, and data: URLs instead of the network) and cover the success, rejection and already-loaded paths. The is-electron test asserted that the current browser is not Electron, which made it depend on the runner: it passes under headless Chrome and fails in an Electron-based browser. Every branch is now driven with an explicit user agent. to-model, to-leaflet-bounds and replace-table-row are pinned as they behave today, with notes: to-model creates its helper without an owner so it always throws, and replace-table-row's `if (rowIndex)` guard skips a match at index 0 and treats a missing row as index -1. Failing tests are down from 45 to 31, of which only three are not generated stubs. Coverage is at statements 585/3763, branches 393/2484, functions 173/898, lines 558/3605. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tensions
The fetch service imports `fetch` from ember-fetch, which was never declared in
package.json and is not installed here. Like ember-cli-string-helpers, it only
worked because the console application happened to provide it. Added as a
dependency.
Several modules read host configuration as soon as they are evaluated — the
fetch service touches config.API.host at module scope — so the dummy app now
supplies the API, socket and osrm sections a host application is expected to
configure. Without them those modules cannot be loaded, let alone measured.
The extension manager imports getExtensionLoader from
'@fleetbase/console/extensions'. That is a function rather than config, so
ember-get-config cannot redirect it; tests register an AMD stub for the module
instead, which keeps production code unchanged.
Rewrote the application serializer test, which called createRecord('application')
for a model that does not exist. It now registers real models and checks the
uuid primary key, the underscored polymorphic type key, and that the read-only
slug is stripped both from a serialized record and from a bare payload.
Coverage reaches 164 of 168 eligible files, up from 162, and failing tests are
down from 31 to 28, of which only two are not generated stubs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… calls Two sources of cross-test interference are gone, and with them the last failures that were not simply untouched generated stubs. ember-local-storage caches its storage objects across owners, so the second test to use a `storageFor` service inherited the previous test's destroyed object and failed with "calling set on destroyed object". Storages are now reset after every test. The fleetbase-api-fetch stub called the util with no stubbing at all, which issued a real network request. Its asynchronous "Failed to fetch" surfaced as a global failure that QUnit attributed to whichever test happened to be running, so unrelated tests failed seemingly at random. It now stubs window.fetch and covers url construction, the namespace override, GET query serialisation, json bodies, default and overridden request options, the bearer token from a stored session, non-2xx responses, fallback responses and network failures. auto-serialize called objectAt on a plain array, the same breakage already fixed in group-by and get-mime-type, and is now covered for arrays, the except list, empty and populated relationships. Also replaced the waypoint-label, timeout, get-pod-methods, get-meta-field-types, mock-response and normalize-polymorphic-type stubs. 323 tests, 25 failing — all of them generated stubs, none behavioural. Coverage: statements 673/4094, branches 465/2628, functions 185/962, lines 643/3930. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
consoleUrl passed window.location.host into extractHostAndPort when no host was supplied. That value is only "hostname:port", which `new URL` cannot parse, so the parse failed and the fallback produced an invalid "https:///path". It now passes a full url built with the current protocol. get-routing-host read waypoints.firstObject, an Ember array property that does not exist on a plain array once prototype extensions are off, so the waypoint branch never matched. This is the fifth instance of that pattern, after group-by, get-mime-type, auto-serialize and find-closest-waypoint, which is also fixed here (objectAt, pushObject, sortBy and firstObject all replaced). New tests cover console-url (query encoding, host and port extraction, explicit and derived hosts, ports, empty subdomains), get-routing-host (per-country servers, waypoints, fallbacks), map-engines (mount paths, route naming, external routes shared across engines, extra services), group-api-events, find-closest-waypoint, leaflet-icon, has-extension, and the two always-true column-filter utils, which are pinned with notes. 359 tests, 21 failing — all untouched generated stubs. Coverage: statements 708/4095, branches 490/2628, functions 189/963, lines 676/3930. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
register-component and register-helper are tested through a real owner: derived and explicit names, the dasherizing of both, and that an existing registration is never overwritten. The serialize helpers get full branch coverage — rewriting backslashed class names onto _type attributes, copying a nested relation type, splitting an embedded relation into the relation and its id, custom primary keys, blank payloads, and passing non-object input straight through. is-relation-missing is pinned rather than changed. Its non-polymorphic branch computes `isset(model, relation_uuid) && !isset(model, '')`, and the empty-string key looks like an unfinished edit: reading a blank path is always falsy, so the negation is always true and the result reduces to "is the foreign key set". The test says so explicitly so the next reader does not have to work it out. 383 tests, 16 failing — all untouched generated stubs. Coverage: statements 736/4095, branches 529/2628, functions 189/963, lines 704/3930. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
app-cache.has() and doesntHave() were always true and always false. They asked
`this.get(k) !== undefined`, but `get` substitutes its default for a missing
value and so never returns undefined. Passing undefined explicitly does not help
either, because a JavaScript default parameter applies whenever the argument is
undefined. Both now read storage directly through a small helper.
notifications.serverError crashed on a null error while reading `.errors`, which
a rejected promise carrying no value would produce. Guarded.
Worth noting for review: ember-cli-notifications and ember-can each ship their
own app/services/{notifications,abilities}.js, which collide with this addon's
re-exports of the same names. Which file wins depends on build order, so
`service:notifications` did not resolve to this addon's subclass in the dummy
app at all. The tests register the classes under test explicitly rather than
relying on that lookup, but the collision is real and may mean the overrides are
not active in consuming applications either.
Also covers table-context and the abilities parse override.
406 tests, 16 failing — all untouched generated stubs.
Coverage: statements 782/4097, branches 555/2630, functions 208/964, lines 746/3932.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two more instances of the prototype-extension pattern, bringing the total to seven: - loader.js pushed onto routesLoaded with pushObject, which does not exist on a plain array once extensions are off. Reassigning the array also invalidates the tracked property properly. - language.js read this.locales with objectAt while building its available locale map, so the map could never be built. The loader is covered across conditional display, selector and element targets, the body fallback for a missing target, message defaulting, overlay removal, and the transition paths that record a route and avoid stacking overlays. The language service is covered with fake intl and fetch services: locale seeding, the country lookup map, locales with no matching country, language listing, lookup by a custom property, persisting a locale change, and a failing lookup leaving the service usable. 422 tests, 16 failing — all untouched generated stubs. Coverage: statements 853/4097, branches 597/2630, functions 224/964, lines 813/3932. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`urlParams` is a getter that builds a new URLSearchParams from window.location.search on every access, so every mutator writes to a throwaway object that is discarded as soon as it returns: setParam, setParamArray, remove no observable effect clear throws, it assigns to a getter-only property updateUrl, getFullUrl, getPathWithParams re-serialise the unchanged current URL The read side works, because it reads live from the URL, and so do the *CurrentUrl methods, which operate on a real URL object and push it to history. Nothing is changed here. Making the mutators work means choosing a storage model — a cached instance that can go stale, or mutating the real URL directly — and that is a design decision for the maintainers, not a typo fix. The tests state plainly which methods are inert so the next reader does not have to rediscover it, and they will fail the moment somebody makes them work, which is the point at which the decision gets made. 17 tests, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tracked-built-ins was imported by contracts/universe-registry.js and services/universe/registry-service.js but never declared, the same class of bug as ember-cli-string-helpers and ember-fetch. It is now a dependency. That does not fully resolve it — the module still is not present in the built app, which is precisely why those two files have never appeared in the coverage report. The UniverseRegistry tests are held back with a note until that resolves. Covers base-contract (option copying, falsy values distinguished from missing ones, chaining, defensive copies out of toObject and getOptions, and validation running through setup), registry (name composition through withNamespace and withSubNamespace, the option kept in step with the name, and the missing-name error), the contracts index re-exports, and the ExtensionBootState and HookRegistry singletons, including that each instance owns its own containers rather than sharing class-level ones. 24 contract tests, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both construction paths are exercised — a bare name, a name with a handler, a
name with an options object, and a full definition object — along with the
defaults, the fluent chaining API (execute, withPriority, once, withId, enable,
disable, setEnabled, withMetadata) and toObject serialisation.
One case is pinned rather than asserted as an error. The definition branch is
gated on `isObject(x) && x.name`, so `new Hook({ handler })` with no name falls
through to the string branch and the object itself is assigned as the name.
Being truthy it passes validation, and the handler is silently dropped, so a
typo'd definition fails somewhere far from the mistake. The test says so.
480 tests, 16 failing — all untouched generated stubs, zero behavioural failures.
Coverage: statements 964/4097, branches 642/2630, functions 268/964, lines 924/3932.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@fromstore works and is covered: it queries lazily on first read, caches so the store is consulted once, defaults its query and options, assigns null when the query rejects, and skips the query entirely when a value has been assigned. @isEqual does not work on Ember 5.4 with ember-decorators 6. The decorated property reads back as undefined regardless of the two source properties, because the inner function hands a ComputedProperty to decoratorWithRequiredParams where a property descriptor is expected, so nothing is installed on the class. Its parameter list is also mislabelled — it declares (target, desc, key, params) where the caller passes (target, key, desc, params) — which is harmless only because neither is used. Nothing is changed. Fixing it means deciding how the property should be defined, which is a maintainer's call; the tests pin the current behaviour with that explanation and will fail as soon as somebody makes it work. 9 decorator tests, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Codecov step sat after the 100% gate, so it only ever ran when coverage was already perfect — which meant it never ran at all, and no report has reached Codecov yet. It now runs before the gate and on a run whose tests failed, so the data flows while the numbers are still climbing. A missing or unreadable report still fails the job, and the gate still fails the build when coverage is short. legacy-from-store is covered: lazy querying, caching, null on rejection, and the assigned-value bypass. It is byte-for-byte identical to from-store — both are exported so both are tested, but one is redundant and the two will drift apart the first time somebody edits only one. The test says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both construction paths, including the widgetId legacy alias and that an explicit id wins over it; the three shapes a component can take (a string path, a plain object, and an ExtensionComponent that gets flattened via toObject); the default flag surviving construction, asDefault and toObject; the merge semantics of withGridOptions and withOptions; withTitle and withRefreshInterval writing into options; that every setter returns the widget; and the missing-id error. 17 tests, 63 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both construction paths: a string path (mirrored as the name), an options object carrying loading and error components, and a component class (stored with the class name as the name and no path). Plus the chaining setters, toObject, and the two toString forms. Worth a maintainer's attention: unlike Hook, Widget and Registry, this constructor never calls super.setup(), so validate() does not run on construction. A component with no engine, or with neither a path nor a class, is built happily and only fails later, somewhere less obvious. The rules themselves are fine — calling validate() directly reports both problems — they are just never enforced. Pinned rather than changed, since adding the call could start throwing for consumers who are currently getting away with it. 11 tests, 39 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both registration paths: a lazy path, where the name is the final segment, and a direct class or function, where the name is derived from the class name. The derivation is covered at its edges — PascalCase split to kebab-case, consecutive capitals handled (HTMLParser becomes html-parser), single words lowercased, named functions treated like classes, and an anonymous function yielding no name. One quirk is pinned as-is: the Helper suffix is stripped after kebab-casing, so FormatDistanceHelper becomes "format-distance-" with a trailing hyphen, while FormatDistance becomes "format-distance". The suffix rule runs against the already-hyphenated string and only removes the word, not the separator. 9 tests, 17 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both construction paths, slug derivation from the title, defaults, and that an explicit false or zero survives rather than being replaced by the default. Also the chaining setters, addItem flattening a MenuItem to its object form while passing plain objects through, addItems, and the _isMenuPanel indicator on toObject. One quirk pinned: the slug is derived with dasherize(title) before super.setup() runs, so a missing title throws a TypeError from inside dasherize and the intended "MenuPanel requires a title" message is never reached. An empty string does reach it. The failure is still loud, just less helpful than intended. 13 tests, 43 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both construction paths, the title seeding text/label/id/slug/view, every default, zero priority and index surviving rather than being defaulted, tag normalisation, nested items and shortcuts, the chaining setters, and toObject. Two problems are pinned rather than changed: The onClick chaining method is unreachable. The constructor assigns `this.onClick = definition.onClick || null`, an instance property that shadows the prototype method of the same name, so the documented `.onClick(handler)` call invokes null and throws. A handler has to be passed in the definition instead. Renaming one of the two would fix it and would be a breaking change either way, so it is a maintainer's call. renderInPlace() sets only the option, not the property, unlike every other setter. toObject still reports the right value because options are spread last, but reading item.renderComponentInPlace directly gives the stale answer. 23 tests, 72 assertions, 0 failing. All twelve contracts are now covered. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
filters.js had three more calls that do not exist on a plain array once prototype extensions are off — pushObject when collecting active filters, and objectAt when reading both controller and route query params. That is ten instances of this pattern now, across group-by, get-mime-type, auto-serialize, find-closest-waypoint, get-routing-host, loader, language and filters. theme is covered across preference resolution (stored user option, then initial theme, then system preference), applying a theme and its body classes, the persist flag, the theme.changed event, toggling, the sandbox environment class, route body classes, and console loader removal. filters is covered across value serialisation (dates, arrays, nested dates, blank filtering), the pending-parameter lifecycle including status "all" and blank values clearing rather than storing, apply writing onto the controller and resetting pagination, and activeFilters reading from the route with managed and blank parameters excluded. Both suites needed a stand-in for the private router microlib the service reads to find the current route; the helper is documented in the test. 17 theme tests and 18 filters tests, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
socket.js pushed onto its tracked channels list with pushObject and read it back with objectAt, neither of which exists on a plain array once prototype extensions are off. That is twelve instances of this pattern now, across nine files. The push is replaced with a reassignment so the tracked property invalidates properly. Covered: client construction from the application socket config, the fallback to window.location.hostname when no hostname is configured, coercion of the secure flag, instance() returning the underlying client, subscribing and tracking channels, waiting on the subscribe listener, tolerating a missing callback, and closeChannels closing every tracked channel and being safe with none. The suite-wide SocketCluster stub keeps this off the network; these tests install a richer stand-in to observe what the service asks for. 10 tests, 15 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers the redirect target, the onboarding flag, the loader overlay, expiry reading, two-factor lookup including its failure path, and session event fan-out to both the events service and the universe. Two findings. getSessionSecondsRemaining subtracts the wrong way round. It computes (now - expiry) instead of (expiry - now), so a session with a minute left reports roughly -60 and an expired one reports a positive number. The magnitude is right and only the sign is wrong, which is exactly the kind of thing a caller may already be compensating for, so it is pinned rather than corrected. This is the third app-tree collision: ember-simple-auth ships app/services/session.js at the same path this addon re-exports, so `service:session` did not resolve to the subclass at all — none of its methods existed on the looked-up instance. Same shape as notifications (ember-cli-notifications) and abilities (ember-can). The test registers the class under a distinct name, but three collisions in one addon is a pattern worth addressing at the source. 12 tests, 20 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers the enable flag (on by default, only an explicit false disables, and nothing is emitted while disabled), the fan-out to both local listeners and the universe, the session events including the three aliases termination emits, the user and organization events with their nullish handling, and the resource events. The resource cases pin the useful details: creation emits both a generic resource.created and a model-specific order.created, safe properties are read off the record, absent or null optional properties are omitted rather than sent as null, explicit properties override the ones read from the resource, and a missing resource still emits. 16 tests, 34 assertions, 0 failing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
getUserPermissions builds `const permissions = []` and then called permissions.pushObjects(...) three times. pushObjects does not exist on a native array once prototype extensions are off, so gathering permissions threw for any user who had any. Replaced with push and a spread. Worth being precise about the distinction, because a blanket substitution would have been wrong: the objectAt calls in the same method are on ember-data relationship arrays, which keep Ember's array methods regardless of the EXTEND_PROTOTYPES setting. Those are correct and are left alone. Only the plain array literal was broken. Covered: permissions applied directly to the user, permissions from the role, permissions from each policy on the role, policies applied directly to the user, all four merged, an empty user, policies with no permissions, a role with neither, and that duplicates are deliberately kept rather than collapsed. The fixtures mimic the ember-data shape the method reads rather than building real records, since this is plain aggregation logic. 9 tests, 0 failing. Fifteen prototype-extension defects fixed across ten files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A filtered run force-loads every addon module but exercises only the tests that match the filter, so it produces a report with the full denominator and almost no numerator — 56/4096 statements across 269 files, which reads as a total collapse. That report overwrote the good full-suite one, and coverage:check reads the same file, so a partial run could make a healthy tree look broken. The reverse is worse: a filter narrow enough to cover its own subset could in principle satisfy the gate on a fraction of the suite. The QUnit.done hook now bails out when QUnit.config.filter, module or testId is set, leaving the previous full-suite artifact intact and saying why. Verified both directions: a filtered run leaves coverage/ absent, and an unfiltered run still writes a credible report — statements 1491/4097 (36.39%), branches 935/2630 (35.55%), functions 376/964 (39%), lines 1445/3932 (36.74%) across 164 addon files. CI was never affected, since it only ever runs the unfiltered suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bulkAction's `remove` callback called selected.removeObject(model). When resolveModelName is supplied the selection has just been through .map(), which returns a native array with no removeObject, so removing a row from the confirmation modal threw. It now filters and republishes the new reference via setOption, which the modal already reads through. That works whether the caller passed a plain or an Ember array. The upload flow had the same shape: `const uploadedFiles = []` followed by uploadedFiles.pushObject(...). Now push. Left alone deliberately: the uploadQueue calls in the same file operate on a value from modalsManager.getOption, whose type this service does not control, so they may legitimately be Ember arrays. Also adds a test-only stub for @fleetbase/ember-ui/utils/smart-humanize, which crud imports. ember-ui cannot be a dependency here because it already depends on @fleetbase/ember-core, so without the stub the crud service cannot be loaded in the dummy app at all — which is why it never appeared in the coverage report. The stub is a faithful copy of the real implementation rather than a simplification. Worth a maintainer's attention: ember-core already ships addon/utils/humanize.js with the same acronym list, so this circular reach is for a near-duplicate of something core already owns. 9 tests covering the empty-selection guards, the modal contract, custom templates, resolveModelName including its non-string branch, and removal — with an explicit regression test for the post-map case that used to throw. Seventeen array defects fixed across eleven files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pattern I had been grepping for was incomplete — it missed removeAt, and
searching the whole addon for the full set of Ember array methods turned up
three more sites:
chat.js — openChannels is a plain array literal, so pushObject and removeAt both
threw; the cached 'open-chats' list comes back from app-cache as a plain array
too, so pushObject and removeObject threw there as well. Opening or closing a
chat channel could not work at all. Reassigned rather than mutated, which also
makes the tracked property invalidate properly.
make-dataset.js — dataset is a plain array literal built with pushObject, then
sorted with sortBy. Worth flagging separately: it sorts by 't', but the points
it builds are {x, y}, so that sort has never done anything. The no-op is
preserved deliberately rather than quietly picking a real key, with a note.
menu-service.js — `A(items).filter(...).sortBy('priority')` looks safe but is
not: filter returns a plain array and drops the Ember mixin, so sortBy was
undefined. Re-wrapped. The two nearby `A(panels).sortBy(...)` calls are correct
and untouched.
Checked and deliberately left alone: macros/group-by.js builds its groups with
A(), so findBy is legitimate; chat's feed/attachments/receipts calls are on
ember-data relationships.
Ten tests cover opening and closing channels, deduplication, id-based matching,
closing something that was never open, and recovery from a corrupted cache entry.
Twenty-four array defects fixed across fourteen files.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
I recorded 32 statements as impossible to cover. Several of those calls were the same mistake I have made six times already — describing the dependency rather than testing it. "Ember always supplies an owner" is true of a service built through the CONTAINER. `RegistryService.create()` builds one directly with no owner at all, which is exactly the shape those `if (!owner)` fallbacks were written for. Injections only throw if the path touches them, and none of these do. "#getApplication's fallbacks are unreachable" was an artefact of my own tests: every extension-manager test gives the universe stub an applicationInstance, so the search short-circuits at the first step. A universe without one falls through to the owner's application. "language:12 is a tracked initialiser the constructor overwrites" — the constructor starts loadAvailableCountries, but that task yields on its first statement, so `countries` is still its declared default when the constructor returns. "deleting window.URL aborts the run" was never tested. I assumed it shared the blast radius of a whole-test document.createElement override, which aborted a run earlier. Scoped to the call it is fine — the framework does not consult window.URL between synchronous statements. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
I asserted a string payload takes downloadjs's btoa branch. It does not: with Blob present — which is every browser this suite can run in — even a string becomes a real Blob and takes the FileReader route, same as the sibling test. The btoa branch is for a browser with no Blob constructor at all, where downloadjs falls back to its own `toString` and `payload instanceof myBlob` throws, because a bound function has no prototype. Genuinely unreachable, and recorded as such rather than faked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
registry-service's #loadHelperFromEngine opened with the same `if (!owner) return null` that registerHelper — its only caller — had already applied on the identical expression. `owner` was not used anywhere else in the method, so the whole block was dead. Same class as the seven fixed earlier. extension-manager's remaining two: #getApplication's last resort, an owner with no `.application`, which is what an EngineInstance looks like. `owner.application` is blanked around the call and RESTORED — an earlier attempt deleted it and never put it back, and Ember's ApplicationInstance#willDestroy reads `this.application._unwatchInstance` during teardown, which took the run down. the hooks #onEngineInstanceBuilt schedules on next(). In every ordinary ordering the boot patch clears them first, because boot resolves as a microtask and next() is a runloop task. An engine whose boot never resolves leaves them in place, which is the only way that callback has anything to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each carries a one-line reason at the code it suppresses, so every one is visible in review rather than hidden in a config file. No file is excluded and no threshold changed — download.js in particular stays measured, so the parts of it that ARE covered keep counting. 4 @Tracked initialisers whose constructor assigns the same field on its very next line, so the default is never read. Suppressed rather than deleted: they document the field's shape, and removing working code to satisfy a metric makes the codebase worse, not better. 2 module-scope `if (isBlank(config.API.host))` fallbacks in adapters/application and services/fetch. Both run at import; whichever module loads first sets the host, so the other can never enter. Not even a dummy-app config change can cover both. 5 in the vendored downloadjs: `location.href = url`, which would navigate the page away and take the run with it, and the no-Blob-constructor branch, which throws on `payload instanceof myBlob` before it can be reached because a bound function has no prototype. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correcting my own reporting first: I have been quoting STATEMENT coverage as "coverage" all along. The gate checks statements, branches, functions and lines per file, and while statements and lines are now at 100%, branches are at 96.63% and functions at 99.79% — 90 branches and 2 functions across 33 files. This is the first batch: default parameters every call site happens to supply, the right-hand side of a `??` whose left side is always set, and the false arm of a guard. Small utils and the one- and two-branch services. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes the four failures from the last run: MockTask's declared no-op is overwritten by its own constructor, applyContextComponentArguments only reaches its false arm with an EMPTY model name (a null one throws inside camelize first), theme's currentTheme needs a currentUser that answers getOption, and authenticatedOptionOwnerId is a getter — the chain is walked by taking its inputs away instead. New coverage: the small-util defaults, corslite's onprogress no-op, the browser-derived whois fallbacks, both legacy decorators' skipped arms, the universe cascade's remaining three guards, hook-service's missing names and non-callable handlers, the registry's non-clearable lists and explicit application instance, and five more download.js routes. Ignored with a reason each: MockTask's dead default, group-api-events' always-true guard, console-url's URL port default / multi-label host / import-time environment, menu-service's and custom-fields-registry's private defaults that their sole callers always supply, corslite's portless arm, chat's RSVP.all else, hook-service's normalizeHook defaults and the serializer's keyForRelationship fallback.
RegistryService built its shared registry in a FIELD INITIALIZER, which runs during construction — before setApplicationInstance can have been called — so the documented "first priority: use applicationInstance if set" could never happen and every service silently fell back to the owner. Resolved lazily now, the same fix HookService's registry already carries. `/* istanbul ignore next */` above a class METHOD does not suppress its parameter defaults, so the four private methods whose sole callers always pass every argument simply lose the dead defaults instead: #normalizeHook, #normalizeMenuPanel, #scopeKey and #enrichProperties. Also fixes the three failures from the last run: hasHook returns undefined rather than false for an unknown name (pinned as it stands), the registry test needed the lazy fix above, and downloadjs schedules its click through `later`, so the assertion has to wait for settled(). New coverage: the fetch verbs' data defaults and a content-disposition with no filename, hasMany serialization and removeReadOnlyAttributes' default, the session's no-transition and message-less failure paths, an export whose format was cleared, and instantiate on the registry's engine-helper path.
resource-action: the router getter's three-link fallback chain (setupTest's
owner always resolves service:router, so the last link needs an owner that
resolves neither), bulkDelete and export with no selection at all, and the
`?? {}` beside each of the four public options fields — which is what a
subclass clearing one of them lands on.
subject-custom-fields: a staged entry whose field record carries no type at
all, a null staged over an existing value, a values bag cleared outright, and
adapter options reaching the update and the deletion rather than only the
create. Plus the re-check inside the afterRender write, which exists because
something else may write the same list first — so the test writes it first.
extension-manager: a base with no dependencies, a hostRouter the application
cannot resolve, an engine that throws on registration, and a primitive default
export.
download.js: `/* istanbul ignore next */` is not honoured on a declarator
inside a var list, so the myBlob assignment is hoisted into its own statement.
Same reason the four private methods lost their dead defaults last commit.
Fixes the serializer assertion from the last run: removeReadOnlyAttributes
returns the payload it was given.
It landed outside that module, so it missed the beforeEach that calls finishLoadingExtensions() and fills application.extensions — setupExtensions then sat in waitForExtensionsLoaded until QUnit's 60s timeout. This is the last uncovered branch in the addon.
The gate is green on all four metrics. Replaces the "what still cannot be covered" list — written when 32 statements were out of reach — with the list of what is marked unreachable and why, and adds #29 (RegistryService built its registry before it could be told where to), #30 (hasHook returns undefined), #31 (MockTask's dead default) and #32 (resource-action's duplicated option fallbacks).
…ring sanitization' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
ember-data 4.12 requires @ember/string as a peer, and resource-action imports ember-cli-string-helpers; neither was declared, so the dummy app failed to load before any test ran.
getRegistry, registerInRegistry and lookupFromRegistry called RegistryService without a list name, so the arguments shifted and nothing was stored or found. getMenuItemsFromRegistry and getMenuPanelsFromRegistry read lists that never exist, so they always returned nothing — which left registered tabs out of panels that use them and made virtualRouteRedirect a no-op. They now default to the 'menu-item' list (as createRegistry does) and the menu getters delegate to MenuService.
Extensions can now add columns, row actions, bulk actions and toolbar
buttons to table views, and header buttons and menu items to details views,
of any engine.
Registry names follow <extension>:<surface>:<resource>:<slot>, where the
extension is the owning engine's console mount segment, the surface is table
or details, and the slot is one of columns, row-actions, bulk-actions or
actions (table), or actions or menu (details). e.g.
fleet-ops:table:driver:columns, ledger:details:invoice:menu.
- contracts: TableColumn, ResourceAction, ActionButton (+ ResourceViewItem)
- universe/resource-view-service: validated register/unregister, sugar per
slot, declare(), merge()/mergeSlot()/mergeRowActions(), queryParamsFor()
so registered filter columns become controller query params
- utils/merge-registered-items: placement by before/after/index/priority
with per-slot defaults, built-ins win id collisions, handlers bound to the
view context, copies so renderers never mutate the registry
- ResourceActionService: tableRegistry/detailsRegistry derived from the mount
prefix and model name, and mergeRegistered()
- universe facade: getService('resource-view') and register* methods
- exported to engines so the host singleton is shared
The merge context now copies property descriptors instead of spreading, so a view can hand over lazy getters (its table is only set up after first render) without them being read during the render that merges. ResourceActionService.mergeRegisteredColumns merges registered columns and row actions for views that render their own <Table>.
Index controllers declare `queryParams = this.<x>Actions.queryParamsFor([...])` so filter params of registered filterable columns become query params.
This was referenced Sep 30, 2026
Merged
Live defects: - crud: bulk-action message no longer doubles the count (#2); the import queue is an Ember array (#11); the modelName option overrides the model's own name in dialogs and the bulk endpoint (#26) - fetch: a bare Content-Type is parsed (#5); a caller's filename wins over content-disposition (#15) - filters: Ember's mapped query-param form is unwrapped to property names (#4) - registry-service: classes are keyed, so they can be looked up (#6); only a Helper subclass is instantiated, however a function helper is written (#12) - universe facade: list names are passed, menu readers ask the menu service, dashboardWidgets reads the 'dashboard' dashboard (#7) - current-user: loadWhois warns when the lookup fell back (#8) - session: promiseCurrentUser aborts and invalidates once (#9); seconds remaining is expiry minus now (#10) - chat: remembering an open channel keeps the list (#13) - subject-custom-fields: writeFieldValue checks for a field first (#14) Latent and behavioural: - MenuItem's chaining setter is withOnClick(); onClick stays the handler (#24) - organization and user account menus are separated by key prefix (#25) - #26: '~'-slug menu items get unique registry keys; transitionMenuItem carries query params; replace-table-row handles index 0 and misses; get-mime-type returns mime types for whole extensions; custom-field panels keep merged saveOptions; sameIds gets its options; loadSubjectCustomFields rethrows (9 of 10 callers already catch); the stub utils do what their names say; legacy-from-store re-exports from-store; legacy-fetch-from reads null until loaded on native class fields Also: fetch.cachedGet never expired a cache exactly a month old (it compared the 'days' component of intervalToDuration); expiry is now elapsed time. The make-dataset tests no longer depend on the machine's timezone. Every pin is rewritten to assert the fixed behaviour. 2319 tests pass locally.
… registries Both land in release/v0.3.25. Conflicts: - universe.js: #90 makes the same registry facade fix as this branch; keep this branch's resource view facade methods on top - universe-test.js, resource-action-test.js: #90 rewrote both; take #90's, and move this branch's tests to universe-resource-view-facade-test.js and resource-action-registries-test.js - package.json: #90 already declares ember-cli-string-helpers (as a runtime dependency) and @ember/string, so this branch's devDependency is dropped
roncodes
added a commit
that referenced
this pull request
Oct 3, 2026
… gaps - universe.js: the registerInResourceView/registerTableColumn/... facade was dropped while resolving the #90/#95 merge conflict (the resolution matched a '// ====' banner as the conflict marker); restored - lookup-user-ip test: the CodeQL autofix compared the hostname against 'geoiplookup.io'; the primary API is json.geoiplookup.io - coverage: drop a registryExtension initializer initialize() always overwrites, resourceView's '?? null' and an unreachable 'context ?? {}'; cover the optional arguments of TableColumn, mergeRegisteredItems, mergeSlot and mergeRowActions, an Ember array as a base list, and a non-string value on a required file custom field
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a standard way for extensions to add columns, row actions, bulk actions and toolbar buttons to any engine's table views, and header buttons and menu items to its details views.
Registry names follow one rule,
<extension>:<surface>:<resource>:<slot>:tablecolumns,row-actions,bulk-actions,actions(toolbar)detailsactions(header buttons),menu("…" items)Examples:
fleet-ops:table:driver:columns,ledger:details:invoice:menu. The extension segment is always the owning engine'sconsole.<x>mount segment. The full guide is indocs/resource-view-registries.md.Changes
TableColumn,ResourceActionandActionButton, sharing aResourceViewItembase: a requiredid, placement withbefore/after/index/priority, andpermission.universe/resource-view-service:register/unregister, plus a helper per slot,declare()andresourceRegistryNames();merge(),mergeSlot()andmergeRowActions();queryParamsFor(), so registered filterable columns become controller query params.utils/merge-registered-items:delete).fn(row, ctx),fn(selectedRows, ctx)andonClick(ctx).ResourceActionService:tableRegistryanddetailsRegistry, derived from the mount prefix and model name (override withregistryResource), plusmergeRegistered(),mergeRegisteredColumns()andqueryParamsFor().universefacade:getService('resource-view')and oneregister*method per slot.Fixes
getRegistry,registerInRegistry,lookupFromRegistry,getMenuItemsFromRegistryandgetMenuPanelsFromRegistrypassed the wrong arguments toRegistryService, so they stored or returned nothing. This is a separate commit.virtualRouteRedirect, which the console'sauth/loginroute calls, has never found a menu item until now. It will now redirect hiddenauth:loginvirtual pages on direct load.@ember/string(an ember-data peer) andember-cli-string-helperswere never declared; they are now devDependencies.Release order
This merges and releases (0.3.25) first. Then:
The engines need their
@fleetbase/ember-core/@fleetbase/ember-uiranges bumped and lockfiles refreshed once these two are published.Testing
ResourceActionService. All pass locally.tests/helpers/universe-registry-stubs.js, because the dummy app can't construct the real registry service (tracked-built-ins,@fleetbase/console/extensions).