feat: tasks management with periodic schedule #1781
Conversation
WalkthroughThis pull request introduces comprehensive task management features across the system. On the backend, two new models—TaskTemplate and TaskNode—are added along with their serializers, views, utilities, and associated permissions. New functions support task scheduling and date handling, with thorough unit tests validating expected behaviors. On the frontend, navigation elements, dynamic calendar redirection, form enhancements for task templates, updated schemas, localization keys, and utility mappings have been added. Additionally, several localization string corrections and adjustments to test cases ensure the new task features integrate consistently within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant F as Frontend (TaskTemplateForm)
participant A as API (TaskTemplateViewSet)
participant M as Backend (TaskTemplate Model)
participant S as Scheduler Utility
U->>F: Fill and submit Task Template form
F->>A: Send API request with form data
A->>M: Create new TaskTemplate
M->>S: Validate schedule (adjust days_of_week)
S-->>M: Return updated schedule
M-->>A: Return created TaskTemplate details
A-->>F: Respond with task template info
sequenceDiagram
participant U as User
participant L as Load Function (+page.server.ts)
participant R as Redirect
U->>L: Request /calendar
L->>L: Compute current year & month
L->>U: 302 Redirect to /calendar/{year}/{month}
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ab-smith
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
documentation/architecture/data-model.md (2)
1594-1598: Clarify "Finding" Terminology and Requirements.
The new "finding" (labeled as "constat") fields are listed here. To improve clarity, please consider explicitly stating whether "finding" and "constat" are synonyms and ensure that the field requirements (ref_id/name/description, severity, and status) align with those used in similar entities elsewhere in the document.
1700-1707: Clarify MVP Exclusions in the Task Scheduling Section.
The documentation states that whenenabledis set to False the schedule is suspended and that subtasks, exceptions, and overdue_behavior will not be included in the MVP. Please ensure that this intent is consistently reflected both in the textual descriptions (e.g., explicitly noting that the subtasks relationship in the diagram is planned for future iterations) and in the JSON schema (by possibly marking exceptions and overdue_behavior as deprecated or unused for MVP).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
documentation/architecture/data-model.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: test (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: startup-docker-compose-test
🔇 Additional comments (1)
documentation/architecture/data-model.md (1)
1617-1631: Task Entity Definitions Look Consistent.
The definitions forTASK_TEMPLATEandTASK_NODEclearly list the required fields (e.g., ref_id, name, description, task_date, schedule_definition, enabled for the template; due_date, status, and observation for the node). Please ensure that these definitions remain consistent with the backend models and API contracts.
| SCHEDULE_JSONSCHEMA = { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "title": "Schedule Definition", | ||
| "type": "object", | ||
| "properties": { | ||
| "interval": { | ||
| "type": "integer", | ||
| "minimum": 1, | ||
| "description": "Number of periods to wait before repeating (e.g., every 2 days, 3 weeks).", | ||
| }, | ||
| "frequency": { | ||
| "type": "string", | ||
| "enum": ["DAILY", "WEEKLY", "MONTHLY", "YEARLY"], | ||
| }, | ||
| "days_of_week": { | ||
| "type": "array", | ||
| "items": {"type": "integer", "minimum": 1, "maximum": 7}, | ||
| "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", | ||
| }, | ||
| "weeks_of_month": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "integer", | ||
| "minimum": -1, | ||
| "maximum": 4, | ||
| }, | ||
| "description": "Optional. for a given weekday, which one in the month (1 for first, -1 for last)", | ||
| }, | ||
| "months_of_year": { | ||
| "type": "array", | ||
| "items": {"type": "integer", "minimum": 1, "maximum": 12}, | ||
| "description": "Optional. Months of the year (1=January, 12=December)", | ||
| }, | ||
| "start_date": { | ||
| "type": ["string"], | ||
| "format": "date", | ||
| "description": "Date when recurrence begins.", | ||
| }, | ||
| "end_date": { | ||
| "type": ["string"], | ||
| "format": "date", | ||
| "description": "Date when recurrence ends.", | ||
| }, | ||
| "occurrences": { | ||
| "type": ["integer", "null"], | ||
| "minimum": 1, | ||
| "description": "Optional. Number of occurrences before recurrence stops.", | ||
| }, | ||
| "overdue_behavior": { | ||
| "type": "string", | ||
| "enum": ["DELAY_NEXT", "NO_IMPACT"], | ||
| "default": "NO_IMPACT", | ||
| "description": "Optional. Behavior when tasks become overdue.", | ||
| }, | ||
| "exceptions": { | ||
| "type": ["object", "null"], | ||
| "description": "Optional. JSON object for future exceptions handling.", | ||
| }, | ||
| }, | ||
| "required": ["interval", "frequency"], | ||
| "additionalProperties": False, | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Inconsistency in days_of_week Range.
Within the SCHEDULE_JSONSCHEMA, the property "days_of_week" is defined to accept integers between 1 and 7, yet its description indicates that days should be expressed as “0=Sunday, 6=Saturday.” To resolve this conflict, consider one of the following updates:
Option 1 (Adjust Range):
-"minimum": 1,
-"maximum": 7,
+"minimum": 0,
+"maximum": 6,Option 2 (Adjust Description):
Update the description so that it correctly reflects a 1–7 scale (e.g., "Optional. Days of the week (1=Monday, 7=Sunday)").
Additionally, note that the fields "exceptions" and "overdue_behavior" are defined in the schema even though the subsequent text specifies that these concepts will not be included in the MVP. It would be helpful to mention that these fields are reserved for future use.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SCHEDULE_JSONSCHEMA = { | |
| "$schema": "https://json-schema.org/draft/2020-12/schema", | |
| "title": "Schedule Definition", | |
| "type": "object", | |
| "properties": { | |
| "interval": { | |
| "type": "integer", | |
| "minimum": 1, | |
| "description": "Number of periods to wait before repeating (e.g., every 2 days, 3 weeks).", | |
| }, | |
| "frequency": { | |
| "type": "string", | |
| "enum": ["DAILY", "WEEKLY", "MONTHLY", "YEARLY"], | |
| }, | |
| "days_of_week": { | |
| "type": "array", | |
| "items": {"type": "integer", "minimum": 1, "maximum": 7}, | |
| "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", | |
| }, | |
| "weeks_of_month": { | |
| "type": "array", | |
| "items": { | |
| "type": "integer", | |
| "minimum": -1, | |
| "maximum": 4, | |
| }, | |
| "description": "Optional. for a given weekday, which one in the month (1 for first, -1 for last)", | |
| }, | |
| "months_of_year": { | |
| "type": "array", | |
| "items": {"type": "integer", "minimum": 1, "maximum": 12}, | |
| "description": "Optional. Months of the year (1=January, 12=December)", | |
| }, | |
| "start_date": { | |
| "type": ["string"], | |
| "format": "date", | |
| "description": "Date when recurrence begins.", | |
| }, | |
| "end_date": { | |
| "type": ["string"], | |
| "format": "date", | |
| "description": "Date when recurrence ends.", | |
| }, | |
| "occurrences": { | |
| "type": ["integer", "null"], | |
| "minimum": 1, | |
| "description": "Optional. Number of occurrences before recurrence stops.", | |
| }, | |
| "overdue_behavior": { | |
| "type": "string", | |
| "enum": ["DELAY_NEXT", "NO_IMPACT"], | |
| "default": "NO_IMPACT", | |
| "description": "Optional. Behavior when tasks become overdue.", | |
| }, | |
| "exceptions": { | |
| "type": ["object", "null"], | |
| "description": "Optional. JSON object for future exceptions handling.", | |
| }, | |
| }, | |
| "required": ["interval", "frequency"], | |
| "additionalProperties": False, | |
| } | |
| ``` | |
| SCHEDULE_JSONSCHEMA = { | |
| "$schema": "https://json-schema.org/draft/2020-12/schema", | |
| "title": "Schedule Definition", | |
| "type": "object", | |
| "properties": { | |
| "interval": { | |
| "type": "integer", | |
| "minimum": 1, | |
| "description": "Number of periods to wait before repeating (e.g., every 2 days, 3 weeks).", | |
| }, | |
| "frequency": { | |
| "type": "string", | |
| "enum": ["DAILY", "WEEKLY", "MONTHLY", "YEARLY"], | |
| }, | |
| "days_of_week": { | |
| "type": "array", | |
| "items": {"type": "integer", "minimum": 0, "maximum": 6}, | |
| "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", | |
| }, | |
| "weeks_of_month": { | |
| "type": "array", | |
| "items": { | |
| "type": "integer", | |
| "minimum": -1, | |
| "maximum": 4, | |
| }, | |
| "description": "Optional. for a given weekday, which one in the month (1 for first, -1 for last)", | |
| }, | |
| "months_of_year": { | |
| "type": "array", | |
| "items": {"type": "integer", "minimum": 1, "maximum": 12}, | |
| "description": "Optional. Months of the year (1=January, 12=December)", | |
| }, | |
| "start_date": { | |
| "type": ["string"], | |
| "format": "date", | |
| "description": "Date when recurrence begins.", | |
| }, | |
| "end_date": { | |
| "type": ["string"], | |
| "format": "date", | |
| "description": "Date when recurrence ends.", | |
| }, | |
| "occurrences": { | |
| "type": ["integer", "null"], | |
| "minimum": 1, | |
| "description": "Optional. Number of occurrences before recurrence stops.", | |
| }, | |
| "overdue_behavior": { | |
| "type": "string", | |
| "enum": ["DELAY_NEXT", "NO_IMPACT"], | |
| "default": "NO_IMPACT", | |
| "description": "Optional. Behavior when tasks become overdue.", | |
| }, | |
| "exceptions": { | |
| "type": ["object", "null"], | |
| "description": "Optional. JSON object for future exceptions handling.", | |
| }, | |
| }, | |
| "required": ["interval", "frequency"], | |
| "additionalProperties": False, | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
documentation/architecture/data-model.md (3)
1602-1633: New Tasks Section and Entity Definitions.
The new "## Tasks" section—including the mermaid diagram and the definitions for TASK_TEMPLATE and TASK_NODE—is comprehensive and well organized. However, note that the relationship on line 1610 (TASK_TEMPLATE }o--o| TASK_TEMPLATE : is_subtask_of) implies support for subtasks, which contradicts the MVP scope (subtasks are not included). Please reconcile this inconsistency by either removing this relationship or clearly annotating it as reserved for future use.
1703-1707: MVP Scope Clarification Regarding Non-Included Concepts.
The documentation clearly states that subtasks, exceptions, and overdue_behavior will not be included in the MVP. Please ensure that any diagram elements (such as the subtask relationship on line 1610) are either removed or explicitly marked as features reserved for future implementation to prevent confusion.
1636-1695: 🛠️ Refactor suggestionReview JSON Schema for schedule_definition.
The provided JSON schema is thorough. One potential issue: thedays_of_weekproperty enforces values between 1 and 7, yet its description indicates a 0-indexed week (0=Sunday, 6=Saturday). To resolve this discrepancy, you might either adjust the validation range or change the description. For example, if you intend to use 0‑based indexing, consider applying the following diff:"days_of_week": { - "items": {"type": "integer", "minimum": 1, "maximum": 7}, - "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", + "items": {"type": "integer", "minimum": 0, "maximum": 6}, + "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", }Otherwise, update the description to reflect a 1–7 scale.
🧹 Nitpick comments (3)
documentation/architecture/data-model.md (3)
1594-1601: Clarify and Validate Updated Finding Definition.
The revised "finding (constat)" fields (ref_id/name/description, severity, and status) are now clearly defined. Please double-check that these changes are consistent with the overall findings model elsewhere in the documentation.
1701-1701: Grammatical Improvement for Schedule Suspension Description.
The sentence on line 1701 could be revised for better grammatical consistency. For example, changing"When enabled is set to False, the schedule is suspended (for recurring task), and generated tasks are hidden (past and future)."
to
"When enabled is set to False, the schedule is suspended (for recurring tasks), and the generated tasks (both past and future) are hidden."
would improve clarity.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1701-~1701: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...he schedule is suspended (for recurring task), and generated tasks are hidden (past ...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
1709-1715: Task Node Generation Strategy.
The explanation for generating future task_nodes (with limits based on frequency) is practical. Please ensure that these frequency limits (e.g., 5 years for yearly, 24 months for monthly, etc.) are validated by appropriate unit tests when implemented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
documentation/architecture/data-model.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
documentation/architecture/data-model.md
[uncategorized] ~1701-~1701: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...he schedule is suspended (for recurring task), and generated tasks are hidden (past ...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: test (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: startup-docker-compose-test
- GitHub Check: build (3.12)
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
backend/core/utils.py (1)
663-693:⚠️ Potential issueReturn a consistent response type for
task_calendar.
When inputs are valid,task_calendarreturns a list. On invalid date format, it returns aResponseobject. This inconsistency can break consumers expecting a uniform return type. Consider consistently returning either a DRFResponseor raising an exception.-if not end_date_param: - start_date = ... - end_date_param = ... ... +# Option: Always return a DRF Response, even on success tasks = _generate_occurrences(template, start_date, end_date) future_tasks.extend(tasks) -return future_tasks +return Response(future_tasks, status=status.HTTP_200_OK)
🧹 Nitpick comments (9)
backend/core/urls.py (1)
85-85: Consider using explicit import instead of star import.The new route for
TaskTemplateViewSetlooks good, but it is imported fromfrom .views import *at line 1. Star imports can lead to namespace pollution and make dependencies less clear.-from .views import * +from .views import ( + FolderViewSet, PerimeterViewSet, RiskMatrixViewSet, + VulnerabilityViewSet, RiskAssessmentViewSet, ThreatViewSet, + RiskScenarioViewSet, AppliedControlViewSet, PolicyViewSet, + RiskAcceptanceViewSet, ReferenceControlViewSet, AssetViewSet, + UserViewSet, UserGroupViewSet, RoleViewSet, RoleAssignmentViewSet, + FrameworkViewSet, EvidenceViewSet, ComplianceAssessmentViewSet, + RequirementViewSet, RequirementAssessmentViewSet, FilteringLabelViewSet, + QualificationViewSet, SecurityExceptionViewSet, FindingsAssessmentViewSet, + FindingViewSet, IncidentViewSet, TimelineEntryViewSet, + TaskTemplateViewSet, get_csrf_token, get_build, UploadAttachmentView, + get_counters_view, get_metrics_view, get_agg_data, get_composer_data, + UserPreferencesView, QuickStartView, ComplianceAssessmentActionPlanList +)🧰 Tools
🪛 Ruff (0.8.2)
85-85:
TaskTemplateViewSetmay be undefined, or defined from star imports(F405)
backend/core/serializers.py (1)
1403-1414: The TaskTemplateReadSerializer implementation looks good but has implicit imports.The serializer implementation correctly extends
BaseModelSerializerand includes the necessary related fields. However, theTaskTemplatemodel is imported from a star import at line 9 (from core.models import *), which can make code dependencies less clear.Consider refactoring the imports to be more explicit:
-from core.models import * +from core.models import ( + Folder, Vulnerability, AppliedControl, RiskAcceptance, Perimeter, + RiskAssessment, Asset, ComplianceAssessment, RequirementNode, + RequirementAssessment, Framework, Evidence, RiskMatrix, + RequirementMappingSet, FilteringLabel, Qualification, SecurityException, + FindingsAssessment, Finding, Incident, TimelineEntry, TaskTemplate +)🧰 Tools
🪛 Ruff (0.8.2)
1412-1412:
TaskTemplatemay be undefined, or defined from star imports(F405)
frontend/messages/fr.json (2)
1419-1420: Clarify or differentiate the translations for “Add Task” and “Add Task Template”.
Both keys ("addTask"and"addTaskTemplate") use the same French text"Ajouter une tâche". This can confuse end users if these operations differ. Consider using a distinct label for the second key (e.g., “Ajouter un modèle de tâche”)."addTask": "Ajouter une tâche", -"addTaskTemplate": "Ajouter une tâche", +"addTaskTemplate": "Ajouter un modèle de tâche",
1452-1452: Check the mismatch between the key and its translation.
The key"isTemplate"is rendered as"Recurrence". If this key truly indicates a template state, the French translation should reflect “Modèle” or “Est un modèle.” If you intended to label recurrence, use the correct key and translation pair.-"isTemplate": "Recurrence" +"isTemplate": "Est un modèle"backend/core/utils.py (1)
538-541: Combine these conditions to streamline logic.
Minor suggestion from static analysis: you can merge these consecutiveif/elifchecks using an “or” condition, avoiding duplication.-if months_of_year and base_date.month > max(months_of_year): - target_year += interval -elif base_date.month == 12: # End of year case +if (months_of_year and base_date.month > max(months_of_year)) or base_date.month == 12: target_year += interval🧰 Tools
🪛 Ruff (0.8.2)
538-541: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
frontend/messages/en.json (2)
1417-1420: Ambiguity in Task-Related Keys
The newly added keys"tasks","tasksNodes","addTask", and"addTaskTemplate"all use very similar labels (e.g. both"tasks"and"tasksNodes"display as "Tasks", and both add actions produce "Add task"). This might confuse users as to the distinction between a regular task and a task template/node. Consider clarifying the labels—for example, using "Add Task Template" for"addTaskTemplate"if its intent is different from adding a normal task.
1429-1433: Generic Recurrence Components
The keys"interval","frequency","each","the", and"of"are very generic. While such words are often needed when dynamically constructing phrases (for example, “Each week” or “The day of”), ensure that their usage context in the UI avoids ambiguity or unintended translations.backend/core/models.py (2)
4198-4247: Consider using a more suitable field type fortask_date, and verify the broad usage of M2M references.
Currently,task_dateis stored as aCharField, which may allow invalid date inputs. Unless there is a specific need to store this data as raw text, adopting aDateFieldorDateTimeFieldcan help with validation, comparisons, and readability.- task_date = models.CharField(null=True, blank=True, verbose_name="Date") + task_date = models.DateField(null=True, blank=True, verbose_name="Date")Additionally, the numerous ManyToMany relationships (
assigned_to,assets,applied_controls,compliance_assessments, etc.) are fine but be mindful of performance or loading issues if they grow large. Confirm this design remains scalable for your requirements.
4264-4296: Optionally track activity dates or statuses for task progression.
TheTaskNodemodel includes adue_dateand astatuschoice but lacks fields for dates of completion, cancellation, or status transitions. If you intend to measure task performance or track user behavior across statuses, consider adding optional fields (e.g.,completed_date) or a workflow table for better data visibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
backend/core/migrations/0067_tasktemplate_tasknode.py(1 hunks)backend/core/models.py(1 hunks)backend/core/serializers.py(1 hunks)backend/core/startup.py(5 hunks)backend/core/tests/test_task.py(1 hunks)backend/core/urls.py(1 hunks)backend/core/utils.py(2 hunks)backend/core/views.py(6 hunks)enterprise/frontend/src/lib/components/SideBar/navData.ts(1 hunks)frontend/messages/en.json(1 hunks)frontend/messages/fr.json(1 hunks)frontend/src/lib/components/Forms/ModelForm.svelte(2 hunks)frontend/src/lib/components/Forms/ModelForm/TaskTemplateForm.svelte(1 hunks)frontend/src/lib/components/SideBar/navData.ts(1 hunks)frontend/src/lib/utils/crud.ts(1 hunks)frontend/src/lib/utils/schemas.ts(2 hunks)frontend/src/lib/utils/table.ts(1 hunks)frontend/src/lib/utils/types.ts(1 hunks)frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- frontend/src/lib/components/SideBar/navData.ts
- frontend/src/lib/utils/types.ts
- enterprise/frontend/src/lib/components/SideBar/navData.ts
- backend/core/tests/test_task.py
- frontend/src/lib/utils/crud.ts
- frontend/src/lib/utils/table.ts
- frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte
- frontend/src/lib/components/Forms/ModelForm.svelte
- backend/core/startup.py
🧰 Additional context used
🧬 Code Graph Analysis (5)
backend/core/serializers.py (2)
backend/core/serializer_fields.py (1)
FieldsRelatedField(24-86)backend/core/models.py (10)
risk_assessments(937-938)risk_assessments(2154-2155)Meta(124-125)Meta(179-180)Meta(207-208)Meta(212-214)Meta(811-813)Meta(877-879)Meta(1011-1013)TaskTemplate(4139-4261)
backend/core/utils.py (1)
backend/core/views.py (2)
calendar(5064-5076)tasks(2192-2209)
backend/core/views.py (2)
backend/core/utils.py (1)
task_calendar(663-693)backend/core/models.py (1)
TaskTemplate(4139-4261)
backend/core/models.py (2)
backend/core/base_models.py (1)
save(106-108)backend/iam/models.py (5)
FolderMixin(182-195)PublishInRootFolderMixin(198-213)User(340-546)save(206-213)save(395-397)
backend/core/urls.py (1)
backend/core/views.py (1)
TaskTemplateViewSet(5056-5076)
🪛 Ruff (0.8.2)
backend/core/serializers.py
1412-1412: TaskTemplate may be undefined, or defined from star imports
(F405)
1418-1418: TaskTemplate may be undefined, or defined from star imports
(F405)
backend/core/utils.py
538-541: Combine if branches using logical or operator
Combine if branches
(SIM114)
backend/core/views.py
2205-2205: TaskTemplate may be undefined, or defined from star imports
(F405)
5057-5057: TaskTemplate may be undefined, or defined from star imports
(F405)
5075-5075: TaskTemplate may be undefined, or defined from star imports
(F405)
backend/core/urls.py
85-85: TaskTemplateViewSet may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: build (22)
🔇 Additional comments (20)
backend/core/serializers.py (1)
1416-1419: The TaskTemplateWriteSerializer looks good but uses an implicit model import.The write serializer is correctly implemented using
BaseModelSerializerand includes all fields from theTaskTemplatemodel. Similar to the read serializer, it relies on star imports for the model.🧰 Tools
🪛 Ruff (0.8.2)
1418-1418:
TaskTemplatemay be undefined, or defined from star imports(F405)
frontend/src/lib/utils/schemas.ts (2)
641-677: Well-implemented TaskTemplateSchema with comprehensive fields.The schema implementation is thorough and covers all the necessary fields for task templates, including recurrence scheduling options with appropriate defaults and constraints. The date formatting in the default function is also correctly implemented.
723-724: TaskTemplateSchema correctly added to SCHEMA_MAP.The schema has been properly registered in the SCHEMA_MAP with the key 'task-templates', which matches the API endpoint.
frontend/src/lib/components/Forms/ModelForm/TaskTemplateForm.svelte (3)
14-22: Clean implementation of form component props and form field proxies.The component properly defines the expected props and sets up form field proxies for reactive state management.
24-49: Well-structured base form fields with appropriate labels and help text.The basic form fields (folder, task_date, is_recurrent) are well implemented with appropriate labels and help text for user guidance.
50-170: Comprehensive schedule configuration with conditional rendering based on frequency.The scheduling options are well-implemented with conditional rendering based on the selected frequency. The implementation correctly handles different recurrence patterns (daily, weekly, monthly, yearly) with appropriate input fields.
backend/core/migrations/0067_tasktemplate_tasknode.py (1)
277-284: Verify the cascade deletion for thegeneratorforeign key.
When you delete aTaskTemplate, all associatedTaskNodeobjects will also be removed (on_delete=CASCADE). If this is not your intended behavior, consider usingPROTECTorSET_NULLinstead.Do you want to keep existing behavior, or should we protect
TaskNodeobjects from accidental template deletions?frontend/messages/en.json (5)
1421-1424: Clear Date and Recurrence Descriptions
The keys"etaOrCompletionDate","etaOrCompletionDateHelpText", and"completionDateHelpText"clearly explain their purpose, which is crucial for tasks management with scheduled dates. The inclusion of these keys helps ensure users understand what each date field represents.
1425-1428: Straightforward Additional Task Labels
The keys"isRecurrentHelpText","date", and"assignedTo"are concise and directly support the task management feature. Their definitions are clear and should integrate well into the UI.
1434-1439: Consistent Time Interval Labels
The labels for time intervals such as"recurrent","day","week","weeks","month","months", and"year"are implemented consistently and will serve well in recurrence settings.
1440-1444: Clear Status and End Date Keys
The keys"endDate","pending","completed", and"cancelled"are straightforward and align with standard status terminology, making them easy for users to understand.
1444-1448: Accurate Ordinal Indicators
The ordinal keys"first","second","third","fourth", and"last"appear to be correctly implemented for use in scheduling contexts. They are clear and conform with expectations for expressing recurrence order.backend/core/views.py (5)
11-11: Appropriate import addition for relative date handling.The import of
relativedeltafromdateutilis necessary for the new task scheduling functionality, enabling more advanced date manipulations than standardtimedelta.
95-95: Good import of task_calendar utility function.Correctly importing the
task_calendarfunction fromcore.utilsthat will be used by the new task management features.
2189-2209: Well-implemented user tasks endpoint with proper date handling.The
tasksaction implementation correctly addresses the previous review feedback by moving default parameter evaluation inside the function body, avoiding the common Python pitfall of mutable default arguments. The method properly filters task templates for the specified user and date range.🧰 Tools
🪛 Ruff (0.8.2)
2205-2205:
TaskTemplatemay be undefined, or defined from star imports(F405)
5059-5076: Well-implemented calendar endpoint with proper date handling.The
calendaraction correctly retrieves tasks for a specified date range with proper default values. The implementation follows the same pattern as thetasksaction in UserViewSet, ensuring consistency across the codebase.🧰 Tools
🪛 Ruff (0.8.2)
5075-5075:
TaskTemplatemay be undefined, or defined from star imports(F405)
5056-5058:✅ Verification successful
New TaskTemplateViewSet for task management.
The TaskTemplateViewSet extends BaseModelViewSet to provide standard CRUD operations for TaskTemplate objects, which is consistent with the existing application architecture.
🏁 Script executed:
#!/bin/bash # Check if TaskTemplate is properly imported or defined grep -r "class TaskTemplate" --include="*.py" .Length of output: 395
Approval: Verified Implementation of TaskTemplateViewSet
The new TaskTemplateViewSet correctly extends BaseModelViewSet using the TaskTemplate model (as defined in
backend/core/models.py). This implementation provides the expected standard CRUD operations for TaskTemplate objects and aligns with our application architecture. Verification scripts confirm that TaskTemplate is properly defined alongside its related serializers. Approved.🧰 Tools
🪛 Ruff (0.8.2)
5057-5057:
TaskTemplatemay be undefined, or defined from star imports(F405)
backend/core/models.py (3)
4138-4139: No concerns at the moment.
4249-4251: No concerns at the moment.
4252-4258: Acknowledged resolution of repeated schedule modifications.
This code ensures thatdays_of_weekvalues outside the standard range are normalized once. This addresses the repeated-modification concerns from earlier reviews.
| "days_of_week": { | ||
| "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", | ||
| "items": { | ||
| "maximum": 7, | ||
| "minimum": 1, | ||
| "type": "integer", | ||
| }, | ||
| "type": "array", | ||
| }, |
There was a problem hiding this comment.
Align days_of_week range with the documented 0=Sunday reference.
The JSON schema states 0=Sunday through 6=Saturday, but the schema’s minimum is 1 and maximum is 7. This mismatch could block Sunday (0) or allow an invalid 7. Adjust either the code or the schema to ensure consistency.
"days_of_week": {
"description": "Optional. Days of the week (0=Sunday, 6=Saturday)",
"items": {
- "minimum": 1,
- "maximum": 7,
+ "minimum": 0,
+ "maximum": 6,
"type": "integer"
},
"type": "array"
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "days_of_week": { | |
| "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", | |
| "items": { | |
| "maximum": 7, | |
| "minimum": 1, | |
| "type": "integer", | |
| }, | |
| "type": "array", | |
| }, | |
| "days_of_week": { | |
| "description": "Optional. Days of the week (0=Sunday, 6=Saturday)", | |
| "items": { | |
| "minimum": 0, | |
| "maximum": 6, | |
| "type": "integer", | |
| }, | |
| "type": "array", | |
| }, |
frontend/messages/en.json
Outdated
| "taskDateHelpText": "The task date or the start of its recurrence", | ||
| "schedule": "Schedule", | ||
| "taskDate": "Date", | ||
| "isTemplate": "Recurrent" |
There was a problem hiding this comment.
Potential Key Mismatch in "isTemplate"
The key "isTemplate" is assigned the value "Recurrent", which is confusing given that "isRecurrent" already exists and is used to indicate recurrence. This suggests a possible mix-up: if "isTemplate" is meant to flag task templates, its value should likely be a term like "Template" or "Task Template" rather than "Recurrent". Please verify the intended behavior and update accordingly to avoid UI confusion.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
backend/core/utils.py (1)
660-691: Inconsistent return types (list vs. DRFResponse).As previously noted in past reviews, returning a raw list vs. a
Responseobject in error cases can lead to confusion and breaks uniform expectations. Consider returning aResponsein all cases or raising an exception.backend/core/migrations/0067_tasktemplate_tasknode.py (1)
18-207:TaskTemplatemodel creation is generally sound; schedule schema range mismatch remains.The model fields and relationships look valid. However, lines 76-79 define
days_of_weekwith"minimum": 1and"maximum": 7, conflicting with the documented 0=Sunday, 6=Saturday range.backend/core/models.py (2)
4260-4262: Overridingcreate()in a model is ineffective.This pattern does not actually override a base manager method in Django’s
models.Model, so this method won't be called when usingModelName.objects.create(). Please remove or refactor to a custom manager, if needed.
4155-4158:⚠️ Potential issueResolve the day-of-week schema inconsistency.
The JSON schema enforces values
1..7, but the description references0..6. This discrepancy can lead to confusion or invalid data.Consider aligning both the schema and the documentation to use days 0..6, for example:
- "items": {"type": "integer", "minimum": 1, "maximum": 7}, + "items": {"type": "integer", "minimum": 0, "maximum": 6}, - "description": "Optional. Days of the week (0=Sunday, 6=Saturday)" + "description": "Optional. Days of the week (0=Sunday, 6=Saturday)"
🧹 Nitpick comments (2)
backend/core/utils.py (2)
322-417: Comprehensive schedule matching logic; watch performance for large date checks.The code covers all frequencies and optional constraints. Just keep an eye on potential performance impacts if called in a tight loop for large date ranges.
538-541: Combine branches as suggested by static analysis (SIM114).These two branches can be merged for simplicity:
-if months_of_year and base_date.month > max(months_of_year): - target_year += interval -elif base_date.month == 12: - target_year += interval +if (months_of_year and base_date.month > max(months_of_year)) or (base_date.month == 12): + target_year += interval🧰 Tools
🪛 Ruff (0.8.2)
538-541: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/core/migrations/0067_tasktemplate_tasknode.py(1 hunks)backend/core/models.py(1 hunks)backend/core/utils.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/core/models.py (2)
backend/core/base_models.py (2)
NameDescriptionMixin(111-124)save(106-108)backend/iam/models.py (4)
FolderMixin(182-195)PublishInRootFolderMixin(198-213)save(206-213)save(395-397)
🪛 Ruff (0.8.2)
backend/core/utils.py
538-541: Combine if branches using logical or operator
Combine if branches
(SIM114)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
backend/core/utils.py (8)
5-15: Imports look good.All the newly introduced imports (calendar, relativedelta, Response, status) are relevant to the scheduling and DRF response handling. No issues here.
284-285: No meaningful change.These are blank lines; nothing to address.
286-288: Conversion logic verifies Sunday → 6, Monday → 0 mapping.
(day - 1) % 7is correct for shifting 0=Sunday to Python's 0=Monday scheme.
291-295: Helper function_get_month_rangeis straightforward and effective.It correctly returns the first and last day of the specified month with no apparent pitfalls.
298-320: Ensure negative indexing for nth weekday is well-tested.The logic for negative indexing (e.g., -1 for the last occurrence) looks correct. However, consider adding or verifying test coverage to avoid off-by-one errors.
419-591: Complex next-occurrence calculation is generally solid; consider edge-case validation.All frequency branches are well-handled. There's a small risk of infinite loops if a malformed schedule yields no date progression. Safeguard by ensuring each frequency branch increments the date or returns None.
Would you like a script to grep for all calls to ensure inputs are validated?
🧰 Tools
🪛 Ruff (0.8.2)
538-541: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
593-607: Dictionary creation for future tasks is concise and clear.No issues noted. The structure provides all necessary task fields.
609-657: Occurrence generation handles cutoffs and intervals well.The loop properly terminates on recurrence end or max occurrences. Keep an eye on
_calculate_next_occurrencereturning None for malformed schedules.backend/core/migrations/0067_tasktemplate_tasknode.py (4)
1-2: Migration auto-generation details.No concerns with the Django auto-generation timestamp.
3-9: Imports and references.Everything needed for the migration is included and properly imported.
11-16: Migration dependencies look correct.They reference existing migrations in both
coreandiam, plus the user model dependency.
208-292:TaskNodemodel creation is correct.Primary key, foreign keys, ManyToMany relationships, and verbose names are well-structured.
backend/core/models.py (2)
4138-4296: Add tests for task management logic.The newly added
TaskTemplateandTaskNodeclasses don't appear to have dedicated tests. Ensuring coverage is crucial to validate scheduling logic, recurrences, day-of-week transformations, etc.Would you like me to open a new issue or generate test stubs for these classes?
4253-4258:⚠️ Potential issueAvoid producing out-of-schema values for
days_of_week.Currently,
day % 7generates a 0 for day=7, which violates the stated1..7range or conflicts with 0..6 usage. Unify the logic to remain consistent with whichever interval you choose. For example, if sticking to 1..7, clamp values to [1,7]:- self.schedule["days_of_week"] = [ - day % 7 if day > 6 else day for day in self.schedule["days_of_week"] - ] + self.schedule["days_of_week"] = [ + 7 if day > 7 else (1 if day < 1 else day) + for day in self.schedule["days_of_week"] + ]Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (15)
frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte (2)
1-14: All imports and type definitions are properly set up.The imports for form components, types, and messages are consistent with the rest of the codebase. Consider running Prettier to address styling issues flagged by the pipeline.
🧰 Tools
🪛 GitHub Actions: Frontend Linters
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
40-48: Evidence selection is well-structured.Allowing multiple evidence entries and the choice to fetch from
optionsEndpoint="evidences"matches project conventions. Remember to run Prettier to address any minor style discrepancies.frontend/src/lib/utils/table.ts (1)
925-931: List view for task nodes is correctly set up.Filtering by
statusadds consistency with other list views. Be sure to address the Prettier warning from the pipeline for overall code style.backend/core/serializers.py (2)
1411-1414: Confirm the exclusion ofschedule.
Excluding the schedule field in the read serializer avoids potentially sensitive data but might limit front-end debugging. If the front-end needs it, consider either returning a portion or gating access by permission.🧰 Tools
🪛 Ruff (0.8.2)
1412-1412:
TaskTemplatemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
1429-1432: Evaluate excludingtask_templatefrom writes.
Preventing direct assignment totask_templatemight disallow re-linking tasks after creation. If re-linking is needed, consider adding a separate endpoint or approach.🧰 Tools
🪛 Ruff (0.8.2)
1431-1431:
TaskNodemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
backend/core/utils.py (2)
284-287: Clarify weekday offset.
Switching from 0=Sunday to 0=Monday is subtle. Document or test thoroughly to prevent indexing errors when combined with other date functions.
320-415: Comprehensive scheduling checks.
This one function covers many frequency variants. Consider splitting monthly/yearly logic to smaller helpers for maintainability and extended test coverage.backend/core/views.py (5)
94-97: Consolidate or clarifyrelativedeltaimports.
The file importsrelativedeltatwice (lines 11 and 97) under different names (relativedeltaandrd). This could cause confusion or accidental misuse. Consider standardizing on a single import style.- from dateutil import relativedelta - from dateutil import relativedelta as rd + from dateutil import relativedelta as rd🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
2190-2212: Avoid passingself=selfto another ViewSet’s instance method.
Here, you callTaskTemplateViewSet.task_calendarand manually passself=self. This can be unconventional since the method expects its ownselfinstance. To avoid confusion or potential side effects, consider extracting the scheduling logic into a standalone utility function or instantiatingTaskTemplateViewSetproperly.- return Response( - TaskTemplateViewSet.task_calendar( - self=self, - task_templates=TaskTemplate.objects.filter(enabled=True, assigned_to=request.user), - start=start, - end=end, - ) - ) + # Optionally refactor into a common utility function or properly instantiate the viewset🧰 Tools
🪛 Ruff (0.8.2)
2207-2207:
TaskTemplatemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
5058-5060: Define explicit imports for TaskTemplate.
Relying on star imports from models forTaskTemplatecan lead to potential confusion. Consider making the import explicit for clarity and to address the static analysis warnings (F405).- from .models import * + from .models import TaskTemplate, TaskNode🧰 Tools
🪛 Ruff (0.8.2)
5059-5059:
TaskTemplatemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
5166-5184: Refactor or unify duplicate scheduling logic.
Thecalendaraction reuses logic similar totask_calendar. You could unify them into a single utility method for DRY compliance and easier maintenance.🧰 Tools
🪛 Ruff (0.8.2)
5182-5182:
TaskTemplatemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
5186-5192: Explicit import forTaskNode.
LikeTaskTemplate, referencingTaskNodefrom star imports can raise F405 warnings. For clarity and correctness, add a direct import.🧰 Tools
🪛 Ruff (0.8.2)
5187-5187:
TaskNodemay be undefined, or defined from star imports(F405)
5192-5192:
TaskNodemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
backend/core/models.py (3)
4138-4143: Consider removing or adding logic to your custom manager.
YourTaskTemplateManagercurrently adds no new functionality beyond Django’s default manager. If you don’t plan to add logic later, you might remove or inline this manager to retain simplicity. Conversely, if you anticipate custom manager logic (e.g., specialized creation flows), consider clarifying it here.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
4148-4204: Validate or utilizeoverdue_behaviorwithin your codebase.
The JSON schema provides anoverdue_behaviorproperty, but there’s no corresponding logic in this file to act on that field. Consider implementing or documenting howoverdue_behavioris intended to affect task scheduling, or remove the property if unneeded.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
4138-4397: Address Ruff formatting suggestions.
Your pipeline reported a Ruff formatting failure. You can typically fix this by running Ruff in “fix” mode (e.g.,ruff check --fix .), ensuring the code style remains consistent and passes lint checks.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/core/migrations/0067_tasktemplate_tasknode.py(1 hunks)backend/core/models.py(1 hunks)backend/core/serializers.py(1 hunks)backend/core/urls.py(1 hunks)backend/core/utils.py(2 hunks)backend/core/views.py(6 hunks)frontend/src/lib/components/Forms/ModelForm.svelte(2 hunks)frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte(1 hunks)frontend/src/lib/utils/crud.ts(1 hunks)frontend/src/lib/utils/schemas.ts(2 hunks)frontend/src/lib/utils/table.ts(2 hunks)frontend/src/lib/utils/types.ts(1 hunks)frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/lib/utils/types.ts
- frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte
- frontend/src/lib/utils/crud.ts
🧰 Additional context used
🧬 Code Graph Analysis (4)
frontend/src/lib/utils/schemas.ts (1)
backend/core/base_models.py (1)
NameDescriptionMixin(111-124)
backend/core/views.py (2)
backend/core/utils.py (2)
_generate_occurrences(608-656)_create_task_dict(591-605)backend/core/models.py (5)
TaskTemplate(4145-4273)TaskNode(4276-4307)delete(777-790)delete(1855-1858)create(2230-2232)
backend/core/models.py (3)
backend/core/base_models.py (2)
NameDescriptionMixin(111-124)save(106-108)backend/iam/models.py (5)
FolderMixin(182-195)PublishInRootFolderMixin(198-213)save(206-213)save(395-397)delete(391-393)backend/core/tests/fixtures.py (1)
applied_controls(58-69)
backend/core/urls.py (1)
backend/core/views.py (2)
TaskTemplateViewSet(5058-5183)TaskNodeViewSet(5186-5192)
🪛 GitHub Actions: Frontend Linters
frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
frontend/src/lib/utils/table.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
frontend/src/lib/utils/schemas.ts
[warning] 1-1: Code style issues found. Run Prettier with --write to fix.
🪛 Ruff (0.8.2)
backend/core/serializers.py
1412-1412: TaskTemplate may be undefined, or defined from star imports
(F405)
1418-1418: TaskTemplate may be undefined, or defined from star imports
(F405)
1425-1425: TaskNode may be undefined, or defined from star imports
(F405)
1431-1431: TaskNode may be undefined, or defined from star imports
(F405)
backend/core/utils.py
536-539: Combine if branches using logical or operator
Combine if branches
(SIM114)
backend/core/views.py
2207-2207: TaskTemplate may be undefined, or defined from star imports
(F405)
5059-5059: TaskTemplate may be undefined, or defined from star imports
(F405)
5122-5122: TaskNode may be undefined, or defined from star imports
(F405)
5123-5123: TaskTemplate may be undefined, or defined from star imports
(F405)
5133-5133: TaskNodeReadSerializer may be undefined, or defined from star imports
(F405)
5141-5141: TaskNode may be undefined, or defined from star imports
(F405)
5156-5156: TaskTemplate may be undefined, or defined from star imports
(F405)
5158-5158: TaskNode may be undefined, or defined from star imports
(F405)
5182-5182: TaskTemplate may be undefined, or defined from star imports
(F405)
5187-5187: TaskNode may be undefined, or defined from star imports
(F405)
5192-5192: TaskNode may be undefined, or defined from star imports
(F405)
backend/core/urls.py
85-85: TaskTemplateViewSet may be undefined, or defined from star imports
(F405)
86-86: TaskNodeViewSet may be undefined, or defined from star imports
(F405)
🪛 GitHub Actions: Backend Linters
backend/core/serializers.py
[warning] Ruff formatting check failed. 1 file would be reformatted.
backend/core/views.py
[warning] Ruff formatting check failed. 1 file would be reformatted.
backend/core/models.py
[warning] Ruff formatting check failed. 1 file would be reformatted.
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (28)
frontend/src/lib/components/Forms/ModelForm.svelte (2)
46-47: No issues found with the new imports.The imports of
TaskTemplateFormandTaskNodeFormappear correct. Good job on keeping the naming consistent.
376-379: Conditional rendering looks correct.The logic to render
<TaskTemplateForm>or<TaskNodeForm>based onURLModelis straightforward and aligns with similar existing code branches.frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte (3)
16-24: Use of<TextField type="date">is appropriate.This meets typical date entry needs. Make sure to confirm client-side date format requirements and potential local/timezone concerns.
33-39: Observation field aligns with standard usage.The
<TextArea>binding looks correct, and the caching approach is consistent with other fields.
25-32:❓ Verification inconclusive
Status dropdown logic appears sound.
Using
model.selectOptions['status']is a clear approach. Verify the content ofselectOptions['status']is always populated to avoid runtime errors.
🏁 Script executed:
#!/bin/bash # Description: Check if 'status' is defined in 'model.selectOptions' across all references. rg -A 3 'model\.selectOptions\[[\"\']status[\"\']\]'Length of output: 116
Below is a revised review comment that reflects the need to re-run verification using an updated shell command due to the earlier quoting error:
Status dropdown logic appears sound. However, please verify that
model.selectOptions['status']is always populated to avoid runtime errors. It looks like the original verification command encountered a quoting issue. To re-run the verification properly, please try the following command:#!/bin/bash # Re-run the verification command with corrected quoting to search for all occurrences of the 'status' key. rg -A 3 "model\.selectOptions\[['\"]status['\"]\]"Once you've confirmed that the key is reliably present, you can consider this review addressed.
frontend/src/lib/utils/table.ts (2)
921-924: Proper configuration for 'task-templates' list view.The head/body fields look aligned with your model fields (
name,description,status,owner).
128-138:✅ Verification successful
New filter definition is consistent.
TASK_STATUS_FILTERfollows the established filter pattern withAutocompleteSelect. Just confirm that the endpoint'task-nodes/status'is valid.
🏁 Script executed:
#!/bin/bash # Description: Check for the existence of 'task-nodes/status' route or usage. rg -A 3 'task-nodes/status'Length of output: 274
Filter configuration verified – no changes required.
- The filter object
TASK_STATUS_FILTERadheres to the established pattern usingAutocompleteSelect.- The search confirms that the endpoint string
'task-nodes/status'appears correctly in the code.- Ensure that the backend supports this endpoint at runtime, though from the static review it is valid.
frontend/src/lib/utils/schemas.ts (3)
641-677: Consider verifying day-of-week alignment between the front-end and the back-end.
The schedule'sdays_of_weekproperty is currently allowed in the range 1–7, whereas the server-side logic uses a 0–6 (Sunday-based) index. This mismatch can cause confusion or dropped tasks.
679-685: Schema definition appears consistent.
TheTaskNodeSchemafields properly align with likely usage. Ensure test coverage for boundary cases (e.g., missingdue_date) to validate the schema's behavior under real inputs.
731-733: SCHEMA_MAP references look correct.
Registering'task-templates'and'task-nodes'fosters consistency. Verify that downstream code handling'timeline-entries'also properly handles these new keys.backend/core/serializers.py (3)
1403-1410: Ensure read serializer coverage in tests.
TaskTemplateReadSerializerfields reference multiple foreign and many-to-many relationships. Verify all these relationships and their FieldsRelatedField usage appear correctly in the returned JSON.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
1416-1420: Write serializer includes all fields.
Allowing allTaskTemplatefields to be written provides flexibility but can risk unintended updates (e.g., changingcreated_at). If needed, restrict them via read-only fields or dedicated endpoints.🧰 Tools
🪛 Ruff (0.8.2)
1418-1418:
TaskTemplatemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
1422-1427: Check TaskNode to TaskTemplate references.
Exposingtask_templatefor reading ensures clarity of the node’s origin. Double-check permissioning to confirm that sensitive info in the template is not inadvertently leaked to unauthorized users.🧰 Tools
🪛 Ruff (0.8.2)
1425-1425:
TaskNodemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
backend/core/utils.py (7)
5-5: Check module usage.
Introducingdatetime, timedelta, dateis in line with the scheduling logic. No apparent conflicts with existing imports.
12-13: Libraries for date manipulation.
Usingcalendaranddateutil.relativedeltais appropriate for handling complex scheduling intervals. Ensure a consistent approach across all date utilities.
282-283: Time-state enhancement.
Returning “today” with a distinct color promotes clearer UI feedback. Validate that time zones are consistently applied when comparingetatonow.
289-294: Month range utility.
Simple and effective. Remember to test for leap years (e.g., February 29).
296-318: Support for nth weekday logic.
Handling negative (last) occurrences is a nice addition. Confirm it correctly handles edge cases, such as a 5th occurrence that doesn't exist in shorter months.
591-606: Creating a task dictionary.
Defines a minimal representation for a “virtual” upcoming task. Confirm the front-end expectsvirtualandstatusfields, and that they're documented.
608-657: Scheduled occurrences generation.
Logical approach to iterate through occurrences. Ensure large date ranges don’t introduce performance issues and thatmax_occurrencesis correctly enforced.backend/core/migrations/0067_tasktemplate_tasknode.py (2)
19-207: Validate schedule JSON schema alignment.
TaskTemplateincludes a JSONField and validator for scheduling. Make sure the front-end’s day-of-week constraints align with the schema’s min/max. Also test for invalid frequencies or intervals.
208-290: Linking tasks to templates.
TaskNodereferencesTaskTemplatewith a ForeignKey. Confirm that deletion policies (cascade vs. restrict) match your intended behavior—especially if ongoing tasks exist.backend/core/views.py (4)
11-12: Imports appear fine.
These imports fromdateutiland Django’s ORM functions look correct for usage throughout the file. No issues spotted.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
5137-5165: Preserve existing TaskNode data on updates.
perform_updatedeletes all existingTaskNodeobjects for the updated template, which can wipe user progress on tasks in progress. This might be unintended. Consider confirming requirements or implementing partial regeneration logic.🧰 Tools
🪛 Ruff (0.8.2)
5141-5141:
TaskNodemay be undefined, or defined from star imports(F405)
5156-5156:
TaskTemplatemay be undefined, or defined from star imports(F405)
5158-5158:
TaskNodemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
1979-1985:✅ Verification successful
Confirm usage of
monthandyearlookups forexpiry_date.
Similar to theetafield, ensure thatmonthandyearlookups are defined or properly supported bydjango-filters. Otherwise, queries could fail.
🏁 Script executed:
#!/bin/bash # Search for references to 'month'/'year' filters in risk acceptance code rg -A 5 'RiskAcceptanceFilterSet'Length of output: 733
Expiry_Date Lookups Verified
The RiskAcceptanceFilterSet in
backend/core/views.pyconfiguresexpiry_datewith lookups includingmonthandyear, consistent with the handling of theetafield. As long as the underlyingexpiry_datemodel field is a DateField or DateTimeField (which natively supports these lookups via Django’s ORM), these lookups will function correctly. No additional custom logic appears to be necessary.🧰 Tools
🪛 Ruff (0.8.2)
1979-1979:
RiskAcceptancemay be undefined, or defined from star imports(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
1312-1312:❓ Verification inconclusive
Verify custom lookups for
monthandyear.
The filter includes"month"and"year"lookups for theetafield. Ensure that they're implemented or supported indjango-filters. Otherwise, it may cause runtime errors or unexpected behavior.
🏁 Script executed:
#!/bin/bash # Search for FilterSet classes that define custom lookups for 'month'/'year' to confirm usage rg -A 5 'def filter_(month|year)'Length of output: 35
I'll run an additional verification by searching for explicit custom filter implementations for
"month"and"year"(i.e. looking fordef filter_monthanddef filter_year) to confirm whether they are provided in the codebase.
🏁 Script executed:
#!/bin/bash # Searching for custom filter methods for 'month' and 'year' in the entire repository rg -n 'def filter_month' . rg -n 'def filter_year' .Length of output: 56
VERIFY BUILT-IN LOOKUP SUPPORT FOR
monthANDyearONeta.
Automated searches did not reveal any custom methods (e.g.filter_monthorfilter_year) in the repository. This indicates that the code likely relies on Django’s native support for date-based lookups. Please verify that theetafield is defined as a DateField or DateTimeField so that themonthandyearlookups are handled automatically by Django’s ORM. If the field isn’t of a date type, or if Django’s built-in lookups do not meet your needs, you’ll need to implement custom filters to avoid potential runtime errors.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
backend/core/models.py (2)
4260-4266: Verify day-of-week transformation logic.
By defaulting Sunday (7) to 0 usingday % 7 if day > 6 else day, you’re mapping 7→0, which conflicts with your declared range of [1..7] for Monday..Sunday. If you intentionally want a zero-based index for Sunday, consider clarifying or rename the field to a 0–6 range. Otherwise, ensure Sunday remains 7 to avoid inconsistency.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
4276-4308: Consider validations and relationships for your TaskNode model.
task_templateis nullable: If tasks must always stem from aTaskTemplate, you may want to mark it as non-nullable.due_date: No validations ensure it isn’t in the past or that it respects theTaskTemplate.schedule. Consider aclean()method or validations if that is a requirement.- Assigned user tracking: Since
TaskTemplatehas anassigned_torelation, determine if eachTaskNodemight also need its own owners or if it inherits from the template.
These decisions can strengthen data integrity and clarify your task model relationships.🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
| router.register(r"task-templates", TaskTemplateViewSet, basename="task-templates") | ||
| router.register(r"task-nodes", TaskNodeViewSet, basename="task-nodes") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Explicitly import TaskTemplateViewSet and TaskNodeViewSet.
According to the static analysis hint (F405), these references may not be defined if they're only brought in via a from .views import * statement. To avoid potential collisions and improve clarity, consider importing these classes explicitly.
Example of how you might improve clarity:
- from .views import *
+ from .views import (
+ FolderViewSet,
+ PerimeterViewSet,
+ TaskTemplateViewSet,
+ TaskNodeViewSet,
+ # ... add other needed imports
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| router.register(r"task-templates", TaskTemplateViewSet, basename="task-templates") | |
| router.register(r"task-nodes", TaskNodeViewSet, basename="task-nodes") | |
| - from .views import * | |
| + from .views import ( | |
| + FolderViewSet, | |
| + PerimeterViewSet, | |
| + TaskTemplateViewSet, | |
| + TaskNodeViewSet, | |
| + # ... add other needed imports | |
| + ) | |
| router.register(r"task-templates", TaskTemplateViewSet, basename="task-templates") | |
| router.register(r"task-nodes", TaskNodeViewSet, basename="task-nodes") |
🧰 Tools
🪛 Ruff (0.8.2)
85-85: TaskTemplateViewSet may be undefined, or defined from star imports
(F405)
86-86: TaskNodeViewSet may be undefined, or defined from star imports
(F405)
|
|
||
| if week < 0: # Last occurrence of weekday | ||
| last_day = calendar.monthrange(target_year, target_month)[1] | ||
| last_date = date(target_year, target_month, last_day) | ||
|
|
||
| # Find last occurrence of this weekday | ||
| days_diff = (last_date.weekday() - python_weekday) % 7 | ||
| if days_diff > 0: | ||
| target_date = last_date - timedelta(days=days_diff) | ||
| else: | ||
| target_date = last_date - timedelta(days=7 - days_diff) | ||
|
|
||
| if target_date.month == target_month: | ||
| possible_dates.append(target_date) | ||
| else: | ||
| target_date = _get_nth_weekday_of_month( | ||
| target_year, target_month, python_weekday, week | ||
| ) | ||
| if target_date: | ||
| possible_dates.append(target_date) | ||
| else: | ||
| # Use same day in next month(s) | ||
| day = min(base_date.day, calendar.monthrange(target_year, target_month)[1]) | ||
| possible_dates.append(date(target_year, target_month, day)) | ||
|
|
||
| # Return earliest date after base_date | ||
| valid_dates = [d for d in possible_dates if d > base_date] | ||
| if valid_dates: | ||
| return min(valid_dates) | ||
|
|
||
| # Try next interval if no valid dates found | ||
| return _calculate_next_occurrence( | ||
| task, base_date + rd.relativedelta(months=interval) | ||
| ) | ||
|
|
||
| elif frequency == "YEARLY": | ||
| months_of_year = schedule.get("months_of_year", []) | ||
| days_of_week = schedule.get("days_of_week", []) | ||
| weeks_of_month = schedule.get("weeks_of_month", []) | ||
|
|
||
| if not months_of_year and not days_of_week and not weeks_of_month: | ||
| return base_date + rd.relativedelta(years=interval) | ||
|
|
||
| target_year = base_date.year | ||
|
|
||
| # If we're past all months in current year, move to next year | ||
| if months_of_year and base_date.month > max(months_of_year): | ||
| target_year += interval | ||
| elif base_date.month == 12: # End of year case | ||
| target_year += interval | ||
|
|
||
| sorted_months = sorted(months_of_year) if months_of_year else [base_date.month] | ||
| possible_dates = [] | ||
|
|
||
| for month in sorted_months: | ||
| if not days_of_week and not weeks_of_month: | ||
| # Same day each year | ||
| last_day_of_month = calendar.monthrange(target_year, month)[1] | ||
| day = min(base_date.day, last_day_of_month) | ||
| possible_dates.append(date(target_year, month, day)) | ||
| elif weeks_of_month and days_of_week: | ||
| # Specific week/day combinations | ||
| for week in sorted(weeks_of_month): | ||
| for day in sorted(days_of_week): | ||
| python_weekday = _convert_to_python_weekday(day) | ||
|
|
||
| if week < 0: # From end of month | ||
| target_date = _get_nth_weekday_of_month( | ||
| target_year, month, python_weekday, week | ||
| ) | ||
| if target_date: | ||
| possible_dates.append(target_date) | ||
| else: | ||
| target_date = _get_nth_weekday_of_month( | ||
| target_year, month, python_weekday, week | ||
| ) | ||
| if target_date: | ||
| possible_dates.append(target_date) | ||
| elif days_of_week: | ||
| # All occurrences of specified weekdays in month | ||
| for day in range(1, calendar.monthrange(target_year, month)[1] + 1): | ||
| check_date = date(target_year, month, day) | ||
| adjusted_weekday = (check_date.weekday() + 1) % 7 | ||
| if adjusted_weekday in days_of_week: | ||
| possible_dates.append(check_date) | ||
| else: | ||
| # Just specified weeks of month | ||
| day = min(base_date.day, calendar.monthrange(target_year, month)[1]) | ||
| possible_dates.append(date(target_year, month, day)) | ||
|
|
||
| # Return earliest date after base_date | ||
| valid_dates = [d for d in possible_dates if d > base_date] | ||
| if valid_dates: | ||
| return min(valid_dates) | ||
|
|
||
| # If no valid dates, try next interval | ||
| return date(target_year + interval, sorted_months[0], 1) | ||
|
|
||
| return None | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complex next occurrence calculation.
The method thoroughly manages daily, weekly, monthly, and yearly intervals, but is quite lengthy. Extracting each frequency case into dedicated helpers could simplify reading and testing.
🧰 Tools
🪛 Ruff (0.8.2)
536-539: Combine if branches using logical or operator
Combine if branches
(SIM114)
backend/core/views.py
Outdated
| def task_calendar(self, task_templates, start=None, end=None): | ||
| """Generate calendar of tasks for the given templates.""" | ||
| tasks_list = [] | ||
|
|
||
| for template in task_templates: | ||
| if not template.is_recurrent: | ||
| tasks_list.append(_create_task_dict(template, template.task_date)) | ||
| continue | ||
|
|
||
| start_date_param = start or template.task_date or datetime.now().date() | ||
| end_date_param = end or template.schedule.get("end_date") | ||
|
|
||
| if not end_date_param: | ||
| start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() | ||
| end_date_param = (start_date + rd.relativedelta(months=1)).strftime( | ||
| "%Y-%m-%d" | ||
| ) | ||
|
|
||
| try: | ||
| start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() | ||
| end_date = datetime.strptime(str(end_date_param), "%Y-%m-%d").date() | ||
| except ValueError: | ||
| return {"error": "Invalid date format. Use YYYY-MM-DD"} | ||
|
|
||
| tasks = _generate_occurrences(template, start_date, end_date) | ||
| tasks_list.extend(tasks) | ||
|
|
||
| processed_tasks_identifiers = set() # Track tasks we've already processed | ||
|
|
||
| # Sort tasks by due date | ||
| sorted_tasks = sorted(tasks_list, key=lambda x: x["due_date"]) | ||
|
|
||
| # Process all past tasks and next 10 upcoming tasks | ||
| current_date = datetime.now().date() | ||
|
|
||
| # First separate past and future tasks | ||
| past_tasks = [task for task in sorted_tasks if task["due_date"] <= current_date] | ||
| next_tasks = [task for task in sorted_tasks if task["due_date"] > current_date] | ||
|
|
||
|
|
||
| # Combined list of tasks to process (past and next 10) | ||
| tasks_to_process = past_tasks + next_tasks | ||
|
|
||
| # Directly modify tasks in the original tasks_list | ||
| for i in range(len(tasks_list)): | ||
| task = tasks_list[i] | ||
| task_date = task["due_date"] | ||
| task_template_id = task["task_template"] | ||
|
|
||
| # Create a unique identifier for this task to avoid duplication | ||
| task_identifier = (task_template_id, task_date) | ||
|
|
||
| # Skip if we've already processed this task | ||
| if task_identifier in processed_tasks_identifiers: | ||
| continue | ||
|
|
||
| # Check if this task should be processed (is in past or next 10) | ||
| if task in tasks_to_process: | ||
| processed_tasks_identifiers.add(task_identifier) | ||
|
|
||
| # Get or create the TaskNode | ||
| task_node, created = TaskNode.objects.get_or_create( | ||
| task_template=TaskTemplate.objects.get(id=task_template_id), | ||
| due_date=task_date, | ||
| defaults={ | ||
| "name": task["name"], | ||
| "description": task["description"], | ||
| "status": "pending", | ||
| }, | ||
| ) | ||
|
|
||
| # Replace the task dictionary with the actual TaskNode in the original list | ||
| tasks_list[i] = TaskNodeReadSerializer(task_node).data | ||
|
|
||
| return tasks_list | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure logic aligns with code comments regarding “next 10 tasks.”
The comment at line 5093 mentions "next 10 upcoming tasks," but the code doesn’t actually slice to 10. This mismatch might cause confusion for maintainers. If the intent is to limit, add a slice; otherwise, update or remove the comment.
- # Process all past tasks and next 10 upcoming tasks
+ # Process all past tasks and all upcoming tasks (comment updated)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def task_calendar(self, task_templates, start=None, end=None): | |
| """Generate calendar of tasks for the given templates.""" | |
| tasks_list = [] | |
| for template in task_templates: | |
| if not template.is_recurrent: | |
| tasks_list.append(_create_task_dict(template, template.task_date)) | |
| continue | |
| start_date_param = start or template.task_date or datetime.now().date() | |
| end_date_param = end or template.schedule.get("end_date") | |
| if not end_date_param: | |
| start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() | |
| end_date_param = (start_date + rd.relativedelta(months=1)).strftime( | |
| "%Y-%m-%d" | |
| ) | |
| try: | |
| start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() | |
| end_date = datetime.strptime(str(end_date_param), "%Y-%m-%d").date() | |
| except ValueError: | |
| return {"error": "Invalid date format. Use YYYY-MM-DD"} | |
| tasks = _generate_occurrences(template, start_date, end_date) | |
| tasks_list.extend(tasks) | |
| processed_tasks_identifiers = set() # Track tasks we've already processed | |
| # Sort tasks by due date | |
| sorted_tasks = sorted(tasks_list, key=lambda x: x["due_date"]) | |
| # Process all past tasks and next 10 upcoming tasks | |
| current_date = datetime.now().date() | |
| # First separate past and future tasks | |
| past_tasks = [task for task in sorted_tasks if task["due_date"] <= current_date] | |
| next_tasks = [task for task in sorted_tasks if task["due_date"] > current_date] | |
| # Combined list of tasks to process (past and next 10) | |
| tasks_to_process = past_tasks + next_tasks | |
| # Directly modify tasks in the original tasks_list | |
| for i in range(len(tasks_list)): | |
| task = tasks_list[i] | |
| task_date = task["due_date"] | |
| task_template_id = task["task_template"] | |
| # Create a unique identifier for this task to avoid duplication | |
| task_identifier = (task_template_id, task_date) | |
| # Skip if we've already processed this task | |
| if task_identifier in processed_tasks_identifiers: | |
| continue | |
| # Check if this task should be processed (is in past or next 10) | |
| if task in tasks_to_process: | |
| processed_tasks_identifiers.add(task_identifier) | |
| # Get or create the TaskNode | |
| task_node, created = TaskNode.objects.get_or_create( | |
| task_template=TaskTemplate.objects.get(id=task_template_id), | |
| due_date=task_date, | |
| defaults={ | |
| "name": task["name"], | |
| "description": task["description"], | |
| "status": "pending", | |
| }, | |
| ) | |
| # Replace the task dictionary with the actual TaskNode in the original list | |
| tasks_list[i] = TaskNodeReadSerializer(task_node).data | |
| return tasks_list | |
| def task_calendar(self, task_templates, start=None, end=None): | |
| """Generate calendar of tasks for the given templates.""" | |
| tasks_list = [] | |
| for template in task_templates: | |
| if not template.is_recurrent: | |
| tasks_list.append(_create_task_dict(template, template.task_date)) | |
| continue | |
| start_date_param = start or template.task_date or datetime.now().date() | |
| end_date_param = end or template.schedule.get("end_date") | |
| if not end_date_param: | |
| start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() | |
| end_date_param = (start_date + rd.relativedelta(months=1)).strftime( | |
| "%Y-%m-%d" | |
| ) | |
| try: | |
| start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() | |
| end_date = datetime.strptime(str(end_date_param), "%Y-%m-%d").date() | |
| except ValueError: | |
| return {"error": "Invalid date format. Use YYYY-MM-DD"} | |
| tasks = _generate_occurrences(template, start_date, end_date) | |
| tasks_list.extend(tasks) | |
| processed_tasks_identifiers = set() # Track tasks we've already processed | |
| # Sort tasks by due date | |
| sorted_tasks = sorted(tasks_list, key=lambda x: x["due_date"]) | |
| # Process all past tasks and all upcoming tasks (comment updated) | |
| current_date = datetime.now().date() | |
| # First separate past and future tasks | |
| past_tasks = [task for task in sorted_tasks if task["due_date"] <= current_date] | |
| next_tasks = [task for task in sorted_tasks if task["due_date"] > current_date] | |
| # Combined list of tasks to process (past and next 10) | |
| tasks_to_process = past_tasks + next_tasks | |
| # Directly modify tasks in the original tasks_list | |
| for i in range(len(tasks_list)): | |
| task = tasks_list[i] | |
| task_date = task["due_date"] | |
| task_template_id = task["task_template"] | |
| # Create a unique identifier for this task to avoid duplication | |
| task_identifier = (task_template_id, task_date) | |
| # Skip if we've already processed this task | |
| if task_identifier in processed_tasks_identifiers: | |
| continue | |
| # Check if this task should be processed (is in past or next 10) | |
| if task in tasks_to_process: | |
| processed_tasks_identifiers.add(task_identifier) | |
| # Get or create the TaskNode | |
| task_node, created = TaskNode.objects.get_or_create( | |
| task_template=TaskTemplate.objects.get(id=task_template_id), | |
| due_date=task_date, | |
| defaults={ | |
| "name": task["name"], | |
| "description": task["description"], | |
| "status": "pending", | |
| }, | |
| ) | |
| # Replace the task dictionary with the actual TaskNode in the original list | |
| tasks_list[i] = TaskNodeReadSerializer(task_node).data | |
| return tasks_list |
🧰 Tools
🪛 Ruff (0.8.2)
5122-5122: TaskNode may be undefined, or defined from star imports
(F405)
5123-5123: TaskTemplate may be undefined, or defined from star imports
(F405)
5133-5133: TaskNodeReadSerializer may be undefined, or defined from star imports
(F405)
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
| # Check if there are any TaskNode instances that are not within the date range | ||
| if self.schedule and "end_date" in self.schedule: | ||
| end_date = self.schedule["end_date"] | ||
| end_date = datetime.strptime(end_date, "%Y-%m-%d").date() | ||
| # Delete TaskNode instances that have a due date after the end date | ||
| TaskNode.objects.filter(task_template=self, due_date__gt=end_date).delete() | ||
| super().save(*args, **kwargs) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Confirm intended deletion of tasks beyond end date.
When end_date is set in the schedule, you’re permanently deleting any TaskNode with a due_date after this end date. Confirm whether a soft-delete or an explicit “archived/cancelled” status might be more appropriate. If strict removal is intended, ensure callers understand the permanent loss of these records.
🧰 Tools
🪛 GitHub Actions: Backend Linters
[warning] Ruff formatting check failed. 1 file would be reformatted.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (12)
frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte (1)
49-49: Missing closing newline at end of file.Add a newline at the end of the file to follow standard coding conventions and avoid potential issues with some version control systems.
-49 +49 +backend/core/models.py (2)
4138-4142: Ensure the manager class is actually utilized.While defining a custom manager
TaskTemplateManagerwith acreate_task_templatemethod is fine, ensure that any external code calls this method for clarity and consistency. Otherwise, consider using the manager’s default.create()directly if no additional logic is provided.
4205-4254: Fields appear consistent and well-organized.The additional fields (
task_date,is_recurrent,ref_id,schedule, etc.) are consistent with the intended task management logic. Includingvalidatorsfor the JSON schema is a sound approach. However, consider adding unit tests to confirm correct validation for edge cases such as empty schedules or missing fields.frontend/src/lib/utils/schemas.ts (2)
641-677: TaskTemplateSchema looks well implemented but could benefit from stricter schedule frequency validation.The schema for task templates is comprehensive, with appropriate default values and validation constraints. However, the
frequencyfield in the schedule is defined as a free-form string, which could lead to invalid values being accepted.Consider using a z.enum() to restrict the frequency to valid values:
schedule: z .object({ interval: z.number().min(1).positive().optional(), - frequency: z.string().optional(), + frequency: z.enum(['DAILY', 'WEEKLY', 'MONTHLY', 'YEARLY']).optional(), weeks_of_month: z.number().min(-1).max(4).array().optional(), days_of_week: z.number().min(1).max(7).array().optional(), months_of_year: z.number().min(1).max(12).array().optional(), end_date: z.string().optional() })
679-685: TaskNodeSchema should include date format validation.The task node schema is well-structured, but the
due_datefield accepts any string without validating the date format.Consider adding date format validation similar to other date fields in the codebase:
due_date: z.string().optional(), + due_date: z.union([z.literal('').transform(() => null), z.string().date()]).nullish(),backend/core/serializers.py (1)
1422-1434: Consider explicit model imports instead of star imports.The models
TaskTemplateandTaskNodeare imported via star imports (from core.models import *), which can make dependencies less clear and cause namespace pollution.Consider explicitly importing these models at the top of the file for better clarity:
from core.models import * + # Explicitly import task models + from core.models import TaskTemplate, TaskNode🧰 Tools
🪛 Ruff (0.8.2)
1426-1426:
TaskNodemay be undefined, or defined from star imports(F405)
1432-1432:
TaskNodemay be undefined, or defined from star imports(F405)
backend/core/views.py (6)
11-11: Consider consolidating redundant imports.The module imports
dateutil.relativedeltatwice - once directly at line 11 and again with an alias at line 97.-from dateutil import relativedelta ... -from dateutil import relativedelta as rd +from dateutil import relativedelta as rdThis would simplify the imports while maintaining all functionality.
Also applies to: 97-97
5095-5095: Update comment to match implementation.The comment mentions "next 10 upcoming tasks" but the code doesn't actually limit to 10 tasks. Either implement the limit or update the comment to avoid confusion for future maintainers.
-# Process all past tasks and next 10 upcoming tasks +# Process all past tasks and all upcoming tasks
5076-5079: Consider handling date parsing more robustly.The code relies on error-prone string conversions for date handling. For instance, when
end_date_paramis None, you convertstart_date_paramto a string, parse it again, and then format it. Consider using native date operations.if not end_date_param: - start_date = datetime.strptime(str(start_date_param), "%Y-%m-%d").date() - end_date_param = (start_date + rd.relativedelta(months=1)).strftime( - "%Y-%m-%d" - ) + # start_date_param is already a date object here + end_date_param = (start_date_param + rd.relativedelta(months=1))
5121-5135: Consider batching database operations.The code creates or retrieves TaskNode instances one at a time in a loop, which could lead to performance issues with large datasets. Consider using bulk_create or bulk_update operations.
# Replace individual database operations with batch operations + task_nodes_to_create = [] + existing_task_nodes = {(tn.task_template_id, tn.due_date): tn for tn in + TaskNode.objects.filter( + task_template_id__in=[t["task_template"] for t in tasks_to_process], + due_date__in=[t["due_date"] for t in tasks_to_process] + )} for i in range(len(tasks_list)): # ...existing code... if task in tasks_to_process: processed_tasks_identifiers.add(task_identifier) - # Get or create the TaskNode - task_node, created = TaskNode.objects.get_or_create( - task_template=TaskTemplate.objects.get(id=task_template_id), - due_date=task_date, - defaults={ - "name": task["name"], - "description": task["description"], - "status": "pending", - }, - ) + # Use existing TaskNode or prepare to create a new one + key = (task_template_id, task_date) + if key in existing_task_nodes: + task_node = existing_task_nodes[key] + else: + task_node = TaskNode( + task_template=TaskTemplate.objects.get(id=task_template_id), + due_date=task_date, + name=task["name"], + description=task["description"], + status="pending" + ) + task_nodes_to_create.append(task_node) + existing_task_nodes[key] = task_node # Replace the task dictionary with the actual TaskNode in the original list tasks_list[i] = TaskNodeReadSerializer(task_node).data + # Bulk create new TaskNodes after the loop + if task_nodes_to_create: + TaskNode.objects.bulk_create(task_nodes_to_create)🧰 Tools
🪛 Ruff (0.8.2)
5123-5123:
TaskNodemay be undefined, or defined from star imports(F405)
5124-5124:
TaskTemplatemay be undefined, or defined from star imports(F405)
5134-5134:
TaskNodeReadSerializermay be undefined, or defined from star imports(F405)
5173-5192: Common pattern detected across calendar-related endpoints.The calendar endpoint implementation in TaskTemplateViewSet is similar to the UserViewSet.tasks endpoint. Consider extracting common functionality into a utility function to avoid duplication and ensure consistent behavior.
🧰 Tools
🪛 Ruff (0.8.2)
5188-5188:
TaskTemplatemay be undefined, or defined from star imports(F405)
5195-5202: Make TaskNodeViewSet consistent with naming conventions.In the
statusmethod, the first parameter is namedsrlfinstead of the standardself. This seems like a typo that should be fixed.@action(detail=False, name="Get Task Node status choices") -def status(srlf, request): +def status(self, request): return Response(dict(TaskNode.TASK_STATUS_CHOICES))🧰 Tools
🪛 Ruff (0.8.2)
5196-5196:
TaskNodemay be undefined, or defined from star imports(F405)
5201-5201:
TaskNodemay be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/core/migrations/0067_tasktemplate_tasknode.py(1 hunks)backend/core/models.py(1 hunks)backend/core/serializers.py(1 hunks)backend/core/tests/test_task.py(1 hunks)backend/core/views.py(6 hunks)frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte(1 hunks)frontend/src/lib/utils/crud.ts(1 hunks)frontend/src/lib/utils/schemas.ts(2 hunks)frontend/src/lib/utils/table.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/lib/utils/crud.ts
- frontend/src/lib/utils/table.ts
- backend/core/migrations/0067_tasktemplate_tasknode.py
🧰 Additional context used
🧬 Code Graph Analysis (5)
backend/core/serializers.py (2)
backend/core/serializer_fields.py (1)
FieldsRelatedField(24-86)backend/core/models.py (4)
risk_assessments(937-938)risk_assessments(2154-2155)TaskTemplate(4144-4272)TaskNode(4275-4306)
frontend/src/lib/utils/schemas.ts (1)
backend/core/base_models.py (1)
NameDescriptionMixin(111-124)
backend/core/models.py (1)
backend/core/base_models.py (2)
NameDescriptionMixin(111-124)save(106-108)
backend/core/tests/test_task.py (1)
backend/core/utils.py (7)
_convert_to_python_weekday(284-286)_get_month_range(289-293)_get_nth_weekday_of_month(296-317)_date_matches_schedule(320-414)_calculate_next_occurrence(417-588)_create_task_dict(591-605)_generate_occurrences(608-656)
backend/core/views.py (2)
backend/core/utils.py (2)
_generate_occurrences(608-656)_create_task_dict(591-605)backend/core/models.py (3)
TaskTemplate(4144-4272)TaskNode(4275-4306)create(2230-2232)
🪛 Ruff (0.8.2)
backend/core/serializers.py
1412-1412: TaskTemplate may be undefined, or defined from star imports
(F405)
1418-1418: TaskTemplate may be undefined, or defined from star imports
(F405)
1426-1426: TaskNode may be undefined, or defined from star imports
(F405)
1432-1432: TaskNode may be undefined, or defined from star imports
(F405)
backend/core/tests/test_task.py
1-1: datetime.datetime imported but unused
Remove unused import
(F401)
1-1: datetime.timedelta imported but unused
Remove unused import
(F401)
2-2: dateutil.relativedelta imported but unused
Remove unused import: dateutil.relativedelta
(F401)
9-9: core.utils._date_matches_schedule imported but unused
Remove unused import
(F401)
10-10: core.utils._calculate_next_occurrence imported but unused
Remove unused import
(F401)
11-11: core.utils._create_task_dict imported but unused
Remove unused import
(F401)
12-12: core.utils._generate_occurrences imported but unused
Remove unused import
(F401)
backend/core/views.py
2207-2207: TaskTemplate may be undefined, or defined from star imports
(F405)
5061-5061: TaskTemplate may be undefined, or defined from star imports
(F405)
5123-5123: TaskNode may be undefined, or defined from star imports
(F405)
5124-5124: TaskTemplate may be undefined, or defined from star imports
(F405)
5134-5134: TaskNodeReadSerializer may be undefined, or defined from star imports
(F405)
5142-5142: TaskNode may be undefined, or defined from star imports
(F405)
5158-5158: TaskTemplate may be undefined, or defined from star imports
(F405)
5163-5163: TaskNode may be undefined, or defined from star imports
(F405)
5188-5188: TaskTemplate may be undefined, or defined from star imports
(F405)
5196-5196: TaskNode may be undefined, or defined from star imports
(F405)
5201-5201: TaskNode may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (15)
frontend/src/lib/components/Forms/ModelForm/TaskNodeForm.svelte (5)
1-14: Component structure looks good!The component is well defined with proper TypeScript typing and appropriate exports. The form component accepts necessary props for validation, model information, and caching behavior which follows good component design principles.
16-24: TextField implementation for due date is appropriate.The due date field is correctly implemented with proper internationalization for labels and help text. The cache binding pattern will help maintain state during form interactions.
25-32: Status Select component uses model data effectively.Good implementation of the Status dropdown that dynamically pulls options from the model's selectOptions, making the component reusable and data-driven.
33-39: TextArea component for observations follows consistent pattern.The observation field maintains the same pattern as other form elements with proper caching behavior and internationalization.
40-48: AutocompleteSelect implementation for evidences is well configured.The evidences field is correctly set up with the multiple attribute and appropriate endpoint configuration. The optionsLabelField="auto" suggests automatic label generation based on the data structure.
backend/core/models.py (3)
4144-4146: Class usage looks good.By assigning
objects = TaskTemplateManager()toTaskTemplate, you enable the new manager. This is a clean approach, and no immediate issues stand out.
4147-4203: JSON schema definition is comprehensive.The
SCHEDULE_JSONSCHEMAproperty thoroughly captures and constrains the schedule data. Ensure that code utilizing this schema properly handles optional fields likeoccurrencesorexceptions, which may be null.
4275-4307: TaskNode definition is concise and functional.The
TaskNodeclass properly references its parentTaskTemplate, includes a clearTASK_STATUS_CHOICESset, and optional fields such asdue_dateandobservation. If you anticipate task lifecycle changes or further states in the future, consider using a dedicated state machine library. Otherwise, this is a straightforward solution.frontend/src/lib/utils/schemas.ts (1)
731-733: LGTM: Schema mapping updated correctly.The new schemas are correctly added to the
SCHEMA_MAPwith appropriate keys.backend/core/tests/test_task.py (1)
62-98: Active test cases look good but should be part of a proper test class.The implemented tests for date utility functions are well-written with clear assertions and comments explaining the logic.
backend/core/serializers.py (2)
1416-1420: TaskTemplateWriteSerializer looks good.The write serializer includes all fields, which is appropriate for create/update operations.
🧰 Tools
🪛 Ruff (0.8.2)
1418-1418:
TaskTemplatemay be undefined, or defined from star imports(F405)
1403-1414:❓ Verification inconclusive
TaskTemplateReadSerializer excludes schedule field without explanation.
The
schedulefield is excluded from the read serializer, which means it won't be returned in API responses. This seems inconsistent with the frontend schema that includes this field.Verify whether excluding the
schedulefield is intentional. If it is, consider documenting why in a comment. If not, consider including it:
🏁 Script executed:
#!/bin/bash # Find where TaskTemplate is used in views to understand the API usage grep -r "TaskTemplate" --include="*.py" backend/core/views.pyLength of output: 557
Action: Clarify the Intent Behind Excluding the
scheduleFieldThe current implementation of the
TaskTemplateReadSerializerinbackend/core/serializers.pyexcludes theschedulefield. Our grep search inbackend/core/views.pyconfirms that whileTaskTemplateis used across the views, there’s no clear indication that theschedulefield is actively required in these API responses. However, given that the frontend schema expects this field, please verify whether this exclusion is intentional.
- If the exclusion is intentional: Add an inline comment documenting why the
schedulefield is omitted.- If it’s unintentional: Update the serializer to include the
schedulefield to ensure consistency with the frontend schema.🧰 Tools
🪛 Ruff (0.8.2)
1412-1412:
TaskTemplatemay be undefined, or defined from star imports(F405)
backend/core/views.py (3)
1981-1986: Great addition of month and year filters!Adding month and year filtering options to date fields enhances the API's querying capabilities, making it more useful for reporting and analytics.
Also applies to: 1312-1312
2190-2214: Proper default parameter handling implemented.You've correctly addressed the previous issue with function calls in default parameters by setting them to None and initializing them inside the function body. This ensures date values are evaluated at runtime rather than module load time.
🧰 Tools
🪛 Ruff (0.8.2)
2207-2207:
TaskTemplatemay be undefined, or defined from star imports(F405)
5060-5193: Verify TaskTemplate star imports.The static analysis tools indicate that
TaskTemplateand related classes may be undefined or imported via star imports. Consider explicit imports instead of relying on star imports for better code maintainability.- from .models import * + from .models import TaskTemplate, TaskNode - from .serializers import * + from .serializers import TaskNodeReadSerializerWhile star imports (
*) work, they make dependencies less explicit and can lead to namespace pollution. Explicit imports are generally considered better practice in Python.🧰 Tools
🪛 Ruff (0.8.2)
5061-5061:
TaskTemplatemay be undefined, or defined from star imports(F405)
5123-5123:
TaskNodemay be undefined, or defined from star imports(F405)
5124-5124:
TaskTemplatemay be undefined, or defined from star imports(F405)
5134-5134:
TaskNodeReadSerializermay be undefined, or defined from star imports(F405)
5142-5142:
TaskNodemay be undefined, or defined from star imports(F405)
5158-5158:
TaskTemplatemay be undefined, or defined from star imports(F405)
5163-5163:
TaskNodemay be undefined, or defined from star imports(F405)
5188-5188:
TaskTemplatemay be undefined, or defined from star imports(F405)
| class Meta: | ||
| verbose_name = "Task template" | ||
| verbose_name_plural = "Task templates" | ||
|
|
||
| def save(self, *args, **kwargs): | ||
| if self.schedule and "days_of_week" in self.schedule: | ||
| # Only modify values that are not already in range 0-6 | ||
| self.schedule["days_of_week"] = [ | ||
| day % 7 if day > 6 else day for day in self.schedule["days_of_week"] | ||
| ] | ||
|
|
||
| # Check if there are any TaskNode instances that are not within the date range | ||
| if self.schedule and "end_date" in self.schedule: | ||
| end_date = self.schedule["end_date"] | ||
| end_date = datetime.strptime(end_date, "%Y-%m-%d").date() | ||
| # Delete TaskNode instances that have a due date after the end date | ||
| TaskNode.objects.filter(task_template=self, due_date__gt=end_date).delete() | ||
| super().save(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate end_date parsing and day-of-week edge cases.
- If
end_dateis an empty string orNone,datetime.strptime(end_date, '%Y-%m-%d')raises an error. Consider an additional check or default behavior forNoneor invalid date strings. - The line
day % 7 if day > 6 else daycan produce0(out of [1..7] range) ifdayis exactly 7. This likely contradicts the schema where Sunday=7.
Below is a suggested fix to preserve Sunday=7 and wrap values strictly above 7:
- self.schedule["days_of_week"] = [
- day % 7 if day > 6 else day for day in self.schedule["days_of_week"]
- ]
+ corrected_days = []
+ for day in self.schedule["days_of_week"]:
+ if day > 7:
+ # wrap around, ensuring Sunday remains 7, not 0
+ remainder = day % 7
+ corrected_days.append(remainder if remainder != 0 else 7)
+ else:
+ corrected_days.append(day)
+ self.schedule["days_of_week"] = corrected_days📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class Meta: | |
| verbose_name = "Task template" | |
| verbose_name_plural = "Task templates" | |
| def save(self, *args, **kwargs): | |
| if self.schedule and "days_of_week" in self.schedule: | |
| # Only modify values that are not already in range 0-6 | |
| self.schedule["days_of_week"] = [ | |
| day % 7 if day > 6 else day for day in self.schedule["days_of_week"] | |
| ] | |
| # Check if there are any TaskNode instances that are not within the date range | |
| if self.schedule and "end_date" in self.schedule: | |
| end_date = self.schedule["end_date"] | |
| end_date = datetime.strptime(end_date, "%Y-%m-%d").date() | |
| # Delete TaskNode instances that have a due date after the end date | |
| TaskNode.objects.filter(task_template=self, due_date__gt=end_date).delete() | |
| super().save(*args, **kwargs) | |
| class Meta: | |
| verbose_name = "Task template" | |
| verbose_name_plural = "Task templates" | |
| def save(self, *args, **kwargs): | |
| if self.schedule and "days_of_week" in self.schedule: | |
| # Only modify values that are not already in range 0-6 | |
| corrected_days = [] | |
| for day in self.schedule["days_of_week"]: | |
| if day > 7: | |
| # wrap around, ensuring Sunday remains 7, not 0 | |
| remainder = day % 7 | |
| corrected_days.append(remainder if remainder != 0 else 7) | |
| else: | |
| corrected_days.append(day) | |
| self.schedule["days_of_week"] = corrected_days | |
| # Check if there are any TaskNode instances that are not within the date range | |
| if self.schedule and "end_date" in self.schedule: | |
| end_date = self.schedule["end_date"] | |
| end_date = datetime.strptime(end_date, "%Y-%m-%d").date() | |
| # Delete TaskNode instances that have a due date after the end date | |
| TaskNode.objects.filter(task_template=self, due_date__gt=end_date).delete() | |
| super().save(*args, **kwargs) |
| # class DummyQuerySet: | ||
| # """Simulates a queryset returning a list of simple objects with an 'id' attribute.""" | ||
|
|
||
| # def __init__(self, ids): | ||
| # self.items = [DummyObject(_id) for _id in ids] | ||
|
|
||
| # def all(self): | ||
| # return self.items | ||
|
|
||
|
|
||
| # class DummyObject: | ||
| # def __init__(self, _id): | ||
| # self.id = _id | ||
|
|
||
|
|
||
| # class DummyTask: | ||
| # """ | ||
| # Simulates a task or template object. | ||
| # Minimum required attributes: | ||
| # - id, name, description, ref_id, task_date, due_date, schedule | ||
| # - assigned_to, assets, applied_controls, compliance_assessments, risk_assessments | ||
| # """ | ||
|
|
||
| # def __init__(self, **kwargs): | ||
| # self.id = kwargs.get("id", 1) | ||
| # self.name = kwargs.get("name", "Dummy Task") | ||
| # self.description = kwargs.get("description", "Description") | ||
| # self.ref_id = kwargs.get("ref_id", "ref1") | ||
| # self.task_date = kwargs.get("task_date") # Must be a date or string | ||
| # self.due_date = kwargs.get("due_date") # Must be a date or None | ||
| # self.schedule = kwargs.get("schedule", {}) | ||
| # self.is_recurrent = kwargs.get("is_recurrent", True) | ||
| # # Relations are simulated by DummyQuerySet | ||
| # self.assigned_to = DummyQuerySet(kwargs.get("assigned_to", [1])) | ||
| # self.assets = DummyQuerySet(kwargs.get("assets", [10])) | ||
| # self.applied_controls = DummyQuerySet(kwargs.get("applied_controls", [20])) | ||
| # self.compliance_assessments = DummyQuerySet( | ||
| # kwargs.get("compliance_assessments", [30]) | ||
| # ) | ||
| # self.risk_assessments = DummyQuerySet(kwargs.get("risk_assessments", [40])) | ||
|
|
There was a problem hiding this comment.
Uncomment the test cases or remove them if not needed yet.
Most of the file contains commented-out test code, including dummy classes and test functions for task scheduling. This suggests incomplete test coverage for the task management features.
Either uncomment these tests if they're ready to be used, or remove them if they're placeholders for future development. Commented-out code can lead to confusion and should be documented if it's intentionally left in the codebase.
If these tests are intended for future implementation, consider adding a TODO comment explaining when they will be implemented.
Also applies to: 103-289
| from datetime import date, datetime, timedelta | ||
| from dateutil import relativedelta as rd | ||
|
|
||
| # Import functions to be tested from core.utils | ||
| from core.utils import ( | ||
| _convert_to_python_weekday, | ||
| _get_month_range, | ||
| _get_nth_weekday_of_month, | ||
| _date_matches_schedule, | ||
| _calculate_next_occurrence, | ||
| _create_task_dict, | ||
| _generate_occurrences, | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused imports flagged by static analysis.
Several imported functions and modules are not used in the active tests, which might confuse future developers.
- from datetime import date, datetime, timedelta
+ from datetime import date
- from dateutil import relativedelta as rd
# Import functions to be tested from core.utils
from core.utils import (
_convert_to_python_weekday,
_get_month_range,
_get_nth_weekday_of_month,
- _date_matches_schedule,
- _calculate_next_occurrence,
- _create_task_dict,
- _generate_occurrences,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from datetime import date, datetime, timedelta | |
| from dateutil import relativedelta as rd | |
| # Import functions to be tested from core.utils | |
| from core.utils import ( | |
| _convert_to_python_weekday, | |
| _get_month_range, | |
| _get_nth_weekday_of_month, | |
| _date_matches_schedule, | |
| _calculate_next_occurrence, | |
| _create_task_dict, | |
| _generate_occurrences, | |
| ) | |
| from datetime import date | |
| # Import functions to be tested from core.utils | |
| from core.utils import ( | |
| _convert_to_python_weekday, | |
| _get_month_range, | |
| _get_nth_weekday_of_month, | |
| ) |
🧰 Tools
🪛 Ruff (0.8.2)
1-1: datetime.datetime imported but unused
Remove unused import
(F401)
1-1: datetime.timedelta imported but unused
Remove unused import
(F401)
2-2: dateutil.relativedelta imported but unused
Remove unused import: dateutil.relativedelta
(F401)
9-9: core.utils._date_matches_schedule imported but unused
Remove unused import
(F401)
10-10: core.utils._calculate_next_occurrence imported but unused
Remove unused import
(F401)
11-11: core.utils._create_task_dict imported but unused
Remove unused import
(F401)
12-12: core.utils._generate_occurrences imported but unused
Remove unused import
(F401)
eric-intuitem
force-pushed
the
feat_tasks_management
branch
from
561fe12 to
8a867fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/core/views.py (1)
5063-5139: 🛠️ Refactor suggestionAdjust or clarify the logic vs. docstring regarding “next 10 upcoming tasks.”
The docstring mentions processing the next 10 tasks, but the code currently processes all upcoming tasks, which might cause confusion. If you truly want only 10 upcoming tasks, consider slicing the list accordingly:
- next_tasks = [task for task in sorted_tasks if task["due_date"] > current_date] - tasks_to_process = past_tasks + next_tasks + next_tasks_limited = [task for task in sorted_tasks if task["due_date"] > current_date][:10] + tasks_to_process = past_tasks + next_tasks_limited🧰 Tools
🪛 Ruff (0.8.2)
5123-5123:
TaskTemplatemay be undefined, or defined from star imports(F405)
5124-5124:
TaskNodemay be undefined, or defined from star imports(F405)
5136-5136:
TaskNodeReadSerializermay be undefined, or defined from star imports(F405)
🧹 Nitpick comments (3)
backend/core/views.py (3)
11-12: Consider removing duplicate imports.You've imported
relativedeltahere, but there's also another import on line 97 (from dateutil import relativedelta as rd). Unifying them into a single import could improve clarity and remove redundancy.
97-97: Avoid duplicating imports forrelativedelta.Since line 11 already imports
relativedelta, you could reuse it instead of aliasing it asrdhere. This helps maintain consistency.
5175-5196: Avoid duplicating scheduling logic.Your
@actionmethod re-invokestask_calendarsimilar to logic inUserViewSet.tasks(...). While the code is fine, consider unifying them to prevent drift and maintenance overhead.🧰 Tools
🪛 Ruff (0.8.2)
5191-5191:
TaskTemplatemay be undefined, or defined from star imports(F405)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/core/views.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/core/views.py (2)
backend/core/utils.py (2)
_generate_occurrences(608-656)_create_task_dict(591-605)backend/core/models.py (3)
TaskTemplate(4144-4272)TaskNode(4275-4306)create(2230-2232)
🪛 Ruff (0.8.2)
backend/core/views.py
2207-2207: TaskTemplate may be undefined, or defined from star imports
(F405)
5061-5061: TaskTemplate may be undefined, or defined from star imports
(F405)
5123-5123: TaskTemplate may be undefined, or defined from star imports
(F405)
5124-5124: TaskNode may be undefined, or defined from star imports
(F405)
5136-5136: TaskNodeReadSerializer may be undefined, or defined from star imports
(F405)
5144-5144: TaskNode may be undefined, or defined from star imports
(F405)
5160-5160: TaskTemplate may be undefined, or defined from star imports
(F405)
5165-5165: TaskNode may be undefined, or defined from star imports
(F405)
5191-5191: TaskTemplate may be undefined, or defined from star imports
(F405)
5199-5199: TaskNode may be undefined, or defined from star imports
(F405)
5204-5204: TaskNode may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: test (3.12)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
backend/core/views.py (5)
94-95: New utils imports look good.Importing
_generate_occurrencesand_create_task_dictfromcore.utilscleanly reflects their usage in the new scheduling logic.
1980-1986: Field-based filters for RiskAcceptance look good.Adding
folder,state,approver, and expanded date filters onexpiry_dateimproves usability and consistency with other filter sets.
5059-5062: New TaskTemplateViewSet class definition is well-structured.Defining a dedicated
TaskTemplateViewSetis a clean approach for managing task templates. Themodel = TaskTemplateattribute is consistent with the codebase’s pattern.🧰 Tools
🪛 Ruff (0.8.2)
5061-5061:
TaskTemplatemay be undefined, or defined from star imports(F405)
5198-5205: TaskNodeViewSet looks good.This viewset complements the
TaskTemplateViewSetby exposing TaskNodes and allowing status or template-based filtering.🧰 Tools
🪛 Ruff (0.8.2)
5199-5199:
TaskNodemay be undefined, or defined from star imports(F405)
5204-5204:
TaskNodemay be undefined, or defined from star imports(F405)
1312-1312:✅ Verification successful
Verify that the custom lookups “month” and “year” are supported.
Django Filter’s date fields typically allow lookups like
exact,lt,gt, etc. Ensure your version supports the “month” and “year” lookups without issues.
🌐 Web query:
Do Django and django-filter natively support "month" and "year" lookups for date fields?💡 Result:
Yes, Django and django-filter natively support "month" and "year" lookups for date fields through Django's ORM and query API. Here's how it works:
Django's Native Support
Basic Field Lookups:
- Use
__yearand__monthdirectly in querysets:# Filter by year Model.objects.filter(date_field__year=2025) # Filter by month (returns results for April across all years) Model.objects.filter(date_field__month=4) # Combined year/month filtering Model.objects.filter(date_field__year=2025, date_field__month=4)Date Extraction Functions:
Django provides database functions likeExtractYearandExtractMonthfor annotations:from django.db.models.functions import ExtractYear, ExtractMonth queryset = Model.objects.annotate( year=ExtractYear('date_field'), month=ExtractMonth('date_field') ).filter(year=2025, month=4)Aggregation by Date Parts:
Usedates()ordatetimes()to group results:# Get distinct year/month combinations Model.objects.dates('date_field', 'month', order='ASC')django-filter Integration
Django-filter automatically supports these lookups when configuring filters:
import django_filters class ModelFilter(django_filters.FilterSet): year = django_filters.NumberFilter(field_name='date_field__year') month = django_filters.NumberFilter(field_name='date_field__month') class Meta: model = Model fields = []This allows URLs like
/?year=2025&month=4to filter results. Both fields use Django's native__yearand__monthlookups under the hood[1][2][5].Notes
- For timezone-aware datetime fields, extraction respects the active timezone[2].
- Week/day lookups (e.g.,
__week_day) are also available but follow Django's unique weekday numbering (Sunday=1, Saturday=7)[2].- PostgreSQL users can leverage
Truncfunctions for more precise date truncation[2][9].These features work across all Django-supported databases without requiring raw SQL or third-party packages.
Citations:
- 1: https://bradmontgomery.net/blog/date-lookups-django/
- 2: https://docs.djangoproject.com/en/5.1/ref/models/database-functions/
- 3: https://docs.djangoproject.com/en/5.1/ref/models/querysets/
- 4: https://forum.djangoproject.com/t/django-add-field-values-as-per-month-and-year-wise-and-consolidate/28687
- 5: https://quepinch.com/s/how-to-extract-year-month-day-from-date-in-django-6ffb297
- 6: https://docs.djangoproject.com/en/5.1/ref/models/fields/
- 7: https://gist.github.com/bee-keeper/e9c69e757b529fcffeda
- 8: https://clivern.com/django-custom-month-and-year-lookup/
- 9: https://django.readthedocs.io/en/stable/ref/models/expressions.html
- 10: https://forum.djangoproject.com/t/using-date-function-with-queryset/11201
Supported Lookups Confirmed
The verification confirms that both Django and django-filter natively support the
monthandyearlookups for date fields. The code inbackend/core/views.py(line 1312) is valid and does not require further modifications.
no name/description for task_node no publish for tasks
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
backend/core/views.py (2)
5093-5095: Update comment to match code behavior regarding task processing.The comment states "Process all past tasks and next 10 upcoming tasks", but the code doesn't actually limit to 10 upcoming tasks - it processes all tasks in the date range.
5137-5161: Consider preserving existing TaskNodes on update.Calling
TaskNode.objects.filter(task_template=...).delete()may discard user updates (e.g. changed statuses) when a template is updated. A more refined approach preserves existing TaskNodes and only adds missing ones, preventing unintended data loss.🧰 Tools
🪛 Ruff (0.8.2)
5141-5141:
TaskNodemay be undefined, or defined from star imports(F405)
5157-5157:
TaskTemplatemay be undefined, or defined from star imports(F405)
backend/core/models.py (1)
4258-4264:⚠️ Potential issueFix Sunday-day-of-week bug.
When
day == 7,day % 7becomes0, which is invalid per the schema (expected range 1–7). This logic will incorrectly map Sunday to0, causing a potential mismatch in scheduling.Apply this diff to preserve Sunday=7 and wrap only values strictly above 7:
- self.schedule["days_of_week"] = [ - day % 7 if day > 6 else day for day in self.schedule["days_of_week"] - ] + corrected_days = [] + for day in self.schedule["days_of_week"]: + if day > 7: + remainder = day % 7 + corrected_days.append(remainder if remainder != 0 else 7) + else: + corrected_days.append(day) + self.schedule["days_of_week"] = corrected_days
🧹 Nitpick comments (6)
backend/core/views.py (4)
11-11: Consolidate duplicate imports of relativedelta.The module
relativedeltafromdateutilis imported twice - once directly and once with the aliasrd. This creates unnecessary duplication and potential confusion.- from dateutil import relativedelta ... - from dateutil import relativedelta as rd + from dateutil import relativedeltaThen update all usages of
rd.relativedeltato simply userelativedelta.Also applies to: 97-97
2190-2213: Consider refactoring static method call to follow OOP principles.The
TaskTemplateViewSet.task_calendar()is called as a static method but withself=self, whereselfis theUserViewSetinstance - not aTaskTemplateViewSetinstance. While this works, it doesn't follow standard OOP design principles.Consider one of these approaches for a cleaner design:
- Make
task_calendara static or class method:@action( detail=True, name="Get user tasks", url_path="tasks/(?P<start>.+)/(?P<end>.+)" ) def tasks( self, request, pk, start=None, end=None, ): if start is None: start = timezone.now().date() if end is None: end = timezone.now().date() + relativedelta.relativedelta(months=1) return Response( TaskTemplateViewSet.task_calendar( - self=self, task_templates=TaskTemplate.objects.filter( enabled=True, assigned_to=request.user ), start=start, end=end, ) )
- Or instantiate the viewset properly:
template_viewset = TaskTemplateViewSet() return Response( template_viewset.task_calendar( task_templates=TaskTemplate.objects.filter( enabled=True, assigned_to=request.user ), start=start, end=end, ) )🧰 Tools
🪛 Ruff (0.8.2)
2207-2207:
TaskTemplatemay be undefined, or defined from star imports(F405)
5204-5207: Remove debug print statement.The print statement
print("coucou42", generator)appears to be debugging code that should be removed before merging.def list(self, request, *args, **kwargs): generator = request.query_params.get("generator") - print("coucou42", generator) return super().list(request, *args, **kwargs)
110-110: Replace star imports with explicit imports for clarity.Star imports (
from .models import *) make it difficult to track class origins and can cause namespace pollution. The static analyzer is flagging thatTaskTemplateandTaskNodemight be undefined or imported from star imports.For better code maintainability and to avoid potential naming conflicts, consider replacing the star import with explicit imports:
- from .models import * + from .models import ( + # Keep existing explicitly needed models + TaskTemplate, + TaskNode, + # Other needed models... + )This makes dependencies clear and prevents potential name collisions.
🧰 Tools
🪛 Ruff (0.8.2)
110-110:
from .models import *used; unable to detect undefined names(F403)
frontend/src/lib/utils/schemas.ts (1)
649-655: Consider extracting date formatting to a utility function.The date formatting logic could be extracted to a reusable utility function since this pattern might be needed elsewhere in the codebase.
+ // In a date utility file + export const getCurrentDateFormatted = () => { + const date = new Date(); + const year = date.getFullYear(); + const month = String(date.getMonth() + 1).padStart(2, '0'); + const day = String(date.getDate()).padStart(2, '0'); + return `${year}-${month}-${day}`; + }; // In this file - task_date: z - .string() - .default(() => { - const date = new Date(); - const year = date.getFullYear(); - const month = String(date.getMonth() + 1).padStart(2, '0'); // Months are zero-based - const day = String(date.getDate()).padStart(2, '0'); - return `${year}-${month}-${day}`; - }) + task_date: z + .string() + .default(getCurrentDateFormatted)frontend/src/lib/utils/crud.ts (1)
956-977: Consider adding filters property to both task-related entries.For consistency with other model mappings that have filterable fields (like applied-controls, assets, etc.), consider adding a filters property to enable filtering in list views.
'task-templates': { name: 'tasktemplate', localName: 'taskTemplate', localNamePlural: 'taskTemplates', verboseName: 'Task template', verboseNamePlural: 'Task templates', reverseForeignKeyFields: [ { field: 'task_template', urlModel: 'task-nodes', disableAddDeleteButtons: true } - ] + ], + filters: [{ field: 'status' }, { field: 'folder' }] }, 'task-nodes': { name: 'tasknode', localName: 'taskNode', localNamePlural: 'taskNodes', verboseName: 'Task node', verboseNamePlural: 'Task nodes', selectFields: [{ field: 'status' }], foreignKeyFields: [ { field: 'task_template', urlModel: 'task-templates' }, { field: 'evidences', urlModel: 'evidences' }, { field: 'folder', urlModel: 'folders' } - ] + ], + filters: [{ field: 'status' }, { field: 'task_template' }] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/core/migrations/0067_tasktemplate_tasknode.py(1 hunks)backend/core/models.py(1 hunks)backend/core/views.py(6 hunks)frontend/src/lib/utils/crud.ts(1 hunks)frontend/src/lib/utils/schemas.ts(2 hunks)frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte
- backend/core/migrations/0067_tasktemplate_tasknode.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/core/views.py (2)
backend/core/utils.py (2)
_generate_occurrences(608-656)_create_task_dict(591-605)backend/core/models.py (5)
TaskTemplate(4143-4271)TaskNode(4274-4305)delete(777-790)delete(1855-1858)create(2229-2231)
backend/core/models.py (2)
backend/core/base_models.py (2)
NameDescriptionMixin(111-124)AbstractBaseModel(8-108)backend/iam/models.py (2)
FolderMixin(182-195)delete(391-393)
🪛 Ruff (0.8.2)
backend/core/views.py
2207-2207: TaskTemplate may be undefined, or defined from star imports
(F405)
5061-5061: TaskTemplate may be undefined, or defined from star imports
(F405)
5122-5122: TaskTemplate may be undefined, or defined from star imports
(F405)
5123-5123: TaskNode may be undefined, or defined from star imports
(F405)
5133-5133: TaskNodeReadSerializer may be undefined, or defined from star imports
(F405)
5141-5141: TaskNode may be undefined, or defined from star imports
(F405)
5157-5157: TaskTemplate may be undefined, or defined from star imports
(F405)
5162-5162: TaskNode may be undefined, or defined from star imports
(F405)
5188-5188: TaskTemplate may be undefined, or defined from star imports
(F405)
5196-5196: TaskNode may be undefined, or defined from star imports
(F405)
5201-5201: TaskNode may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: build (3.12)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
backend/core/views.py (1)
5176-5192: The calendar method correctly handles default parameters.The implementation properly initializes date parameters inside the function rather than in the signature, resolving the potential issue with function calls in default parameters.
🧰 Tools
🪛 Ruff (0.8.2)
5188-5188:
TaskTemplatemay be undefined, or defined from star imports(F405)
backend/core/models.py (3)
4137-4141: Clean minimal override in the custom manager.No immediate concerns. The override is minimal and functional if you plan to add more logic later.
4143-4257: Fields and JSON schema definition appear well-structured.The class-level configuration for
TaskTemplateand its schema fields are consistent and match the intended scheduling functionality.
4274-4306: Implementation ofTaskNodelooks solid.The model definition is straightforward, fields align with expected usage, and there are no apparent issues with the new fields.
frontend/src/lib/utils/schemas.ts (3)
641-677: LGTM: TaskTemplateSchema correctly implements task scheduling functionality.The TaskTemplateSchema provides a comprehensive structure for task templates with recurrence functionality. The implementation handles both one-time and recurring tasks through the schedule object with appropriate constraints on frequency values.
679-684: LGTM: TaskNodeSchema is well-structured for individual task instances.The TaskNodeSchema properly captures the key attributes needed for task instances that are created from templates.
731-732: LGTM: Schema map updated correctly with new schemas.Both schemas have been properly added to the SCHEMA_MAP, making them available for the application's form validation system.
frontend/src/lib/utils/crud.ts (2)
956-965: LGTM: Task templates model mapping is properly defined.The task-templates entry in URL_MODEL_MAP properly defines the relationship with task nodes and uses consistent naming conventions.
966-977: LGTM: Task nodes model mapping is well-defined with proper relationships.The task-nodes entry correctly defines its relationships with task templates, evidences, and folders, and makes the status field selectable for UI filtering.
| # Check if there are any TaskNode instances that are not within the date range | ||
| if self.schedule and "end_date" in self.schedule: | ||
| end_date = self.schedule["end_date"] | ||
| end_date = datetime.strptime(end_date, "%Y-%m-%d").date() | ||
| # Delete TaskNode instances that have a due date after the end date | ||
| TaskNode.objects.filter(task_template=self, due_date__gt=end_date).delete() | ||
| super().save(*args, **kwargs) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider soft deletion or an alternative to purging out-of-range tasks.
Automatically deleting TaskNode instances with a due date past the end_date can lead to permanent data loss. You might prefer to mark them as canceled or archived for future reference.
avoids showing UUIDs
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
frontend/messages/en.json (1)
1452-1453: Potential inconsistency in "isTemplate" key valueThe key
"isTemplate"is assigned the value"Recurrent", which appears to duplicate the purpose of the"isRecurrent"key (line 1424). This could lead to confusion in the UI where both keys refer to the same concept but might be used in different contexts.What's the difference between a template and recurrence in task management?
🧹 Nitpick comments (1)
frontend/messages/en.json (1)
1430-1448: Consider adding pluralization for calendar termsWhile "weeks" and "months" are included, other calendar terms like "days" and "years" are missing their plural forms. This might cause localization issues in certain UI contexts where plurals are needed.
+ "days": "Days", + "years": "Years",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/core/serializers.py(1 hunks)frontend/messages/en.json(1 hunks)frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/routes/(app)/(internal)/calendar/[year]/[month]/+page.svelte
🧰 Additional context used
🧬 Code Graph Analysis (1)
backend/core/serializers.py (2)
backend/core/serializer_fields.py (1)
FieldsRelatedField(24-86)backend/core/models.py (11)
risk_assessments(937-938)risk_assessments(2154-2155)Meta(124-125)Meta(179-180)Meta(207-208)Meta(212-214)Meta(811-813)Meta(877-879)Meta(1011-1013)TaskTemplate(4143-4271)TaskNode(4274-4305)
🪛 Ruff (0.8.2)
backend/core/serializers.py
1412-1412: TaskTemplate may be undefined, or defined from star imports
(F405)
1418-1418: TaskTemplate may be undefined, or defined from star imports
(F405)
1431-1431: TaskNode may be undefined, or defined from star imports
(F405)
1437-1437: TaskNode may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: test (3.12)
- GitHub Check: build (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
backend/core/serializers.py (4)
1403-1414: TaskTemplateReadSerializer is implemented correctlyThis serializer properly extends BaseModelSerializer and defines appropriate fields for serializing TaskTemplate instances, including related fields like folder, assets, applied_controls, etc. The exclusion of "schedule" field from serialization is likely intentional since it might contain complex JSON data that needs special handling in the frontend.
🧰 Tools
🪛 Ruff (0.8.2)
1412-1412:
TaskTemplatemay be undefined, or defined from star imports(F405)
1416-1420: WriteSerializer correctly exposes all fieldsThe TaskTemplateWriteSerializer effectively implements the write operations for TaskTemplate objects by exposing all fields through
fields = "__all__". This is appropriate for creating and updating task templates.🧰 Tools
🪛 Ruff (0.8.2)
1418-1418:
TaskTemplatemay be undefined, or defined from star imports(F405)
1422-1433: TaskNodeReadSerializer correctly implements name derivationThis serializer properly extends BaseModelSerializer and includes a method to derive the name from the associated task_template. The task_template field is correctly serialized using FieldsRelatedField, and the folder is included for proper context.
🧰 Tools
🪛 Ruff (0.8.2)
1431-1431:
TaskNodemay be undefined, or defined from star imports(F405)
1435-1438: TaskNodeWriteSerializer correctly excludes the template referenceThe TaskNodeWriteSerializer properly excludes the "task_template" field since this relationship is likely managed elsewhere in your application flow (probably set based on URL routing or other context).
🧰 Tools
🪛 Ruff (0.8.2)
1437-1437:
TaskNodemay be undefined, or defined from star imports(F405)
frontend/messages/en.json (1)
1417-1457: New localization keys for task management are well structuredThe added localization keys provide comprehensive coverage for the new task management features, including task status indicators, scheduling terminology, and helpful descriptions for various task fields.
backend/core/serializers.py
Outdated
| class TaskTemplateReadSerializer(BaseModelSerializer): | ||
| folder = FieldsRelatedField() | ||
| assets = FieldsRelatedField(many=True) | ||
| applied_controls = FieldsRelatedField(many=True) | ||
| compliance_assessments = FieldsRelatedField(many=True) | ||
| risk_assessments = FieldsRelatedField(many=True) | ||
| assigned_to = FieldsRelatedField(many=True) | ||
|
|
||
| class Meta: | ||
| model = TaskTemplate | ||
| exclude = ["schedule"] | ||
|
|
||
|
|
||
| class TaskTemplateWriteSerializer(BaseModelSerializer): | ||
| class Meta: | ||
| model = TaskTemplate | ||
| fields = "__all__" | ||
|
|
||
|
|
||
| class TaskNodeReadSerializer(BaseModelSerializer): | ||
| task_template = FieldsRelatedField() | ||
| folder = FieldsRelatedField() | ||
| name = serializers.SerializerMethodField() | ||
|
|
||
| def get_name(self, obj): | ||
| return obj.task_template.name if obj.task_template else "" | ||
|
|
||
| class Meta: | ||
| model = TaskNode | ||
| fields = "__all__" | ||
|
|
||
|
|
||
| class TaskNodeWriteSerializer(BaseModelSerializer): | ||
| class Meta: | ||
| model = TaskNode | ||
| exclude = ["task_template"] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify model imports to resolve static analysis warnings
The static analysis tool reports that TaskTemplate and TaskNode may be undefined or imported via star imports. Consider explicitly importing these models instead of relying on star imports for better code clarity.
🏁 Script executed:
#!/bin/bash
# Check how TaskTemplate and TaskNode are imported in the serializers.py file
grep -n "import" backend/core/serializers.py | grep -E "TaskTemplate|TaskNode|core.models"Length of output: 118
Action Required: Replace Wildcard Imports with Explicit Imports
The static analysis tool warns that TaskTemplate and TaskNode may be undefined due to the wildcard import. The grep output confirms that from core.models import * is used. Please replace the star import with explicit imports. For instance, update the import on line 9 of backend/core/serializers.py to:
from core.models import TaskTemplate, TaskNode # include other models as neededThis change will improve code clarity and resolve the static analysis warnings.
🧰 Tools
🪛 Ruff (0.8.2)
1412-1412: TaskTemplate may be undefined, or defined from star imports
(F405)
1418-1418: TaskTemplate may be undefined, or defined from star imports
(F405)
1431-1431: TaskNode may be undefined, or defined from star imports
(F405)
1437-1437: TaskNode may be undefined, or defined from star imports
(F405)
| "taskNode": "Task occurence", | ||
| "taskNodes": "Tasks occurences", | ||
| "associatedTaskNodes": "Instances review", | ||
| "nextOccurence": "Next occurence", | ||
| "taskTemplate": "Task template" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve naming consistency for task nodes
The terms "Task occurence" (line 1453) and "Tasks occurences" (line 1454) contain a spelling error ("occurence" instead of "occurrence") and inconsistent capitalization/spacing compared to other entity names.
- "taskNode": "Task occurence",
- "taskNodes": "Tasks occurences",
- "associatedTaskNodes": "Instances review",
- "nextOccurence": "Next occurence",
+ "taskNode": "Task Occurrence",
+ "taskNodes": "Task Occurrences",
+ "associatedTaskNodes": "Occurrences Review",
+ "nextOccurence": "Next Occurrence",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "taskNode": "Task occurence", | |
| "taskNodes": "Tasks occurences", | |
| "associatedTaskNodes": "Instances review", | |
| "nextOccurence": "Next occurence", | |
| "taskTemplate": "Task template" | |
| "taskNode": "Task Occurrence", | |
| "taskNodes": "Task Occurrences", | |
| "associatedTaskNodes": "Occurrences Review", | |
| "nextOccurence": "Next Occurrence", | |
| "taskTemplate": "Task template" |
- create task nodes at creation of task_template. - avoid deleting task_nodes if they are still valid, thanks to a to_delete field.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
backend/core/serializers.py (5)
1435-1439:⚠️ Potential issueTaskNodeWriteSerializer correctly excludes task_template field.
This is a good design choice as it prevents modification of the parent relationship during updates.
The static analysis tool warns that
TaskNodemay be undefined due to the wildcard import on line 9.🧰 Tools
🪛 Ruff (0.8.2)
1437-1437:
TaskNodemay be undefined, or defined from star imports(F405)
9-9:⚠️ Potential issueReplace wildcard import with explicit imports.
The new task serializers rely on models that are imported via wildcard import, which makes dependencies harder to track.
As noted in previous review comments, please replace the star import with explicit imports:
-from core.models import * +from core.models import ( + TaskTemplate, TaskNode, Folder, RoleAssignment, Permission, FilteringLabel, + ValidationError, ReferentialObjectMixin, RiskMatrix, Vulnerability, + RiskAcceptance, Perimeter, RiskAssessment, Asset, ReferenceControl, + AppliedControl, Finding, Policy, UserGroup, Role, + RequirementNode, Evidence, RequirementAssessment, ComplianceAssessment, + Framework, RequirementMappingSet, SecurityException, FindingsAssessment, + TimelineEntry, Incident, LoadedLibrary, StoredLibrary, Threat +)🧰 Tools
🪛 Ruff (0.8.2)
9-9:
from core.models import *used; unable to detect undefined names(F403)
1403-1414:⚠️ Potential issueTaskTemplateReadSerializer looks good but has import warnings.
The serializer is well-structured and consistent with other read serializers in the file, properly including related fields using FieldsRelatedField.
The static analysis tool warns that
TaskTemplatemay be undefined due to the wildcard import. Replace the star import on line 9 with explicit imports as recommended in the previous review.🧰 Tools
🪛 Ruff (0.8.2)
1412-1412:
TaskTemplatemay be undefined, or defined from star imports(F405)
1416-1420:⚠️ Potential issueWrite serializer implementation is correct.
This write serializer follows the pattern established in other parts of the codebase, allowing all fields to be written.
The static analysis tool warns that
TaskTemplatemay be undefined due to the wildcard import on line 9.🧰 Tools
🪛 Ruff (0.8.2)
1418-1418:
TaskTemplatemay be undefined, or defined from star imports(F405)
1422-1433:⚠️ Potential issueTaskNodeReadSerializer has a helpful name SerializerMethodField.
Good implementation of the
get_namemethod to ensure the task name is available even when accessing through the TaskNode model.The static analysis tool warns that
TaskNodemay be undefined due to the wildcard import on line 9.🧰 Tools
🪛 Ruff (0.8.2)
1431-1431:
TaskNodemay be undefined, or defined from star imports(F405)
backend/core/models.py (1)
4265-4271: [Duplicate from prior advice] Consider soft delete instead of permanent removal.
You’re physically deletingTaskNodeobjects beyond theend_date, which can lead to permanent data loss. A softer approach—such as marking them as expired—might be more reversible and auditable.
🧹 Nitpick comments (7)
backend/core/views.py (5)
5058-5062: NewTaskTemplateViewSetdefinition.
Introducing a dedicated viewset for task templates seems consistent with the new scheduling logic. Verify that the relevant serializer forTaskTemplateis leveraged; consider explicitly importing theTaskTemplatemodel instead of relying on wildcard imports for clarity.🧰 Tools
🪛 Ruff (0.8.2)
5061-5061:
TaskTemplatemay be undefined, or defined from star imports(F405)
5063-5137: Mismatch in “next 10 upcoming tasks.”
Intask_calendar, the comment suggests processing “all past tasks and next 10 upcoming tasks,” but the code does not limit the future tasks to 10; it simply concatenates all. Update the comment or enforce the intended 10-task limit to avoid confusion.- # Process all past tasks and next 10 upcoming tasks + # Process all past tasks and all upcoming tasks🧰 Tools
🪛 Ruff (0.8.2)
5122-5122:
TaskTemplatemay be undefined, or defined from star imports(F405)
5123-5123:
TaskNodemay be undefined, or defined from star imports(F405)
5134-5134:
TaskNodeReadSerializermay be undefined, or defined from star imports(F405)
5138-5171: Consider partial sync to prevent potential data disruption.
Currently,_sync_task_nodessoft-deletes all existingTaskNodeitems before regenerating them, which could cause unexpected behavior if a large or complex schedule only needs partial updates. If this is intentional, proceed. Otherwise, consider refining to preserve the unaffected nodes or statuses.🧰 Tools
🪛 Ruff (0.8.2)
5138-5138:
TaskTemplatemay be undefined, or defined from star imports(F405)
5140-5140:
TaskNodemay be undefined, or defined from star imports(F405)
5156-5156:
TaskTemplatemay be undefined, or defined from star imports(F405)
5162-5162:
TaskNodemay be undefined, or defined from star imports(F405)
5169-5169:
TaskNodemay be undefined, or defined from star imports(F405)
5194-5201: Watch for performance overhead on frequent updates.
perform_createandperform_updateimmediately trigger_sync_task_nodes. IfTaskTemplateobjects are frequently updated, the repeated regeneration of tasks may slow the system. Consider a background job or throttling approach for performance-critical workloads.
5203-5215: NewTaskNodeViewSetprovides direct task control.
The new viewset for task nodes adds straightforward actions. Consider adding concurrency checks or explicit merges for potential parallel updates. Also confirm that owners or assignees can only modify tasks they are permitted to see.🧰 Tools
🪛 Ruff (0.8.2)
5204-5204:
TaskNodemay be undefined, or defined from star imports(F405)
5209-5209:
TaskNodemay be undefined, or defined from star imports(F405)
5212-5212:
TaskNodemay be undefined, or defined from star imports(F405)
backend/core/models.py (2)
4138-4145: Consider removing or enhancing the custom create method.
YourTaskTemplateManager.create_task_templatecurrently just passes through tosuper().create()without adding any specialized logic or documentation. If you don't need special processing, removing this method reduces clutter; otherwise, consider adding validations or docstrings for clarity.
4274-4304: Clarify usage of both “cancelled” status andto_deletefield.
Having astatus="cancelled"option and a separateto_deleteboolean risks confusion about how tasks are ultimately handled. Consider consolidating them or clearly documenting the difference, particularly if “to_delete” is for permanent removal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/core/migrations/0067_tasktemplate_tasknode.py(1 hunks)backend/core/models.py(1 hunks)backend/core/serializers.py(1 hunks)backend/core/views.py(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/core/migrations/0067_tasktemplate_tasknode.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/core/serializers.py (2)
backend/core/serializer_fields.py (1)
FieldsRelatedField(24-86)backend/core/models.py (11)
risk_assessments(937-938)risk_assessments(2154-2155)Meta(124-125)Meta(179-180)Meta(207-208)Meta(212-214)Meta(811-813)Meta(877-879)Meta(1011-1013)TaskTemplate(4143-4271)TaskNode(4274-4307)
backend/core/views.py (2)
backend/core/utils.py (2)
_generate_occurrences(608-656)_create_task_dict(591-605)backend/core/models.py (3)
TaskTemplate(4143-4271)TaskNode(4274-4307)create(2229-2231)
backend/core/models.py (2)
backend/core/base_models.py (2)
save(106-108)AbstractBaseModel(8-108)backend/iam/models.py (5)
FolderMixin(182-195)User(340-546)save(206-213)save(395-397)delete(391-393)
🪛 Ruff (0.8.2)
backend/core/serializers.py
1412-1412: TaskTemplate may be undefined, or defined from star imports
(F405)
1418-1418: TaskTemplate may be undefined, or defined from star imports
(F405)
1431-1431: TaskNode may be undefined, or defined from star imports
(F405)
1437-1437: TaskNode may be undefined, or defined from star imports
(F405)
backend/core/views.py
2207-2207: TaskTemplate may be undefined, or defined from star imports
(F405)
5061-5061: TaskTemplate may be undefined, or defined from star imports
(F405)
5122-5122: TaskTemplate may be undefined, or defined from star imports
(F405)
5123-5123: TaskNode may be undefined, or defined from star imports
(F405)
5134-5134: TaskNodeReadSerializer may be undefined, or defined from star imports
(F405)
5138-5138: TaskTemplate may be undefined, or defined from star imports
(F405)
5140-5140: TaskNode may be undefined, or defined from star imports
(F405)
5156-5156: TaskTemplate may be undefined, or defined from star imports
(F405)
5162-5162: TaskNode may be undefined, or defined from star imports
(F405)
5169-5169: TaskNode may be undefined, or defined from star imports
(F405)
5188-5188: TaskTemplate may be undefined, or defined from star imports
(F405)
5204-5204: TaskNode may be undefined, or defined from star imports
(F405)
5209-5209: TaskNode may be undefined, or defined from star imports
(F405)
5212-5212: TaskNode may be undefined, or defined from star imports
(F405)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: startup-functional-test (3.12)
- GitHub Check: startup-docker-compose-test
- GitHub Check: enterprise-startup-functional-test (3.12)
- GitHub Check: functional-tests (3.12, chromium)
- GitHub Check: enterprise-functional-tests (3.12, chromium)
- GitHub Check: build (3.12)
- GitHub Check: test (3.12)
- GitHub Check: enterprise-startup-docker-compose-test
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
backend/core/views.py (5)
11-12: Imports look good.
Added imports fromdateutiland expandeddjango.db.modelsreferences appear to be needed for scheduling logic and aggregate queries.
94-97: Approved new util imports for task features.
Importing_generate_occurrencesand_create_task_dictalongsiderelativedelta as rdaligns well with recurring task scheduling needs.
1312-1312: Month/year lookups for 'eta' field.
Expanding filter options to include"month","year"introduces more fine-grained date filtering. Please ensure unit tests cover these new lookups.
1980-1985: Expanded filter options for expiry_date.
As with theetafield filters, adding"month"and"year"can improve date-based queries for risk acceptances. Please confirm thorough testing in both typical and edge-case date queries.
5172-5193: Verify user-based permissions for view-wide calendar retrieval.
@action(calendar)fetches tasks for all enabled templates without filtering by assigned user or verifying if anyone can view them. Ensure that user permission checks align with the application's security requirements.🧰 Tools
🪛 Ruff (0.8.2)
5188-5188:
TaskTemplatemay be undefined, or defined from star imports(F405)
backend/core/models.py (3)
4204-4253: Consider adding usage documentation or indexing for new relationships.
These new fields (assigned_to,assets,applied_controls, etc.) effectively link tasks to multiple objects. If you anticipate large datasets or frequent queries by these relationships, you might consider adding indexes or references in your documentation for query optimization. Otherwise, this is a straightforward and clean declaration.
4254-4257: No concerns with the Meta configuration.
The meta options specifying verbose names appear consistent and clear.
4258-4263: Watch out for unexpected zero-day mismatch.
The modulo logic may produce zero for Sunday if the user supplies day=7. Verify that Sunday remains represented as 7 if that’s your intended design; otherwise, adjust the operation or schema range accordingly.
| SCHEDULE_JSONSCHEMA = { | ||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||
| "title": "Schedule Definition", | ||
| "type": "object", | ||
| "properties": { | ||
| "interval": { | ||
| "type": "integer", | ||
| "minimum": 1, | ||
| "description": "Number of periods to wait before repeating (e.g., every 2 days, 3 weeks).", | ||
| }, | ||
| "frequency": { | ||
| "type": "string", | ||
| "enum": ["DAILY", "WEEKLY", "MONTHLY", "YEARLY"], | ||
| }, | ||
| "days_of_week": { | ||
| "type": "array", | ||
| "items": {"type": "integer", "minimum": 1, "maximum": 7}, | ||
| "description": "Optional. Days of the week (Monday=1, Sunday=7)", | ||
| }, | ||
| "weeks_of_month": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "integer", | ||
| "minimum": -1, | ||
| "maximum": 4, | ||
| }, | ||
| "description": "Optional. for a given weekday, which one in the month (1 for first, -1 for last)", | ||
| }, | ||
| "months_of_year": { | ||
| "type": "array", | ||
| "items": {"type": "integer", "minimum": 1, "maximum": 12}, | ||
| "description": "Optional. Months of the year (1=January, 12=December)", | ||
| }, | ||
| "end_date": { | ||
| "type": ["string", "null"], | ||
| "format": "date", | ||
| "description": "Optional. Date when recurrence ends.", | ||
| }, | ||
| "occurrences": { | ||
| "type": ["integer", "null"], | ||
| "minimum": 1, | ||
| "description": "Optional. Number of occurrences before recurrence stops.", | ||
| }, | ||
| "overdue_behavior": { | ||
| "type": "string", | ||
| "enum": ["DELAY_NEXT", "NO_IMPACT"], | ||
| "default": "NO_IMPACT", | ||
| "description": "Optional. Behavior when tasks become overdue.", | ||
| }, | ||
| "exceptions": { | ||
| "type": ["object", "null"], | ||
| "description": "Optional. JSON object for future exceptions handling.", | ||
| }, | ||
| }, | ||
| "required": ["interval", "frequency"], | ||
| "additionalProperties": False, | ||
| } | ||
|
|
There was a problem hiding this comment.
Validate handling of day=7 in days_of_week.
Your JSON schema enforces 1 <= day <= 7, but the code later performs day % 7 if day > 6 else day, which can yield 0 when day=7. This conflicts with the schema’s allowed range. Ensure you handle Sunday=7 consistently without producing an out-of-range 0.
Summary by CodeRabbit
New Features
TaskTemplateandTaskNode.UI & Navigation
Localization
Tests
Documentation