⚙️ 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.
| ) | ||
| if fields_filter_kwargs: | ||
| qst = qst.filter(**fields_filter_kwargs) | ||
| return qst |
There was a problem hiding this comment.
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)
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.
| 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() |
There was a problem hiding this comment.
🟢 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
fieldsandfieldsets. The conditionif data.get('fields'):is falsy for an empty list[], so_update_fieldswon't be called and existing fields (withfieldset=None) won't be deleted. However,if data.get('fieldsets') is not None:will pass for an empty list, causing_update_fieldsetsto delete all fieldsets. If the intent is to clear all items when an empty list is provided, thefieldscheck should also useis not Noneinstead 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.
| 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.
🟢 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, callingservice._validate()will raiseTypeError: 'NoneType' object is not callablefor any rule whosetypedoes not have a corresponding_validate_{type}method. Looking atFieldsetTemplateRuleService._validate(), it doesvalidator = getattr(self, f'_validate_{self.instance.type}', None)and then callsvalidator(**kwargs)without checking ifvalidatorisNone. Since only_validate_sum_equalexists, 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.
| def partial_update(self, **update_kwargs) -> Model: | ||
| self._validate(**update_kwargs) |
There was a problem hiding this comment.
🟡 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)
| ) | ||
| service.partial_update( | ||
| value=fields_values.get(task_field.api_name), | ||
| value=fields_values[field.api_name], |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit 4e44624. Configure here.
| """ 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.
| 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') |
There was a problem hiding this comment.
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)
Reviewed by Cursor Bugbot for commit 4e44624. Configure here.
…d not requred fields
| else: | ||
| total += float(field.value) | ||
| values_exists = True | ||
| if values_exists and total != float(self.instance.value): |
There was a problem hiding this comment.
🟡 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)
| except TypeError: | ||
| pass | ||
| fieldsets_api_names.append(elem['fieldset']['api_name']) | ||
| except (TypeError, ValueError): |
There was a problem hiding this comment.
🟡 Medium templates/template.py:202
The exception handler on line 202 catches ValueError instead of KeyError. When elem['fieldset']['api_name'] is accessed and either key is missing, KeyError is raised, not ValueError. This causes an unhandled exception for malformed fieldset link data instead of skipping the element. Change to except (TypeError, KeyError): to match the dictionary access pattern.
- except (TypeError, ValueError):
+ except (TypeError, KeyError):🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file backend/src/processes/serializers/templates/template.py around line 202:
The exception handler on line 202 catches `ValueError` instead of `KeyError`. When `elem['fieldset']['api_name']` is accessed and either key is missing, `KeyError` is raised, not `ValueError`. This causes an unhandled exception for malformed fieldset link data instead of skipping the element. Change to `except (TypeError, KeyError):` to match the dictionary access pattern.
Evidence trail:
File: backend/src/processes/serializers/templates/template.py lines 200-203 at REVIEWED_COMMIT. Line 201: `fieldsets_api_names.append(elem['fieldset']['api_name'])`. Line 202: `except (TypeError, ValueError):`. Python dict subscript `d[key]` raises `KeyError` when key is absent, not `ValueError`. See Python docs: https://docs.python.org/3/library/stdtypes.html#dict
… "start_task" actions
| skip_value: bool = False, | ||
| **kwargs, | ||
| ): | ||
| fields_data = fields_data or {} |
There was a problem hiding this comment.
Fieldset fields type annotation wrong, List[Dict] vs dict
Medium Severity
_create_fields declares fields_data as Optional[List[Dict]] but defaults it to {} (an empty dict) on line 51 with fields_data = fields_data or {}. The caller passes a dict (mapping api_name to value), which is correct for the .get() call on line 63, but the type annotation is misleading and the default should be {} not confusingly mixed with the List[Dict] annotation. More critically, if someone passes an empty list [], or {} converts it to a dict, which is fine, but passing an actual list would break .get().
Reviewed by Cursor Bugbot for commit e5d253f. Configure here.
… "start_task" actions
…ates/id/fields response
…ates/id/fields response
…to backend/fieldsets/45773__fieldsets
…mpleted_or_skipped" for backward compatibility
| WHERE pt.rule_id = rt.id | ||
| AND pt.operator = 'completed' | ||
| AND pt.field_type = 'task' | ||
| """) |
There was a problem hiding this comment.
Migration RunSQL indentation creates invalid operations list
Low Severity
The second migrations.RunSQL(...) call at line 197 is indented with an extra level inside the first migrations.RunSQL call's parenthetical context, making it appear as an argument rather than a separate operation. However, examining the Python syntax, the comma on line 196 closes the first RunSQL, and the second RunSQL at line 197 is actually a separate list element — but with misleading indentation. The actual Python parsing should be verified, as incorrect indentation here could cause either a syntax error or the second migration being treated as an argument to the first.
Reviewed by Cursor Bugbot for commit 8c8f090. Configure here.
…mpleted_or_skipped" in the drafts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 8 total unresolved issues (including 7 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 f34b880. Configure here.
| from django.db import models # noqa : PLC0415 | ||
| if isinstance(instance, models.Manager): | ||
| instance = instance.first() | ||
| return super().to_representation(instance) |
There was a problem hiding this comment.
KickoffListSerializer.to_representation missing None guard
Medium Severity
KickoffListSerializer.to_representation resolves a Manager to its first item but doesn't handle the None case when no kickoff exists. KickoffSerializer correctly returns {'fields': [], 'fieldsets': []} for None, but KickoffListSerializer passes None to super().to_representation(), which will crash.
Reviewed by Cursor Bugbot for commit f34b880. Configure here.
1 participant
Note
Medium Risk
Adds new database models/migrations plus template/workflow serialization and field-creation paths to support grouped fields with rule validation; this touches workflow execution and data writes, so regressions could affect task/kickoff field population and condition evaluation.
Overview
Adds FieldSet support for templates and runtime workflows, introducing
FieldsetTemplate/FieldsetTemplateRule(plus kickoff/task link through-models) and workflow-levelFieldSet/FieldSetRulewith layout/label metadata and rule validation (e.g.sum_equal).Template APIs/serialization are extended to accept/emit
fieldsetson kickoffs and tasks, persist their ordered links, and include fieldset fields when resolving output fields; workflow creation, update/versioning, events, and task completion now also create/update fieldsets and re-validate their rules.Extends task condition operators with
skippedandcompleted_or_skipped, updates allowed-operator validation, resolvers, and comparator handling, and migrates existing task predicates/drafts fromcompletedtocompleted_or_skipped. Also centralizesBaseModelService.delete(), removes redundant no-op_create_related/deleteoverrides, and adds generic fields/helpers for account/template-scoped lookups andapi_namelist serialization.Reviewed by Cursor Bugbot for commit f34b880. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add FieldSet support to templates, kickoffs, and tasks in the process engine
FieldsetTemplate,FieldsetTemplateRule,FieldsetTemplateKickoff, andFieldsetTemplateTaskTemplatemodels alongside runtimeFieldSetandFieldSetRuleworkflow models, with a database migration in 0252_add_fieldsets.py./templates/{id}/fieldsets(list, create) and/templates/fieldsets/{id}(retrieve, partial update, delete) viaFieldsetTemplateViewSetandFieldSetTemplateService.fieldsetson kickoff and tasks, persist link records, and clone fieldsets (with rules and fields) when copying a template.FieldSetobjects from templates, apply field values into them, and validate fieldset rules (e.g.SUM_EQUAL) on save.SKIPPEDandCOMPLETED_OR_SKIPPEDPredicateOperatorvalues; task condition resolution andget_tasks_parentsnow respect these new operators.fieldsetsarrays throughout.FieldMixinno longer inheritsAccountBaseMixin; any model relying on that inheritance path will lose those fields unless it re-declares them.Macroscope summarized f34b880.