48849 frontend [ fieldsets ] Advanced fieldset rules - #344
48849 frontend [ fieldsets ] Advanced fieldset rules#344Maria-Lordwill wants to merge 48 commits into
Conversation
… to disabled state
…to frontend/fieldsets/48741__provide_information_regarding_the_binding_of_the_field_set_to_specific_templates
…ge, convert px to rem
…to frontend/fieldsets/48741__provide_information_regarding_the_binding_of_the_field_set_to_specific_templates
…ent field in run/kickoff, truncate large text placeholder in run
…llipsis, and icon spacing
…d description inputs
… linked fieldsets
…ails, and FilterSelect integration
…to frontend/fieldsets/48741__provide_information_regarding_the_binding_of_the_field_set_to_specific_templates
…esets_2' into frontend/fieldsets/48849__advanced_fieldset_rules
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟠 High
Updating a fieldset with an existing field raises AttributeError instead of saving because FieldSetTemplateService._update_fields forwards rulesets to FieldTemplateService.partial_update, which attempts setattr(field, 'rulesets', value) on the reverse relation. Remove rulesets before the call, or handle it through the field-ruleset service.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/services/fieldsets/fieldset.py around line 238:
Updating a fieldset with an existing field raises `AttributeError` instead of saving because `FieldSetTemplateService._update_fields` forwards `rulesets` to `FieldTemplateService.partial_update`, which attempts `setattr(field, 'rulesets', value)` on the reverse relation. Remove `rulesets` before the call, or handle it through the field-ruleset service.
| ) | ||
| service.partial_update(**rule_data) | ||
| rule_api_names.add(rule_api_name) | ||
| service.partial_update(**ruleset_data) |
There was a problem hiding this comment.
🟡 Medium fieldsets/fieldset.py:403
Updating an existing ruleset with a new fields list persists text fields in a sum rule without revalidating its unchanged groups_or, so invalid numeric validation behavior is accepted. FieldsetTemplateRuleSetService.partial_update only validates groups when they are created or updated; revalidate the existing sum groups after updating fields.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/services/fieldsets/fieldset.py around line 403:
Updating an existing ruleset with a new `fields` list persists text fields in a sum rule without revalidating its unchanged `groups_or`, so invalid numeric validation behavior is accepted. `FieldsetTemplateRuleSetService.partial_update` only validates groups when they are created or updated; revalidate the existing sum groups after updating `fields`.
|
|
||
| def validate_rules(self) -> bool: | ||
| rules = list(self.instance.rules.order_by('type').all()) | ||
| rules = list(self.instance.rulesets.order_by('type').all()) |
There was a problem hiding this comment.
🟠 High fieldsets/fieldset.py:83
validate_rules raises Django FieldError before validating any rules because FieldSetRuleSet has no type field for order_by('type'); the rule operators are stored on nested groups_and records. Update the query and grouping logic to use the nested operator field.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/services/workflows/fieldsets/fieldset.py around line 83:
`validate_rules` raises Django `FieldError` before validating any rules because `FieldSetRuleSet` has no `type` field for `order_by('type')`; the rule operators are stored on nested `groups_and` records. Update the query and grouping logic to use the nested operator field.
| ) | ||
| rule_ids.append(rule.id) | ||
| fieldset.rules.exclude(id__in=rule_ids).delete() | ||
| fieldset.rulesets.exclude(id__in=rule_ids).delete() |
There was a problem hiding this comment.
🟠 High workflows/kickoff_version.py:119
Updating a kickoff version can delete unrelated FieldSetRuleSet records and leave the requested rules only in deprecated FieldSetRule rows. _update_fieldset_rules collects IDs from FieldSetRule.objects but applies them to fieldset.rulesets, so the new relation is never populated and its records are removed when their IDs are absent. Create/update the new ruleset model and use those records for this cleanup.
Also found in 1 other location(s)
backend/src/processes/services/tasks/task_version.py:236
_update_fieldset_rulesstill creates/updates legacyFieldSetRulerows, but line 236 treats their IDs as IDs in the newFieldSetRuleSetreverse relation. Updating a task version can therefore delete unrelated new rulesets whose IDs are absent from (or collide with) the legacy IDs, while never creating/updating the requested new rulesets.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/services/workflows/kickoff_version.py around line 119:
Updating a kickoff version can delete unrelated `FieldSetRuleSet` records and leave the requested rules only in deprecated `FieldSetRule` rows. `_update_fieldset_rules` collects IDs from `FieldSetRule.objects` but applies them to `fieldset.rulesets`, so the new relation is never populated and its records are removed when their IDs are absent. Create/update the new ruleset model and use those records for this cleanup.
Also found in 1 other location(s):
- backend/src/processes/services/tasks/task_version.py:236 -- `_update_fieldset_rules` still creates/updates legacy `FieldSetRule` rows, but line 236 treats their IDs as IDs in the new `FieldSetRuleSet` reverse relation. Updating a task version can therefore delete unrelated new rulesets whose IDs are absent from (or collide with) the legacy IDs, while never creating/updating the requested new rulesets.
| + {formatMessage({ id: 'fieldsets.add-rule' })} | ||
| </button> | ||
| </div> | ||
| <FieldsetRulesets |
There was a problem hiding this comment.
🟡 Medium FieldsetDetails/FieldsetDetails.tsx:373
Deleting the only condition leaves an empty, undeletable ruleset, so validateFieldsetRules rejects the changes and the user cannot save or recover without discarding the entire edit. FieldsetRulesets always calls deleteGroupAnd, which removes the condition but leaves the parent ruleset; use deleteRuleset for the final condition or provide a way to add a new condition.
Also found in 2 other location(s)
frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetRulesets/FieldsetRulesets.tsx:84
The delete button always calls
deleteGroupAnd, even when this is the ruleset's only condition. Deleting that condition leaves the ruleset with an emptygroupsOr; this component provides no way to delete that ruleset or add a condition back, andvalidateFieldsetRulesrejects it. The user is therefore stuck with an unsaveable ruleset and must reload, losing other edits. UsedeleteRulesetfor the final condition or expose controls to recover the empty ruleset.
frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetRulesets/utils.ts:122
deleteGroupAndremoves the containinggroupOrwhen its last condition is deleted, but leaves the parent ruleset in place. In the supplied UI this delete button is also used for the sole condition created bycreateEmptyRuleset, and there is no control that callsaddGroupAndordeleteRuleset; after clicking it, the ruleset hasgroupsOr: [], renders no condition inputs, and cannot be repaired or removed. The user is left with an invalid, undeletable ruleset that prevents saving until the entire edit is discarded.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetDetails.tsx around line 373:
Deleting the only condition leaves an empty, undeletable ruleset, so `validateFieldsetRules` rejects the changes and the user cannot save or recover without discarding the entire edit. `FieldsetRulesets` always calls `deleteGroupAnd`, which removes the condition but leaves the parent ruleset; use `deleteRuleset` for the final condition or provide a way to add a new condition.
Also found in 2 other location(s):
- frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetRulesets/FieldsetRulesets.tsx:84 -- The delete button always calls `deleteGroupAnd`, even when this is the ruleset's only condition. Deleting that condition leaves the ruleset with an empty `groupsOr`; this component provides no way to delete that ruleset or add a condition back, and `validateFieldsetRules` rejects it. The user is therefore stuck with an unsaveable ruleset and must reload, losing other edits. Use `deleteRuleset` for the final condition or expose controls to recover the empty ruleset.
- frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetRulesets/utils.ts:122 -- `deleteGroupAnd` removes the containing `groupOr` when its last condition is deleted, but leaves the parent ruleset in place. In the supplied UI this delete button is also used for the sole condition created by `createEmptyRuleset`, and there is no control that calls `addGroupAnd` or `deleteRuleset`; after clicking it, the ruleset has `groupsOr: []`, renders no condition inputs, and cannot be repaired or removed. The user is left with an invalid, undeletable ruleset that prevents saving until the entire edit is discarded.
| for rule_template in ruleset_templates: | ||
| service = FieldSetRuleService(user=self.user) | ||
| service.create( | ||
| instance_template=rule_template, |
There was a problem hiding this comment.
🟠 High fieldsets/fieldset.py:73
Creating a workflow from a template containing a new FieldSetTemplateRuleSet raises AttributeError instead of cloning its ruleset. _create_rules passes the new ruleset to the legacy FieldSetRuleService, which expects type and value fields that the nested-group ruleset does not provide; use the ruleset-aware cloning service instead.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/services/workflows/fieldsets/fieldset.py around line 73:
Creating a workflow from a template containing a new `FieldSetTemplateRuleSet` raises `AttributeError` instead of cloning its ruleset. `_create_rules` passes the new ruleset to the legacy `FieldSetRuleService`, which expects `type` and `value` fields that the nested-group ruleset does not provide; use the ruleset-aware cloning service instead.
| field = models.ForeignKey( | ||
| TaskField, | ||
| on_delete=models.CASCADE, | ||
| related_name='rulesets', |
There was a problem hiding this comment.
🟠 High workflows/fields.py:125
Versioning a field with legacy rules raises a Django model-type TypeError and aborts workflow creation. TaskField.rulesets is the reverse-FK manager for FieldRuleSet, but _link_rules passes it FieldSetRule instances; use the legacy TaskField.fieldset_rulesets relation (or otherwise convert the objects) when linking those rules.
Also found in 3 other location(s)
backend/src/processes/services/tasks/field.py:399
_link_rulesstill queries deprecatedFieldSetRuleobjects, but now passes them toself.instance.rulesets.set(...).TaskField.rulesetsis the reverse foreign-key manager forFieldRuleSet.field, so it cannot acceptFieldSetRuleinstances. Creating a task field whose template has any ruleset reaches this path and raises a model-type error, aborting workflow/task creation. The fieldset ruleset relation is instead exposed throughTaskField.fieldset_rulesets.
backend/src/processes/services/tasks/task_version.py:252
rulesis a queryset of legacyFieldSetRuleobjects, butfield.rulesetsis the reverse FK manager for newFieldRuleSetobjects. Callingfield.rulesets.set(rules)with the legacy model instances raises a relation type/value error whenever a versioned field has rules, so task version updates fail instead of linking rules.
backend/src/processes/services/workflows/kickoff_version.py:135
The queryset assigned at line 135 contains legacy
FieldSetRuleinstances, whereasfield.rulesetsmanages newFieldRuleSetinstances. For any kickoff field with nonempty rules,.set(rules)rejects the wrong model type and aborts the kickoff version update.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/models/workflows/fields.py around line 125:
Versioning a field with legacy rules raises a Django model-type `TypeError` and aborts workflow creation. `TaskField.rulesets` is the reverse-FK manager for `FieldRuleSet`, but `_link_rules` passes it `FieldSetRule` instances; use the legacy `TaskField.fieldset_rulesets` relation (or otherwise convert the objects) when linking those rules.
Also found in 3 other location(s):
- backend/src/processes/services/tasks/field.py:399 -- `_link_rules` still queries deprecated `FieldSetRule` objects, but now passes them to `self.instance.rulesets.set(...)`. `TaskField.rulesets` is the reverse foreign-key manager for `FieldRuleSet.field`, so it cannot accept `FieldSetRule` instances. Creating a task field whose template has any ruleset reaches this path and raises a model-type error, aborting workflow/task creation. The fieldset ruleset relation is instead exposed through `TaskField.fieldset_rulesets`.
- backend/src/processes/services/tasks/task_version.py:252 -- `rules` is a queryset of legacy `FieldSetRule` objects, but `field.rulesets` is the reverse FK manager for new `FieldRuleSet` objects. Calling `field.rulesets.set(rules)` with the legacy model instances raises a relation type/value error whenever a versioned field has rules, so task version updates fail instead of linking rules.
- backend/src/processes/services/workflows/kickoff_version.py:135 -- The queryset assigned at line 135 contains legacy `FieldSetRule` instances, whereas `field.rulesets` manages new `FieldRuleSet` instances. For any kickoff field with nonempty rules, `.set(rules)` rejects the wrong model type and aborts the kickoff version update.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| <div className={kickoffStyles['kick-off-input__dropdown']}> | ||
| <FieldsetFlowRowDropdown | ||
| headerTitle={title} | ||
| headerTitle={apiNameBinding} |
There was a problem hiding this comment.
Dropdown header shows internal API name
Medium Severity
The fieldset row menu now passes apiNameBinding into headerTitle, which is rendered as the dropdown header. Users see the internal binding id instead of the fieldset title that was shown before.
Reviewed by Cursor Bugbot for commit 5ca2c38. Configure here.
| <InfiniteScroll | ||
| dataLength={fieldsetsList.length} | ||
| next={() => dispatch(loadFieldsets({ offset: offset + 1 }))} | ||
| next={() => !isLoading && dispatch(loadFieldsets({ offset: offset + 1 }))} |
There was a problem hiding this comment.
🟡 Medium Fieldsets/Fieldsets.tsx:44
Pagination can stop permanently after a sorting reload: InfiniteScroll invokes next while the reload is in progress, line 44 returns without dispatching loadFieldsets, and dataLength may not change when the first sorted page replaces the previous page. Because the scroll component only rearms after a dataLength change, later pages are never requested. Remove this guard (or make hasMore false while loading) so an invoked next is not recorded without loading the next page.
| next={() => !isLoading && dispatch(loadFieldsets({ offset: offset + 1 }))} | |
| next={() => dispatch(loadFieldsets({ offset: offset + 1 }))} |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @frontend/src/public/components/Fieldsets/Fieldsets.tsx around line 44:
Pagination can stop permanently after a sorting reload: `InfiniteScroll` invokes `next` while the reload is in progress, line 44 returns without dispatching `loadFieldsets`, and `dataLength` may not change when the first sorted page replaces the previous page. Because the scroll component only rearms after a `dataLength` change, later pages are never requested. Remove this guard (or make `hasMore` false while loading) so an invoked `next` is not recorded without loading the next page.
| 'fieldsets.leave-unsaved-message': 'Ваши изменения не сохранены.', | ||
| 'fieldsets.leave-unsaved-stay': 'Продолжить редактирование', | ||
| 'fieldsets.leave-unsaved-leave': 'Отменить', | ||
| 'fieldsets.leave-unsaved-leave': 'Убрать', |
There was a problem hiding this comment.
🟡 Medium locales/ru_RU.ts:1435
The fieldsets.leave-unsaved-leave button is labeled Убрать (“Remove”), so Russian users are not told that confirmLeave will discard their unsaved edits. Use a discard-specific translation such as Отменить изменения or Не сохранять, matching the English Discard action.
| 'fieldsets.leave-unsaved-leave': 'Убрать', | |
| 'fieldsets.leave-unsaved-leave': 'Отменить изменения', |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @frontend/src/public/lang/locales/ru_RU.ts around line 1435:
The `fieldsets.leave-unsaved-leave` button is labeled `Убрать` (“Remove”), so Russian users are not told that `confirmLeave` will discard their unsaved edits. Use a discard-specific translation such as `Отменить изменения` or `Не сохранять`, matching the English `Discard` action.
…to backend/fieldsets/48345__add_rulesets_2
…plateRuleSetSerializer
| color: var(--pneumatic-color-notification1); | ||
| background: transparent; | ||
| border: 0; | ||
| opacity: 0; |
There was a problem hiding this comment.
🟡 Medium FieldsetDetails/FieldsetDetails.css:485
Keyboard users can focus the real delete button while .rule-remove-btn remains invisible, because visibility currently depends only on .rule-row:hover or (hover: none). Add a :focus-visible rule so the focused destructive control is visible.
- opacity: 0;
+ opacity: 0;
+
+ &:focus-visible {
+ opacity: 1;
+ }🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetDetails.css around line 485:
Keyboard users can focus the real delete button while `.rule-remove-btn` remains invisible, because visibility currently depends only on `.rule-row:hover` or `(hover: none)`. Add a `:focus-visible` rule so the focused destructive control is visible.
| field.rulesets.set(rules) | ||
| else: | ||
| field.rules.clear() | ||
| field.rulesets.clear() |
There was a problem hiding this comment.
Version update keeps flat rules API
High Severity
_update_fieldset_rules still update_or_creates FieldSetRule from flat type/value, then deletes via fieldset.rulesets. Callers still pass fieldset_data.get('rules'), and _update_field_rules still reads rules and assigns FieldSetRule rows onto field.rulesets. Template version sync drops or corrupts nested rulesets.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 9a04fcb. Configure here.
| type=shared_rule.type, | ||
| value=shared_rule.value, | ||
| api_name=f'{fieldset.api_name}-ruleset-1', | ||
| type=shared_ruleset.type, |
There was a problem hiding this comment.
Fixture creates ruleset with type
Medium Severity
create_test_fieldset_template creates FieldSetTemplateRuleSet with type=shared_ruleset.type, but that model has no type. __str__ on both template and workflow ruleset models also interpolates self.type, which will raise if invoked.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 9a04fcb. Configure here.
…eldsets and template edit
…esets_2' into frontend/fieldsets/48849__advanced_fieldset_rules
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 80e88cf. Configure here.
| &:hover .rule-remove-btn { | ||
| opacity: 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
Rule borders always removed
Low Severity
The new .rule-row block adds :last-child { border-bottom: none }, but each .rule-row sits alone as the last child inside its own .rule-item, so every row matches and loses its separator. The block also duplicates the existing .rule-row definition instead of extending it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 80e88cf. Configure here.
2 participants
Note
High Risk
Breaking API and schema change for fieldset rules, plus a data migration that rewrites existing rule rows. Incomplete wiring of field-level rulesets and leftover
rulesvsrulesetsusage in workflow version updates could drop or mis-apply rules at runtime.Overview
Replaces fieldset
ruleswith nestedrulesets(OR groups of AND conditions) so users can combine sum comparisons (sum_equal,sum_greater_than,sum_less_than) instead of a single flatsum_equalrule.Backend adds template and workflow models for fieldset and field-level rulesets (
FieldSet*RuleSet/Field*RuleSetplus GroupOr/GroupAnd). OldFieldsetTemplateRule/FieldSetRulestay but are marked deprecated. A data migration copies existing fieldset template rules into the new hierarchy. Template APIs, fieldset services, clone/sync, and workflow instantiation now persist and validaterulesets. Unique constraints on field/fieldset templates now includeaccount.Frontend fieldset editor uses a new
FieldsetRulesetsUI (add/delete rulesets, AND/OR regrouping, field picker, custom messages) and validators for nested sum rules. Payload field isrulesetsthroughout.Breaking: fieldset request/response no longer uses
rules. Field-level show/validate rulesets are modeled and serialized on field templates, but fieldset field creation still dropsrulesetsfor now.Reviewed by Cursor Bugbot for commit 00c9dd7. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace flat fieldset
ruleswith hierarchicalrulesets(nested OR/AND groups)FieldSetTemplateRuleSet→FieldSetTemplateRuleGroupOr→FieldSetTemplateRuleGroupAnd, plus equivalent field-level ruleset models, each with operator/value semantics instead of a flat rule type/value pairFieldRuleType(SHOW, VALIDATOR),FieldRuleOperator(EQUAL, GREATER_THAN, LESS_THAN), and extendsFieldSetRuleOperatorwith SUM_GREATER_THAN and SUM_LESS_THANFieldSetTemplateService,FieldsetTemplateRuleSetService,TaskUpdateVersionService,KickoffUpdateVersionService,TaskFieldService) to create, upsert byapi_name, and prune nested rulesets with validation for SUM operators at the AND-group levelFieldsetTemplateRulerows into the new ruleset/group/group-and structure and reassigns M2M fields; duplicate(fieldset_id, api_name)rows are skippedFieldsetDetailspage with a newFieldsetRulesetscomponent tree (cards, rule items, field selectors), updates types, validators, factories, and i18n strings to the rulesets modelrulestorulesetswith nestedgroups_or/groups_and; any out-of-tree consumers sending the oldrulespayload will break. LegacyFieldsetTemplateRuleandFieldSetRulemodels remain but are deprecated. Unique constraints onFieldTemplateandFieldsetTemplatenow includeaccount, which may affect existing cross-account dataMacroscope summarized 00c9dd7.