hr_timesheet_sheet* fixes: back to list after remove a timesheet. bulk unfreeze confirmed and pending sheets

This commit is contained in:
Luis 2026-10-01 09:38:17 +02:00
parent 8b0d0bb6cc
commit 0323b6672e
11 changed files with 354 additions and 34 deletions

View file

@ -304,5 +304,9 @@ class Sheet(models.Model):
active_model="hr_timesheet.sheet",
active_ids=self.ids,
active_id=self.ids[0] if self.ids else False,
# The button lives in the header of the sheet form: once the
# sheet is gone that form cannot be restored, and the wizard
# has to send the user somewhere else.
sheet_delete_cascade_from_form=True,
),
}

View file

@ -8,6 +8,13 @@ section that depends on the configured policy. Nothing is deleted until
**Delete permanently** is pressed, and the button is hidden when something
blocks the deletion.
Deleting from the form leaves the user on the list of timesheet sheets, since
the form he was on shows a sheet that no longer exists: *My Timesheet Sheets*,
or *All Timesheet Sheets* when a timesheet administrator deletes the sheet of
another employee. The deleted sheet is not kept in the breadcrumb. Deleting
several sheets from the **Action** menu of a list leaves that list where it
was, with its filters and its grouping.
A plain ``sheet.unlink()`` from a script or a server action raises a
``UserError`` that points to the button, so the confirmation step cannot be
skipped by accident. Code that really means to delete does

View file

