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] google_calendar: ignore no partner created #2

Open
wants to merge 1 commit into
base: 14.0
Choose a base branch
from

Conversation

ntsirintanis
Copy link

No description provided.

@ntsirintanis ntsirintanis self-assigned this Apr 18, 2024
@ntsirintanis
Copy link
Author

this concerns odoo#10570. Haven't tested it yet, and looking for a way to actually do that

@thomaspaulb
Copy link
Member

@ntsirintanis The link you posted points to something else I think?

Did you mean one of:

odoo#78678

odoo#82454

@ntsirintanis
Copy link
Author

@ntsirintanis The link you posted points to something else I think?

Did you mean one of:

odoo#78678

odoo#82454

haha. Yes, by 10570 I meant our internal issue, but githut tlinked it automatically to some other issue and I did not notice that.


partners = self.env['mail.thread']._mail_find_partner_from_emails(emails, records=self, force_create=True, extra_domain=[('type', '!=', 'private')])
for attendee in zip(emails, partners, google_attendees):
email = attendee[0]
Copy link
Member

Choose a reason for hiding this comment

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

Question: the 15.0 fix which was merged looks decidedly simpler. Did you have to include other changes that were done from 14.0->15.0?

If yes -> is it possible to cherry-pick these, or at least put them in a separate commit from the actual fix for this issue?

If no -> can you explain a bit how you arrived at the current code

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I had to change _odoo_attendee_commands() method significantly, as it is basically different from the 15.0 version. The difference that required these changes is how partners are retreived. As you can see, cherry-picking won't work here.

email = attendee.get('email')

partners = self.env['mail.thread']._mail_find_partner_from_emails(emails, records=self, force_create=True, extra_domain=[('type', '!=', 'private')])
for attendee in zip(emails, partners, google_attendees):
Copy link
Member

Choose a reason for hiding this comment

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

The 15.0 fixes fixes both Google and Microsoft. What we're after is a fix for Microsoft. This PR looks like it's only addressing Google?

Copy link
Author

Choose a reason for hiding this comment

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

The microsoft part of 15.0-fix is not applicable here, because the microsoft_calendar module has evolved. In addition, the method in microsoft_calendar that is edited in the fix, is identical between 14.0 and 15.0

Copy link
Author

Choose a reason for hiding this comment

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

So what I did, is to basically migrate what is in the fix commit you shared. And now I want to test it, already asked Niels if he is able to do so too.

Copy link
Member

Choose a reason for hiding this comment

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

OK yes, I see now the Microsoft side has evolved, so that fix is not applicable anymore to it.

migrate what is in the fix commit you shared

Which commit do you mean here?

testing by Niels

Will be difficult for him if your change is on the Google side, because BMAir just uses the Microsoft part.

Copy link
Author

Choose a reason for hiding this comment

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

this one https://github.com/odoo/odoo/pull/82454/files.

I will check out microsoft_calendar (v14) again, to see if there's something I can do about it. The gist here is that the fix for 15 is not applicable to 14, at least not directly.

Copy link
Author

Choose a reason for hiding this comment

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

ok got it, pushing another commit asap

Copy link
Member

Choose a reason for hiding this comment

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

OK. I just looked into it myself as well, just going from the original traceback. I'm a bit sad about my bad judgement because I thought this would be an easy backport, but it's not. Anyway, what I've found (you may have already found it yourself as well):

  • message_partner_info_from_emails has a possibility for partner to be Falsy, and only when force_create=True and some other condition is satisfied, will it create a partner and otherwise it will just add a Falsy value for partner to the dict, probably an empty partner recordset since .id still works on it
  • command_attendee will then contain the Falsy value, eventually leading to the creation error of the attendee

Copy link
Member

Choose a reason for hiding this comment

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

The first behavior does not seem changed in newer Odoo versions. Maybe/hopefully they fixed the second behavior to check for falsy partner values before adding the record.

attendee_commands += [(0, 0, {'state': attendee.get('responseStatus'), 'partner_id': partner.id})]
if attendee[2].get('self'):
partner = self.env.user.partner_id
elif attendee[1]:
Copy link
Author

Choose a reason for hiding this comment

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

@thomaspaulb basically this elif resolves the falsy partner problem. Just need to test it. I will do the same for microsoft_calendar, almost done, will create an additional PR for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants