Skip to content

46129 frontend [ templates ] Add FieldSets#205

Open
Maria-Lordwill wants to merge 63 commits into
masterfrom
frontend/templates/46129__add_fieldsets
Open

46129 frontend [ templates ] Add FieldSets#205
Maria-Lordwill wants to merge 63 commits into
masterfrom
frontend/templates/46129__add_fieldsets

Conversation

@Maria-Lordwill
Copy link
Copy Markdown
Collaborator

@Maria-Lordwill Maria-Lordwill commented Apr 24, 2026

Note

Medium Risk
Adds new database models/migrations and threads fieldset/rule creation + validation through template save, workflow run, and task/kickoff update paths, which can affect data integrity and completion flows. Most changes are additive but touch core workflow field collection/serialization and versioning logic.

Overview
Adds FieldSets to both templates (FieldsetTemplate, rules, and kickoff/task link tables) and running workflows (FieldSet, FieldSetRule), including a large migration that also lets FieldTemplate/TaskField belong to a fieldset and associate to rules.

Updates template/workflow APIs/serializers and versioning to include fieldsets alongside standalone fields (kickoff + tasks), and expands Template.get_*_output_fields / Workflow.get_*_output_fields to return fields coming from linked fieldsets as well.

Implements services to create/update/delete fieldset templates and to instantiate/validate workflow fieldsets (including a sum_equal rule) during kickoff/task creation and during kickoff/task field updates/completions; also adds supporting generic serializer fields and a default BaseModelService.delete() implementation. Minor: flips backend .env example ENABLE_LOGGING default to no and refreshes i18n pot creation dates/strings.

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

Note

Add FieldSets support to templates, workflows, tasks, and kickoffs

  • Introduces FieldsetTemplate, FieldSet, and associated rule/link models for both template and workflow layers, allowing fields to be grouped into ordered, named sets attached to kickoffs or tasks.
  • Adds backend API endpoints (GET/POST /templates/{id}/fieldsets, GET/PATCH/DELETE /templates/fieldsets/{id}) via FieldsetTemplateViewSet and wires them into the URL router.
  • Extends serializers for templates, kickoffs, tasks, and workflow events to include fieldsets arrays; public and embedded template endpoints also expose kickoff fieldsets.
  • Updates Workflow.get_kickoff_fields, Workflow.get_tasks_fields, Template.get_kickoff_fields, and Template.get_tasks_fields to return fields from both direct assignments and fieldset-linked fields.
  • Adds frontend Redux slice, sagas, selectors, and API clients for fieldsets; integrates fieldset loading into dashboard, clone, and run workflow flows so fieldset fields receive dataset selections.
  • New UI components: Fieldsets listing page, FieldsetDetails edit page, FieldsetModal create/rename modal, FieldsetCard, MergedOutputList, FieldsetFieldGroup, FieldsetIconPicker, and OutputFormTaskMerged for merged field+fieldset editing in kickoff and task forms.
  • KickoffRedux, TaskForm, TaskCard, WorkflowEditPopup, WorkflowModal, and KickoffEdit are updated to render, validate, persist, and submit fieldset fields alongside standalone fields.
  • Versioning schemas (KickoffSchemaV1, TaskSchemaV1) and TaskUpdateVersionService/KickoffUpdateVersionService are extended to handle fieldset create/update/delete lifecycle.
  • Risk: storageOutputs API has changed (entries now keyed under a data property); any code relying on the old tasks_outputs localStorage shape will not read existing stored values correctly.

Macroscope summarized c753989.

@Maria-Lordwill Maria-Lordwill added bug Something isn't working Frontend Web client changes request labels Apr 24, 2026
Comment thread backend/src/processes/services/tasks/task_version.py
Comment thread backend/src/processes/services/templates/fieldsets/fieldset_rule.py
Comment thread backend/src/processes/services/workflows/fieldsets/fieldset_rule.py
Comment thread backend/src/processes/models/templates/fieldset.py
Comment thread backend/src/processes/serializers/templates/task.py Outdated
Comment thread backend/src/processes/services/workflows/fieldsets/fieldset.py
for field in self.instance.fields.all():
if field.value not in self.NULL_VALUES:
total += float(field.value)
if 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:22

