Skip to content

⚙️ 45773 backend [ templates ] Add FieldSets#211

Open
pneumojoseph wants to merge 17 commits into
masterfrom
backend/fieldsets/45773__fieldsets
Open

⚙️ 45773 backend [ templates ] Add FieldSets#211
pneumojoseph wants to merge 17 commits into
masterfrom
backend/fieldsets/45773__fieldsets

Conversation

@pneumojoseph
Copy link
Copy Markdown
Collaborator

@pneumojoseph pneumojoseph commented May 6, 2026

Note

Medium Risk
Adds new database models/migrations and threads fieldset creation/validation through template run, workflow updates, and task completion, which can affect data integrity and API payloads. Query changes (new joins/ordering and expanded field selection) may impact performance and field ordering semantics.

Overview
Adds FieldSets as a first-class concept for grouping fields in both templates and running workflows, including new models/migration (FieldsetTemplate/FieldsetTemplateRule and runtime FieldSet/FieldSetRule) plus enums for layout/label position and a sum_equal rule type.

Updates template/task/kickoff serializers and versioning schemas to accept/emit fieldsets, adds filtering/queryset support and prefetches, and expands Template.get_*_output_fields / Workflow.get_*_output_fields to include fields coming from linked fieldsets.

Extends workflow execution paths to materialize fieldsets when starting tasks/kickoffs, link field rules onto created TaskFields, and validate fieldset rules during kickoff updates, task completion, and version updates; also centralizes default delete() behavior in BaseModelService and removes redundant overrides.

Reviewed by Cursor Bugbot for commit c53259f. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add FieldSets to templates and workflows for grouping and validating form fields

  • Introduces FieldsetTemplate, FieldsetTemplateRule, FieldSet, and FieldSetRule models (fieldset.py, fieldset.py) to group fields within kickoffs and tasks, with ordering, layout, label position, and rule validation (e.g. SUM_EQUAL).
  • Adds new service classes (FieldSetTemplateService, FieldSetService, FieldSetRuleService) to create, update, validate, and delete fieldsets and their nested fields and rules transactionally.
  • Extends KickoffUpdateVersionService and TaskUpdateVersionService to fully synchronize fieldsets (create/update/delete) including nested rules and fields during template and workflow version updates.
  • Exposes new REST endpoints: GET/POST /templates/{id}/fieldsets and GET/PATCH/DELETE /templates/fieldsets/{id} via FieldsetTemplateViewSet.
  • Updates serializers for tasks, kickoffs, workflow events, and public templates to include a fieldsets array in API responses; TASK_COMPLETE events include populated fieldset data when present.
  • TaskField and FieldTemplate gain a fieldset FK and rules M2M; get_kickoff_fields and get_tasks_fields on both Workflow and Template now include fields reachable via fieldset links.
  • Risk: workflow detail and task queries now prefetch fieldsets__fields and exclude fieldset-bound fields from the standalone output list, changing response composition for existing clients.

Macroscope summarized c53259f.

@pneumojoseph pneumojoseph added the Backend API changes request label May 6, 2026
Comment thread backend/src/processes/models/templates/fieldset.py
Comment thread backend/src/processes/serializers/workflows/kickoff_value.py
)()

def __str__(self):
return self.name
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FieldsetTemplateRule.__str__ references non-existent name field

Medium Severity

The FieldsetTemplateRule.__str__ method attempts to return self.name, but the FieldsetTemplateRule model does not define a name field. This causes an AttributeError when the object is converted to a string.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cd8ef70. Configure here.

if field.value not in self.NULL_VALUES:
total += float(field.value)
if total != float(self.instance.value):
raise FieldsetServiceException(MSG_FS_0002(self.instance.value))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Float equality check causes spurious validation failures

Medium Severity

_validate_sum_equal sums field values as float and checks total != float(self.instance.value) using exact equality. Floating-point arithmetic accumulation (e.g., 0.1 + 0.2 ≠ 0.3) can cause valid inputs to fail validation. The template-side validator in fieldset_rule.py uses the same fragile pattern.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cd8ef70. Configure here.

)
if fields_filter_kwargs:
qst = qst.filter(**fields_filter_kwargs)
return qst
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed try/except makes kickoff lookup crash-prone

Medium Severity

