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

[15.0][MIG] account_check_deposit #1301

Closed
wants to merge 58 commits into from

Conversation

alexis-via
Copy link
Contributor

No description provided.

bguillot and others added 30 commits December 7, 2021 10:48
* Clean code
* Add access rules for account_check_deposit
* Change print button name
* [FIX] fix datamodel structure, indeed check_payment_ids should be a m2o. In the view we still keep the widget
* [REF] remove useless state cancel, only draft/done is sufficent for now, forbid to delete a done deposit, fix move_line name need to be the equal to the line.ref for the reconciliation
* [IMP] improve reconciliation between the check and the check deposit
* [FIX] set correctly the date on the move line generated
* [FIX] fix reconciliation of check
* [IMP] check deposit improve view and add tab for move line generated
* [IMP] add total amount and flag to know if the deposit have been reconcile, the best will be to add a workflow
* [FIX] fix error when replaying commit
* [REF] remove dependency on sale_quick_payment as check can be registred in various way like native voucher
* [CLEAN] pep8 cleanup no code change
* [REF] clean remove useless code, default value are called during the create
* [FIX] fix wrong spelling
* [REF] V7 clean up
Works with a single account journal
Usability and many other enhancements
… for the payment, one for the deposit), hide unreconcile entry which a link to a deposit
Convert webkit report to qweb
* Move description from __openerp__.py to README.rst
* Move files in sub-directories
* FIX access rights issue when user was not accounting manager
* create move and move line at the same time
* add tests
* Add option to have the counter-part of the deposit move directly to the bank account
  Use account.config.settings page
* Remove data that create a bank journal because it blocks the loading of the chart of accounts when
  the module is installed at the same time as the chart of account (via dependencies)
…rnal if the offsetting account is the bank account
Currently translated at 100,0% (61 of 61 strings)

Translation: account-financial-tools-11.0/account-financial-tools-11.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-11-0/account-financial-tools-11-0-account_check_deposit/de/
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
Add print button (feature up-ported from v10)
Add option to post moves (feature up-ported from v10)
Sequence now takes into account the deposit date
Customize the filename of the report
Update README with v12 labels
oca-transbot and others added 18 commits December 7, 2021 10:48
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-financial-tools-13.0/account-financial-tools-13.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-13-0/account-financial-tools-13-0-account_check_deposit/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-financial-tools-13.0/account-financial-tools-13.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-13-0/account-financial-tools-13-0-account_check_deposit/
Update documentation
Don't duplicate labels between name of account.move.line and ref of
account.move, now that name of account.move.line is not required any
more.
Add button "Get All Received Checks"
Add default value for journal_id and bank_journal_id when there is only 1 possible value
Add hide/show options on lines
Improve multi-company handling
Add chatter message when you click on the button to get all checks
Add unicity SQL constraint
Currently translated at 84.3% (54 of 64 strings)

Translation: account-financial-tools-14.0/account-financial-tools-14.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-14-0/account-financial-tools-14-0-account_check_deposit/fr/
Currently translated at 100.0% (64 of 64 strings)

Translation: account-financial-tools-14.0/account-financial-tools-14.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-14-0/account-financial-tools-14-0-account_check_deposit/pt/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: account-financial-tools-14.0/account-financial-tools-14.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-14-0/account-financial-tools-14-0-account_check_deposit/
Currently translated at 100.0% (90 of 90 strings)

Translation: account-financial-tools-14.0/account-financial-tools-14.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-14-0/account-financial-tools-14-0-account_check_deposit/pt/
Currently translated at 67.7% (61 of 90 strings)

Translation: account-financial-tools-14.0/account-financial-tools-14.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-14-0/account-financial-tools-14-0-account_check_deposit/fr/
Currently translated at 70.0% (63 of 90 strings)

