[IMP] hr_expense: usability improvements #138387

alialfie · 2023-10-11T13:02:19Z

This commit contains a few small improvements to Expense app.

Primary button moved to the left.
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

Enterprise PR: odoo/enterprise/pull/50136

robodoo · 2023-10-11T13:03:06Z

h4818

Nicely done.
I think a QUnit test is important, in case of changes to ListRenderer or x2manyField

addons/hr_expense/static/src/views/expense_list_view.js

addons/hr_expense/models/hr_expense.py

addons/hr_expense/static/src/views/expense_list_view.js

addons/hr_expense/tests/test_expenses.py

addons/hr_expense/views/hr_expense_views.xml

addons/hr_expense/static/src/views/expense_list_view.js

JulienAlardot · 2023-10-24T11:24:49Z

Sorry for the hellish rebase you'll have on that one :p (also, check your missing EOL :) )

alialfie · 2023-10-25T11:03:50Z

Rebased and ready!

JulienAlardot

Remind me why you are not using the hr_expense.message_main_attachment_id again?

addons/hr_expense/models/hr_expense.py

alialfie · 2023-10-26T10:49:14Z

Remind me why you are not using the hr_expense.message_main_attachment_id again?

PO request. When an expense line is clicked in a sheet, the last uploaded attachment in that line is shown, and message_main_attachment_id is not necessarily the latest.

JulienAlardot · 2023-10-26T12:15:49Z

Remind me why you are not using the hr_expense.message_main_attachment_id again?

PO request. When an expense line is clicked in a sheet, the last uploaded attachment in that line is shown, and message_main_attachment_id is not necessarily the latest.

After discussions with Laura

We want to display the main attachment
If we add a new attachment through the "Attach receipt" it becomes the main attachment
If we add a new attachment through the "Add attachments" it keeps the previous one

The last 2 changes are handled by register_as_main_attachment parameter force=True (need to check but it may already be properly set, else it is an easy fix

The reason behind the "last attachment" was because of simplicity (from what another PO told her), here it's definitely not simpler

Note that you would have to edit the attach_document method and check that its overrides are now extends of it

(Also, note that right now as you are never sorting the ids you only show the oldest one and not the latest)

In odoo/pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in odoo/pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again.

In /pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in /pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes #155417 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>

In odoo/pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in odoo/pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. X-original-commit: 0624487

In /pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in /pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes #164640 X-original-commit: 0624487 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>

In odoo/pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in odoo/pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes odoo#155417 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>

In /pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in /pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes #155417 closes #164645 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com> Signed-off-by: Ali Alfie (alal) <alal@odoo.com>

In odoo/pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in odoo/pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes odoo#155417 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>

In /pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in /pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes #155417 closes #165117 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>

In odoo/pull/138387, a new feature was added where clicking on an expense line in an expense sheet will highlight the receipts of that line in the attachment viewer if any. A JS test was included for this feature that mocked the fetching of the sheet's attachment. Since this changed in the backend in odoo/pull/142029, the feature no longer worked but the test never broke since it was mocking the backend. This commit adds a tour to prevent that from happening again. closes odoo#155417 closes odoo#165117 Signed-off-by: Habib Ayob (ayh) <ayh@odoo.com>

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch from 67c3e1b to eec21f0 Compare October 11, 2023 13:02

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch from eec21f0 to 9d3d33d Compare October 11, 2023 13:05

alialfie requested a review from h4818 October 11, 2023 13:12

alialfie marked this pull request as draft October 11, 2023 13:33

C3POdoo added the RD research & development, internal work label Oct 11, 2023

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch 2 times, most recently from 2456f4a to 35e9fe0 Compare October 12, 2023 10:14

alialfie marked this pull request as ready for review October 12, 2023 11:09

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch from 35e9fe0 to 1614e22 Compare October 12, 2023 15:48

h4818 requested changes Oct 13, 2023

View reviewed changes

h4818 reviewed Oct 13, 2023

View reviewed changes

addons/hr_expense/static/src/views/expense_list_view.js Outdated Show resolved Hide resolved

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch 9 times, most recently from 9556d08 to 3be712c Compare October 19, 2023 15:04

alialfie requested a review from h4818 October 20, 2023 06:49

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch from 3be712c to e1aa199 Compare October 25, 2023 09:53

JulienAlardot requested changes Oct 26, 2023

View reviewed changes

addons/hr_expense/models/hr_expense.py Outdated Show resolved Hide resolved

addons/hr_expense/models/hr_expense.py Outdated Show resolved Hide resolved

alialfie force-pushed the master-imp_hr_expense_misc_modifs-alal branch 2 times, most recently from 675791b to e8e6eaf Compare October 27, 2023 07:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IMP] hr_expense: usability improvements #138387

[IMP] hr_expense: usability improvements #138387

alialfie commented Oct 11, 2023 •

edited

robodoo commented Oct 11, 2023 •

edited

h4818 left a comment

JulienAlardot commented Oct 24, 2023

alialfie commented Oct 25, 2023

JulienAlardot left a comment

alialfie commented Oct 26, 2023

JulienAlardot commented Oct 26, 2023

[IMP] hr_expense: usability improvements #138387

[IMP] hr_expense: usability improvements #138387

Conversation

alialfie commented Oct 11, 2023 • edited

robodoo commented Oct 11, 2023 • edited

h4818 left a comment

Choose a reason for hiding this comment

JulienAlardot commented Oct 24, 2023

alialfie commented Oct 25, 2023

JulienAlardot left a comment

Choose a reason for hiding this comment

alialfie commented Oct 26, 2023

JulienAlardot commented Oct 26, 2023

alialfie commented Oct 11, 2023 •

edited

robodoo commented Oct 11, 2023 •

edited