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

[16.0][MIG] account_payment_mode_auto_reconcile: Migrate to version 16.0 #537

Open
wants to merge 8 commits into
base: 16.0
Choose a base branch
from

Conversation

sonhd91
Copy link

@sonhd91 sonhd91 commented Mar 10, 2023

@sonhd91 sonhd91 marked this pull request as ready for review March 10, 2023 08:16
@sonhd91 sonhd91 mentioned this pull request Mar 16, 2023
5 tasks
@grindtildeath
Copy link
Contributor

@sonhd91 can you please rebase to have a runboat to test?

@sonhd91 sonhd91 force-pushed the 16.0-mig-account_payment_mode_auto_reconcile branch from 00341af to f440e10 Compare June 10, 2023 03:34
@grindtildeath
Copy link
Contributor

@sonhd91 Look to work well, thx 👍

However, the view on invoices looks broken with the alert message message not displayed properly, can you please fix? 🙏
On v16 runbot:
image

@sonhd91 sonhd91 force-pushed the 16.0-mig-account_payment_mode_auto_reconcile branch from f440e10 to e97a4aa Compare June 15, 2023 06:15
@sonhd91
Copy link
Author

sonhd91 commented Jun 15, 2023

@sonhd91 Look to work well, thx 👍

However, the view on invoices looks broken with the alert message message not displayed properly, can you please fix? 🙏 On v16 runbot: image

I updated it, please check again

@rousseldenis
Copy link

@sonhd91 Could you rebase ?

Copy link

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

Code and functional review

@sbidoul sbidoul added this to the 16.0 milestone Jan 1, 2024
Copy link

github-actions bot commented May 5, 2024

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.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 5, 2024
grindtildeath and others added 8 commits May 6, 2024 10:36
Override write instead of using onchange

Add missing utf-8 comment

Fix warning message

Restrict to customer invoices

Add readme and load tests

Improvements after reviews

Improve doc

Unreconcile only automatically reconciled payments
* Add auto reconcile only same journal

Add an option to auto reconcile only credits that are on the same
journal than the invoice.
This make sure that unrelated payment will not be used automatically.

* Update account_payment_mode_auto_reconcile/models/account_invoice.py

Co-Authored-By: Akim Juillerat <[email protected]>

* Update account_payment_mode_auto_reconcile/tests/test_partner_auto_reconcile.py

Co-Authored-By: Akim Juillerat <[email protected]>
This small change is needed since action_invoice_open filters
invoices that can be open from other invoices that cannot.

Therefore using invoice_validate here allows for better
customizations by ensuring that the invoices to be handled are
really open (i.e. have a move generated).
As using reverse is not reliable to ensure oldest credits
will be reconciled first, use a function to sort the dicts
according to their id and allows inheritance.
@sonhd91 sonhd91 force-pushed the 16.0-mig-account_payment_mode_auto_reconcile branch from e97a4aa to 17386a6 Compare May 6, 2024 03:36
@sonhd91
Copy link
Author

sonhd91 commented May 6, 2024

Code and functional review

@rousseldenis
Sorry, I missed this, branch rebased

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label May 12, 2024
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Copy link

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.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 15, 2024
@github-actions github-actions bot closed this Oct 20, 2024
@grindtildeath grindtildeath added migration no stale Use this label to prevent the automated stale action from closing this PR/Issue. and removed stale PR/Issue without recent activity, it'll be soon closed automatically. labels Oct 21, 2024
@grindtildeath grindtildeath reopened this Oct 21, 2024
Comment on lines 16 to 15
states={"draft": [("readonly", False)], "open": [("readonly", False)]}
states={"draft": [("readonly", False)], "posted": [("readonly", False)]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need this? AFAIU it means readonly is only applied on cancel?

continue
payment_mode = invoice.payment_mode_id
invoice_lines = invoice.line_ids.filtered(
lambda line: line.account_type == "asset_receivable"
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 about that one, but somehow you're restricting this feature to customer invoices. I guess it would make more sense to use the filtering on payment term lines?

invoice_lines = invoice.line_ids.filtered(lambda l: l.display_type == 'payment_term')

def auto_reconcile_credits(self, partial_allowed=True):
for invoice in self:
if not invoice.has_outstanding:
invoice._compute_payments_widget_to_reconcile_info()
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems weird to me that you have to explicitely call this before accessing the value 😕

)
refund_id = cls.refund_wiz.reverse_moves().get("res_id")
cls.refund = cls.env["account.move"].browse(refund_id)
cls.payment = cls.env["account.payment"].create(
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to create a payment here? 🤔

self.invoice_copy.write({"payment_mode_id": False})
self.assertEqual(self.invoice_copy.residual, 1500)
self.assertEqual(auto_rec_invoice.state, "paid")
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO you should still assert invoice was paid.

@thienvh332
Copy link

Hi @grindtildeath ,
I have created #735 to update the changes from your review. Could you have a look it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved migration no stale Use this label to prevent the automated stale action from closing this PR/Issue. ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants