From bff86e441e36203bc526813cf9d9803053f63821 Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 20 Feb 2024 14:23:48 +0100 Subject: [PATCH 01/12] [IMP] cooperator: Improve form Better names, move important fields to the start Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 3 ++- cooperator/views/subscription_request_view.xml | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index 603f3f5f4..6ed00997a 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -224,6 +224,7 @@ def _compute_subscription_amount(self): ("new", "New Cooperator"), ("increase", "Increase number of share"), ], + string="Type of Subscription", default="new", readonly=True, states={"draft": [("readonly", False)]}, @@ -258,7 +259,7 @@ def _compute_subscription_amount(self): ) partner_id = fields.Many2one( "res.partner", - string="Cooperator", + string="Existing Contact", readonly=True, states={"draft": [("readonly", False)]}, ) diff --git a/cooperator/views/subscription_request_view.xml b/cooperator/views/subscription_request_view.xml index 6337c6e6b..b1f0b4af2 100644 --- a/cooperator/views/subscription_request_view.xml +++ b/cooperator/views/subscription_request_view.xml @@ -97,10 +97,12 @@ SPDX-License-Identifier: AGPL-3.0-or-later attrs="{'invisible': [('active', '=', True)]}" /> + - + + - - From 0b31765be8ca7350eb0103eb99ee532b608bc864 Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 20 Feb 2024 14:24:33 +0100 Subject: [PATCH 02/12] [IMP] cooperator[_website]: Factor out already_cooperator Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 13 +++++++++---- cooperator/views/subscription_request_view.xml | 1 - cooperator_website/controllers/main.py | 11 ----------- cooperator_website/views/subscription_template.xml | 14 -------------- 4 files changed, 9 insertions(+), 30 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index 6ed00997a..ad11245a6 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -125,8 +125,6 @@ def _adapt_create_vals_and_membership_from_partner(self, vals, partner): # handle draft and waiting requests. if member or self.search(pending_requests_domain): vals["type"] = "increase" - if member: - vals["already_cooperator"] = True if not cooperative_membership: cooperative_membership = partner.create_cooperative_membership(company_id) elif not cooperative_membership.cooperator: @@ -170,6 +168,14 @@ def _compute_name(self): if part ) + @api.depends("partner_id.member") + def _compute_already_cooperator(self): + for sub_request in self: + if sub_request.partner_id: + sub_request.already_cooperator = sub_request.partner_id.member + else: + sub_request.already_cooperator = False + @api.depends("iban", "skip_iban_control") def _compute_is_valid_iban(self): for sub_request in self: @@ -188,7 +194,7 @@ def _compute_subscription_amount(self): already_cooperator = fields.Boolean( string="I'm already cooperator", readonly=True, - states={"draft": [("readonly", False)]}, + compute="_compute_already_cooperator", ) # previously, this was a normal field. it is now computed, and is used for @@ -496,7 +502,6 @@ def onchange_partner(self): partner = self.partner_id if partner: self.is_company = partner.is_company - self.already_cooperator = partner.member if partner.bank_ids: self.iban = partner.bank_ids[0].acc_number if partner.member: diff --git a/cooperator/views/subscription_request_view.xml b/cooperator/views/subscription_request_view.xml index b1f0b4af2..b3290f143 100644 --- a/cooperator/views/subscription_request_view.xml +++ b/cooperator/views/subscription_request_view.xml @@ -100,7 +100,6 @@ SPDX-License-Identifier: AGPL-3.0-or-later - -
From 56c25e9ecfb8c9b3a927ec21f43ecbc98085b104 Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 20 Feb 2024 14:17:21 +0100 Subject: [PATCH 03/12] [IMP] cooperator: Title sections in subscription form view Signed-off-by: Carmen Bianca BAKKER --- cooperator/views/subscription_request_view.xml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cooperator/views/subscription_request_view.xml b/cooperator/views/subscription_request_view.xml index b3290f143..5ba13b239 100644 --- a/cooperator/views/subscription_request_view.xml +++ b/cooperator/views/subscription_request_view.xml @@ -102,6 +102,8 @@ SPDX-License-Identifier: AGPL-3.0-or-later + + + + @@ -145,7 +149,7 @@ SPDX-License-Identifier: AGPL-3.0-or-later - + Date: Tue, 20 Feb 2024 14:18:37 +0100 Subject: [PATCH 04/12] [FIX] cooperator: Make subscription type mandatory Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index ad11245a6..0d94018f6 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -232,6 +232,7 @@ def _compute_subscription_amount(self): ], string="Type of Subscription", default="new", + required=True, readonly=True, states={"draft": [("readonly", False)]}, ) From 28bb786c95b3cc6453fcf3db63c7b481d705377d Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 20 Feb 2024 14:19:10 +0100 Subject: [PATCH 05/12] =?UTF-8?q?[IMP]=20cooperator:=20Clarify=20that=20ac?= =?UTF-8?q?count=20=E2=84=96=20is=20bank=20account=20=E2=84=96?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index 0d94018f6..fd00f84f3 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -260,7 +260,7 @@ def _compute_subscription_amount(self): states={"draft": [("readonly", False)]}, ) iban = fields.Char( - string="Account Number", + string="Bank Account Number", readonly=True, states={"draft": [("readonly", False)]}, ) From a01d4eb47a06116064f350295c499d229d6a0147 Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 20 Feb 2024 16:34:00 +0100 Subject: [PATCH 06/12] [IMP] cooperator: Write test case for onchange partner_id Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 1 - cooperator/tests/test_cooperator.py | 39 ++++++++++++++++++++++- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index fd00f84f3..cd6b40146 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -486,7 +486,6 @@ def _compute_subscription_amount(self): def get_person_info(self, partner): self.firstname = partner.firstname - self.name = partner.name self.lastname = partner.lastname self.email = partner.email self.birthdate = partner.birthdate_date diff --git a/cooperator/tests/test_cooperator.py b/cooperator/tests/test_cooperator.py index a628a3b3f..a6c390c88 100644 --- a/cooperator/tests/test_cooperator.py +++ b/cooperator/tests/test_cooperator.py @@ -9,7 +9,7 @@ from odoo import fields from odoo.exceptions import AccessError, UserError, ValidationError -from odoo.tests.common import TransactionCase, users +from odoo.tests.common import Form, TransactionCase, users from .cooperator_test_mixin import CooperatorTestMixin @@ -892,6 +892,43 @@ def test_existing_partner_company_dependent_fields_with_membership(self): cooperative_membership.generic_rules_approved, ) + def test_partner_existing(self): + """ + Test that selecting an existing partner automatically changes certain + fields to default values. + """ + partner = self.env["res.partner"].create( + { + "firstname": "Test", + "lastname": "Partner", + "email": "test@example.com", + "birthdate_date": "2018-01-01", + "gender": "other", + "street": "Example Street 1", + "city": "Brussels", + "zip": "1000", + "country_id": self.env.ref("base.be").id, + "phone": "1234", + "lang": "en_US", + } + ) + with Form(self.env["subscription.request"]) as request_form: + request_form.partner_id = partner + self.assertEqual(request_form.firstname, partner.firstname) + self.assertEqual(request_form.lastname, partner.lastname) + self.assertEqual(request_form.email, partner.email) + self.assertEqual(request_form.birthdate, partner.birthdate_date) + self.assertEqual(request_form.gender, partner.gender) + self.assertEqual(request_form.address, partner.street) + self.assertEqual(request_form.city, partner.city) + self.assertEqual(request_form.zip_code, partner.zip) + self.assertEqual(request_form.country_id, partner.country_id) + self.assertEqual(request_form.phone, partner.phone) + self.assertEqual(request_form.lang, partner.lang) + + # This is to make sure that the form can be saved. + request_form.share_product_id = self.share_x + @freeze_time("2023-06-21") def test_partner_company_dependent_fields_with_membership(self): """ From 168c3d04dc41480e25983f4b08b73d5539a8b32d Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Tue, 20 Feb 2024 17:47:02 +0100 Subject: [PATCH 07/12] [IMP] cooperator: Force 'increase' type if already cooperator Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 13 +++++++++++-- cooperator/tests/test_cooperator.py | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index cd6b40146..d228e7c4f 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -192,7 +192,7 @@ def _compute_subscription_amount(self): ) already_cooperator = fields.Boolean( - string="I'm already cooperator", + string="Already a cooperator", readonly=True, compute="_compute_already_cooperator", ) @@ -228,7 +228,7 @@ def _compute_subscription_amount(self): type = fields.Selection( [ ("new", "New Cooperator"), - ("increase", "Increase number of share"), + ("increase", "Increase number of shares"), ], string="Type of Subscription", default="new", @@ -848,6 +848,15 @@ def validate_subscription_request(self): if self.ordered_parts <= 0: raise UserError(_("Number of share must be greater than 0.")) + if self.already_cooperator and self.type != "increase": + raise UserError( + _( + "Partner %s is already a cooperator, subscription type" + " must be 'Increase number of shares'." + ) + % self.partner_id.name + ) + partner = self.setup_partner() invoice = self.create_invoice(partner) diff --git a/cooperator/tests/test_cooperator.py b/cooperator/tests/test_cooperator.py index a6c390c88..1c5380b09 100644 --- a/cooperator/tests/test_cooperator.py +++ b/cooperator/tests/test_cooperator.py @@ -912,8 +912,12 @@ def test_partner_existing(self): "lang": "en_US", } ) + partner.create_cooperative_membership(self.env.company) + partner.member = True with Form(self.env["subscription.request"]) as request_form: + self.assertEqual(request_form.type, "new") request_form.partner_id = partner + self.assertEqual(request_form.type, "increase") self.assertEqual(request_form.firstname, partner.firstname) self.assertEqual(request_form.lastname, partner.lastname) self.assertEqual(request_form.email, partner.email) @@ -929,6 +933,23 @@ def test_partner_existing(self): # This is to make sure that the form can be saved. request_form.share_product_id = self.share_x + def test_constrain_type_if_already_cooperator(self): + """ + If a partner is already a cooperator, don't allow the 'new' type. + """ + partner = self.env["res.partner"].create({"name": "Test Partner"}) + partner.create_cooperative_membership(self.env.company) + partner.member = True + vals = self.get_dummy_subscription_requests_vals() + vals["type"] = "new" + subscription_request = self.env["subscription.request"].create(vals) + subscription_request.partner_id = partner + self.assertTrue(subscription_request.already_cooperator) + with self.assertRaises(UserError): + subscription_request.validate_subscription_request() + subscription_request.type = "increase" + subscription_request.validate_subscription_request() + @freeze_time("2023-06-21") def test_partner_company_dependent_fields_with_membership(self): """ From 9da1b253327c377243213d0731276cdac2832419 Mon Sep 17 00:00:00 2001 From: Victor Champonnois Date: Wed, 21 Feb 2024 10:01:12 +0100 Subject: [PATCH 08/12] [IMP] cooperator: subscription information form in backend --- .../views/subscription_request_view.xml | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/cooperator/views/subscription_request_view.xml b/cooperator/views/subscription_request_view.xml index 5ba13b239..06a4fb3d6 100644 --- a/cooperator/views/subscription_request_view.xml +++ b/cooperator/views/subscription_request_view.xml @@ -148,24 +148,21 @@ SPDX-License-Identifier: AGPL-3.0-or-later + - - - - - + - - + + @@ -180,6 +177,8 @@ SPDX-License-Identifier: AGPL-3.0-or-later groups="!base.group_multi_company" invisible="True" /> + + From c5a6b2007246b3a08a9fc6c0b0d8abbc6f3a7e7c Mon Sep 17 00:00:00 2001 From: Victor Champonnois Date: Mon, 26 Feb 2024 15:24:15 +0100 Subject: [PATCH 09/12] [IMP] cooperator: subscription.request type is now computed --- cooperator/models/subscription_request.py | 12 +++++++++--- cooperator/views/subscription_request_view.xml | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index d228e7c4f..e5f59035a 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -169,12 +169,16 @@ def _compute_name(self): ) @api.depends("partner_id.member") - def _compute_already_cooperator(self): + def _compute_type(self): for sub_request in self: if sub_request.partner_id: sub_request.already_cooperator = sub_request.partner_id.member + sub_request.type = ( + "increase" if sub_request.partner_id.member else "new" + ) else: sub_request.already_cooperator = False + sub_request.type = "new" @api.depends("iban", "skip_iban_control") def _compute_is_valid_iban(self): @@ -191,10 +195,12 @@ def _compute_subscription_amount(self): sub_request.share_product_id.list_price * sub_request.ordered_parts ) + # The field is technically kind of superfluous, but it's also handy as a + # shorthand. already_cooperator = fields.Boolean( string="Already a cooperator", readonly=True, - compute="_compute_already_cooperator", + compute="_compute_type", ) # previously, this was a normal field. it is now computed, and is used for @@ -231,10 +237,10 @@ def _compute_subscription_amount(self): ("increase", "Increase number of shares"), ], string="Type of Subscription", + compute="_compute_type", default="new", required=True, readonly=True, - states={"draft": [("readonly", False)]}, ) state = fields.Selection( [ diff --git a/cooperator/views/subscription_request_view.xml b/cooperator/views/subscription_request_view.xml index 06a4fb3d6..2d367854d 100644 --- a/cooperator/views/subscription_request_view.xml +++ b/cooperator/views/subscription_request_view.xml @@ -100,8 +100,8 @@ SPDX-License-Identifier: AGPL-3.0-or-later - + Date: Mon, 26 Feb 2024 16:49:02 +0100 Subject: [PATCH 10/12] [FIX] cooperator: tests the field type is computed such that it's 'increase' only if the sub request is linked with a partner that is a member. This contradicts previous tests according the which a sub request for a non member partner should have a type 'increase' --- cooperator/tests/test_cooperator.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/cooperator/tests/test_cooperator.py b/cooperator/tests/test_cooperator.py index 1c5380b09..a97d21e5a 100644 --- a/cooperator/tests/test_cooperator.py +++ b/cooperator/tests/test_cooperator.py @@ -290,8 +290,8 @@ def test_create_subscription_from_non_cooperator_company_partner(self): def test_create_multiple_subscriptions_from_non_cooperator_partner(self): """ - Test that creating a subscription from a partner that has no parts yet - creates a subscription request with the correct type. + Test that creating a subscription from a partner is not member yet + creates a subscription request with type 'new'. """ partner = self.env["res.partner"].create( { @@ -301,12 +301,12 @@ def test_create_multiple_subscriptions_from_non_cooperator_partner(self): subscription_request = self.create_dummy_subscription_from_partner(partner) self.assertEqual(subscription_request.type, "new") subscription_request2 = self.create_dummy_subscription_from_partner(partner) - self.assertEqual(subscription_request2.type, "increase") + self.assertEqual(subscription_request2.type, "new") def test_create_multiple_subscriptions_from_non_cooperator_company_partner(self): """ - Test that creating a subscription from a company partner that has no - parts yet creates a subscription request with the correct type. + Test that creating a subscription from a company partner is not member yet + creates a subscription request with the type 'new'. """ partner = self.env["res.partner"].create( { @@ -321,7 +321,7 @@ def test_create_multiple_subscriptions_from_non_cooperator_company_partner(self) subscription_request2 = self.create_dummy_subscription_from_company_partner( partner ) - self.assertEqual(subscription_request2.type, "increase") + self.assertEqual(subscription_request2.type, "new") def test_create_subscription_from_cooperator_partner(self): """ @@ -935,19 +935,16 @@ def test_partner_existing(self): def test_constrain_type_if_already_cooperator(self): """ - If a partner is already a cooperator, don't allow the 'new' type. + If a partner is already a cooperator, check that type is 'increase' """ partner = self.env["res.partner"].create({"name": "Test Partner"}) partner.create_cooperative_membership(self.env.company) partner.member = True vals = self.get_dummy_subscription_requests_vals() - vals["type"] = "new" subscription_request = self.env["subscription.request"].create(vals) subscription_request.partner_id = partner self.assertTrue(subscription_request.already_cooperator) - with self.assertRaises(UserError): - subscription_request.validate_subscription_request() - subscription_request.type = "increase" + self.assertEqual(subscription_request.type, "increase") subscription_request.validate_subscription_request() @freeze_time("2023-06-21") From 1d7e5dc8333f45d2228b4eca95a20ce8d662a842 Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Mon, 4 Mar 2024 11:21:30 +0100 Subject: [PATCH 11/12] [FIX] cooperator: Remove superfluous checks This is dead code since type and already_cooperator are both computed Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index e5f59035a..770681a0d 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -754,14 +754,7 @@ def _find_or_create_partner(self): if partner: return partner domain = [] - if self.already_cooperator: - raise UserError( - _( - "The checkbox already cooperator is" - " checked please select a cooperator." - ) - ) - elif self.is_company and self.company_register_number: + if self.is_company and self.company_register_number: domain = [ ( "company_register_number", @@ -854,15 +847,6 @@ def validate_subscription_request(self): if self.ordered_parts <= 0: raise UserError(_("Number of share must be greater than 0.")) - if self.already_cooperator and self.type != "increase": - raise UserError( - _( - "Partner %s is already a cooperator, subscription type" - " must be 'Increase number of shares'." - ) - % self.partner_id.name - ) - partner = self.setup_partner() invoice = self.create_invoice(partner) From ab5ce536948c153025020194e1ad1e0e0fe6159a Mon Sep 17 00:00:00 2001 From: Carmen Bianca BAKKER Date: Mon, 4 Mar 2024 11:25:52 +0100 Subject: [PATCH 12/12] [FIX] cooperator: Remove dead code that adjusts 'type' type is computed. This code is now superfluous. Signed-off-by: Carmen Bianca BAKKER --- cooperator/models/subscription_request.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/cooperator/models/subscription_request.py b/cooperator/models/subscription_request.py index 770681a0d..ee44004e1 100644 --- a/cooperator/models/subscription_request.py +++ b/cooperator/models/subscription_request.py @@ -115,16 +115,6 @@ def _adapt_create_vals_and_membership_from_partner(self, vals, partner): company_id = vals.get("company_id", self.env.company.id) company_id = self.env["res.company"].browse(company_id) cooperative_membership = partner.get_cooperative_membership(company_id) - member = cooperative_membership and cooperative_membership.member - pending_requests_domain = [ - ("company_id", "=", company_id.id), - ("partner_id", "=", partner.id), - ("state", "in", ("draft", "waiting", "done")), - ] - # we don't use partner.coop_candidate because we want to also - # handle draft and waiting requests. - if member or self.search(pending_requests_domain): - vals["type"] = "increase" if not cooperative_membership: cooperative_membership = partner.create_cooperative_membership(company_id) elif not cooperative_membership.cooperator: @@ -510,8 +500,6 @@ def onchange_partner(self): self.is_company = partner.is_company if partner.bank_ids: self.iban = partner.bank_ids[0].acc_number - if partner.member: - self.type = "increase" if partner.is_company: self.company_name = partner.name self.company_email = partner.email