fix(fusion_clock): code-review hardening [19.0.5.0.1]
- _cron_generate: per-rule savepoint isolation (one bad rule can't abort the whole daily batch) - fclk_attach_recurrence: clear an existing recurrence first (no orphaned rule generating forever) - fclk_apply_planner_cell: collapse split rows (search was limit=1 after the UNIQUE drop, orphaning extras) - fclk_release_shift: reject non-posted/open shifts (raw-POST guard) - delete_open_shift: report success=false when nothing was deleted + JS surfaces it - _generate: log before removing an empty recurrence Tests added for collapse, re-attach, draft-release. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -96,7 +96,9 @@ class FusionClockScheduleRecurrence(models.Model):
|
||||
seed = Schedule.search(
|
||||
[('recurrence_id', '=', rec.id)], order='schedule_date desc', limit=1)
|
||||
if not seed:
|
||||
# No anchor row -> nothing to repeat; drop the empty rule.
|
||||
# No anchor row left -> nothing to repeat; drop the empty rule.
|
||||
_logger.info(
|
||||
"Fusion Clock: recurrence %s has no shifts left; removing it.", rec.id)
|
||||
rec.unlink()
|
||||
continue
|
||||
anchor = Schedule.search(
|
||||
@@ -150,5 +152,13 @@ class FusionClockScheduleRecurrence(models.Model):
|
||||
|
||||
@api.model
|
||||
def _cron_generate(self):
|
||||
"""Roll every recurrence's horizon forward (called daily)."""
|
||||
self.search([])._generate()
|
||||
"""Roll every recurrence's horizon forward (called daily). Each rule is
|
||||
isolated in its own savepoint so one bad recurrence can't abort
|
||||
generation for all the others."""
|
||||
for rec in self.search([]):
|
||||
try:
|
||||
with self.env.cr.savepoint():
|
||||
rec._generate()
|
||||
except Exception:
|
||||
_logger.exception(
|
||||
"Fusion Clock: recurrence %s failed to generate; skipping.", rec.id)
|
||||
|
||||
@@ -332,16 +332,21 @@ class FusionClockSchedule(models.Model):
|
||||
if not employee.exists() or not date_obj:
|
||||
raise ValidationError(_("Invalid employee or schedule date."))
|
||||
|
||||
existing = self.search([
|
||||
# The UNIQUE(employee, date) constraint was dropped to allow split
|
||||
# shifts, so a day can hold several rows. The planner cell is the single
|
||||
# authoritative entry for the day: an edit collapses to one row, a clear
|
||||
# removes them all. (Open shifts have no employee, so they never match.)
|
||||
same_day = self.search([
|
||||
('employee_id', '=', employee.id),
|
||||
('schedule_date', '=', date_obj),
|
||||
], limit=1)
|
||||
], order='id')
|
||||
existing = same_day[:1]
|
||||
old_value = self.fclk_snapshot(existing)
|
||||
parsed = self.fclk_values_from_planner_payload(payload, employee)
|
||||
|
||||
if parsed.get('clear'):
|
||||
if existing:
|
||||
existing.unlink()
|
||||
if same_day:
|
||||
same_day.unlink()
|
||||
new_schedule = self.browse()
|
||||
new_value = ''
|
||||
else:
|
||||
@@ -373,6 +378,7 @@ class FusionClockSchedule(models.Model):
|
||||
if existing:
|
||||
existing.write(vals)
|
||||
new_schedule = existing
|
||||
(same_day - existing).unlink() # collapse any split rows into one
|
||||
else:
|
||||
new_schedule = self.create(vals)
|
||||
new_value = new_schedule.fclk_display_value()
|
||||
@@ -446,6 +452,10 @@ class FusionClockSchedule(models.Model):
|
||||
schedule = schedule.sudo()
|
||||
if not schedule:
|
||||
raise ValidationError(_("Pick a shift to repeat first."))
|
||||
# Replace any existing recurrence so we never orphan a rule that would
|
||||
# otherwise keep generating shifts forever.
|
||||
if schedule.recurrence_id:
|
||||
self.fclk_clear_recurrence(schedule)
|
||||
rule = self.env['fusion.clock.schedule.recurrence'].sudo().create({
|
||||
'repeat_interval': int(repeat_vals.get('repeat_interval') or 1),
|
||||
'repeat_unit': repeat_vals.get('repeat_unit') or 'week',
|
||||
@@ -517,6 +527,8 @@ class FusionClockSchedule(models.Model):
|
||||
schedule = schedule.sudo()
|
||||
if not schedule or schedule.employee_id != employee.sudo():
|
||||
raise ValidationError(_("You can only release your own shift."))
|
||||
if schedule.state != 'posted' or schedule.is_open:
|
||||
raise ValidationError(_("Only a posted shift assigned to you can be released."))
|
||||
cutoff = schedule.company_id.fclk_self_unassign_days_before or 0
|
||||
if (schedule.schedule_date - fields.Date.today()).days < cutoff:
|
||||
raise ValidationError(_("It is too late to release this shift."))
|
||||
|
||||
Reference in New Issue
Block a user