@ -44,7 +44,7 @@ class TestHrTimesheetSheetDeleteCascade(TransactionCase):
cls.employee = cls._new_employee("Cascade Employee", cls.user)
cls.employee_other = cls._new_employee("Other Employee", cls.user_other)
cls.employee_span = cls._new_employee("Span Employee", cls.user_span)
cls._new_employee("Cascade Manager", cls.user_manager)
cls.employee_manager = cls._new_employee("Cascade Manager", cls.user_manager)
cls.project = cls.env["project.project"].create(
{
@ -215,6 +215,16 @@ class TestHrTimesheetSheetDeleteCascade(TransactionCase):
.create({})
)
def _wizard_from_form(self, sheets, user=None):
user = user or self.user
action = sheets.with_user(user).action_delete_sheet()
return (
self.env["hr_timesheet.sheet.delete.wizard"]
.with_user(user)
.with_context(**action["context"])
.create({})
)
def _leave_balance(self, employee):
leave_type = self.leave_type.with_context(employee_id=employee.id)
leave_type.invalidate_recordset()
@ -357,6 +367,63 @@ class TestHrTimesheetSheetDeleteCascade(TransactionCase):
self.assertEqual(action["res_model"], "hr_timesheet.sheet.delete.wizard")
self.assertEqual(action["context"]["active_ids"], self.sheet.ids)
# Navigation once the sheet the user was looking at is gone
def test_delete_from_form_returns_to_the_sheet_list(self):
wizard = self._wizard_from_form(self.sheet)
action = wizard.action_confirm_delete()
self.assertEqual(action["type"], "ir.actions.act_window")
self.assertEqual(action["res_model"], "hr_timesheet.sheet")
self.assertEqual(action["target"], "main")
self.assertIn("tree", action["view_mode"])
self.assertFalse(self.sheet.exists())
def test_delete_from_form_returns_own_sheets_to_an_employee(self):
action = self._wizard_from_form(self.sheet).action_confirm_delete()
self.assertEqual(
action["xml_id"],
"hr_timesheet_sheet.act_hr_timesheet_sheet_my_timesheets",
)
def test_delete_from_form_returns_own_sheets_to_a_manager(self):
manager_sheet = self._new_sheet(self.employee_manager)
action = self._wizard_from_form(
manager_sheet, user=self.user_manager
).action_confirm_delete()
self.assertEqual(
action["xml_id"],
"hr_timesheet_sheet.act_hr_timesheet_sheet_my_timesheets",
)
def test_delete_of_another_employee_sheet_returns_all_sheets(self):
action = self._wizard_from_form(
self.sheet_other, user=self.user_manager
).action_confirm_delete()
self.assertEqual(
action["xml_id"],
"hr_timesheet_sheet.act_hr_timesheet_sheet_all_timesheets",
)
self.assertFalse(self.sheet_other.exists())
def test_delete_from_list_stays_on_the_list(self):
action = self._wizard(self.sheet).action_confirm_delete()
self.assertEqual(action, {"type": "ir.actions.act_window_close"})
self.assertFalse(self.sheet.exists())
def test_wizard_is_not_bound_to_the_sheet_form(self):
action = self.env.ref(
"hr_timesheet_sheet_delete_cascade"
".action_hr_timesheet_sheet_delete_wizard"
)
self.assertNotIn("form", action.binding_view_types)
def test_blocked_deletion_navigates_nowhere(self):
self.regular_line.timesheet_invoice_id = self._invoice()
wizard = self._wizard_from_form(self.sheet)
with self.assertRaises(UserError):
wizard.action_confirm_delete()
self.assertTrue(self.sheet.exists())
# RF-02
def test_cascade_deletes_timesheets(self):

View file

@ -107,7 +107,46 @@ class HrTimesheetSheetDeleteWizard(models.TransientModel):
)
return Markup("").join(parts)
def _delete_cascade_list_action_xml_id(self):
"""Hook for extensions
The list the user plausibly comes from. Deleting the sheet of another
employee requires *Timesheets / Administrator*, and that sheet is only
listed by *All Timesheet Sheets*; every other case is a user looking at
his own sheets, and *My Timesheet Sheets* is the only list of sheets an
employee outside that group has a menu for.
"""
self.ensure_one()
own_sheets = self.sheet_ids.user_id == self.env.user
is_manager = self.env.user.has_group("hr_timesheet.group_timesheet_manager")
if own_sheets or not is_manager:
return "hr_timesheet_sheet.act_hr_timesheet_sheet_my_timesheets"
return "hr_timesheet_sheet.act_hr_timesheet_sheet_all_timesheets"
def _delete_cascade_next_action(self):
"""Hook for extensions
Called before the deletion, while the sheets can still be read.
Opened from a list, the caller is left on that list: closing the dialog
reloads it and keeps its filters, its grouping and its selection, and
there is no dead record on screen. Opened from the form of a sheet that
no longer exists, closing the dialog would leave that form on screen
with the values of a deleted record: return the list of sheets instead,
with ``target="main"`` so the deleted form is dropped from the
breadcrumb rather than kept one step behind.
"""
self.ensure_one()
if not self.env.context.get("sheet_delete_cascade_from_form"):
return {"type": "ir.actions.act_window_close"}
action = self.env["ir.actions.act_window"]._for_xml_id(
self._delete_cascade_list_action_xml_id()
)
action["target"] = "main"
return action
def action_confirm_delete(self):
self.ensure_one()
next_action = self._delete_cascade_next_action()
self.sheet_ids._with_delete_cascade().unlink()
return {"type": "ir.actions.act_window_close"}
return next_action

View file

@ -55,6 +55,11 @@
name="binding_model_id"
ref="hr_timesheet_sheet.model_hr_timesheet_sheet"
/>
<field name="binding_view_types">list,kanban,form</field>
<!--
Not bound to the form view: there the Delete Sheet button of the
header is the only entry point, and it is the one that tells the
wizard the sheet form has to be left behind once the sheet is gone.
-->
<field name="binding_view_types">list,kanban</field>
</record>
</odoo>

View file

@ -6,8 +6,8 @@ msgid ""
msgstr ""
"Project-Id-Version: Odoo Server 16.0\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2026-09-15 07:59+0000\n"
"PO-Revision-Date: 2026-09-15 07:59+0000\n"
"POT-Creation-Date: 2026-09-30 16:47+0000\n"
"PO-Revision-Date: 2026-09-30 16:47+0000\n"
"Last-Translator: \n"
"Language-Team: \n"
"MIME-Version: 1.0\n"
@ -57,6 +57,11 @@ msgstr ""
msgid "Frozen Sheets"
msgstr ""
#. module: hr_timesheet_sheet_no_review
#: model:ir.model.fields,field_description:hr_timesheet_sheet_no_review.field_hr_timesheet_sheet_unfreeze_wizard__sheet_ids
msgid "Frozen Sheets To Release"
msgstr ""
#. module: hr_timesheet_sheet_no_review
#: model:ir.model.fields,field_description:hr_timesheet_sheet_no_review.field_hr_timesheet_sheet_unfreeze_wizard__id
msgid "ID"

