From b01cd6ff4842699895cb19e0edf3b8f253b42a63 Mon Sep 17 00:00:00 2001 From: "Ali Alfie (alal)" Date: Tue, 10 Oct 2023 14:42:05 +0200 Subject: [PATCH] [IMP] hr_expense: usability improvements This commit contains a few small improvements to the Expense app. - "View Report" button moved to the left if an attachment has been uploaded. - Reports are auto saved when created from an expense. - "Attach Receipt" button now sets the uploaded attachment as the main attachment. - Expense lines in a report are no longer editable. When a line is clicked, the main attachment for that expense (if any) is shown in the attachment previewer. task-3539382 closes odoo/odoo#138387 Related: odoo/enterprise#50136 Signed-off-by: Habib Ayob (ayh) --- addons/hr_expense/__manifest__.py | 3 + addons/hr_expense/models/hr_expense.py | 29 ++-- .../static/src/views/expense_line_widget.js | 50 +++++++ .../static/tests/helpers/discuss.js | 29 ++++ .../tests/helpers/model_definitions_setup.js | 5 + .../tests/views/expense_line_widget_tests.js | 128 ++++++++++++++++++ addons/hr_expense/views/hr_expense_views.xml | 8 +- 7 files changed, 228 insertions(+), 24 deletions(-) create mode 100644 addons/hr_expense/static/src/views/expense_line_widget.js create mode 100644 addons/hr_expense/static/tests/helpers/discuss.js create mode 100644 addons/hr_expense/static/tests/helpers/model_definitions_setup.js create mode 100644 addons/hr_expense/static/tests/views/expense_line_widget_tests.js diff --git a/addons/hr_expense/__manifest__.py b/addons/hr_expense/__manifest__.py index 6f8a9ff47cf..9abed1f3607 100644 --- a/addons/hr_expense/__manifest__.py +++ b/addons/hr_expense/__manifest__.py @@ -69,6 +69,9 @@ This module also uses analytic accounting and is compatible with the invoice on 'web.report_assets_common': [ 'hr_expense/static/src/scss/hr_expense.scss', ], + 'web.qunit_suite_tests': [ + 'hr_expense/static/tests/**/*.js', + ], }, 'license': 'LGPL-3', } diff --git a/addons/hr_expense/models/hr_expense.py b/addons/hr_expense/models/hr_expense.py index cae3ae21f5c..18790b95c8e 100644 --- a/addons/hr_expense/models/hr_expense.py +++ b/addons/hr_expense/models/hr_expense.py @@ -480,8 +480,8 @@ class HrExpense(models.Model): ) def attach_document(self, **kwargs): - # To override - pass + """When an attachment is uploaded as a receipt, set it as the main attachment.""" + self.message_main_attachment_id = kwargs['attachment_ids'][-1] def create_expense_from_attachments(self, attachment_ids=None, view_type='list'): """ @@ -653,29 +653,16 @@ class HrExpense(models.Model): def action_submit_expenses(self): if self.filtered(lambda expense: not expense.is_editable): raise UserError(_('You are not authorized to edit this expense.')) - context_vals = self._get_default_expense_sheet_values() - action_values = { + sheets = self.env['hr.expense.sheet'].create(self._get_default_expense_sheet_values()) + return { 'name': _('New Expense Reports'), 'type': 'ir.actions.act_window', 'res_model': 'hr.expense.sheet', + 'context': self.env.context, + 'views': [[False, "list"], [False, "form"]] if len(sheets) > 1 else [[False, "form"]], + 'domain': [('id', 'in', sheets.ids)], + 'res_id': sheets.id if len(sheets) == 1 else False, } - if len(context_vals) > 1: - sheets = self.env['hr.expense.sheet'].create(context_vals) - action_values.update({ - 'views': [[False, "list"], [False, "form"]], - 'domain': [('id', 'in', sheets.ids)], - 'context': self.env.context, - }) - else: - context_vals_def = {} - for key in context_vals[0]: - context_vals_def['default_' + key] = context_vals[0][key] - action_values.update({ - 'views': [[False, "form"]], - 'target': 'current', - 'context': {f'default_{key}': value for key, value in context_vals[0].items()}, - }) - return action_values def action_get_attachment_view(self): self.ensure_one() diff --git a/addons/hr_expense/static/src/views/expense_line_widget.js b/addons/hr_expense/static/src/views/expense_line_widget.js new file mode 100644 index 00000000000..d51ef735274 --- /dev/null +++ b/addons/hr_expense/static/src/views/expense_line_widget.js @@ -0,0 +1,50 @@ +/** @odoo-module */ + +import { registry } from "@web/core/registry"; +import { useService } from "@web/core/utils/hooks"; + +import { ListRenderer } from "@web/views/list/list_renderer"; +import { X2ManyField, x2ManyField } from "@web/views/fields/x2many/x2many_field"; + +export class ExpenseLinesListRenderer extends ListRenderer { + setup() { + super.setup(); + this.threadService = useService("mail.thread"); + } + + /** @override **/ + async onCellClicked(record, column, ev) { + const sheetId = this.env.model.root.resId; + const sheetThread = this.threadService.getThread('hr.expense.sheet', sheetId); + const attachmentId = record.data.message_main_attachment_id[0] + + if (attachmentId) { + sheetThread.update({ mainAttachment: sheetThread.attachments.find((attachment) => attachment.id === attachmentId) }); + } + super.onCellClicked(record, column, ev); + } +} + +export class ExpenseLinesWidget extends X2ManyField { + static components = { + ...X2ManyField.components, + ListRenderer: ExpenseLinesListRenderer, + }; + + setup() { + super.setup(); + this.canOpenRecord = false; + } + + get isMany2Many() { + // The field is used like a many2many to allow for adding existing lines to the sheet. + return true; + } +} + +export const expenseLinesWidget = { + ...x2ManyField, + component: ExpenseLinesWidget, +}; + +registry.category("fields").add("expense_lines_widget", expenseLinesWidget); diff --git a/addons/hr_expense/static/tests/helpers/discuss.js b/addons/hr_expense/static/tests/helpers/discuss.js new file mode 100644 index 00000000000..edecfeb0ebf --- /dev/null +++ b/addons/hr_expense/static/tests/helpers/discuss.js @@ -0,0 +1,29 @@ +/* @odoo-module */ + +import { patch } from "@web/core/utils/patch"; +import { MockServer } from "@web/../tests/helpers/mock_server"; + +patch(MockServer.prototype, { + /** + * @override + */ + async _mockRouteMailThreadData(thread_model, thread_id, request_list) { + if (thread_model === "hr.expense.sheet" && request_list.includes("attachments")) { + const res = await super._mockRouteMailThreadData(thread_model, thread_id, request_list); + const sheet = this.pyEnv["hr.expense.sheet"].searchRead([ + ["id", "=", thread_id], + ]); + const attachments = this.pyEnv["ir.attachment"].searchRead([ + ["res_id", "in", sheet[0].expense_line_ids], + ["res_model", "=", "hr.expense"], + ]); + if (Array.isArray(res["attachments"])) { + res["attachments"] = this._mockIrAttachment_attachmentFormat(attachments.map((attachment) => attachment.id)); + } else { + res["attachments"].push(this._mockIrAttachment_attachmentFormat(attachments.map((attachment) => attachment.id))); + } + return res; + } + return super._mockRouteMailThreadData(thread_model, thread_id, request_list); + } +}); diff --git a/addons/hr_expense/static/tests/helpers/model_definitions_setup.js b/addons/hr_expense/static/tests/helpers/model_definitions_setup.js new file mode 100644 index 00000000000..c063da77d63 --- /dev/null +++ b/addons/hr_expense/static/tests/helpers/model_definitions_setup.js @@ -0,0 +1,5 @@ +/** @odoo-module **/ + +import { addModelNamesToFetch } from '@bus/../tests/helpers/model_definitions_helpers'; + +addModelNamesToFetch(['hr.expense', 'hr.expense.sheet']); diff --git a/addons/hr_expense/static/tests/views/expense_line_widget_tests.js b/addons/hr_expense/static/tests/views/expense_line_widget_tests.js new file mode 100644 index 00000000000..9b22c0735f7 --- /dev/null +++ b/addons/hr_expense/static/tests/views/expense_line_widget_tests.js @@ -0,0 +1,128 @@ +/** @odoo-module **/ + +import { startServer } from "@bus/../tests/helpers/mock_python_environment"; + +import { getOrigin } from "@web/core/utils/urls"; +import { click, contains } from "@web/../tests/utils"; +import { nextTick } from "@web/../tests/helpers/utils"; + +import { start } from "@mail/../tests/helpers/test_utils"; +import { patchUiSize, SIZES } from "@mail/../tests/helpers/patch_ui_size"; +import { ROUTES_TO_IGNORE as MAIL_ROUTES_TO_IGNORE } from "@mail/../tests/helpers/webclient_setup"; + +const ROUTES_TO_IGNORE = [ + "/mail/init_messaging", + "/mail/thread/messages", + "/mail/load_message_failures", + "/web/dataset/call_kw/ir.attachment/register_as_main_attachment", + "/mail/thread/data", + ...MAIL_ROUTES_TO_IGNORE, +]; + +QUnit.module("Views", {}, function () { + QUnit.module("ExpenseLineWidget"); + + const OpenPreparedView = async (assert, size, sheet) => { + const views = { + "hr.expense.sheet,false,form": + `
+ + + + + + + + + + + + +
+
+ + + +
+ `, + }; + patchUiSize({ size: size }); + const { openView } = await start({ + serverData: { views }, + mockRPC: function (route, args) { + if (ROUTES_TO_IGNORE.includes(route)) { + return; + } + if (route.includes("/web/static/lib/pdfjs/web/viewer.html")) { + return Promise.resolve(); + } + const method = args.method || route; + const model = args.model !== undefined ? args.model : args.thread_model; + assert.step(`${method}/${model}`); + }, + }); + await openView({ + res_model: "hr.expense.sheet", + res_id: sheet, + views: [[false, "form"]], + }); + }; + + QUnit.test("ExpenseLineWidget test attachments change on expense line click", async (assert) => { + const pyEnv = await startServer(); + const sheet = pyEnv["hr.expense.sheet"].create({ name: "Expense Sheet"}) + const expense_lines = pyEnv["hr.expense"].create([ + { name: "Lunch", sheet_id: sheet}, + { name: "Taxi", sheet_id: sheet}, + { name: "Misc", sheet_id: sheet}, + ]) + const attachmentIds = pyEnv["ir.attachment"].create([ + { res_id: expense_lines[0], res_model: "hr.expense", mimetype: "application/pdf" }, + { res_id: expense_lines[0], res_model: "hr.expense", mimetype: "application/pdf" }, + { res_id: expense_lines[1], res_model: "hr.expense", mimetype: "application/pdf" }, + { res_id: expense_lines[1], res_model: "hr.expense", mimetype: "application/pdf" }, + ]); + pyEnv['hr.expense.sheet'].write([sheet], {expense_line_ids: expense_lines}) + pyEnv['hr.expense'].write([expense_lines[0]], {message_main_attachment_id: attachmentIds[1]}) + pyEnv['hr.expense'].write([expense_lines[1]], {message_main_attachment_id: attachmentIds[2]}) + + await OpenPreparedView(assert, SIZES.XXL, sheet); + + await contains(".o_data_row", { count: 3 }); + await nextTick(); + assert.verifySteps([ + "get_views/hr.expense.sheet", + "web_read/hr.expense.sheet", + ]); + // Default attachment is the last one. + await contains( + `.o_attachment_preview iframe[data-src='/web/static/lib/pdfjs/web/viewer.html?file=${encodeURIComponent( + getOrigin() + "/web/content/4" + )}#pagemode=none']` + ); + await click(":nth-child(1 of .o_data_row) :nth-child(1 of .o_data_cell)"); + // Attachment is switched to the main attachment in expense line one. + await contains( + `.o_attachment_preview iframe[data-src='/web/static/lib/pdfjs/web/viewer.html?file=${encodeURIComponent( + getOrigin() + "/web/content/2" + )}#pagemode=none']` + ); + assert.verifySteps([], "no extra rpc should be done"); + // No change since line three has no attachments. + await click(":nth-child(3 of .o_data_row) :nth-child(1 of .o_data_cell)"); + await contains( + `.o_attachment_preview iframe[data-src='/web/static/lib/pdfjs/web/viewer.html?file=${encodeURIComponent( + getOrigin() + "/web/content/2" + )}#pagemode=none']` + ); + assert.verifySteps([], "no extra rpc should be done"); + await click(":nth-child(2 of .o_data_row) :nth-child(1 of .o_data_cell)"); + // Attachment is switched to the main attachment in expense line two. + await contains( + `.o_attachment_preview iframe[data-src='/web/static/lib/pdfjs/web/viewer.html?file=${encodeURIComponent( + getOrigin() + "/web/content/3" + )}#pagemode=none']` + ); + assert.verifySteps([], "no extra rpc should be done"); + }); +}); diff --git a/addons/hr_expense/views/hr_expense_views.xml b/addons/hr_expense/views/hr_expense_views.xml index af1e5b8202c..ea0be8c9024 100644 --- a/addons/hr_expense/views/hr_expense_views.xml +++ b/addons/hr_expense/views/hr_expense_views.xml @@ -127,6 +127,7 @@
@@ -862,7 +863,7 @@ - + +