Translation: account-financial-tools-14.0/account-financial-tools-14.0-account_check_deposit
Translate-URL: https://translation.odoo-community.org/projects/account-financial-tools-14-0/account-financial-tools-14-0-account_check_deposit/it/
@rafaelbn
Copy link
Member

/ocabot migration account_check_deposit

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Mar 24, 2022
@OCA-git-bot OCA-git-bot mentioned this pull request Mar 24, 2022
31 tasks
Copy link
Contributor

@petrus-v petrus-v left a comment

Choose a reason for hiding this comment

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

Code review and functional test.

Few details but mainly styling !

Comment on lines +146 to +149
if deposit.move_id:
for line in deposit.move_id.line_ids:
if not company_cur.is_zero(line.debit) and line.reconciled:
reconcile = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Myself note:

  • can we have more than one line with debit > 0 ? I don't think so
  • why use the last line state only ? because we expect to get one record

Should be the same with sugar syntax

Suggested change
if deposit.move_id:
for line in deposit.move_id.line_ids:
if not company_cur.is_zero(line.debit) and line.reconciled:
reconcile = True
if deposit.move_id.line_ids.filtered(lambda line: not company_cur.is_zero(line.debit) and line.reconciled):
reconcile = True

Comment on lines +333 to +337
if self.journal_id:
if self.journal_id.currency_id:
self.currency_id = self.journal_id.currency_id
else:
self.currency_id = self.journal_id.company_id.currency_id
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose in v15 we can go straight forwards but won't block for that

Suggested change
if self.journal_id:
if self.journal_id.currency_id:
self.currency_id = self.journal_id.currency_id
else:
self.currency_id = self.journal_id.company_id.currency_id
self.currency_id = self.journal_id.currency_id or self.journal_id.company_id.currency_id

"summary": "Manage deposit of checks to the bank",
"author": "Akretion, Tecnativa, Odoo Community Association (OCA)",
"website": "https://github.com/OCA/account-financial-tools",
"depends": ["account"],
Copy link
Contributor

@petrus-v petrus-v Jun 9, 2022

Choose a reason for hiding this comment

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

we should probably add account_usability module in order to be able to access to the accounting sub menu ?

string="Number of Checks",
tracking=True,
)
is_reconcile = fields.Boolean(
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure to understand the attempt of this field, I can't see any real usage ?

Comment on lines +158 to +166
if rec.journal_id:
for line in rec.journal_id.inbound_payment_method_line_ids:
if (
line.payment_method_id.code == "manual"
and line.payment_account_id
):
in_hand_check_account_id = line.payment_account_id.id
break
rec.in_hand_check_account_id = in_hand_check_account_id
Copy link
Contributor

Choose a reason for hiding this comment

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

styling suggestion

Suggested change
if rec.journal_id:
for line in rec.journal_id.inbound_payment_method_line_ids:
if (
line.payment_method_id.code == "manual"
and line.payment_account_id
):
in_hand_check_account_id = line.payment_account_id.id
break
rec.in_hand_check_account_id = in_hand_check_account_id
rec.in_hand_check_account_id = rec.journal_id.inbound_payment_method_line_ids.filtered(
lambda line: line.payment_method_id.code == "manual"
).payment_account_id

Copy link

@gregory-moka gregory-moka left a comment

Choose a reason for hiding this comment

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

Fonctionnal review : works fine

@dreispt
Copy link
Member

dreispt commented Aug 10, 2022

/ocbot merge nobump

<template id="report_checkdeposit">
<t t-call="web.html_container">
<t t-foreach="docs" t-as="o">
<t t-call="web.internal_layout">

Choose a reason for hiding this comment

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

I'll suggest to use the external layout for this document, as it has to be given to the bank. It's a way to "communicate" to external partner.
<t t-call="web.external_layout">

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are certainly right ! I'm not a qweb expert, I have more experience on py3o.

@victoralmau
Copy link
Member

Superseed by #1504

@pedrobaeza pedrobaeza closed this Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.