View file

@ -4,6 +4,7 @@
import logging
from odoo import api, models
from odoo.osv import expression
_logger = logging.getLogger(__name__)
@ -17,14 +18,24 @@ class HrTimesheetSheet(models.Model):
return [("state", "in", ("confirm", "done"))]
@api.model
def _unfreeze_sheets(self):
def _unfreeze_sheets(self, sheets=None):
"""Send frozen sheets back to draft. Returns the number of sheets affected.
Without ``sheets``, every frozen sheet the current user can see is
released, which is the historical behaviour of this hook. With
``sheets``, only those of them that are still frozen are released: the
wizard passes the recordset it showed the user, so the confirmation
message counts what the warning announced and not what the database
happens to hold by the time the button is pressed.
The update is issued in SQL on purpose: going through ``write()`` would
trigger the sheet recomputation and one tracking message per sheet, on a
volume that is measured in thousands of records.
"""
sheets = self.search(self._unfreeze_domain())
domain = self._unfreeze_domain()
if sheets is not None:
domain = expression.AND([domain, [("id", "in", sheets.ids)]])
sheets = self.search(domain)
if not sheets:
_logger.info("No frozen timesheet sheets to unfreeze.")
return 0

View file

@ -9,6 +9,12 @@ To release the sheets that were left frozen by the review workflow, go to
many sheets, how many timesheet entries and how many hours are about to be
released. Confirming sets those sheets back to draft and clears their reviewer.
The wizard releases exactly the sheets it listed when it was opened, and the
confirmation message counts them. A sheet frozen by somebody else while the
dialog was on screen is not released: it shows up the next time the wizard is
opened. Only the frozen sheets the user can see are listed, so on a
multi-company database the figures follow the company switcher.
Installing the module never changes the state of a sheet on its own: unfreezing
is always an explicit decision of the timesheet manager. Running the wizard
twice is safe — the second run reports zero sheets and changes nothing.
@ -19,4 +25,11 @@ once and the reviewer of the approved ones is lost.
To decide which sheets count as frozen, override
``hr_timesheet.sheet._unfreeze_domain()``. For instance, an installation that
wants to keep its approved sheets as historical record can narrow it down to
``[("state", "=", "confirm")]``.
``[("state", "=", "confirm")]``. The domain is applied twice: once to list the
sheets in the dialog, and again when the button is pressed, so a sheet it keeps
out is never released.
``hr_timesheet.sheet._unfreeze_sheets()`` takes an optional recordset. Called
with no argument it releases every frozen sheet, as before; called with a
recordset it releases only those of them that the domain still considers
frozen.

View file

