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

[FIX] account: Remove partner on changing partner_type or payment_type #32069

Closed
wants to merge 1 commit into from

Conversation

sswapnesh
Copy link
Contributor

Description of the issue/feature this PR addresses: Account module (Customer/Vendor Payment)

Current behavior before PR:

  • Create Payment (Customer)
  • Select Partner Type --> Customer
  • Select Partner --> It will be Customer (As domain will be applied based on Partner Type)
  • Change Partner Type -->Vendor

Still Customer is there which should be removed as Partner Type is changed now

Desired behavior after PR is merged: Remove partner when partner_type or payment_type is changed

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added seen 🙂 CI 🤖 Robodoo has seen passing statuses labels Mar 23, 2019
@@ -486,6 +486,7 @@ def _compute_reconciled_invoice_ids(self):
@api.onchange('partner_type')
def _onchange_partner_type(self):
self.ensure_one()
self.partner_id = False
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not self.env["res.partner"] as an empty record?

@sswapnesh
Copy link
Contributor Author

sswapnesh commented Mar 25, 2019 via email

@MiquelRForgeFlow
Copy link
Contributor

The difference is that False is a boolean, so for example, if you do later (in another onchange or some other place) self.partner_id.company_id, then it will break, as boolean doesn't have company_id field. But if you do as empty record, then it doesn't break.

@sswapnesh
Copy link
Contributor Author

Thanks
Let's see what odoo says on this.

@sswapnesh
Copy link
Contributor Author

Ping @qdp-odoo

@qdp-odoo
Copy link
Contributor

qdp-odoo commented Apr 3, 2019

I dunno

@avw-odoo ?

@sswapnesh
Copy link
Contributor Author

Or Maybe @mart-e can verify.

@sswapnesh
Copy link
Contributor Author

or Maybe someone from accounting team ✋
/cc @smetl @csnauwaert

@smetl
Copy link
Contributor

smetl commented Aug 21, 2019

If your partner is both a customer/vendor, why removing it?

@sswapnesh
Copy link
Contributor Author

sswapnesh commented Aug 21, 2019

If your partner is both a customer/vendor, why removing it?

Ideally, Partner shouldn't be removed but handling that case would be too much for Stable version.

IMO, This is enough to Prevent wrong Payment where Customer/Supplier are Identical.

@smetl Do you want me to go ahead and fix this entirely ? (To handle the case where Partner is Customer and Supplier)

@smetl
Copy link
Contributor

smetl commented Aug 21, 2019

Let's check with @avw-odoo

@sswapnesh
Copy link
Contributor Author

@avw-odoo Can you please look into that? (At least for v13)

@smetl
Copy link
Contributor

smetl commented Sep 20, 2019

@sswapnesh In 13.0, partner_type disappeared (customer & vendor)

@sswapnesh
Copy link
Contributor Author

Oh yes, You are Right.
I will check all these again.
Let's close it than.

@sswapnesh sswapnesh closed this Sep 20, 2019
@robodoo robodoo added closed 💔 and removed CI 🤖 Robodoo has seen passing statuses labels Sep 20, 2019
@sswapnesh
Copy link
Contributor Author

Actually this was for 12.0

@sswapnesh sswapnesh reopened this Sep 20, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed closed 💔 labels Sep 20, 2019
@smetl
Copy link
Contributor

smetl commented Sep 23, 2019

@sswapnesh Discussed with @avw-odoo . We decide to not change the actual behavior.

@smetl smetl closed this Sep 23, 2019
@robodoo robodoo added closed 💔 and removed CI 🤖 Robodoo has seen passing statuses labels Sep 23, 2019
@sswapnesh sswapnesh deleted the patch-9 branch September 23, 2019 17:33
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.

None yet

6 participants