[FIX] account move line default_get #25298
Conversation
|
I don't get what is the problem, but you shouldn't change so much code if you want this to be clear for having it merged. You need to sign the CLA also. |
|
@pedrobaeza The diff is 10 lines. Are you seeing something else than me? |
|
10 lines which doesn't differ in fact, because you want to add a new cyclomatic complexity level with no benefit, so please remove that extra |
|
@pedrobaeza Thanks for the tip! I simplified the PR. |
|
Is this applicable also to other branches? @qdp-odoo can you review this one? |
|
@pedrobaeza Well, V9.0 and V10.0 both didn't include a default get for account_move_line. So either it was solved in a different way, or it's isn't included at all. V8.0 has a completely different implementation (Which by the way is very slow. in computation.). |
|
OK, thanks for the insight. Take into account that master is an alpha version, so some problems are expected to happen. Any way, it's good to have this fixed. |
| for line in self._context['line_ids']: | ||
| if line[2]: | ||
| balance += line[2].get('debit', 0) - line[2].get('credit', 0) | ||
| for line in self.env['account.move'].resolve_2many_commands('line_ids', self._context['line_ids']): |
There was a problem hiding this comment.
resolve_2many_commands is defined on the model level, not specifically on account.move so you can use self.resolve_2many_commands
There was a problem hiding this comment.
You can pass an extra parameter with the fields you need. fields=['debit', 'credit']
There was a problem hiding this comment.
@StefanRijnhart I think you mean self.move_id.resolve_2many_commands(...). I will update with your comments, thanks!
There was a problem hiding this comment.
I'm sorry, you are right. It's the line_ids field of account.move of course.
There was a problem hiding this comment.
@StefanRijnhart I updated the PR and fixed the CLA to the corporate version in a separate commit.
There was a problem hiding this comment.
PR is looking good! I think legal wise, Stijn Houben needs to commit the addition to the corporate CLS
There was a problem hiding this comment.
Well, I discussed it internally with the Dynapps developers and they agreed I should do this: #25336. ;-)
|
nice catch. Landed in master, at revision 50ee1c9 Thank you all for the bug report, the fix and the reviews :-) |
When creating a journal entry manually, the value proposed as next line was not taken into consideration all existing lines. Was PR #25298. Courtesy of Pieter Paulussen (Dynapps)
Backport of 50ee1c9. Previous version was buggy, as it was already patched by 54c2c08 and 862c245. When creating a journal entry manually, the computed value for the next line was not taking into consideration all existing lines. Was PR odoo#25298. Courtesy of Pieter Paulussen (Dynapps)
Description of the issue/feature this PR addresses:
The default get does not support multiple account_move_line's calculated debit/credit.
Current behavior before PR:
The move line's debit and credit are calculated from a hardcoded counterpart. (line[2]).
Desired behavior after PR is merged:
Return a calculated debit/credit from all move lines related to the move.
--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr