⚙️ 45773 backend [ templates ] Add FieldSets#211
Conversation
…d/fieldsets endpoint
…off and FieldSetTemplate models
…ntermediate m2m models
…to backend/fieldsets/45773__fieldsets
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| 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)) |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit cd8ef70. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| 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, | ||
| ) |
There was a problem hiding this comment.
🟢 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
TaskFieldinstances for fields without fieldsets (lines 109-116),fieldset_idis not passed toTaskFieldService.create(). However,TaskFieldService._create_relatedchecksinstance_template.rules.all().exists()and calls_link_rules, which unconditionally accesseskwargs['fieldset_id'](visible in the references). If a field template without a fieldset has associated rules, this will raise aKeyError.
backend/src/processes/services/tasks/task.py:218
Calling
service.create()without passingfieldset_idwill cause aKeyErrorinTaskFieldService._link_rulesif the field template has rules. The_link_rulesmethod (added in this commit) accesseskwargs['fieldset_id']with direct dictionary access, butcreate_fields_from_templateonly passesworkflow_id,task_id, andskip_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.
| class FieldSet( | ||
| BaseApiNameModel, | ||
| BaseFieldSetMixin, | ||
| AccountBaseMixin, | ||
| ): | ||
|
|
||
| class Meta: | ||
| ordering = ['-id'] | ||
|
|
||
| workflow = models.ForeignKey( |
There was a problem hiding this comment.
🟡 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() |
There was a problem hiding this comment.
🟠 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) |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 4e44624. Configure here.
…d not requred fields
| class FieldsetTemplateSerializer( | ||
| ModelSerializer, | ||
| CustomValidationErrorMixin, | ||
| ): |
There was a problem hiding this comment.
🟡 Medium templates/fieldset.py:38
FieldsetTemplateSerializer lists ModelSerializer before CustomValidationErrorMixin, so run_validation, to_internal_value, and is_valid resolve to ModelSerializer's implementations. The custom error enrichment methods in CustomValidationErrorMixin are shadowed and never execute, breaking the enriched validation error format. Consider reversing the inheritance order to CustomValidationErrorMixin, ModelSerializer to match SharedFieldsetTemplateSerializer and FieldsetTemplateRuleSerializer.
| class FieldsetTemplateSerializer( | |
| ModelSerializer, | |
| CustomValidationErrorMixin, | |
| ): | |
| class FieldsetTemplateSerializer( | |
| CustomValidationErrorMixin, | |
| ModelSerializer, | |
| ): |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/serializers/templates/fieldset.py around lines 38-41:
`FieldsetTemplateSerializer` lists `ModelSerializer` before `CustomValidationErrorMixin`, so `run_validation`, `to_internal_value`, and `is_valid` resolve to `ModelSerializer`'s implementations. The custom error enrichment methods in `CustomValidationErrorMixin` are shadowed and never execute, breaking the enriched validation error format. Consider reversing the inheritance order to `CustomValidationErrorMixin, ModelSerializer` to match `SharedFieldsetTemplateSerializer` and `FieldsetTemplateRuleSerializer`.
Evidence trail:
backend/src/processes/serializers/templates/fieldset.py lines 38-41: `class FieldsetTemplateSerializer(ModelSerializer, CustomValidationErrorMixin)` — ModelSerializer listed first. Lines 16-18: `class FieldsetTemplateRuleSerializer(CustomValidationErrorMixin, ModelSerializer)` — mixin listed first (correct). Lines 83-86: `class SharedFieldsetTemplateSerializer(CustomValidationErrorMixin, ModelSerializer)` — mixin listed first (correct). backend/src/generics/mixins/serializers.py lines 356, 381, 430: `CustomValidationErrorMixin` defines `run_validation`, `to_internal_value`, and `is_valid`. Python MRO (C3 linearization) places `ModelSerializer` → `Serializer` (DRF) before `CustomValidationErrorMixin`, so Serializer's default implementations shadow the mixin's overrides.
| template=template, | ||
| task=instance, | ||
| ) | ||
| if raw_due_date_created: |
There was a problem hiding this comment.
🟢 Low templates/task.py:549
In update(), AnalyticService.templates_task_due_date_created() fires at line 549-556 before create_or_update_related_one() creates the due date at lines 618-629. If due date creation fails, the analytics event is incorrectly recorded. Move the analytics call to after line 629, matching the correct order in create() where it fires after creation (lines 508-515 after 496-507).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/serializers/templates/task.py around line 549:
In `update()`, `AnalyticService.templates_task_due_date_created()` fires at line 549-556 before `create_or_update_related_one()` creates the due date at lines 618-629. If due date creation fails, the analytics event is incorrectly recorded. Move the analytics call to after line 629, matching the correct order in `create()` where it fires after creation (lines 508-515 after 496-507).
Evidence trail:
backend/src/processes/serializers/templates/task.py lines 496-516 (create method: create_or_update_related_one at 496-507, then analytics at 508-515), lines 518-630 (update method: analytics at 549-556, create_or_update_related_one at 618-629)
| fieldset = existing_fieldsets[fieldset_api_name] | ||
| update_kwargs = {} | ||
| if fieldset.order != fieldset_data['order']: | ||
| update_kwargs['order'] = fieldset_data['order'] |
There was a problem hiding this comment.
Missing order key crashes update
Medium Severity
When updating an existing fieldset by api_name, create_or_update_fieldsets compares fieldset_data['order'] without a default. Payloads that omit order raise KeyError even though the serializer treats order as optional elsewhere.
Reviewed by Cursor Bugbot for commit b023b11. Configure here.
There was a problem hiding this comment.
🟢 Low
When generating api_name for task fields that lack one, line 106 uses TaskTemplate.api_name_prefix (which is 'task') instead of FieldTemplate.api_name_prefix (which is 'field'). This causes task fields to receive api_names prefixed with task- instead of field-, inconsistent with kickoff fields (line 82-84) which correctly use FieldTemplate.api_name_prefix.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/services/templates/template.py around line 104:
When generating `api_name` for task fields that lack one, line 106 uses `TaskTemplate.api_name_prefix` (which is `'task'`) instead of `FieldTemplate.api_name_prefix` (which is `'field'`). This causes task fields to receive api_names prefixed with `task-` instead of `field-`, inconsistent with kickoff fields (line 82-84) which correctly use `FieldTemplate.api_name_prefix`.
Evidence trail:
backend/src/processes/services/templates/template.py lines 82-84 (kickoff fields use FieldTemplate.api_name_prefix) and lines 99-107 (task fields use TaskTemplate.api_name_prefix). backend/src/processes/models/templates/fields.py:33 (FieldTemplate.api_name_prefix = 'field'), lines 42-53 (FieldTemplate has FKs to both Kickoff and TaskTemplate). backend/src/processes/models/templates/task.py:48 (TaskTemplate.api_name_prefix = 'task').
| fs_api_names = set() | ||
| for fs_data in data or []: | ||
| order = fs_data['kickoff_links'][0]['order'] | ||
| fieldset, _ = FieldSet.objects.update_or_create( |
There was a problem hiding this comment.
Kickoff version fieldset order key
High Severity
Kickoff workflow versioning reads fieldset order from kickoff_links[0]['order'], but version snapshots from FieldSetSchemaV1 expose order on the fieldset object. Applying a template version with kickoff fieldsets can raise KeyError and abort sync.
Reviewed by Cursor Bugbot for commit 48bd6c8. Configure here.
| shared_fieldset = models.ForeignKey( | ||
| 'FieldsetTemplate', | ||
| on_delete=models.SET_NULL, | ||
| related_name='child_fieldsets', |
There was a problem hiding this comment.
Shared fieldset delete mismatch
Medium Severity
The model sets shared_fieldset to on_delete=SET_NULL, but migration 0254 creates the FK with CASCADE. Deleting a shared fieldset may cascade-delete template copies at the DB while the ORM expects nulling, causing inconsistent lifecycle behavior.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 48bd6c8. Configure here.
| shared_fieldset_id = AccountPrimaryKeyRelatedField( | ||
| queryset=FieldsetTemplate.objects.all(), | ||
| required=True, | ||
| ) |
There was a problem hiding this comment.
Shared fieldset ID unfiltered
Medium Severity
FieldsetTemplateSerializer resolves shared_fieldset_id against all account fieldsets, not only is_shared=True library rows. Attaching a template copy or non-shared row as the shared source can duplicate wrong definitions.
Reviewed by Cursor Bugbot for commit 48bd6c8. Configure here.
| rule_data['fields'] = [ | ||
| fields_map[old_api_name] | ||
| for old_api_name in rule_data.get('fields', []) | ||
| ] |
There was a problem hiding this comment.
Rule field remap KeyError
Medium Severity
_replace_api_names remaps rule fields through fields_map without guarding missing keys. If serialized rule data references a field api_name not present in fields, cloning a shared fieldset raises KeyError instead of validation error.
Reviewed by Cursor Bugbot for commit 314ba8a. Configure here.
| 'ordering': ['-id'], | ||
| }, | ||
| bases=(src.generics.mixins.models.SoftDeleteMixin, models.Model), | ||
| ), |
There was a problem hiding this comment.
Migration omits model columns
High Severity
Migration 0254_add_fieldsets defines FieldsetTemplate without title, order, is_shared, shared_fieldset, or direct task/kickoff FKs, while runtime models and services read and write those fields. Fresh migrations leave the ORM out of sync with the database schema.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 314ba8a. Configure here.
| UniqueConstraint( | ||
| fields=['api_name', 'fieldset'], | ||
| condition=Q(is_deleted=False), | ||
| name='fieldsettemplate_api_name_template_unique', |
There was a problem hiding this comment.
🟠 High templates/fieldset.py:107
FieldsetTemplateRule uses the constraint name 'fieldsettemplate_api_name_template_unique', which duplicates the existing constraint on FieldsetTemplate and mismatches the migration name 'fieldsettemplate_rule_api_name_template_unique'. Django requires unique constraint names across tables, and the mismatch between model and migration state causes makemigrations to generate spurious auto-detected migrations. Rename the constraint to 'fieldsettemplate_rule_api_name_template_unique'.
| name='fieldsettemplate_api_name_template_unique', | |
| name='fieldsettemplate_rule_api_name_template_unique', |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/models/templates/fieldset.py around line 107:
`FieldsetTemplateRule` uses the constraint name `'fieldsettemplate_api_name_template_unique'`, which duplicates the existing constraint on `FieldsetTemplate` and mismatches the migration name `'fieldsettemplate_rule_api_name_template_unique'`. Django requires unique constraint names across tables, and the mismatch between model and migration state causes `makemigrations` to generate spurious auto-detected migrations. Rename the constraint to `'fieldsettemplate_rule_api_name_template_unique'`.
Evidence trail:
backend/src/processes/models/templates/fieldset.py lines 30-35 (FieldsetTemplate constraint name 'fieldsettemplate_api_name_template_unique'), lines 103-108 (FieldsetTemplateRule uses same constraint name). backend/src/processes/migrations/0255_add_shared_fieldsets.py lines 267-273 (migration uses 'fieldsettemplate_rule_api_name_template_unique' for FieldsetTemplateRule). backend/src/processes/migrations/0256_auto_20260622_1939.py lines 120-122 and 147-149 (only touches FieldsetTemplate constraint, not FieldsetTemplateRule). git_grep for 'fieldsettemplate_rule_api_name_template_unique' shows it only exists in migration 0255 line 272, never in model code.
| fieldsets_api_names.add(fieldset.api_name) | ||
| instance.fieldsets.exclude(api_name__in=fieldsets_api_names).delete() | ||
|
|
||
| def get_draft_fieldsets(self, fieldsets_data: Any): |
There was a problem hiding this comment.
🟠 High templates/mixins.py:308
get_draft_fieldsets fetches the shared FieldsetTemplate with FieldsetTemplate.objects.get(id=shared_fieldset_id, is_shared=True) and uses its data without any account scoping. A caller can pass another account's shared fieldset ID, and the server will copy that fieldset's fields and rules into the draft, exposing private template structure across accounts. Scope the lookup to the current account, for example by filtering on account_id from the template or user.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/serializers/templates/mixins.py around line 308:
`get_draft_fieldsets` fetches the shared `FieldsetTemplate` with `FieldsetTemplate.objects.get(id=shared_fieldset_id, is_shared=True)` and uses its data without any account scoping. A caller can pass another account's shared fieldset ID, and the server will copy that fieldset's `fields` and `rules` into the draft, exposing private template structure across accounts. Scope the lookup to the current account, for example by filtering on `account_id` from the template or user.
| ) | ||
| service.validate_rules() | ||
| except FieldsetServiceException as ex: | ||
| self.raise_validation_error(message=ex.message) |
There was a problem hiding this comment.
Kickoff update not transactional
High Severity
Kickoff field updates apply each TaskField via partial_update before validate_rules runs, with no enclosing transaction. If a fieldset rule fails validation, earlier field values may already be saved while the API returns an error.
Reviewed by Cursor Bugbot for commit 16d69c8. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
There are 15 total unresolved issues (including 12 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 0b969af. Configure here.
| del task_fieldsets[i] | ||
| else: | ||
| new_task_fieldsets.append(task_fieldsets[i]) | ||
| task['fieldsets'] = new_kickoff_fieldsets |
There was a problem hiding this comment.
Tasks get kickoff fieldsets
High Severity
In update_draft_fieldset, the task loop builds new_task_fieldsets but assigns task['fieldsets'] from new_kickoff_fieldsets, so draft tasks receive kickoff fieldset data instead of their own. Matching task fieldsets also append to task_fieldsets rather than new_task_fieldsets, so replacements are often dropped.
Reviewed by Cursor Bugbot for commit 0b969af. Configure here.
| ) | ||
| else: | ||
| print('--- duplicate task fieldset - removed') | ||
| link.delete() |
There was a problem hiding this comment.
Migration skips task fieldsets
High Severity
handle reuses one updated flag for both kickoff M2M links and task M2M links. After the first kickoff link creates a template fieldset, every task link is treated as a duplicate and deleted without creating a replacement runtime fieldset.
Reviewed by Cursor Bugbot for commit 0b969af. Configure here.
| f'---|--- new task fieldset: ' | ||
| f'task: "{task["name"]}"', | ||
| ) | ||
| updated = True |
There was a problem hiding this comment.
Draft fieldset flag shared
Medium Severity
In update_draft_fieldset, a single updated flag spans kickoff and all tasks. After the kickoff fieldset is replaced, matching task draft fieldsets with the same api_name are removed as duplicates instead of updated.
Reviewed by Cursor Bugbot for commit 0b969af. Configure here.
| model_name='fieldsettemplate', | ||
| name='task', | ||
| field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='fieldsets', to='processes.TaskTemplate'), | ||
| ), |
There was a problem hiding this comment.
🟡 Medium migrations/0255_add_shared_fieldsets.py:55
This migration adds fieldsettemplate.task_id and fieldsettemplate.kickoff_id as nullable foreign keys but never copies existing links from the processes_fieldsettemplate_tasktemplate / ..._kickoff through tables. Pre-existing FieldsetTemplate rows created by 0254_add_fieldsets get task_id and kickoff_id set to NULL, so after deploy they drop out of task.fieldsets, kickoff.fieldsets, and Template.get_*_output_fields() until the separate migrate_fieldsets command is manually run. Add a RunSQL (or RunPython) operation to this migration that backfills task_id and kickoff_id from the through tables.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/migrations/0255_add_shared_fieldsets.py around line 55:
This migration adds `fieldsettemplate.task_id` and `fieldsettemplate.kickoff_id` as nullable foreign keys but never copies existing links from the `processes_fieldsettemplate_tasktemplate` / `..._kickoff` through tables. Pre-existing `FieldsetTemplate` rows created by `0254_add_fieldsets` get `task_id` and `kickoff_id` set to `NULL`, so after deploy they drop out of `task.fieldsets`, `kickoff.fieldsets`, and `Template.get_*_output_fields()` until the separate `migrate_fieldsets` command is manually run. Add a `RunSQL` (or `RunPython`) operation to this migration that backfills `task_id` and `kickoff_id` from the through tables.
| kickoff['fieldsets'] = new_kickoff_fieldsets | ||
|
|
||
| # Update matching fieldsets in tasks | ||
| for task in draft.get('tasks') or []: |
There was a problem hiding this comment.
🟡 Medium commands/migrate_fieldsets.py:44
In update_draft_fieldset, the task loop mutates task_fieldsets with del task_fieldsets[i] during for i in range(len(task_fieldsets)). When a task has multiple matching fieldsets and updated is already True, deleting an earlier element shrinks the list, but the loop still advances toward the original final index, so task_fieldsets[i] raises IndexError and aborts the migration. Also, the first match appends fieldset_data to task_fieldsets (growing it) instead of to new_task_fieldsets, so it gets dropped. Consider building new_task_fieldsets consistently without index-based deletion.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/management/commands/migrate_fieldsets.py around line 44:
In `update_draft_fieldset`, the task loop mutates `task_fieldsets` with `del task_fieldsets[i]` during `for i in range(len(task_fieldsets))`. When a task has multiple matching fieldsets and `updated` is already `True`, deleting an earlier element shrinks the list, but the loop still advances toward the original final index, so `task_fieldsets[i]` raises `IndexError` and aborts the migration. Also, the first match appends `fieldset_data` to `task_fieldsets` (growing it) instead of to `new_task_fieldsets`, so it gets dropped. Consider building `new_task_fieldsets` consistently without index-based deletion.
| fieldset_api_name=old_shared_fieldset.api_name, | ||
| ) | ||
|
|
||
| updated = False |
There was a problem hiding this comment.
🟠 High commands/migrate_fieldsets.py:128
The migration uses a single updated flag shared across all deprecated kickoff and task links. After the first link creates a replacement FieldsetTemplate, every subsequent link hits the duplicate ... removed branch and is deleted without creating its own replacement. If an old shared fieldset is attached to multiple tasks or to both a kickoff and tasks, the migration silently drops fieldsets from every location beyond the first.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @backend/src/processes/management/commands/migrate_fieldsets.py around line 128:
The migration uses a single `updated` flag shared across all deprecated kickoff and task links. After the first link creates a replacement `FieldsetTemplate`, every subsequent link hits the `duplicate ... removed` branch and is deleted without creating its own replacement. If an old shared fieldset is attached to multiple tasks or to both a kickoff and tasks, the migration silently drops fieldsets from every location beyond the first.
1 participant
Note
High Risk
Large schema and workflow-path changes (task complete, kickoff create/update, template drafts) plus a one-off data migration; ordering of workflow fields now includes
fieldset_id, which may affect consumers relying on sort order.Overview
Adds fieldset support so kickoff and task fields can be grouped in templates and in running workflows, with optional shared definitions reused across templates.
New persistence includes
FieldsetTemplate/FieldSet(and rule models), migrations 0254–0255, and amigrate_fieldsetscommand to reshape legacy shared fieldsets and draft JSON. Template and workflow APIs now exposefieldsetson kickoffs and tasks; kickoff/task save flows create or sync template fieldsets fromshared_fieldset_id, and workflow start/complete paths instantiate fieldsets and runsum_equalrule checks.Supporting changes:
FieldSetTemplateService/ runtimeFieldSetService, DRF fields forapi_name-scoped relations,BaseModelServicegains defaultdeleteand optionalaccount, kickoff/output field queries include fields inside fieldsets, andSystemVariable.TASK_VARSis expanded to match workflow name variables. Minor cleanup removesmigrate_presets, moves “fields-only” template serializers totemplate_fields.py, and drops localTemplateFilterfromfilters.py.Reviewed by Cursor Bugbot for commit 0b969af. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add FieldSets to templates, workflows, and kickoffs with validation rules
FieldsetTemplateandFieldSetmodels (with correspondingFieldsetTemplateRule/FieldSetRule) to group fields within kickoffs and tasks at both template and workflow runtime levels./fieldsetsREST API (viaSharedFieldsetTemplateViewSet) to create, list, retrieve, update, and delete shared fieldset templates scoped to an account.fieldsetsalongsidefields, persisting them viaFieldsetMixin.create_or_update_fieldsets.FieldSetServiceandFieldSetTemplateServiceto handle fieldset creation from templates, field instantiation, and rule validation (e.g.SUM_EQUALsemantics with OR logic across multiple rules).TaskUpdateVersionServiceandKickoffUpdateVersionService.fieldsetsarray forTASK_COMPLETEevents andnullotherwise.migrate_fieldsetsmanagement command to migrate existing shared fieldset templates to the new structure.FieldMixinno longer inheritsAccountBaseMixin;WorkflowQuerySetordering now includesfieldset_id; database environment variables no longer have fallback defaults and will beNoneif unset.Macroscope summarized 0b969af.