[16.0][MIG] sale_expense_manual_reinvoice: Migration to 16.0#296
[16.0][MIG] sale_expense_manual_reinvoice: Migration to 16.0#296
Conversation
9eb578f to
122eebb
Compare
|
Anybody to validate the PR ? |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Currently translated at 100.0% (24 of 24 strings) Translation: hr-expense-15.0/hr-expense-15.0-sale_expense_manual_reinvoice Translate-URL: https://translation.odoo-community.org/projects/hr-expense-15-0/hr-expense-15-0-sale_expense_manual_reinvoice/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: hr-expense-15.0/hr-expense-15.0-sale_expense_manual_reinvoice Translate-URL: https://translation.odoo-community.org/projects/hr-expense-15-0/hr-expense-15-0-sale_expense_manual_reinvoice/
Currently translated at 100.0% (24 of 24 strings) Translation: hr-expense-15.0/hr-expense-15.0-sale_expense_manual_reinvoice Translate-URL: https://translation.odoo-community.org/projects/hr-expense-15-0/hr-expense-15-0-sale_expense_manual_reinvoice/es/
Currently translated at 100.0% (24 of 24 strings) Translation: hr-expense-15.0/hr-expense-15.0-sale_expense_manual_reinvoice Translate-URL: https://translation.odoo-community.org/projects/hr-expense-15-0/hr-expense-15-0-sale_expense_manual_reinvoice/it/
122eebb to
1857650
Compare
|
Hello @ivantodorovich can you check this migration PR please. Thank you |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
|
@pedrobaeza @Saran440 Hello, sorry for the ping, but I haven't received any response in 9 months. Could you please check this PR? Thank you 😃 |
|
/ocabot migration sale_expense_manual_reinvoice You can review other PRs, and ask in exchange that they review yours. |
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failure is caused by a database connection error during the Odoo startup process, which prevents the test environment from initializing properly. This is not a code logic issue in the module itself but rather an infrastructure or configuration problem in the test environment (likely related to Runboat setup).
2. Suggested Fix
There is no code fix required for the test failure itself, as it's a setup issue. However, to make the module more robust, consider adding a check for expense_id in _sale_can_be_reinvoice to prevent potential errors if self.expense_id is not set:
# In sale_expense_manual_reinvoice/models/account_move_line.py, line ~12
def _sale_can_be_reinvoice(self):
# OVERRIDE to skip automatic reinvoicing of expense lines, when needed
res = super()._sale_can_be_reinvoice()
if self.expense_id:
return self.expense_id.product_id.expense_mode != "manual" and res
return resThis already exists in the diff and is correct. The fix is already in place.
3. Additional Code Issues
-
In
hr_expense.py, the_compute_manual_reinvoicemethod assumes thatfields.first(rec.analytic_line_ids)always returns a record. If there are no analytic lines, this could raise an error. It should be guarded:# In sale_expense_manual_reinvoice/models/hr_expense.py, line ~37 def _compute_manual_reinvoice(self): for rec in self: first_line = fields.first(rec.analytic_line_ids) for fname in ["manual_reinvoice", "manual_reinvoice_done", "manual_reinvoice_discarded"]: rec[fname] = first_line[fname] if first_line else False
-
In
account_analytic_line.py, the_compute_manual_reinvoice_donemethod usesrec.so_linebut does not ensureso_lineis properly set or exists. This could cause issues if the sale line is not created or is invalid.
4. Test Improvements
To improve test coverage, add the following test cases using TransactionCase or SavepointCase:
a. Test manual_reinvoice field behavior
- Create an expense with
expense_mode = "manual"and verify thatmanual_reinvoiceis set correctly. - Create an expense with
expense_mode = "auto"and verify thatmanual_reinvoiceisFalse.
b. Test action_manual_reinvoice method
- Test that calling
action_manual_reinvoice()on a line withmanual_reinvoice = FalseraisesUserError. - Test that calling
action_manual_reinvoice()on a line already re-invoiced raisesUserError. - Test successful reinvoice of a manual expense line.
c. Test _sale_can_be_reinvoice override
- Ensure that when
expense_id.product_id.expense_mode == "manual", the method returnsFalse. - Ensure that when
expense_id.product_id.expense_mode == "auto", the method returns the parent result.
d. Test manual_reinvoice_discarded behavior
- Test that expenses marked as discarded are hidden from the "Expenses to Reinvoice" list.
- Test that discarded expenses can be un-discarded.
e. Test hr_expense compute fields
- Test that
manual_reinvoice,manual_reinvoice_done, andmanual_reinvoice_discardedare computed correctly from relatedanalytic_line_ids.
Use tagged tests for better organization:
@tagged('post_install', 'manual_reinvoice')
def test_manual_reinvoice_flow(self):
...Summary
The test failure is due to a database connection issue in the test environment, not a code bug. The module code is mostly solid, but minor robustness improvements are suggested in hr_expense.py and account_analytic_line.py. Add tests for edge cases and override behavior to ensure full coverage.
⚠️ PR Aging Alert: CRITICAL
This PR by @MarwanBHL has been waiting for 355 days — that is over 11 months without being merged or closed.
🔴 Zero human reviews in 355 days. This contributor invested their time to improve this module. The PSC owes them at least a response — even a "needs changes" is better than silence.
💤 Last activity was 70 days ago.
Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
Migration to 16.0