Enhance shift constraints validation and error handling in forms and templates

djangorota/djangorota/templates/rota/partials/shift_form_modal.html

@@ -22,6 +22,7 @@ function _close_shift_modal_targets(){
         <div class="box" id="constraints-editor">
           <h4 class="title is-6">Shift constraints</h4>
           <p class="help">Add constraint entries. The form stores a JSON list; this editor helps build that list.</p>
+          <div id="constraint-errors" class="notification is-danger" style="display:none; margin-bottom:0.5em;"></div>
           <div id="constraint-list"></div>
           <div style="margin-top:0.5em;">
             <button type="button" class="button is-small" id="add-constraint">Add constraint</button>
@@ -45,7 +46,7 @@ function _close_shift_modal_targets(){
               {name: "weeks", label: "Weeks (comma-separated integers)", type: "list_int"},
               {name: "start_date", label: "Start date", type: "date"},
               {name: "end_date", label: "End date", type: "date"},
-              {name: "days", label: "Days (integer)", type: "int"},
+              {name: "days", label: "Duration (days)", type: "int", required: true},
               {name: "ignore_shifts", label: "Ignore shifts (comma-separated)", type: "list"},
               {name: "exclude_days", label: "Exclude days (comma-separated)", type: "list"},
               {name: "allow_self", label: "Allow self", type: "bool"}
@@ -55,7 +56,7 @@ function _close_shift_modal_targets(){
               {name: "weeks", label: "Weeks (comma-separated integers)", type: "list_int"},
               {name: "start_date", label: "Start date", type: "date"},
               {name: "end_date", label: "End date", type: "date"},
-              {name: "days", label: "Days (integer)", type: "int"},
+              {name: "days", label: "Duration (days)", type: "int", required: true},
               {name: "ignore_shifts", label: "Ignore shifts (comma-separated)", type: "list"},
               {name: "exclude_days", label: "Exclude days (comma-separated)", type: "list"},
               {name: "allow_self", label: "Allow self", type: "bool"}
@@ -70,8 +71,31 @@ function _close_shift_modal_targets(){
 
           const textarea = document.getElementById('id_constraints');
           const listEl = document.getElementById('constraint-list');
+          const errorsEl = document.getElementById('constraint-errors');
           const addBtn = document.getElementById('add-constraint');
 
+          function validateConstraintsArray(data){
+            const msgs = [];
+            if(!Array.isArray(data)) return ["Constraints must be a JSON list"];
+            data.forEach((c, idx)=>{
+              if(!c || typeof c !== 'object'){
+                msgs.push(`Constraint #${idx+1} must be an object`);
+                return;
+              }
+              const name = c.name || c.type || null;
+              if(!name){ msgs.push(`Constraint #${idx+1} missing 'name'`); return; }
+              if(name==='pre' || name==='post'){
+                const opts = c.options || {};
+                // days (duration) is required
+                const daysVal = (c.days!==undefined && c.days!==null) ? c.days : (opts.days!==undefined && opts.days!==null ? opts.days : null);
+                if(daysVal===null){
+                  msgs.push(`Constraint #${idx+1} (pre/post) requires 'Duration (days)' to be set`);
+                }
+              }
+            });
+            return msgs;
+          }
+
           function parseInitial(){
             let val = textarea.value || '';
             if(!val.trim()) return [];
@@ -106,6 +130,12 @@ function _close_shift_modal_targets(){
               }catch(e){ /* ignore */ }
             }
             syncTextarea();
+            // validate and show errors
+            try{
+              const v = validateConstraintsArray(data);
+              if(v && v.length){ errorsEl.style.display='block'; errorsEl.innerHTML = v.map(m=>`<div>${m}</div>`).join(''); }
+              else { errorsEl.style.display='none'; errorsEl.innerHTML=''; }
+            }catch(e){ /* ignore */ }
           }
 
           function renderConstraintRow(c, idx){

djangorota/rota/forms.py

@@ -663,6 +663,26 @@ class ShiftForm(forms.Form):
             parsed = json.loads(val)
             if not isinstance(parsed, list):
                 raise forms.ValidationError("Constraints must be a JSON list of constraint objects")
+            # Validate each constraint object for basic correctness
+            errors = []
+            for i, c in enumerate(parsed):
+                if not isinstance(c, dict):
+                    errors.append(f"Constraint #{i+1} must be an object")
+                    continue
+                name = c.get('name') or c.get('type')
+                if not name:
+                    errors.append(f"Constraint #{i+1} missing 'name'")
+                    continue
+                if name in ('pre', 'post'):
+                    opts = c.get('options', {}) or {}
+                    # days is now required: check top-level or under options
+                    days_val = c.get('days') if c.get('days') is not None else opts.get('days')
+                    if days_val is None:
+                        errors.append(
+                            f"Constraint #{i+1} ('{name}') requires a 'days' duration (number of days the constraint applies for)."
+                        )
+            if errors:
+                raise forms.ValidationError(errors)
             return parsed
         except forms.ValidationError:
             raise

rota_generator/shifts.py

@@ -68,9 +68,21 @@ SHIFT_BOUNDS = {
 #]
 
 class BaseShiftConstraint(BaseModel):
-    weeks: Optional[List[int]] = None
+    weeks: Optional[List[int]] = Field(
-    start_date: Optional[datetime.date] = None
+        None,
-    end_date: Optional[datetime.date] = None
+        description=(
+            "List of week numbers when this constraint is active. "
+            "These are selectors that limit WHEN the constraint applies; they must not be used together with start_date/end_date."
+        ),
+    )
+    start_date: Optional[datetime.date] = Field(
+        None,
+        description="Inclusive start date for the period when this constraint is active (cannot be used with `weeks`).",
+    )
+    end_date: Optional[datetime.date] = Field(
+        None,
+        description="Inclusive end date for the period when this constraint is active (cannot be used with `weeks`).",
+    )
 
     model_config = ConfigDict(arbitrary_types_allowed=True, extra="forbid")
 
@@ -90,18 +102,41 @@ class NightConstraint(BaseShiftConstraint):
     options: Any = None
 
 class PreShiftConstraint(BaseShiftConstraint):
-    days: Optional[int] = None
+    """Pre-shift constraint.
-    ignore_shifts: List[str] = []
+
-    exclude_days: List[str] = []
+    Semantics:
-    exclude_dates: List[datetime.date] = []
+    - `weeks`, `start_date`, `end_date` (inherited) are selectors that determine WHEN the constraint is active (which weeks or date window).
-    allow_self: bool = True
+    - `days` is a DURATION: the number of days the constraint applies for (e.g. 3 means apply the constraint for 3 days relative to the shift). This field is required.
+    - `ignore_shifts`, `exclude_days`, `exclude_dates` further refine which shifts/dates are considered.
+    """
+    days: int = Field(
+        ...,
+        description=(
+            "Duration in days: the number of days the constraint applies for. "
+            "This is different from selector fields (weeks/start_date/end_date) which select WHEN the constraint is active."
+        ),
+    )
+    ignore_shifts: List[str] = Field(default_factory=list, description="List of shift names to ignore when applying this constraint")
+    exclude_days: List[str] = Field(default_factory=list, description="Weekday names to exclude (e.g. ['Mon','Tue'])")
+    exclude_dates: List[datetime.date] = Field(default_factory=list, description="Specific dates to exclude from the constraint")
+    allow_self: bool = Field(True, description="Allow the same worker to satisfy this constraint (default: True)")
 
 class PostShiftConstraint(BaseShiftConstraint):
-    days: Optional[int] = None
+    """Post-shift constraint.
-    ignore_shifts: List[str]  = []
+
-    exclude_days: List[str] = []
+    See PreShiftConstraint docs: selectors (weeks/start_date/end_date) choose WHEN the constraint is active; `days` is a duration and is required.
-    exclude_dates: List[datetime.date] = []
+    """
-    allow_self: bool = True
+    days: int = Field(
+        ...,
+        description=(
+            "Duration in days: the number of days the constraint applies for after the shift. "
+            "Different from selector fields which choose which weeks/dates the constraint is active on."
+        ),
+    )
+    ignore_shifts: List[str] = Field(default_factory=list, description="List of shift names to ignore when applying this constraint")
+    exclude_days: List[str] = Field(default_factory=list, description="Weekday names to exclude (e.g. ['Mon','Tue'])")
+    exclude_dates: List[datetime.date] = Field(default_factory=list, description="Specific dates to exclude from the constraint")
+    allow_self: bool = Field(True, description="Allow the same worker to satisfy this constraint (default: True)")
 
 class BalanceAcrossGroupsConstraint(BaseShiftConstraint):
     """"""