Skip to content

[16.0][REF] sale_advance_payment: remove widget dependency#3799

Merged
OCA-git-bot merged 2 commits intoOCA:16.0from
factorlibre:16.0-fix-sale_advance_payment-outstanding-widget-performance
Jul 22, 2025
Merged

[16.0][REF] sale_advance_payment: remove widget dependency#3799
OCA-git-bot merged 2 commits intoOCA:16.0from
factorlibre:16.0-fix-sale_advance_payment-outstanding-widget-performance

Conversation

@ljsalvatierra-factorlibre
Copy link
Copy Markdown
Contributor

Improve performance by searching directly the related invoice lines

Improve performance by searching directly the related invoice lines
@ljsalvatierra-factorlibre ljsalvatierra-factorlibre marked this pull request as ready for review July 9, 2025 06:10
@ljsalvatierra-factorlibre
Copy link
Copy Markdown
Contributor Author

continue

# Domain search (most efficient for large datasets)
advance_payment_lines = self.env["account.move.line"].search(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ljsalvatierra-factorlibre For a better performance, do the search before the loop, then, filter records on payment_move_ids.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi! Thank you for your review! You mean like this?

def _post(self, soft=True):
        # Automatic reconciliation of payment when invoice confirmed.
        res = super()._post(soft=soft)
        sale_orders = self.mapped("line_ids.sale_line_ids.order_id")
        all_payment_move_ids = sale_orders.mapped("account_payment_ids.move_id").ids
        advance_payment_lines = self.env["account.move.line"].search(
            [
                ("move_id", "in", all_payment_move_ids),
                (
                    "account_id.account_type",
                    "in",
                    ("asset_receivable", "liability_payable"),
                ),
                ("reconciled", "=", False),
                ("parent_state", "=", "posted"),
            ]
        )
        for move in self:
            sale_order = move.mapped("line_ids.sale_line_ids.order_id")
            if not sale_order:
                continue

            payment_move_ids = sale_order.account_payment_ids.move_id.ids
            if not payment_move_ids:
                continue

            payment_lines = advance_payment_lines.filtered(
                lambda x: x.move_id.id in payment_move_ids
            )

            for line in payment_lines:
                move.js_assign_outstanding_line(line_id=line.id)
        return res

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @rousseldenis did you have time to review my proposed solution?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes!

@rousseldenis
Copy link
Copy Markdown
Contributor

But @ljsalvatierra-factorlibre, this is great as I thought also computing things on widget values was a nonsense! 😃

Copy link
Copy Markdown

@PabloMartFL PabloMartFL left a comment

Choose a reason for hiding this comment

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

Functional review okey, tested on a local enviroment.

To validate an invoice, before the fix, it took about 7-8 seconds
With the fix, invoice validation takes less than 1 second.

Copy link
Copy Markdown
Contributor

@ValentinVinagre ValentinVinagre left a comment

Choose a reason for hiding this comment

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

Have sense. Will you bring it to higher versions?

@ljsalvatierra-factorlibre
Copy link
Copy Markdown
Contributor Author

Have sense. Will you bring it to higher versions?

There is already a PR for 18.0 migration.

I dont have plans nor time to do it honestly.

@ValentinVinagre
Copy link
Copy Markdown
Contributor

Have sense. Will you bring it to higher versions?

There is already a PR for 18.0 migration.

I dont have plans nor time to do it honestly.

So I won't validate it if it doesn't move to the rest of the versions.

@ljsalvatierra-factorlibre
Copy link
Copy Markdown
Contributor Author

Have sense. Will you bring it to higher versions?

There is already a PR for 18.0 migration.
I dont have plans nor time to do it honestly.

So I won't validate it if it doesn't move to the rest of the versions.

Ok

@OCA-git-bot
Copy link
Copy Markdown
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). 🤖

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot merge minor

@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-3799-by-rousseldenis-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 1538810 into OCA:16.0 Jul 22, 2025
13 checks passed
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 74e209f. Thanks a lot for contributing to OCA. ❤️

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.

6 participants