Floating-point comparison total != float(self.instance.value) incorrectly rejects valid field sets due to precision loss. For example, when fields sum to 0.3 via 0.1 + 0.2, the strict equality check fails because 0.1 + 0.2 != 0.3 in IEEE 754 arithmetic. Consider using a tolerance-based comparison like abs(total - float(self.instance.value)) > epsilon.

-        if total != float(self.instance.value):
+        if abs(total - float(self.instance.value)) > 1e-9:
🚀 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 22:

Floating-point comparison `total != float(self.instance.value)` incorrectly rejects valid field sets due to precision loss. For example, when fields sum to `0.3` via `0.1 + 0.2`, the strict equality check fails because `0.1 + 0.2 != 0.3` in IEEE 754 arithmetic. Consider using a tolerance-based comparison like `abs(total - float(self.instance.value)) > epsilon`.

Evidence trail:
backend/src/processes/services/workflows/fieldsets/fieldset_rule.py lines 16-24 at REVIEWED_COMMIT - shows the `_validate_sum_equal` method with `total != float(self.instance.value)` comparison on line 22. IEEE 754 floating-point precision issues are well-documented (e.g., Python docs on floating point: https://docs.python.org/3/tutorial/floatingpoint.html).

Comment on lines +19 to +21
def _validate(self, **kwargs):
field_type = kwargs.get('type')

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 templates/field_template.py:19

In FieldTemplateService.partial_update, _validate checks field_type == FieldType.USER against kwargs.get('type'), but during partial updates type may not be provided. When updating only is_required=False on an existing USER field, field_type becomes None and the validation incorrectly passes, allowing a USER field to become non-required. Consider using self.instance.type as a fallback when type is not in update_kwargs.

    def _validate(self, **kwargs):
-        field_type = kwargs.get('type')
+        field_type = kwargs.get('type', self.instance.type if self.instance else None)
🚀 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 19-21:

In `FieldTemplateService.partial_update`, `_validate` checks `field_type == FieldType.USER` against `kwargs.get('type')`, but during partial updates `type` may not be provided. When updating only `is_required=False` on an existing `USER` field, `field_type` becomes `None` and the validation incorrectly passes, allowing a `USER` field to become non-required. Consider using `self.instance.type` as a fallback when `type` is not in `update_kwargs`.

Evidence trail:
backend/src/processes/services/templates/field_template.py lines 18-27 (_validate method), lines 33-40 (partial_update method), backend/src/generics/base/service.py lines 13-30 (BaseModelService.__init__ showing self.instance is set from constructor)

Comment on lines +37 to +42
def _validate(self, **kwargs):

""" 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:37

In _validate, validator is fetched with getattr(..., None), but validator(**kwargs) is called unconditionally. When self.instance.type has no matching _validate_{type} method, this raises TypeError: 'NoneType' object is not callable instead of a clear validation error.

    def _validate(self, **kwargs):

        """ Call after objects save """

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

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

In _validate_rules, calling service._validate() will crash with TypeError: 'NoneType' object is not callable for any rule whose type does not have a corresponding _validate_{type} method. The referenced FieldsetTemplateRuleService._validate() method uses getattr(self, f'_validate_{self.instance.type}', None) and then calls the result without checking if it's None. Only _validate_sum_equal exists in the visible code, so any rule with a different type will fail.

🚀 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 lines 37-42:

In `_validate`, `validator` is fetched with `getattr(..., None)`, but `validator(**kwargs)` is called unconditionally. When `self.instance.type` has no matching `_validate_{type}` method, this raises `TypeError: 'NoneType' object is not callable` instead of a clear validation error.

Evidence trail:
backend/src/processes/services/templates/fieldsets/fieldset_rule.py lines 37-38 at REVIEWED_COMMIT (getattr with None default, unconditional call)
backend/src/processes/enums.py lines 747-756 at REVIEWED_COMMIT (FieldSetRuleType only defines 'sum_equal')
backend/src/processes/models/mixins.py lines 349-359 at REVIEWED_COMMIT (BaseFieldSetRuleMixin defines type field with choices=FieldSetRuleType.CHOICES - application-level validation only)

Also found in 1 other location(s):
- backend/src/processes/services/templates/fieldsets/fieldset.py:113 -- In `_validate_rules`, calling `service._validate()` will crash with `TypeError: 'NoneType' object is not callable` for any rule whose `type` does not have a corresponding `_validate_{type}` method. The referenced `FieldsetTemplateRuleService._validate()` method uses `getattr(self, f'_validate_{self.instance.type}', None)` and then calls the result without checking if it's `None`. Only `_validate_sum_equal` exists in the visible code, so any rule with a different type will fail.

Comment thread frontend/src/public/components/KickoffEdit/KickoffEdit.tsx
}
"""

if data.get('fields'):
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 workflows/kickoff_version.py:180

When data contains {'fields': [], 'fieldsets': []}, the empty fields list is skipped due to truthiness check on line 180, but empty fieldsets is processed via is not None check on line 182. This causes _update_fieldsets to delete all fieldsets while _update_fields leaves orphaned fields intact — inconsistent behavior for empty list inputs. Consider changing line 180 to if data.get('fields') is not None: to match the fieldsets handling.

Also found in 1 other location(s)

backend/src/processes/services/tasks/task_version.py:66

If field_data.get('selections') returns an empty list [], the condition on line 66 evaluates to falsy, so the method exits without deleting existing selections. This means when a field previously had selections but the new data has 'selections': [], the old selections will incorrectly remain in the database instead of being cleared.

🚀 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 180:

When `data` contains `{'fields': [], 'fieldsets': []}`, the empty `fields` list is skipped due to truthiness check on line 180, but empty `fieldsets` is processed via `is not None` check on line 182. This causes `_update_fieldsets` to delete all fieldsets while `_update_fields` leaves orphaned fields intact — inconsistent behavior for empty list inputs. Consider changing line 180 to `if data.get('fields') is not None:` to match the fieldsets handling.

Evidence trail:
backend/src/processes/services/workflows/kickoff_version.py lines 180-183 (condition checks), lines 63-77 (_update_fields with delete at line 76-78), lines 136-163 (_update_fieldsets with delete at lines 161-163). Verified at REVIEWED_COMMIT.

Also found in 1 other location(s):
- backend/src/processes/services/tasks/task_version.py:66 -- If `field_data.get('selections')` returns an empty list `[]`, the condition on line 66 evaluates to falsy, so the method exits without deleting existing selections. This means when a field previously had selections but the new data has `'selections': []`, the old selections will incorrectly remain in the database instead of being cleared.

Comment thread frontend/src/public/components/TemplateEdit/FieldsetPicker/FieldsetPicker.tsx Outdated
Comment thread backend/src/processes/services/templates/fieldsets/fieldset.py
Comment thread backend/src/processes/services/workflows/fieldsets/fieldset_rule.py
Comment thread backend/src/processes/models/templates/fieldset.py
Comment thread backend/src/processes/serializers/workflows/kickoff_value.py
Comment thread backend/src/processes/models/workflows/workflow.py
Comment thread backend/src/processes/services/workflow_action.py
Comment thread backend/src/processes/models/templates/fieldset.py
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 deletion at line 94 removes ALL TaskField objects with task=self.instance, including fields that belong to fieldsets. Since _update_fields runs before _update_fieldsets (lines 531-532), and field_ids only tracks non-fieldset fields, existing fieldset fields are deleted before _update_fieldsets can recreate them. The deletion should filter to only remove fields without a fieldset using self.instance.output.filter(fieldset__isnull=True).exclude(id__in=field_ids).delete().

🚀 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 deletion at line 94 removes ALL `TaskField` objects with `task=self.instance`, including fields that belong to fieldsets. Since `_update_fields` runs before `_update_fieldsets` (lines 531-532), and `field_ids` only tracks non-fieldset fields, existing fieldset fields are deleted before `_update_fieldsets` can recreate them. The deletion should filter to only remove fields without a fieldset using `self.instance.output.filter(fieldset__isnull=True).exclude(id__in=field_ids).delete()`.

Evidence trail:
backend/src/processes/services/tasks/task_version.py lines 82-94 (_update_fields: field_ids only collects non-fieldset fields, then deletes all output not in field_ids), lines 91 (fieldset=None), line 94 (self.instance.output.exclude(id__in=field_ids).delete()), lines 531-532 (_update_fields called before _update_fieldsets), lines 167-187 (_update_field uses TaskField.objects.update_or_create with fieldset in lookup), lines 231-244 (_update_fieldset_fields), lines 246-276 (_update_fieldsets). backend/src/processes/models/workflows/fields.py lines 47-53 (TaskField.task FK with related_name='output'), lines 60-65 (TaskField.fieldset FK nullable), line 76 (objects = BaseSoftDeleteManager). backend/src/generics/managers.py lines 6-8 (BaseSoftDeleteManager filters is_deleted=False). backend/src/generics/querysets.py lines 59-61 (BaseQuerySet.delete does soft delete via update is_deleted=True).

Comment thread backend/src/processes/services/templates/fieldsets/fieldset.py
Comment on lines +246 to +254
def _update_fieldsets(self, data: Optional[List]) -> None:

fieldset_api_names = set()
for fieldset_data in data or []:
task_link = next(
link for link in fieldset_data['task_links']
if link['task_api_name'] == self.instance.api_name
)
order = task_link['order']
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 tasks/task_version.py:246

In _update_fieldsets, the next() call at line 250 raises StopIteration if no task_link matches self.instance.api_name, causing the entire update operation to fail. Consider providing a default value with None and handling the missing case gracefully.

        fieldset_api_names = set()
         for fieldset_data in data or []:
-            task_link = next(
-                link for link in fieldset_data['task_links']
-                if link['task_api_name'] == self.instance.api_name
-            )
-            order = task_link['order']
+            task_link = next(
+                (link for link in fieldset_data['task_links']
+                 if link['task_api_name'] == self.instance.api_name),
+                None
+            )
+            if task_link is None:
+                continue
+            order = task_link['order']
             fieldset, _ = FieldSet.objects.update_or_create(
🚀 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 246-254:

In `_update_fieldsets`, the `next()` call at line 250 raises `StopIteration` if no `task_link` matches `self.instance.api_name`, causing the entire update operation to fail. Consider providing a default value with `None` and handling the missing case gracefully.

Evidence trail:
backend/src/processes/services/tasks/task_version.py lines 246-253 (the `next()` call without default), lines 498-532 (`update_from_version` calling `_update_fieldsets`), backend/src/processes/services/versioning/schemas.py lines 84-93 (FieldsetTemplateTaskTemplateSchemaV1), lines 100-135 (FieldSetSchemaV1 with task_links), lines 266-295 (TaskSchemaV1 with fieldsets), backend/src/processes/services/workflows/workflow_version.py lines 61-80 (_update_tasks_from_version passing data).

service.validate_rules()
except FieldsetServiceException as ex:
self.raise_validation_error(message=ex.message)
for field_template in kickoff.fields.filter(fieldset__isnull=True):
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 workflows/kickoff_value.py:113

When creating fields without a fieldset at lines 113-120, fieldset_id is not passed to TaskFieldService.create(). If the field template has rules, _link_rules accesses kwargs['fieldset_id'] with direct key access and raises KeyError, which escapes the except (TaskFieldException, FieldsetServiceException) block at line 121. Pass fieldset_id=None for fields without a fieldset.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/serializers/workflows/kickoff_value.py around line 113:

When creating fields without a fieldset at lines 113-120, `fieldset_id` is not passed to `TaskFieldService.create()`. If the field template has rules, `_link_rules` accesses `kwargs['fieldset_id']` with direct key access and raises `KeyError`, which escapes the `except (TaskFieldException, FieldsetServiceException)` block at line 121. Pass `fieldset_id=None` for fields without a fieldset.

Evidence trail:
backend/src/processes/serializers/workflows/kickoff_value.py lines 113-121 (REVIEWED_COMMIT): `service.create()` called without `fieldset_id`; except block catches only `TaskFieldException, FieldsetServiceException`.
backend/src/generics/base/service.py lines 65-70: `BaseModelService.create(**kwargs)` passes kwargs to `_create_related(**kwargs)`.
backend/src/processes/services/tasks/field.py line 326: `_create_related` calls `_link_rules(instance_template, **kwargs)` if rules exist.
backend/src/processes/services/tasks/field.py line 375: `_link_rules` uses `kwargs['fieldset_id']` (direct key access, raises KeyError if missing).
backend/src/processes/models/templates/fields.py lines 59-68: `FieldTemplate.fieldset` is nullable, and `rules` M2M has no constraint preventing association on fieldset-less fields.

Comment thread backend/src/processes/models/templates/fieldset.py Outdated
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.

Floating-point equality check in sum validation is unreliable

Medium Severity

_validate_sum_equal accumulates field values using float() addition and then compares the total with exact equality (!=). Floating-point arithmetic can produce rounding errors (e.g., 0.1 + 0.2 != 0.3), causing valid inputs to fail validation spuriously. Using Decimal or an epsilon-based comparison would be more reliable.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5944362. Configure here.

Comment thread frontend/src/public/components/TemplateEdit/FieldsetPicker/FieldsetPicker.tsx Outdated
Comment thread frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetDetails.tsx Outdated
…sorted list across workflow log, task log and kickoff of workflow modal
].sort((a, b) => a.order - b.order);


if (isTruncated && items.length > 0) {
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 KickoffOutputs/KickoffOutputs.tsx:98

When isTruncated is true and the first item is a fieldset with an empty fields array, accessing firstItem.data.fields[0] at line 104 returns undefined. Passing undefined to renderSingleOutput causes output.type access at line 72 to throw a runtime TypeError. Consider adding a guard to check firstItem.data.fields.length > 0 before accessing the first element.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file frontend/src/public/components/KickoffOutputs/KickoffOutputs.tsx around line 98:

When `isTruncated` is true and the first item is a fieldset with an empty `fields` array, accessing `firstItem.data.fields[0]` at line 104 returns `undefined`. Passing `undefined` to `renderSingleOutput` causes `output.type` access at line 72 to throw a runtime TypeError. Consider adding a guard to check `firstItem.data.fields.length > 0` before accessing the first element.

Evidence trail:
frontend/src/public/components/KickoffOutputs/KickoffOutputs.tsx lines 98-104 (isTruncated branch accessing fields[0] without length check), line 72 (renderSingleOutput accessing output.type), line 49 (entry guard checks fieldsets array but not individual fieldset.fields). frontend/src/public/types/template.ts line 180-192 (IFieldsetData.fields typed as IExtraField[] allowing empty array).

…aticapp/pneumaticworkflow into frontend/templates/46129__add_fieldsets
…aticapp/pneumaticworkflow into frontend/templates/46129__add_fieldsets
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 6 total unresolved issues (including 5 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 ac887b8. 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.

KickoffOnlyFieldsSerializer crashes when kickoff is None

Medium Severity

KickoffOnlyFieldsSerializer.to_representation converts a Manager to an instance via .first() but doesn't handle the case where .first() returns None. Unlike the sibling KickoffSerializer.to_representation which returns {'fields': [], 'fieldsets': []} for None, this serializer passes None to super().to_representation(), which will crash with an AttributeError.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ac887b8. Configure here.

Maria-Lordwill and others added 3 commits May 8, 2026 11:33
…in workflow run, kickoff edit and task completion
…aticapp/pneumaticworkflow into frontend/templates/46129__add_fieldsets
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.

🟢 Low 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)

Comment on lines +52 to +55
const orderedOutputs: TOutputItem[] = [
...(outputs || []).map((field): TOutputItem => ({ kind: 'field', order: field.order, data: field })),
...(fieldsets || []).map((fs): TOutputItem => ({ kind: 'fieldset', order: fs.order!, data: fs })),
].sort((a, b) => b.order - a.order);
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 KickoffOutputs/KickoffOutputs.tsx:52

Using fs.order! on line 54 asserts that order is defined, but IFieldsetData.order is optional (order?: number). When fieldsets have undefined order values, the sort comparison b.order - a.order on line 55 produces NaN, causing unpredictable output ordering.

-  const orderedOutputs: TOutputItem[] = [
-    ...(outputs || []).map((field): TOutputItem => ({ kind: 'field', order: field.order, data: field })),
-    ...(fieldsets || []).map((fs): TOutputItem => ({ kind: 'fieldset', order: fs.order!, data: fs })),
-  ].sort((a, b) => b.order - a.order);
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file frontend/src/public/components/KickoffOutputs/KickoffOutputs.tsx around lines 52-55:

Using `fs.order!` on line 54 asserts that `order` is defined, but `IFieldsetData.order` is optional (`order?: number`). When fieldsets have `undefined` order values, the sort comparison `b.order - a.order` on line 55 produces `NaN`, causing unpredictable output ordering.

Evidence trail:
frontend/src/public/types/template.ts:191 — `order?: number;` in IFieldsetData interface
frontend/src/public/components/KickoffOutputs/KickoffOutputs.tsx:54 — `order: fs.order!` non-null assertion
frontend/src/public/components/KickoffOutputs/KickoffOutputs.tsx:55 — `.sort((a, b) => b.order - a.order)` sort comparison
frontend/src/public/components/KickoffOutputs/types.ts:3-5 — TOutputItem requires `order: number`
frontend/src/public/components/TemplateEdit/TaskOutputFlow/mergeTaskOutputFlow.ts:78 — similar code correctly handles optional order with `part.data.order ?? 0`

Comment on lines +83 to +95
useEffect(() => {
const id = Number(matchParamId);
if (Number.isNaN(id)) {
history.push(fieldsetListRoute);
return;
}

dispatch(setTemplateId(Number(matchTemplateId)));

if (fieldset?.id === id) return;

dispatch(loadCurrentFieldset({ id }));
}, [matchParamId, matchTemplateId]);
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 FieldsetDetails/FieldsetDetails.tsx:83

matchTemplateId is not validated before being converted to a number, so Number('abc') produces NaN and dispatches it to the Redux store. This allows invalid routes like /templates/abc/fieldsets/123/ to proceed instead of redirecting. Consider adding a check similar to lines 84–88 to validate matchTemplateId and redirect if it is not a valid number.

  useEffect(() => {
    const id = Number(matchParamId);
    if (Number.isNaN(id)) {
      history.push(fieldsetListRoute);
      return;
    }
    
    const templateId = Number(matchTemplateId);
    if (Number.isNaN(templateId)) {
      history.push(ERoutes.Templates);
      return;
    }
    
    dispatch(setTemplateId(templateId));
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetDetails.tsx around lines 83-95:

`matchTemplateId` is not validated before being converted to a number, so `Number('abc')` produces `NaN` and dispatches it to the Redux store. This allows invalid routes like `/templates/abc/fieldsets/123/` to proceed instead of redirecting. Consider adding a check similar to lines 84–88 to validate `matchTemplateId` and redirect if it is not a valid number.

Evidence trail:
frontend/src/public/components/Fieldsets/FieldsetDetails/FieldsetDetails.tsx lines 58 (matchTemplateId from route params), 83-90 (validation for matchParamId but not matchTemplateId, dispatch of Number(matchTemplateId) without NaN check)

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

Labels

bug Something isn't working Frontend Web client changes request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants