[IMP] cooperator[_website]: Improve form

[carmenbianca]

Internal task: https://gestion.coopiteasy.be/web#id=11130&action=479&model=project.task&view_type=form&menu_id=536

[victor-champonnois]

Functional review

Great, the interface is much better !

Some issues:

• french translations should be updated, but I think we can fix this afterwards in weblate. We just need to remember it.
• to me it's weird that the field "existing contact" stays visible even when the type of subscription is "new cooperator". The fact that it's among the first field makes this even more weird. Is there a reason to keep it visible ? This means that the field should be set to null when the type of subscription is set to new cooperator (after having previously selected an "existing contact").

I think, we can also improve significantly the "Subscription Information" section, at low cost. I am writing a commit to do that.

[victor-champonnois]

I think, we can also improve significantly the "Subscription Information" section, at low cost. I am writing a commit to do that.

done

to me it's weird that the field "existing contact" stays visible even when the type of subscription is "new cooperator". The fact that it's among the first field makes this even more weird. Is there a reason to keep it visible ? This means that the field should be set to null when the type of subscription is set to new cooperator (after having previously selected an "existing contact").

I think this should be considered, I am not sure how the case "existing contact is set" and type of subscription is "new cooperator" is handled, and whether we should prevent it directly from the interface. @huguesdk what do you think ?

[victor-champonnois]

I think this should be considered, I am not sure how the case "existing contact is set" and type of subscription is "new cooperator" is handled, and whether we should prevent it directly from the interface. @huguesdk what do you think ?

nevermind I see it's handled here. It could be nice if it was handled directly in the form, but to me it's OK like this

[victor-champonnois]

cooperative/cooperator/models/subscription_request.py

Lines 750 to 756 in 7ce1aea

	if self.already_cooperator:
	raise UserError(
	_(
	"The checkbox already cooperator is"
	" checked please select a cooperator."
	)
	)

Shouldn't this code be removed or altered now that there is no checkbox ?

[huguesdk]

@victor-champonnois about the “new cooperator” type for an existing contact: it is counter-intuitive, but it is valid and important: this is to allow to create a subscription (to become a cooperator) for a contact (res.partner) that already exists in the database but is not yet a cooperator.

what should be prevented, though, are these combinations:

• type is increase, existing contact is none (existing contact must become required in case of increase) or existing contact is not already a cooperator
• type is new cooperator, existing contact is set to a contact that is already a cooperator

moreover, when an existing contact is selected, all the fields corresponding to the contact properties and not related to the subscription request itself should become read-only, because the contact details will not be updated from the subscription request.

[huguesdk]

@victor-champonnois @carmenbianca i realize that the subscription type should not be editable: it should be read-only and computed from the existing contact field:

• if it is set and the contact is already member: increase
• otherwise: new

it should just be there for information, and i think that a more logical place would be after the existing contact field. what do you think?

[victor-champonnois]

Yes it sounds like a good idea !

[victor-champonnois]

@huguesdk I've implemented your idea. (cf last commit). @carmenbianca

[victor-champonnois]

... and fixed the tests (including some other test errors). Strange, this behavior was in contradiction with one of the test (a sub request for an existing partner that is not yet member was expected to be of type "increase").

[carmenbianca]

Rebased this on top of 6eef0d9 because otherwise there would be merge conflicts and the tests wouldn't run.

[carmenbianca]

Made some small fixes on top of @victor-champonnois 's work. Ready for review+merge I think

[github-actions]

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.

[huguesdk]

this field should be stored (and probably not computed but updated with an onchange), otherwise its value will change over time and it is not possible to search or group by it. same for already_cooperator.

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause of the Test Failure

The test failure is caused by a missing database connection during test execution, likely due to an incorrect or incomplete database setup in the test environment (runboat). This prevents Odoo from initializing and running the tests. It is not related to the logic changes in the PR itself.

---

2. Suggested Fix

