chore(release): 0.25.0 - #330
Merged
Merged
Conversation
The rule set lived inline in UserManagementController, in two near-identical copies — one in store(), one in update(). That is against the project's own convention that validation belongs in a Form Request, and it is why the two copies had drifted: only one of them carried the worker-code uniqueness ignore in a form that meant anything. StoreUserRequest holds the shared set; UpdateUserRequest extends it and overrides only what an edit changes — uniqueness ignores the record, the password becomes optional, and workforce values the record already holds stay acceptable. That last one is a deliberate behaviour change: editing somebody whose crew or wage group has since been deactivated now saves. Before, their own stored value was refused because the pickers no longer offered it. The worker screen already carries this fix, via the same trait. One form still writes to two tables — the account, and the personnel record addressed through worker_* keys. The mapping stays in the controller; only the rules moved.
A module that contributes a field to a core form has to reach three places: the form must show it, the request must accept it, and something must store it. This is the middle one, and without it the other two are useless — validated() returns only the keys the rule set names, so a field nobody declared is dropped between the browser and the controller, silently and with no error to show. The four Form Requests behind the user and worker screens now pass their rules through FilterRegistry, the same mechanism the tab matrix, the soft-delete list and the sync shapes already use. With no module listening the array comes back untouched, so the community path costs nothing. The context carries the record being edited — a module needs it to write its own `unique … ignore` — and whether this is a create or an edit. Two conventions are documented on the trait rather than enforced: keys are prefixed `module_<name>_<field>`, because the rule set is one flat array shared with core and an unprefixed key will eventually replace a core rule; and a filter receives the complete set and *can* weaken core rules, which is a real hole to weigh before accepting a third-party module. This is the validation half of the arrangement a separately distributed module needs. Such a module cannot deliver working React into a released install — the page globs are resolved at build time — so its field has to be rendered by core code from data the module supplies.
HookRegistry has been complete and entirely unused since it was written. This
puts it to work on the two screens a separately distributed module needs, and
closes the last of the three gaps such a module faces: the field is shown, it is
accepted (the rule filter, previous commit), and now it is stored.
Shown: the user and worker forms resolve `display.admin.{users,workers}.form
.fields` and hand the contributions to ModuleFields.jsx, which draws them from
data. That indirection is not a preference — a module installed from a ZIP can
never deliver working React, because the page globs are expanded by Vite when
core is built. Contributing data core renders is the only thing that works.
ALLOWED grows the keys that describe a field. `type` is checked against the
three controls core can draw and anything else is dropped with a line in the
log, the same degradation the browser already applies to a component this build
does not contain. `required` is cosmetic: what the backend demands is decided by
the module's validation rule, not by its contribution.
Stored: dispatch() resolves a `persist.<page>` point for its effect, called
inside the controller's transaction. A module's write belongs to the same unit
of work as the record it hangs off — otherwise a failure there leaves the
account saved and its field silently missing. A listener that throws therefore
rolls the save back, which is intended and is tested. The worker controller had
no transaction at all; it has one now.
Two things the tests caught rather than review: resolving the hooks in both
formData() and edit() ran a module's callback twice, the first time with no
record to read; and a hook point's name contains dots, so assertInertia's paths
read every segment as another level of nesting.
Verified in the running app with a stand-in module: the field renders with its
label, help text and asterisk, the server's own validation error appears under
it, and the persist hook receives the value.
refactor(users): move user administration validation into Form Requests
The rule listed code, views, validation messages, seeders and comments, and was read as being about source files. It is not: this repository is public, and a pull request description is the first thing a stranger reads. Naming the surfaces explicitly is cheaper than finding out which ones somebody assumed were exempt.
…er capture Settings → System has offered a choice between a keyboard-wedge reader and manual entry since it was merged. The request validates it, the controller passed it to the page as a prop — and the page never read the prop. Picking `manual` changed nothing, and since the station has no input field of its own, that mode left the operator with no way to enter a code at all. The setting's own description promised them "a visible field" that did not exist. The station now reads the prop: `hid` keeps the document-level capture, `manual` detaches it and shows the field the description always described. The capture moved to lib/useScanBuffer.js, split in two: createScanBuffer holds the rules — buffer, Enter, 500 ms discard, ignore keys while a field has focus — free of React and the DOM so they can be tested under the `node` environment this project's Vitest setup uses, and the hook wires it to the document. The hook reads its callback through a ref, so a re-render no longer detaches the listener mid-scan. Ten unit tests for the rules, four feature tests for the setting reaching the page. Verified in the running app both ways: with `manual` the field appears, with `hid` it is gone and typing a code followed by Enter still reaches the server, which answered "Unknown EAN" — the whole path.
feat(extension): render and store the form fields a module contributes
fix(packaging): honour the scanner mode setting, and extract the reader capture
…c-surfaces docs: say that English-first covers pull requests, issues and commits
Two ends of a module's life that were not guarded. `requires_core` has been in every manifest since modules existed and nothing ever read it. A module built against extension points this core does not have installed cleanly, enabled cleanly, and then quietly did nothing — its fields never rendered, its listeners were never called, and the only symptom was a support conversation. Both install and enable now check it and name the two versions. Enable is checked as well as install, because install-time checking does not cover a module shipped with the image or one installed before core was rolled back — and enable is the step that runs its migrations. CoreVersionConstraint is deliberately not a Composer-grade resolver: one optional operator and one version, which is all any manifest here uses. An unreadable constraint is refused rather than guessed at — a typo that quietly means "any version" is the exact outcome this is meant to prevent. A module declaring nothing is unaffected, which is most of them. uninstall() now calls the module's own uninstall hook first, while its classes are still on disk; afterwards there is nothing left to load, so the hook never ran at all. It is the module's only chance to undo what it did outside its own tables — permissions it registered, settings it wrote. A hook that throws aborts the uninstall and leaves the directory in place, so it can be retried once whatever it tripped over is dealt with; deleting anyway would run half an uninstall and lose the other half. Migrations are still not rolled back, and the success message now says so rather than leaving it to be discovered. Also moves the runInstaller docblock back onto runInstaller — it had been left stranded above migrationsPath, describing the wrong method.
feat(modules): enforce requires_core, and run a module's uninstall hook
Two seams, and deliberately no new mechanism. A display region on the workstation page, resolved the same way the admin forms resolve theirs, so a module that has something to say about the person at the machine has somewhere to say it. A module distributed as a ZIP cannot ship working React into a released install, so the contribution is data and core draws it — the existing Hook component already does, unchanged. The rule filter from the user and worker Form Requests, applied to starting and completing a step. Without a declared rule `validated()` drops an unknown key between the browser and the controller, silently; with one, a module's own field arrives and is validated server-side rather than trusted. The context names the operation as 'start' or 'complete' rather than the trait's create/edit wording: both are POSTs, and which one it is, is the whole distinction here. Nothing else turned out to be necessary. StepStarted and StepCompleted are already dispatched by BatchStepEventObserver, inside BatchService's transaction and on every path that moves a step, so a module hears about the work itself without core growing another hook for it. Not covered: the API step-completion request, which keeps its own rule set. The station flow this serves is the operator panel; widening it can wait for a module that needs it.
feat(extension): let a module reach the operator's station screen
…ies, ModuleTestCase Four generic seams a module reaches the product through, none naming a module: - MenuRegistry::addOperatorItem() renders a module's screen as a tab on the operator top bar (Inertia link, path-prefix highlight), bridged as moduleNav.operator. - modules/<Name>/lang/<locale>.json is merged under core's translations at bootstrap, so a module ships its own strings without touching lang/. - ImportRegistry::entities() runs through the import.entities filter, so a module can add an entity to Admin → Import; the instance cache is rebuilt when the list changes. - Tests\Support\ModuleTestCase registers a module's provider per test and migrates its directory inside the test transaction. Sidebar entries a module contributed are tinted with the accent colour, and now highlight on their own pages: modules register absolute URLs, which the path-based active check never matched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej2yusocapyJ9KsNcDv63v
- Move SeamThingImporter to its own file so it autoloads when a test runs alone - Match operator tab prefixes at a path boundary (/operator/team vs /operator/teams) - Keep the core shift cell when a module's component is missing from the build - Route the remaining work-order quantity inputs through QuantityField - Extract mergeMessages() and test that core strings win over module ones Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Feat/module operator hooks
`vendor/bin/rr get-binary` asks api.github.com for the release list, so every build depended on GitHub being reachable and on DNS answering at that moment. Five services in docker-compose.yml build from this Dockerfile, which means a single `docker compose up --build` made that call several times over in parallel — enough for a resolver to return NODATA and fail the install outright, with an error naming RoadRunner rather than the network. The binary now comes from the official image. The version is pinned to the `spiral/roadrunner` release in composer.lock, because Octane loads the PHP side from vendor and executes this binary and the two are meant to match; the comment says to bump them together. Verified: the image builds to completion and `/var/www/html/rr --version` reports 2025.1.14, the same release composer.lock resolves. This does not make the build offline — composer and npm still reach the network. It removes one dependency that was being hit N times for no reason.
…ithub fix(docker): take the RoadRunner binary from its image, not from GitHub
The rsync rules that keep locally installed modules out of a release were
written unanchored:
--exclude='modules/*'
An rsync pattern without a leading slash matches at any depth, and this
repository has a second directory called `modules`:
`backend/resources/js/Pages/admin/modules`. So every release package was built
without those three pages. Vite then had nothing to compile, the Inertia
resolver found no component, and the UI answered "Page unavailable" — on the one
screen that uploads and enables a module, which is the only way to install one
on the web build at all.
The patterns are now anchored to the transfer root, meaning backend/modules and
nothing else. Verified by running the real rules against this repository: the old
set produced 225 of 228 pages and dropped exactly Index, Install and Store; the
anchored set produces 228, and backend/modules still contains only the three
bundled examples and README.
Also adds the check that would have caught it. The existing guard compares the
Dockerfile's COPY paths against the package, and it passed here because the
directory it copies was present — merely emptied. The new one asserts every
tracked file under backend/resources is in the package: that tree is entirely
source that ships, so anything missing from it is a mistake by definition.
Against a package built with the old rules it reports the three files and fails.
Reporting became opt-in in 0.24.3 and the web setup wizard asks about it on its
admin step. A Docker install never reaches that wizard: docker-entrypoint.sh
creates the admin account and writes storage/installed, which is explicitly
there to skip it. So nobody who installed with install.sh or install.ps1 was
ever asked — the question existed, on a screen that population never sees.
The prompt defaults to no, and an unattended run counts as no rather than as
consent, which is the same rule the application applies to a setting nobody
ever set.
Writing OPENMES_TELEMETRY to .env is not on its own enough, and that is the
part worth reading carefully. TelemetrySettings::enabled() treats the env var
as a veto — set and falsy forces reporting off — but what turns it ON is the
stored setting, and an absent row means off. So three pieces had to line up:
- install.sh / install.ps1 ask and write OPENMES_TELEMETRY to .env;
- docker-compose.yml passes it into the backend container, which is where the
scheduler that sends reports actually runs (entrypoint, not a sidecar) — it
was not in that environment list, so the variable would have gone nowhere;
- the entrypoint records the choice in system_settings, guarded on the
application not being marked installed yet, so it cannot overwrite a choice
an administrator later makes in Settings → System.
Verified each link rather than assuming it: the prompt answers no to silence,
"", and "n" and yes only to y/yes; `docker compose config` shows
OPENMES_TELEMETRY reaching the backend service; and the entrypoint's snippet
writes true and false into system_settings.telemetry_enabled against a real
database.
A module installed through Admin → Modules → Install brought a working backend and a dead frontend: its React pages were not in the bundle, because the bundle was compiled before the module existed and nothing rebuilds it on a running system — the production image deletes node_modules right after building. Every page of such a module rendered the missing-page card, while its routes answered normally. A module now ships its own compiled JS. Core loads it before the first render and reads what it registered as a third source in the resolver. Precedence is unchanged: core > compiled-in module > runtime module. The contract (20 entries: react, @inertiajs/react, @openmes/ui and 14 @core paths) lives in packages/module and is the single source of the two lists that would otherwise be maintained by hand on both sides — core generates its runtime from it, a module build its externals. A module therefore carries no React of its own (two instances on a page break hooks) and builds without a core checkout, since externals are never resolved at build time. Versioning: core advertises apiVersion, a module records the version it was built against, and the loader skips a module on mismatch instead of letting it bind to a contract that no longer holds. Three things only surfaced by running it, and they are encoded here: - Inertia resolves the initial page's component BEFORE setup(), so modules are awaited in the resolver rather than loaded in setup — otherwise a cold load of a module page fails while client-side visits succeed, which reads like a caching bug. - The initial payload lives in a <script type="application/json">, not in the root element's data-page. Reading the wrong one fails silently: module loading never starts and every module page 404s in the resolver. - A namespace from `import * as` carries no __esModule flag, so a module bundler's interop helper treats it as CommonJS and hands back the whole namespace as the default export. The page then renders an object as a component and React throws #130. The runtime re-wraps each namespace. Assets are cache-busted on content, not mtime: publishing copies the file, so mtime would change on every enable/disable cycle and discard the browser's cache for nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRWsLoNeVWdsSHve6vUmxs
…les-admin-screen fix(release): stop the package from stripping the Modules admin screen
…tallers feat(install): ask about usage reports in the install scripts
Load module pages at runtime, without rebuilding the frontend
ModuleManager::loadEnabled() registered each enabled module's provider inside a try/catch, and the comment promised "a bad module never prevents the application from booting". It only half held. Laravel calls a provider's boot() later, while booting every provider, so an exception thrown there escapes that catch: the module that threw takes every route with it. Seen with a module built against a newer core — it called a menu helper with an argument this version does not accept. The result was a 502 on the whole installation, including the admin screen needed to disable the module again; recovery meant deleting its directory by hand. Providers are now wrapped in ModuleProviderGuard, which runs register() and boot() itself and swallows what either throws, logging it and reporting it. A module that fails simply contributes nothing — no routes, menus or listeners — and the rest of the application is untouched. The guard is registered with force: true because the container deduplicates providers by class name. Every module's guard is the same class, so without it the second module's guard would be mistaken for the first one's and that module would never load at all — a quieter bug than the one being fixed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QRWsLoNeVWdsSHve6vUmxs
Keep a failing module from taking the application down
Dev/tracebility
…n entry Two commits on develop changed how modules behave and left no trace in the changelog, which is where anyone upgrading looks first: d200222 — a module installed after the fact now has a working frontend. Its pages used to render the missing-page card while its routes answered normally, because the bundle is built before the module exists and the production image deletes node_modules right after. 1a339fb — a failing module no longer takes the whole application down. An exception in a provider's boot() escaped the try/catch around registration and took every route with it, including the screen needed to disable the module. Also folds the Unreleased section back into one Added, one Fixed and one Changed. Each merged branch appended its own heading, so the section had nine and the same kind of change appeared in three places. Changelog only — no version bump, no code.
feat: import log
docs(changelog): record the two module changes that shipped without an entry
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: Mes-Open/OpenMes/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
4 participants
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.
Release 0.25.0. MINOR — new capability, nothing breaking.
Two files:
backend/config/version.php→v0.25.0, and theUnreleasedsection closed under a dated heading. Everything below is already ondevelopand reviewed.What this release is about
Modules stop being a build-time arrangement.
Until now a module installed through Admin → Modules brought a working backend and a dead frontend: its React pages were never in the bundle, because the bundle is compiled before the module exists and the production image deletes
node_modulesright after building. Every page of an uploaded module rendered the missing-page card while its routes answered normally.A module now ships its own compiled JS, which core loads before the first render and reads as a third source in the page resolver — core, then a compiled-in module, then a runtime one. Alongside that, the seams a module needs on core's own screens: fields on the user and worker forms, a region on the operator's station, and validation rules that survive
validated().Added
packages/moduleso core's runtime and a module's externals come from one list rather than two kept in step by hand.requires_coreis enforced at install and at enable. Every manifest has carried it since modules existed and nothing read it, so a module built against extension points this core lacks installed cleanly and then quietly did nothing.install.shorinstall.ps1was ever asked. The prompt defaults to no, and an unattended run counts as no rather than as consent.Fixed
boot()escaped thetry/catcharound registration and took every route with it — including the admin screen needed to disable that module.modules, includingresources/js/Pages/admin/modules, so the one screen that installs a module was missing from every package.up --build.Verification
/admin/modules(the screen the packaging bug removed); module-owned screens; a module-contributed field arriving on the user form.