[FIX] partner_firstname: Multiple creation with "name"

[SirAionTech]

This happens after https://github.com/OCA/partner-contact/pull/1647.

Steps:

1. Create 2 partners in one create, only assigning their name (also see added test):

   self.env["res.partner"].create(
      [
          {
              "name": "Test partner1",
          },
          {
              "name": "Test partner2",
          },
      ]
   )

Actual behavior: Exception raised:

odoo.addons.partner_firstname.exceptions.EmptyNamesError: ("Error(s) with partner 1425's name.", 'No name is set.')

Expected behavior: 2 partners created

[SirAionTech]

@payen000 @hugho-ad @luisg123v @moylop260 you were involved in https://github.com/OCA/partner-contact/pull/1647, could you please have a look?

[payen000]

Hello @SirAionTech,

I would only add to the current review comments that the commit message [FIX] partner_firstname: Multiple creation with "name" could probably be more descriptive, like [FIX] partner_firstname: Avoid duplicate partner creation with "name" (or something different that points the attention to the issue being fixed). Then the commit description would clarify the details.

[SirAionTech]

Hello @SirAionTech,

I would only add to the current review comments that the commit message [FIX] partner_firstname: Multiple creation with "name" could probably be more descriptive, like [FIX] partner_firstname: Avoid duplicate partner creation with "name" (or something different that points the attention to the issue being fixed). Then the commit description would clarify the details.

Hi,

[FIX] partner_firstname: Avoid duplicate partner creation with "name"

Not sure what I would be avoiding here, could you please clarify that? Right now the commit message states that it is [FIX]ing Multiple creation with "name" because when trying to create multiple partners with only the name field, an exception is raised, so it needs [FIX]ing; I added the exception message being raised in the commit description, does it look better now?

[SirAionTech]

Missing commit description.

Added, see also https://github.com/OCA/partner-contact/pull/1742#issuecomment-2071660402.

[payen000]

Hello @SirAionTech,

Not sure what I would be avoiding here, could you please clarify that?

Of course!

From reviewing the partner's create method, it seems that previous to your fix all partners would be created on each iteration (as opposed to only one partner per iteration, as your change correctly does). I mean, if I sent a list of 3 partner value dicts, I would end up with 9 partners instead of the intended 3, effectively duplicating the partners on each iteration, no?

I thought at first that your commit message would be focused on that idea, but I now see that the focus of your commit message is in preventing the error that you describe in the commit message, so I think you can ignore my suggestion :)

[SirAionTech]

From reviewing the partner's create method, it seems that previous to your fix all partners would be created on each iteration (as opposed to only one partner per iteration, as your change correctly does). I mean, if I sent a list of 3 partner value dicts, I would end up with 9 partners instead of the intended 3, effectively duplicating the partners on each iteration, no?

I thought at first that your commit message would be focused on that idea, but I now see that the focus of your commit message is in preventing the error that you describe in the commit message, so I think you can ignore my suggestion :)

I see, I have clarified what I am fixing in the PR description too.

[moylop260]

/ocabot merge patch

[OCA-git-bot]

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

[OCA-git-bot]

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