feat(web): add OPropertiesField for form and list summary - #271
Conversation
- Expose BaseModel.ResolveProperties on the web store so Form loads schema⊕map without Browse attaching resolved values. - Add OPropertiesField with PP7 light controls, full-map replace writes, schema∩map list summary, and empty/unknown-type safe rendering. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR exposes property resolution through ChangesProperties field resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant OPropertiesField
participant WebModelStore
participant BaseModel
participant resolveProperties
OPropertiesField->>WebModelStore: ResolveProperties(record, fieldName, options)
WebModelStore->>BaseModel: ResolveProperties(record, fieldName, options)
BaseModel->>resolveProperties: Resolve property map and effective schema
resolveProperties-->>BaseModel: ResolvedPropertyItem[]
BaseModel-->>WebModelStore: ResolvedPropertyItem[]
WebModelStore-->>OPropertiesField: ResolvedPropertyItem[]
OPropertiesField->>OPropertiesField: Write normalized property map
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
- Use (v: any) update handlers matching other O*Field components so ElSwitch assignability passes CI vue-tsc. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/web/web/components/field/OPropertiesField.vue`:
- Around line 264-288: Update reloadResolved to maintain a monotonically
increasing request id and capture the id for each ResolveProperties call. Only
apply resolvedItems updates, skipped-item warnings, and error handling when the
captured id is still current, so older responses cannot overwrite newer record
or containerId results. Add a test that starts two requests, resolves the newer
one first, then verifies the older response does not change the resolved schema.
- Around line 36-91: Associate each property label and control in the field
template for accessibility. Add a stable, unique matching id to every rendered
control and set the corresponding label’s for attribute, or apply
itemLabel(item) as each control’s aria-label; ensure all branches, including
boolean, numeric, text, date, selection, and fallback controls, receive the
association.
- Around line 62-69: Update the datetime branch in the date-picker binding to
convert stored values with utcToUserWallDate and picker updates with
userWallDateToUtc followed by formatUtcIso before calling onItemWrite. Keep
date-only handling unchanged, and remove reliance on the literal [Z]
value-format suffix for datetime conversion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e9ea6f5-43d8-44d7-aad4-af66f05d6b38
📒 Files selected for processing (6)
modules/core/service/orm/model/index.tsmodules/core/service/orm/model/model.tsmodules/web/web/components/field/OPropertiesField.test.tsmodules/web/web/components/field/OPropertiesField.vuemodules/web/web/components/field/oproperties_helpers.tsmodules/web/web/stores/modelStore.ts
- Wire label for/id on every property control for accessible naming. - Convert datetime values through utcToUserWallDate / userWallDateToUtc / formatUtcIso instead of a literal [Z] value-format. - Ignore stale ResolveProperties responses with a generation guard, and cover race / UTC / a11y in unit tests. Co-authored-by: Cursor <cursoragent@cursor.com>
- Expand helper and component unit tests across PP7 controls, races, and fallbacks. - Exercise BaseModel.ResolveProperties in properties resolve BE tests. - Extract propertiesFieldKey and drop an unreachable sync generation guard. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/web/web/components/field/oproperties_helpers.ts (1)
96-126: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAvoid ordinary assignment for
__proto__.
assertValidPropertyDefinitionItemsaccepts__proto__, and the resolver passes it through. The dynamic assignments create no own key and can changenext's prototype, so the property is omitted from the submitted map. UseObject.definePropertyor reject__proto__during validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/web/web/components/field/oproperties_helpers.ts` around lines 96 - 126, Update buildFullPropertiesMap and writePropertyValue to safely persist the schema key "__proto__" using an own-property definition rather than ordinary assignment, including when copying previous, value, default, or explicitly written values. Preserve the existing filtering and replacement behavior for all other property names.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/web/web/components/field/OPropertiesField.test.ts`:
- Around line 145-153: Update the test factory’s store selection logic to
distinguish an explicitly provided store value, including undefined, from an
omitted store by checking whether the store property exists on opts. Preserve
the existing default-store behavior only when the property is absent, so the
missing-store test exercises the intended path.
---
Outside diff comments:
In `@modules/web/web/components/field/oproperties_helpers.ts`:
- Around line 96-126: Update buildFullPropertiesMap and writePropertyValue to
safely persist the schema key "__proto__" using an own-property definition
rather than ordinary assignment, including when copying previous, value,
default, or explicitly written values. Preserve the existing filtering and
replacement behavior for all other property names.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ceb4a9ce-3d2e-4623-ba57-667743655adf
📒 Files selected for processing (4)
modules/core/service/orm/model/properties_resolve.test.tsmodules/web/web/components/field/OPropertiesField.test.tsmodules/web/web/components/field/OPropertiesField.vuemodules/web/web/components/field/oproperties_helpers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- modules/web/web/components/field/OPropertiesField.vue
- Build replace maps with Object.create(null) so "__proto__" is an own data key. - Copy field values via own-key enumeration in toView for the same reason. - Let the test factory treat store: undefined as an explicit missing store. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
User description
Summary
BaseModel.ResolveProperties(andWebModelStore.ResolveProperties) so Form loads schema⊕map without Browse attaching resolved values (PP3).OPropertiesFieldwith PP7 light controls, full-map replace writes, schema∩map list summary, empty-schema safety, and unknown-type skip.Test plan
go run . test unit web --fe(896 passed, no Vue warn / stderr noise)go run . test unit web(BE + FE)go run . test typecheck webgo run . test unit core --beMade with Cursor
PR Type
Enhancement, Tests
Description
Expose
BaseModel.ResolvePropertiesandWebModelStore.ResolvePropertiesfor schema+map resolution.Add
OPropertiesFieldcomponent with form editing, display, and list summary of resolved property items.Introduce helpers (
filterRenderablePropertyItems,buildFullPropertiesMap,writePropertyValue, etc.) for item filtering, full-map replace writes, and schema∩map intersection count.Add comprehensive unit tests covering rendering, item write, summary text, empty schema, and container-driven re-resolve.
File Walkthrough
index.ts
Export property‑resolve types and helpersmodules/core/service/orm/model/index.ts
ResolvePropertiesOptions,PROPERTIES_V1_TYPES,normalizePropertiesMap,isPlainPropertiesMap,PropertyItemDefinition,and
ResolvedPropertyItemfor wider access.model.ts
Expose ResolveProperties on BaseModelmodules/core/service/orm/model/model.ts
resolvePropertiesandResolvePropertiesOptions.BaseModel.ResolvePropertiesstatic method that callsresolvePropertiesand returnsResolvedPropertyItem[].modelStore.ts
Add ResolveProperties to the web model store typemodules/web/web/stores/modelStore.ts
ResolvePropertiesservice method to theWebModelStoreinterface.
oproperties_helpers.ts
Add helpers for render, full‑map write, and summary countmodules/web/web/components/field/oproperties_helpers.ts
filterRenderablePropertyItems,normalizeSelectionOptions,countSchemaMapIntersection,buildFullPropertiesMap,writePropertyValue.normalisation, and full-map replace writes.
OPropertiesField.vue
Add OPropertiesField component for editing and list summarymodules/web/web/components/field/OPropertiesField.vue
(switch, number, textarea, date picker, select, etc.) based on the
resolved schema.
modes.
containerIdprop changes.OPropertiesField.test.ts
Add unit tests for OPropertiesField and helpersmodules/web/web/components/field/OPropertiesField.test.ts
OPropertiesFieldand its helpers.intersection, and full‑map write helpers.
full map on edit, shows summary count, handles empty schema, and
re‑resolves on container change.
Summary by CodeRabbit
New Features
Bug Fixes