Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] account_reversal_usability #1771

Closed

Conversation

IT-Ideas
Copy link

@IT-Ideas IT-Ideas commented Nov 4, 2023

This commit adds the account_reversal_usability module which purpose is to improve the usability of the move line reverse functionality by adding:

  • a field that allows tagging a move line as needing to be reversed. The value of the field gets automatically set to False once a reversed entry is made.
  • a filter in the search view that allows selecting the moves that are flagged as needing to be reversed.
  • the reverse move in the form view

Fixes #1761

@IT-Ideas IT-Ideas force-pushed the 16.0-add-account_reversal_usability-lst branch 4 times, most recently from f51b290 to 821b282 Compare November 4, 2023 13:36
@IT-Ideas
Copy link
Author

IT-Ideas commented Nov 4, 2023

Hello @sbidoul ,
Here is the PR you asked for.
I am not an accounting super star, but I don't quite see why the diff could trigger the issues in the tests. Do you have any idea?
Thanks!

@IT-Ideas IT-Ideas force-pushed the 16.0-add-account_reversal_usability-lst branch 3 times, most recently from 3d66cd8 to ce3e7ec Compare November 6, 2023 08:21
_inherit = "account.move"

to_be_reversed = fields.Boolean(
compute="_compute_to_be_reversed",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this field can be managed by account.move.reversal

class AccountMoveReversal(models.TransientModel):

    _inherit = "account.move.reversal"

    def reverse_moves(self):
        self.move_ids.write({"to_be_reversed": False})
        return super().reverse_moves()

Copy link
Author

@IT-Ideas IT-Ideas Nov 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As agreed, we finally go for overriding the account.move._reverse_moves

This commit adds the account_reversal_usability module which purpose is to
improve the usability of the move line reverse functionality by adding:

* a field that allows tagging a move line as needing to be reversed.
  The value of the field gets automatically set to False once a reversed
  entry is made.
* a filter in the search view that allows selecting the moves that are
  flagged as needing to be reversed.
* the reverse move in the form view
@IT-Ideas IT-Ideas force-pushed the 16.0-add-account_reversal_usability-lst branch from ce3e7ec to 7f5f9f2 Compare November 6, 2023 10:03
Copy link
Contributor

@sbejaoui sbejaoui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of comments/question, otherwise looks good to me. Technical review and functional test on runboat.

the field gets automatically set to False once a reversed entry is made.
* a filter in the search view that allows selecting the moves that are flagged as
needing to be reversed.
* the reverse move in the form view""",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do this "big" summary display well in Odoo? Otherwise we can keep (the first sentence may be enough).


def _reverse_moves(self, default_values_list=None, cancel=False):
res = super()._reverse_moves(default_values_list, cancel)
self.to_be_reversed = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it now work to assign a value to a set of multiple records at once?

@IT-Ideas
Copy link
Author

IT-Ideas commented Nov 7, 2023

Closing this PR as already handled in #1725

@IT-Ideas IT-Ideas closed this Nov 7, 2023
@sbidoul sbidoul deleted the 16.0-add-account_reversal_usability-lst branch November 7, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ADD] account_reversal_usability
4 participants