get_kickoff_output_fields now calls self.kickoff.get() without a try/except for ObjectDoesNotExist. The previous code gracefully returned an empty queryset when no kickoff existed. Any caller (including get_fields, get_kickoff_fields_values, get_fields_markdown_values) will now crash with an unhandled exception if no kickoff is present.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cd8ef70. Configure here.

from django.db import models # noqa : PLC0415
if isinstance(instance, models.Manager):
instance = instance.first()
return super().to_representation(instance)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing None check in KickoffOnlyFieldsSerializer.to_representation

Medium Severity

KickoffOnlyFieldsSerializer.to_representation converts a Manager to an instance via instance.first() but doesn't handle the case where first() returns None. The sibling KickoffSerializer.to_representation correctly checks for None and returns a default dict, but this serializer passes None to super().to_representation(), which will crash.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit cd8ef70. Configure here.

@pneumojoseph pneumojoseph self-assigned this May 6, 2026
Comment thread backend/src/processes/models/templates/fieldset.py
Comment thread backend/src/processes/serializers/templates/kickoff.py
Comment on lines +361 to +374
def _link_rules(
self,
instance_template: FieldTemplate,
**kwargs,
):

rule_api_names = set(
instance_template.rules.values_list('api_name', flat=True),
)
rules = FieldSetRule.objects.filter(
account=self.account,
fieldset_id=kwargs['fieldset_id'],
api_name__in=rule_api_names,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low tasks/field.py:361

_link_rules accesses kwargs['fieldset_id'] unconditionally, which raises KeyError when fieldset_id is omitted from kwargs. This crashes when a template has rules but _create_instance was called without a fieldset (it uses kwargs.get('fieldset_id') at line 291, making it optional there). Consider using kwargs.get('fieldset_id') and handling the None case, or ensure fieldset_id is required consistently.

-        rules = FieldSetRule.objects.filter(
-            account=self.account,
-            fieldset_id=kwargs['fieldset_id'],
-            api_name__in=rule_api_names,
-        )
Also found in 2 other location(s)

backend/src/processes/serializers/workflows/kickoff_value.py:111

When creating TaskField instances for fields without fieldsets (lines 109-116), fieldset_id is not passed to TaskFieldService.create(). However, TaskFieldService._create_related checks instance_template.rules.all().exists() and calls _link_rules, which unconditionally accesses kwargs['fieldset_id'] (visible in the references). If a field template without a fieldset has associated rules, this will raise a KeyError.

backend/src/processes/services/tasks/task.py:218

Calling service.create() without passing fieldset_id will cause a KeyError in TaskFieldService._link_rules if the field template has rules. The _link_rules method (added in this commit) accesses kwargs['fieldset_id'] with direct dictionary access, but create_fields_from_template only passes workflow_id, task_id, and skip_value. If any field template excluded from fieldsets has associated rules (field_template.rules.all().exists() is True), the code will crash.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/services/tasks/field.py around lines 361-374:

`_link_rules` accesses `kwargs['fieldset_id']` unconditionally, which raises `KeyError` when `fieldset_id` is omitted from kwargs. This crashes when a template has rules but `_create_instance` was called without a fieldset (it uses `kwargs.get('fieldset_id')` at line 291, making it optional there). Consider using `kwargs.get('fieldset_id')` and handling the None case, or ensure fieldset_id is required consistently.

Evidence trail:
backend/src/processes/services/tasks/field.py line 291: `fieldset_id=kwargs.get('fieldset_id')` (optional); line 373: `fieldset_id=kwargs['fieldset_id']` (required, raises KeyError if missing); line 325: conditional call to `_link_rules` when `instance_template.rules.all().exists()`; backend/src/processes/services/tasks/task.py lines 217-222: `service.create()` called without `fieldset_id`; backend/src/generics/base/service.py line 66-69: `create()` passes same kwargs to both `_create_instance` and `_create_related`; backend/src/processes/models/templates/fields.py line 68-71: `rules = models.ManyToManyField('processes.FieldsetTemplateRule', ...)` on FieldTemplate.

Also found in 2 other location(s):
- backend/src/processes/serializers/workflows/kickoff_value.py:111 -- When creating `TaskField` instances for fields without fieldsets (lines 109-116), `fieldset_id` is not passed to `TaskFieldService.create()`. However, `TaskFieldService._create_related` checks `instance_template.rules.all().exists()` and calls `_link_rules`, which unconditionally accesses `kwargs['fieldset_id']` (visible in the references). If a field template without a fieldset has associated rules, this will raise a `KeyError`.
- backend/src/processes/services/tasks/task.py:218 -- Calling `service.create()` without passing `fieldset_id` will cause a `KeyError` in `TaskFieldService._link_rules` if the field template has rules. The `_link_rules` method (added in this commit) accesses `kwargs['fieldset_id']` with direct dictionary access, but `create_fields_from_template` only passes `workflow_id`, `task_id`, and `skip_value`. If any field template excluded from fieldsets has associated rules (`field_template.rules.all().exists()` is True), the code will crash.

Comment on lines +66 to +79
if field_data.get('selections'):
selection_ids = set()
for selection_data in field_data['selections']:
selection, __ = (
FieldSelection.objects.update_or_create(
field=field,
api_name=selection_data['api_name'],
defaults={
'value': selection_data['value'],
},
)
)
selection_ids.add(selection.id)
field.selections.exclude(id__in=selection_ids).delete()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low tasks/task_version.py:66

When field_data.get('selections') is falsy (empty list, None, or missing key), the method exits early without deleting existing selections. This leaves stale selections in the database when a field's template is updated to remove all selections. Consider adding an else branch to clear all selections when the input is empty or missing.

        if field_data.get('selections'):
            selection_ids = set()
            for selection_data in field_data['selections']:
                selection, __ = (
                    FieldSelection.objects.update_or_create(
                        field=field,
                        api_name=selection_data['api_name'],
                        defaults={
                            'value': selection_data['value'],
                        },
                    )
                )
                selection_ids.add(selection.id)
            field.selections.exclude(id__in=selection_ids).delete()
+        else:
+            field.selections.all().delete()
Also found in 1 other location(s)

backend/src/processes/services/workflows/kickoff_version.py:181

Inconsistent handling of empty lists between fields and fieldsets. The condition if data.get('fields'): is falsy for an empty list [], so _update_fields won't be called and existing fields (with fieldset=None) won't be deleted. However, if data.get('fieldsets') is not None: will pass for an empty list, causing _update_fieldsets to delete all fieldsets. If the intent is to clear all items when an empty list is provided, the fields check should also use is not None instead of truthiness.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/services/tasks/task_version.py around lines 66-79:

When `field_data.get('selections')` is falsy (empty list, `None`, or missing key), the method exits early without deleting existing selections. This leaves stale selections in the database when a field's template is updated to remove all selections. Consider adding an `else` branch to clear all selections when the input is empty or missing.

Evidence trail:
backend/src/processes/services/tasks/task_version.py lines 60-79 (REVIEWED_COMMIT): `_update_field_selections` only processes/deletes inside `if field_data.get('selections'):` block, no else branch.
backend/src/processes/services/tasks/task_version.py lines 213-229 (REVIEWED_COMMIT): `_update_field_rules` in the same file has `else: field.rules.clear()` for the empty case - showing the intended pattern.
backend/src/processes/services/tasks/task_version.py lines 88-94 (REVIEWED_COMMIT): `_update_fields` places the delete outside the `if data:` block, correctly handling empty data.
backend/src/processes/tests/test_services/test_tasks/test_task_version_service.py lines 2235-2267 (REVIEWED_COMMIT): Test `test__update_field_selections__selections_empty__skip` creates no pre-existing selections before asserting count==0, so it doesn't test the stale data scenario.
backend/src/processes/services/workflows/kickoff_version.py lines 44-62 (REVIEWED_COMMIT): Same pattern in KickoffUpdateVersionService.

Also found in 1 other location(s):
- backend/src/processes/services/workflows/kickoff_version.py:181 -- Inconsistent handling of empty lists between `fields` and `fieldsets`. The condition `if data.get('fields'):` is falsy for an empty list `[]`, so `_update_fields` won't be called and existing fields (with `fieldset=None`) won't be deleted. However, `if data.get('fieldsets') is not None:` will pass for an empty list, causing `_update_fieldsets` to delete all fieldsets. If the intent is to clear all items when an empty list is provided, the `fields` check should also use `is not None` instead of truthiness.

Comment on lines +16 to +25
class FieldSet(
BaseApiNameModel,
BaseFieldSetMixin,
AccountBaseMixin,
):

class Meta:
ordering = ['-id']

workflow = models.ForeignKey(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium workflows/fieldset.py:16

FieldSet inherits from BaseApiNameModel but does not implement the abstract api_name_prefix property. When save() is called without an existing api_name, _create_api_name() calls self.api_name_prefix, which returns None from the abstract method's pass statement. This causes create_api_name(None) to receive an invalid prefix. Consider adding an api_name_prefix property to FieldSet or make the field non-auto-generated if no prefix is needed.

+class FieldSet(
+    BaseApiNameModel,
+    BaseFieldSetMixin,
+    AccountBaseMixin,
+):
+
+    class Meta:
+        ordering = ['-id']
+
+    @property
+    def api_name_prefix(self) -> str:
+        return 'fieldset'
+
+    workflow = models.ForeignKey(
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/models/workflows/fieldset.py around lines 16-25:

`FieldSet` inherits from `BaseApiNameModel` but does not implement the abstract `api_name_prefix` property. When `save()` is called without an existing `api_name`, `_create_api_name()` calls `self.api_name_prefix`, which returns `None` from the abstract method's `pass` statement. This causes `create_api_name(None)` to receive an invalid prefix. Consider adding an `api_name_prefix` property to `FieldSet` or make the field non-auto-generated if no prefix is needed.

Evidence trail:
backend/src/processes/models/workflows/fieldset.py (new file, REVIEWED_COMMIT) - lines 16-47: FieldSet class without api_name_prefix; lines 50-64: FieldSetRule class without api_name_prefix.
backend/src/processes/models/base.py lines 8-27 (REVIEWED_COMMIT): BaseApiNameModel defines abstract property api_name_prefix with pass body, _create_api_name calls create_api_name(self.api_name_prefix), save() auto-generates api_name.
backend/src/processes/utils/common.py lines 160-162 (REVIEWED_COMMIT): create_api_name uses f'{prefix}-{salt}'.
backend/src/processes/models/templates/fieldset.py line 39 (REVIEWED_COMMIT): FieldsetTemplate correctly sets api_name_prefix = 'fieldset'; line 146: FieldsetTemplateRule sets api_name_prefix = 'fieldsetrule'.
git_grep for 'api_name_prefix' in fieldset.py returned no matches.

selection_ids.add(selection.id)
field.selections.exclude(id__in=selection_ids).delete()
self._update_field_selections(field, field_data)
self.instance.output.exclude(id__in=field_ids).delete()
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High tasks/task_version.py:94

In _update_fields, the delete query self.instance.output.exclude(id__in=field_ids).delete() removes all TaskField objects not in field_ids, including fields that belong to fieldsets. Since _update_fields runs before _update_fieldsets, this deletes existing fieldset fields before they can be updated. Consider filtering the delete to only fields where fieldset is None to preserve fieldset fields.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/services/tasks/task_version.py around line 94:

In `_update_fields`, the delete query `self.instance.output.exclude(id__in=field_ids).delete()` removes all `TaskField` objects not in `field_ids`, including fields that belong to fieldsets. Since `_update_fields` runs before `_update_fieldsets`, this deletes existing fieldset fields before they can be updated. Consider filtering the delete to only fields where `fieldset` is `None` to preserve fieldset fields.

Evidence trail:
backend/src/processes/models/workflows/fields.py:48-51 (task FK with related_name='output'), backend/src/processes/models/workflows/fields.py:61-67 (fieldset FK, nullable), backend/src/processes/models/workflows/fields.py:78 (objects = BaseSoftDeleteManager), backend/src/processes/services/tasks/task_version.py:84-94 (_update_fields method, delete at line 94), backend/src/processes/services/tasks/task_version.py:92 (_update_field called with fieldset=None), backend/src/processes/services/tasks/task_version.py:211-218 (_update_fieldset_fields has its own scoped cleanup), backend/src/processes/services/tasks/task_version.py:270-271 (_update_fields called before _update_fieldsets in update_from_version), backend/src/generics/querysets.py:67 (BaseQuerySet.delete does soft delete), backend/src/generics/managers.py:7-8 (BaseSoftDeleteManager filters is_deleted=False)

""" Call after objects save """

validator = getattr(self, f'_validate_{self.instance.type}', None)
validator(**kwargs)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low fieldsets/fieldset_rule.py:42

In _validate, validator is assigned None as a default (line 41), then called unconditionally on line 42. If self.instance.type doesn't match any _validate_{type} method, calling None(**kwargs) raises TypeError: 'NoneType' object is not callable. Consider checking validator is not None before calling, or remove the None default to raise AttributeError instead.

        validator = getattr(self, f'_validate_{self.instance.type}', None)
-        validator(**kwargs)
+        if validator:
+            validator(**kwargs)
Also found in 1 other location(s)

backend/src/processes/services/templates/fieldsets/fieldset.py:117

In _validate_rules, calling service._validate() will raise TypeError: 'NoneType' object is not callable for any rule whose type does not have a corresponding _validate_{type} method. Looking at FieldsetTemplateRuleService._validate(), it does validator = getattr(self, f'_validate_{self.instance.type}', None) and then calls validator(**kwargs) without checking if validator is None. Since only _validate_sum_equal exists, any rule with a different type will crash.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/services/templates/fieldsets/fieldset_rule.py around line 42:

In `_validate`, `validator` is assigned `None` as a default (line 41), then called unconditionally on line 42. If `self.instance.type` doesn't match any `_validate_{type}` method, calling `None(**kwargs)` raises `TypeError: 'NoneType' object is not callable`. Consider checking `validator is not None` before calling, or remove the `None` default to raise `AttributeError` instead.

Evidence trail:
backend/src/processes/services/templates/fieldsets/fieldset_rule.py lines 41-42 (getattr with None default, then unconditional call); backend/src/processes/enums.py lines 747-759 (FieldSetRuleType with only SUM_EQUAL); backend/src/processes/models/mixins.py lines 349-359 (type is CharField with choices, not enforced at DB level)

Also found in 1 other location(s):
- backend/src/processes/services/templates/fieldsets/fieldset.py:117 -- In `_validate_rules`, calling `service._validate()` will raise `TypeError: 'NoneType' object is not callable` for any rule whose `type` does not have a corresponding `_validate_{type}` method. Looking at `FieldsetTemplateRuleService._validate()`, it does `validator = getattr(self, f'_validate_{self.instance.type}', None)` and then calls `validator(**kwargs)` without checking if `validator` is `None`. Since only `_validate_sum_equal` exists, any rule with a different type will crash.

Comment on lines +35 to +36
def partial_update(self, **update_kwargs) -> Model:
self._validate(**update_kwargs)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium templates/field_template.py:35

In partial_update, _validate receives only the update kwargs, not the full instance state. When partial_update(is_required=False) is called on a type='user' field without passing type, _validate sees field_type=None and skips the FieldType.USER check, allowing an invalid update that violates the is_required=True requirement. Consider passing the merged instance state and update kwargs to _validate so existing field values are considered during validation.

     def partial_update(self, **update_kwargs) -> Model:
-        self._validate(**update_kwargs)
+        merged_state = {
+            'type': self.instance.type,
+            'is_required': self.instance.is_required,
+        }
+        merged_state.update(update_kwargs)
+        self._validate(**merged_state)
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/services/templates/field_template.py around lines 35-36:

In `partial_update`, `_validate` receives only the update kwargs, not the full instance state. When `partial_update(is_required=False)` is called on a `type='user'` field without passing `type`, `_validate` sees `field_type=None` and skips the `FieldType.USER` check, allowing an invalid update that violates the `is_required=True` requirement. Consider passing the merged instance state and update kwargs to `_validate` so existing field values are considered during validation.

Evidence trail:
backend/src/processes/services/templates/field_template.py lines 19-28 (_validate), lines 35-36 (partial_update calls _validate with only update_kwargs); backend/src/generics/base/service.py lines 28-33 (self.instance is set in __init__); backend/src/processes/services/templates/fieldsets/fieldset.py lines 86-92 (caller that invokes partial_update with field_data from external input)

Comment thread backend/src/processes/services/templates/fieldsets/fieldset.py
Comment thread backend/src/processes/services/workflows/fieldsets/fieldset.py
)
service.partial_update(
value=fields_values.get(task_field.api_name),
value=fields_values[field.api_name],
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task completion skips required-field validation for unsubmitted fields

High Severity

The complete_task_for_user function now only validates task output fields explicitly provided in fields_values. Required fields omitted from fields_values are silently skipped, bypassing validation and allowing tasks to complete without all necessary information.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4e44624. Configure here.

""" Call after objects save """

validator = getattr(self, f'_validate_{self.instance.type}', None)
validator(**kwargs)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validator dispatch calls None for unknown rule types

Low Severity

FieldsetTemplateRuleService._validate uses getattr(self, ..., None) with a None default, then immediately calls validator(**kwargs). If the rule type doesn't match any _validate_* method (e.g., due to data corruption or future types), this calls None() producing a confusing TypeError: 'NoneType' object is not callable instead of a clear error.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4e44624. Configure here.

self._create_related(**kwargs)
self._create_actions(**kwargs)
if kwargs.get('skip_validation') is False:
self.validate(**kwargs)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skip validation check uses identity instead of truthiness

Medium Severity

FieldSetRuleService.create uses kwargs.get('skip_validation') is False (identity check), so when skip_validation is not passed (returns None), None is False evaluates to False and validation is always skipped. The caller _create_rules passes skip_validation=kwargs.get('skip_value') which is None during kickoff creation, meaning rule validation never runs inside create. The condition likely intended not kwargs.get('skip_validation') or kwargs.get('skip_validation') is not True.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4e44624. Configure here.

tasks_filter = {'task__template_id': self.id, **tasks_filter_kwargs}
fieldset_filter = {'fieldset__tasks__template_id': self.id}
for key, value in tasks_filter_kwargs.items():
_key = key.replace('task', 'tasks')
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fragile string replace converts unintended key substrings

Low Severity

Template.get_tasks_output_fields uses key.replace('task', 'tasks') to convert filter kwargs for the fieldset M2M path. This replaces ALL occurrences of the substring task in the key, so a key like task__subtask__id would become tasks__subtasks__id, producing an invalid lookup. The Workflow version avoids this by simply prepending fieldset__ without rewriting.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4e44624. Configure here.

else:
total += float(field.value)
values_exists = True
if values_exists and total != float(self.instance.value):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium fieldsets/fieldset_rule.py:27

In _validate_sum_equal, when self.instance.value is None and values_exists is True, float(self.instance.value) throws TypeError: float() argument must be a string or a number, not 'NoneType'. Since BaseFieldSetRuleMixin.value allows null=True, validation fails on valid database state. Consider handling the None case before converting to float.

-        if values_exists and total != float(self.instance.value):
+        if values_exists and (self.instance.value is None or total != float(self.instance.value)):
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/services/workflows/fieldsets/fieldset_rule.py around line 27:

In `_validate_sum_equal`, when `self.instance.value` is `None` and `values_exists` is `True`, `float(self.instance.value)` throws `TypeError: float() argument must be a string or a number, not 'NoneType'`. Since `BaseFieldSetRuleMixin.value` allows `null=True`, validation fails on valid database state. Consider handling the `None` case before converting to float.

Evidence trail:
backend/src/processes/models/mixins.py:349-359 (BaseFieldSetRuleMixin with value field having null=True), backend/src/processes/services/workflows/fieldsets/fieldset_rule.py:16-30 (_validate_sum_equal with no None guard before float(self.instance.value) at line 27), backend/src/processes/services/templates/fieldsets/fieldset_rule.py:23-31 (template service _validate_sum_equal which DOES check `if not value` before float conversion), backend/src/processes/services/workflows/fieldsets/fieldset_rule.py:56-60 (partial_update path that saves then validates)

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 9 total unresolved issues (including 8 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c53259f. Configure here.

)
selection_ids.add(selection.id)
field.selections.exclude(id__in=selection_ids).delete()
self._update_field_selections(field, field_data)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task version update deletes fieldset fields incorrectly

High Severity

TaskUpdateVersionService._update_fields deletes all task output fields not in field_ids via self.instance.output.exclude(id__in=field_ids).delete(), which incorrectly includes fields belonging to fieldsets. The equivalent KickoffUpdateVersionService._update_fields correctly filters with fieldset__isnull=True before deleting. Without this filter, fieldset fields are wiped out by _update_fields before _update_fieldsets can process them, causing data loss during task version updates.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c53259f. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend API changes request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant