[WIP] [16.0] [IMP] resource_booking: handle partner_ids (booking attendees) safely

[rrebollo]

📝 Description

This PR enhances the resource_booking module by introducing a safeguard against potential issues arising from empty partner_ids when managing booking attendees.

🛠️ Problem

In scenarios where partner_ids is empty or undefined, the system could encounter unexpected behaviors or errors during attendee processing.

✅ Solution

~~Implemented a check to ensure that partner_ids is properly initialized and handled, preventing potential exceptions and ensuring consistent behavior.~~ Following suggestion from reviewers we just use partner_ids[:1] instead.

🔍 Impact

• Enhances the robustness of the booking attendee management process.
• Prevents potential errors related to empty partner_ids.
• Improves overall system stability and user experience.

📎 Related Issues

N/A

More context: OCA/e-commerce#1048 @BinhexTeam

[OCA-git-bot]

Hi @ows-cloud, @pedrobaeza, some modules you are maintaining are being modified, check this out!

[rrebollo]

ping @Christian-RB

[pedrobaeza]

Isn't partner_ids mandatory? In which cases you should have empty partners?

[rrebollo]

Isn't partner_ids mandatory? In which cases you should have empty partners?

Did you have a chance to review OCA/e-commerce#1048] for additional context?
In short: this occurs when a booking is made via the e-commerce frontend by a non-logged-in user.

[pedrobaeza]

Sorry, but after reading your issue, my concerns remains the same. Field partner_ids are the attendees, and they should always be filled when creating the meeting. In fact, you are using self.partner_id as a fallback, when this field comes from partner_ids:

https://github.com/OCA/calendar/blob/65bd60a6abc46ddfc155e92874d08ae6478d4978/resource_booking/models/resource_booking.py#L268

so something else should be happening. If the problem is just in name_get, we can safeguard it adding a regression tests that proves the case, but for _prepare_meeting_vals the fallback will have the same problem of being False. The difference is that in the compute field [:1] is used, so it doesn't fail, so in any case, that should be used also in the code instead of going to self.partner_id.

But anyway, I insist that not having attendees seems the problem itself, and public user should not be the reason, as at the end, you have to login/create user when checking out, ending with a partner.

[rrebollo]

Thanks a lot for your analysis, @pedrobaeza 🙏

You're absolutely right to question this — but just to clarify, partner_id is also marked as readonly=False, so the value might come either from the compute method or from direct assignment. There are indeed some assignations scattered across the different addons that orchestrate this feature.

I completely agree with your point that "something else could be happening." We're not trying to fix that broader "something else" here — it would likely require a deeper analysis, possibly even a redesign of parts of website_sale_resource_booking (which hasn’t been merged yet in v16).

Our intention with this PR was simply to provide a safe fallback, so that users could at least complete the workflow successfully. There’s additional logic during the creation and update of booking records that helps reconcile any inconsistencies introduced by this minimal patch.

That said, I’ll convert this PR to draft and mark it as WIP. Later, I’ll revisit it with a fresh perspective and take more time to analyze it properly.

[pedrobaeza]

Thanks for your understanding. I try to avoid safeguards in general, as they hide real issues, making more difficult to identify the source of the problem, but I agree these modules may be a labyrinth of things that makes it very hard, so let's try a bit to find the underlying issue, and if not, we can at least provide the [:1] fallback.

[brambuijs]

@Christian-RB We are running into this issue on v16. 500 internal server error after reserving a spot. Implemented the above mentioned fix and that resolved the issue.

Can the PR be merged?

[rrebollo]

@pedrobaeza can we finally merge this?

[OCA-git-bot]

This PR looks fantastic, let's merge it! Prepared branch 16.0-ocabot-merge-pr-155-by-pedrobaeza-bump-patch, awaiting test results.

[OCA-git-bot]

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