The database connection error is not due to code changes in the PR but likely due to misconfiguration in the CI or test environment (e.g., runboat). However, to ensure the PR is compatible with OCA standards and Odoo 16.0, verify that all tests are run in a proper database context, and that the test setup does not rely on external services or unconfigured databases.

If this is a local environment issue, the following should be checked:

• Ensure odoo.conf is correctly configured with database settings.
• Ensure that the database is created and accessible.
• Run tests using odoo-bin -d <database_name> -i cooperator --test-enable.

---

3. Additional Code Issues

a. Inconsistent logic in _compute_type

• File: cooperator/models/subscription_request.py
• Lines: 161–171
• Issue: The _compute_type method computes type and already_cooperator based on partner_id.member, but it doesn't handle the case where partner_id is False properly. It assumes partner_id exists, which may not be true in all contexts (e.g., during creation or form handling).
• Fix: Ensure that partner_id is checked for existence before accessing partner_id.member.

@api.depends("partner_id.member")
def _compute_type(self):
    for sub_request in self:
        if sub_request.partner_id and sub_request.partner_id.member:
            sub_request.already_cooperator = True
            sub_request.type = "increase"
        elif sub_request.partner_id:
            sub_request.already_cooperator = False
            sub_request.type = "new"
        else:
            sub_request.already_cooperator = False
            sub_request.type = "new"

b. Incorrect test expectations

• File: cooperator/tests/test_cooperator.py
• Lines: 304, 324
• Issue: Tests now expect type = "new" for second subscription from same partner, but this is inconsistent with the new logic that sets type = "increase" if partner_id.member = True. The test should reflect that the second subscription should be "increase" if the partner is already a cooperator.
• Fix: Update test expectations to match logic:
  • If partner_id.member = True, the second subscription should be "increase".

---

4. Test Improvements

a. Add a test case for partner with no membership

• File: cooperator/tests/test_cooperator.py
• Suggested test:

def test_subscription_from_non_member_partner(self):
    partner = self.env["res.partner"].create({"name": "Test Partner"})
    vals = self.get_dummy_subscription_requests_vals()
    vals["partner_id"] = partner.id
    subscription_request = self.env["subscription.request"].create(vals)
    self.assertFalse(subscription_request.already_cooperator)
    self.assertEqual(subscription_request.type, "new")

b. Test onchange_partner logic

• File: cooperator/tests/test_cooperator.py
• Suggested test:

def test_onchange_partner_sets_type_correctly(self):
    partner = self.env["res.partner"].create({"name": "Test Partner"})
    partner.create_cooperative_membership(self.env.company)
    partner.member = True

    with Form(self.env["subscription.request"]) as form:
        form.partner_id = partner
        self.assertEqual(form.type, "increase")
        self.assertTrue(form.already_cooperator)

c. Use SavepointCase for complex test scenarios

• For tests that modify state or rely on database integrity, prefer using odoo.tests.common.SavepointCase to isolate tests and avoid side effects.

d. Test form behavior with existing partner

• File: cooperator/tests/test_cooperator.py
• Suggested test:

def test_form_onchange_partner_updates_fields(self):
    partner = self.env["res.partner"].create({
        "name": "Test Partner",
        "firstname": "John",
        "lastname": "Doe",
        "email": "john@example.com",
    })
    partner.create_cooperative_membership(self.env.company)
    partner.member = True

    with Form(self.env["subscription.request"]) as form:
        form.partner_id = partner
        self.assertEqual(form.type, "increase")
        self.assertEqual(form.firstname, "John")
        self.assertEqual(form.lastname, "Doe")
        self.assertEqual(form.email, "john@example.com")

These tests align with OCA patterns and ensure correctness of the new logic around already_cooperator and type fields.

---

⚠️ PR Aging Alert: CRITICAL

This PR by @carmenbianca has been waiting for 755 days — that is over 25 months without being merged or closed.

Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)

---

Reciprocal Review Request

Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):

My open PRs across OCA:

• server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
• hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
• stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
• stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
• stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
• purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix

Reviewing each other's work helps the whole community move forward. Thank you!

---

Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b