@ -4,11 +4,12 @@
import os
from unittest.mock import patch
from dateutil.relativedelta import relativedelta
from lxml import etree
from odoo.exceptions import AccessError, UserError
from odoo.modules.module import get_manifest, get_module_path
from odoo.tests.common import TransactionCase
from odoo.tests.common import Form, TransactionCase
MODULE = "hr_timesheet_sheet_no_review"
REVIEW_ACTIONS = [
@ -285,7 +286,62 @@ class TestHrTimesheetSheetNoReview(TransactionCase):
"%s must only install technical records" % MODULE,
)
def test_wizard_reports_pending_sheets(self):
# The figures reach the interface
def test_wizard_reports_pending_sheets_in_the_interface(self):
"""The path the user walks: default_get() + onchange(), no create().
Opening an action with ``target="new"`` never creates the record: the
client builds a virtual one and the server decides what to compute from
the dependency graph. A compute with no ``@api.depends`` is skipped and
the dialog reports zero on a database full of frozen sheets.
"""
form = Form(self.wizard_model)
self.assertEqual(form.sheet_count, self.baseline["sheets"] + 1)
self.assertEqual(form.timesheet_count, self.baseline["timesheets"] + 1)
self.assertAlmostEqual(
form.timesheet_hours, self.baseline["hours"] + 3.0, places=2
)
self.assertNotIn("Nothing to unfreeze", form.summary_html)
self.assertIn(self.sheet, form.sheet_ids)
def test_wizard_reports_pending_sheets_through_raw_onchange(self):
"""Same flow, called the way the web client calls it.
``Form`` is a faithful client, but it is still test code. This drives
``onchange()`` directly with the field list of the real view, so the
test also fails if the view stops exposing ``sheet_ids`` and
``default_get()`` is therefore never asked for it.
"""
arch = self._arch(
"hr_timesheet_sheet_no_review.hr_timesheet_sheet_unfreeze_wizard_form",
"form",
model=self.wizard_model._name,
)
names = [node.get("name") for node in arch.xpath("//field")]
self.assertIn("sheet_ids", names)
values = self.wizard_model.onchange({}, [], {name: "1" for name in names})[
"value"
]
self.assertEqual(values["sheet_count"], self.baseline["sheets"] + 1)
self.assertEqual(values["timesheet_count"], self.baseline["timesheets"] + 1)
self.assertAlmostEqual(
values["timesheet_hours"], self.baseline["hours"] + 3.0, places=2
)
self.assertNotIn("Nothing to unfreeze", values["summary_html"])
def test_wizard_figures_depend_on_a_real_field(self):
"""Guard: the figures must keep a dependency to be computed at all."""
for name in ("sheet_count", "timesheet_count", "timesheet_hours"):
field = self.wizard_model._fields[name]
self.assertEqual(
self.env.registry.field_depends[field],
("sheet_ids",),
"%s must depend on sheet_ids or it is never computed in the UI" % name,
)
def test_wizard_reports_pending_sheets_from_code(self):
"""``create({})`` is the shell path, used by the diagnosis probe."""
wizard = self.wizard_model.create({})
self.assertEqual(wizard.sheet_count, self.baseline["sheets"] + 1)
self.assertEqual(wizard.timesheet_count, self.baseline["timesheets"] + 1)
@ -296,17 +352,22 @@ class TestHrTimesheetSheetNoReview(TransactionCase):
def test_wizard_reports_nothing_to_unfreeze(self):
self.sheet_model._unfreeze_sheets()
wizard = self.wizard_model.create({})
self.assertEqual(wizard.sheet_count, 0)
self.assertEqual(wizard.timesheet_count, 0)
self.assertEqual(wizard.timesheet_hours, 0.0)
form = Form(self.wizard_model)
self.assertEqual(form.sheet_count, 0)
self.assertEqual(form.timesheet_count, 0)
self.assertEqual(form.timesheet_hours, 0.0)
self.assertEqual(len(form.sheet_ids), 0)
self.assertIn("Nothing to unfreeze", form.summary_html)
wizard = form.save()
self.assertIn("Nothing to unfreeze", wizard.summary_html)
# Unfreezing
def test_unfreeze_releases_sheets(self):
self.assertEqual(self.sheet.state, "confirm")
wizard = self.wizard_model.create({})
expected = wizard.sheet_count
result = wizard.action_unfreeze()
form = Form(self.wizard_model)
expected = form.sheet_count
result = form.save().action_unfreeze()
self.assertEqual(self.sheet.state, "draft")
self.assertFalse(self.sheet.reviewer_id)
self.assertEqual(
@ -314,24 +375,105 @@ class TestHrTimesheetSheetNoReview(TransactionCase):
)
self.assertIn(str(expected), result["params"]["message"])
def test_unfreeze_only_releases_the_sheets_it_announced(self):
"""The confirmation must count what the warning announced.
The wizard is built, and only then another sheet is frozen. That sheet
was not on screen, so it is not released and it is not counted.
"""
wizard = Form(self.wizard_model).save()
announced = wizard.sheet_count
late_sheet = self.sheet_model.create(
{
"employee_id": self.employee.id,
"company_id": self.company.id,
"date_start": self.sheet.date_start + relativedelta(months=1),
"date_end": self.sheet.date_end + relativedelta(months=1),
}
)
late_sheet.action_timesheet_confirm()
result = wizard.action_unfreeze()
self.assertIn(str(announced), result["params"]["message"])
self.assertEqual(self.sheet.state, "draft")
self.assertEqual(late_sheet.state, "confirm")
def test_unfreeze_ignores_sheets_outside_the_domain(self):
"""Operating on a given recordset must not relax the domain.
``sheet_ids`` comes from the client and can be tampered with. The
sheets that ``_unfreeze_domain()`` keeps out stay out, whatever the
wizard is told to release.
"""
draft_sheet = self.sheet_model.create(
{
"employee_id": self.employee.id,
"company_id": self.company.id,
"date_start": self.sheet.date_start + relativedelta(months=2),
"date_end": self.sheet.date_end + relativedelta(months=2),
}
)
self.assertEqual(draft_sheet.state, "draft")
kept_id = self.sheet.id
with patch.object(
type(self.sheet_model),
"_unfreeze_domain",
lambda records: [
("state", "in", ("confirm", "done")),
("id", "!=", kept_id),
],
):
self.assertNotIn(self.sheet, Form(self.wizard_model).sheet_ids)
wizard = self.wizard_model.create(
{"sheet_ids": [(6, 0, (self.sheet + draft_sheet).ids)]}
)
result = wizard.action_unfreeze()
self.assertEqual(self.sheet.state, "confirm")
self.assertEqual(draft_sheet.state, "draft")
self.assertEqual(result["params"]["type"], "info")
def test_unfreeze_is_idempotent(self):
self.assertTrue(self.sheet_model._unfreeze_sheets())
self.assertEqual(self.sheet_model._unfreeze_sheets(), 0)
wizard = self.wizard_model.create({})
result = wizard.action_unfreeze()
result = Form(self.wizard_model).save().action_unfreeze()
self.assertEqual(
self.sheet_model.search_count([("state", "in", ("confirm", "done"))]), 0
)
self.assertEqual(result["params"]["type"], "info")
self.assertEqual(self.sheet.state, "draft")
def test_unfreeze_twice_from_the_interface_is_idempotent(self):
first = Form(self.wizard_model).save()
second = Form(self.wizard_model).save()
self.assertTrue(first.action_unfreeze()["params"]["type"] == "success")
result = second.action_unfreeze()
self.assertEqual(result["params"]["type"], "info")
self.assertEqual(self.sheet.state, "draft")
self.assertFalse(self.sheet.reviewer_id)
def test_unfrozen_lines_are_editable(self):
with self.assertRaises(UserError):
self.line.with_user(self.user).write({"unit_amount": 5.0})
self.wizard_model.create({}).action_unfreeze()
Form(self.wizard_model).save().action_unfreeze()
self.line.with_user(self.user).write({"unit_amount": 5.0})
self.assertEqual(self.line.unit_amount, 5.0)
def test_unfreeze_sheets_hook_keeps_its_signature(self):
"""``_unfreeze_sheets()`` is public: calling it with no argument must
keep releasing every frozen sheet."""
self.assertEqual(
self.sheet_model._unfreeze_sheets(), self.baseline["sheets"] + 1
)
self.assertEqual(self.sheet.state, "draft")
def test_unfreeze_sheets_accepts_a_recordset(self):
self.assertEqual(self.sheet_model._unfreeze_sheets(self.sheet), 1)
self.assertEqual(self.sheet.state, "draft")
self.assertEqual(self.sheet_model._unfreeze_sheets(self.sheet), 0)
no_sheet = self.sheet_model.browse()
self.assertEqual(self.sheet_model._unfreeze_sheets(no_sheet), 0)
def test_unfreeze_domain_hook(self):
self.assertEqual(
self.sheet_model._unfreeze_domain(),
@ -355,4 +497,6 @@ class TestHrTimesheetSheetNoReview(TransactionCase):
def test_wizard_is_restricted_to_managers(self):
with self.assertRaises(AccessError):
self.wizard_model.with_user(self.user).create({})
self.assertTrue(self.wizard_model.with_user(self.manager).create({}))
manager_form = Form(self.wizard_model.with_user(self.manager))
self.assertTrue(manager_form.sheet_count)
self.assertTrue(manager_form.save())

View file

@ -10,6 +10,10 @@ class HrTimesheetSheetUnfreezeWizard(models.TransientModel):
_name = "hr_timesheet.sheet.unfreeze.wizard"
_description = "Unfreeze Pending Timesheet Sheets"
sheet_ids = fields.Many2many(
comodel_name="hr_timesheet.sheet",
string="Frozen Sheets To Release",
)
sheet_count = fields.Integer(
string="Frozen Sheets",
compute="_compute_frozen_data",
@ -27,21 +31,36 @@ class HrTimesheetSheetUnfreezeWizard(models.TransientModel):
compute="_compute_summary_html",
)
# No @api.depends: the figures come from the state of the whole database,
# not from any field of the wizard. The wizard is created anew every time
# the action is opened, so they are computed once, when the dialog is read.
@api.model
def default_get(self, fields_list):
res = super().default_get(fields_list)
if "sheet_ids" in fields_list:
sheet_model = self.env["hr_timesheet.sheet"]
sheets = sheet_model.search(sheet_model._unfreeze_domain())
res["sheet_ids"] = [fields.Command.set(sheets.ids)]
return res
@api.depends("sheet_ids")
def _compute_frozen_data(self):
sheet_model = self.env["hr_timesheet.sheet"]
sheets = sheet_model.search(sheet_model._unfreeze_domain())
groups = self.env["account.analytic.line"].read_group(
[("sheet_id", "in", sheets.ids)], ["unit_amount"], []
)
timesheet_count = groups[0]["__count"] if groups else 0
timesheet_hours = (groups[0]["unit_amount"] if groups else 0.0) or 0.0
"""The figures describe ``sheet_ids``, not the database.
Depending on a real field is what makes the figures appear in the
interface: opening an action on a transient model builds the record
with ``default_get()`` + ``onchange()``, and that flow seeds every
field of the view into the cache before computing anything. A compute
with no dependency is then a cache hit on the default value, and the
dialog reports zero.
"""
line_model = self.env["account.analytic.line"]
for wizard in self:
sheets = wizard.sheet_ids
groups = line_model.read_group(
[("sheet_id", "in", sheets.ids)], ["unit_amount"], []
)
group = groups[0] if groups else {}
wizard.sheet_count = len(sheets)
wizard.timesheet_count = timesheet_count
wizard.timesheet_hours = timesheet_hours
wizard.timesheet_count = group.get("__count") or 0
wizard.timesheet_hours = group.get("unit_amount") or 0.0
@api.depends("sheet_count", "timesheet_count", "timesheet_hours")
def _compute_summary_html(self):
@ -61,7 +80,7 @@ class HrTimesheetSheetUnfreezeWizard(models.TransientModel):
def action_unfreeze(self):
self.ensure_one()
count = self.env["hr_timesheet.sheet"]._unfreeze_sheets()
count = self.env["hr_timesheet.sheet"]._unfreeze_sheets(self.sheet_ids)
if count:
message = _("%s timesheet sheet(s) have been set back to draft.", count)
else:

View file

@ -9,6 +9,12 @@
<field name="model">hr_timesheet.sheet.unfreeze.wizard</field>
<field name="arch" type="xml">
<form string="Unfreeze Pending Sheets">
<!--
The figures depend on sheet_ids, and the onchange flow that
builds the dialog only asks default_get() for the fields of
the view. Drop this field and the dialog reports zero.
-->
<field name="sheet_ids" invisible="1" />
<field name="summary_html" nolabel="1" readonly="1" />
<group attrs="{'invisible': [('sheet_count', '=', 0)]}">
<field name="sheet_count" />