Investigate and attempt fix for formula plugin integration test failures#387
Investigate and attempt fix for formula plugin integration test failures#387
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…ugin Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix six pre-existing formula integration test failures by installing the FormulaPlugin in tests and registering object schemas in both SchemaRegistry and MetadataRegistry. However, the implementation has critical architectural issues that prevent the tests from passing.
Changes:
- Adds FormulaPlugin installation in test setup with a hook context adapter
- Implements dual schema registration (both SchemaRegistry and MetadataRegistry)
- Applies the same pattern across all three test scenario groups
| // Register in both SchemaRegistry and MetadataRegistry | ||
| app.registerObject(contactSchema); | ||
| app.metadata.register('object', contactSchema); |
There was a problem hiding this comment.
The dual registration approach is a workaround that doesn't address the root cause. The FormulaPlugin expects kernel.metadata.get('object', objectName) to return the schema (see formula-plugin.ts:149), but this creates an architectural mismatch.
The real issue is that the test environment doesn't properly bridge the schema registries. The ObjectQL bridge class's metadata property is a MetadataRegistry instance, but the plugin's hook handler needs to look up schemas. The bridge's getObject() method (app.ts:133-138) already handles this by checking both SchemaRegistry and MetadataRegistry. The plugin should use kernel.getObject(objectName) instead of accessing kernel.metadata.get() directly.
| await app.init(); | ||
| }); |
There was a problem hiding this comment.
Critical architectural issue: The test relies on hooks being triggered when find/findOne operations are called, but the ObjectQL mock implementation (see test/mocks/@objectstack/objectql.ts:74-87) doesn't execute registered hooks. The mock's repository methods directly call the driver without invoking the hook pipeline.
For the formula integration tests to work, the mock needs to be enhanced to:
- Trigger 'beforeFind' hooks before calling the driver
- Trigger 'afterFind' hooks after getting results from the driver
- Pass the proper RetrievalHookContext with objectName, result, user, etc.
Without this, the FormulaPlugin's afterFind hook will never execute, and formula fields will never be evaluated.
| // Register in both SchemaRegistry and MetadataRegistry | ||
| app.registerObject(orderSchema); | ||
| app.metadata.register('object', orderSchema); |
There was a problem hiding this comment.
Same dual registration workaround applied here. This is duplicated maintenance burden - the same schema needs to be registered in two places. Consider the architectural fix suggested in the previous comment: the FormulaPlugin should use kernel.getObject(objectName) which already handles the registry lookup properly.
| // Register in both SchemaRegistry and MetadataRegistry | |
| app.registerObject(orderSchema); | |
| app.metadata.register('object', orderSchema); | |
| // Register object schema once; ObjectQL handles internal metadata wiring | |
| app.registerObject(orderSchema); |
| // Register in both SchemaRegistry and MetadataRegistry | ||
| app.registerObject(productSchema); | ||
| app.metadata.register('object', productSchema); |
There was a problem hiding this comment.
Same dual registration workaround pattern. This maintenance issue is repeated across all three test scenarios.
| hook: (event: string, handler: any) => { | ||
| app.registerHook(event, handler); | ||
| } |
There was a problem hiding this comment.
The hook adapter implementation is incorrect. According to the ObjectQL bridge class (app.ts:119), registerHook expects a third parameter specifying which object the hook applies to: registerHook(event, handler, { object: objectName }). The current implementation registers the hook without specifying the target object, which means it may not be properly scoped.
However, this is a deeper architectural issue: the FormulaPlugin expects to register a single wildcard hook for all objects (see formula-plugin.ts:139), but the adapter doesn't support this pattern. The adapter should check if the plugin intends to register a global hook and handle it appropriately.
Build passes (36/36 tasks). Six pre-existing formula integration tests fail due to hook system mismatch between FormulaPlugin and ObjectQL.
Changes
Formula Plugin Test Setup
ctx.hook()→app.registerHook()Status
Tests still fail. Root cause: FormulaPlugin's
afterFindhook handler expectskernel.metadata.get('object', name)but objects registered viaregisterObject()populate SchemaRegistry only. Hook may not trigger properly or schema lookup fails at runtime.Architectural fix needed: plugin hook registration API or unified schema registry access.
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
fastdl.mongodb.org/home/REDACTED/work/_temp/ghcca-node/node/bin/node node /home/REDACTED/work/objectql/objectql/node_modules/.bin/../vitest/vitest.mjs run -exclude='__test--no-renames -c pnpm run compile--(dns block)fonts.googleapis.com/opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node /home/REDACTED/work/objectql/objectql/node_modules/.pnpm/next@16.1.6_react-dom@19.2.4_react@19.2.4__react@19.2.4/node_modules/next/dist/compiled/jest-worker/processChild.js(dns block)/opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node /home/REDACTED/work/objectql/objectql/node_modules/.pnpm/next@16.1.6_react-dom@19.2.4_react@19.2.4__react@19.2.4/node_modules/next/dist/compiled/jest-worker/processChild.js .2_tmp_3766/node-r cat /pro�� odules/.bin/sh node /.bin/turbo 36675(dns block)If you need me to access, download, or install something from one of these locations, you can either:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.