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.

Sign up for GitHub

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
Open

[FIX] google_calendar: ignore no partner created #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 17 additions & 10 deletions addons/google_calendar/models/calendar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,22 +109,29 @@ def _odoo_attendee_commands(self, google_event):
if google_event.exists(self.env):
existing_attendees = self.browse(google_event.odoo_id(self.env)).attendee_ids
attendees_by_emails = {tools.email_normalize(a.email): a for a in existing_attendees}
for attendee in google_attendees:
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.

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.

state = attendee[2].get('status', {}).get('response', 'accepted')
if email in attendees_by_emails:
# Update existing attendees
attendee_commands += [(1, attendees_by_emails[email].id, {'state': attendee.get('responseStatus')})]
attendee_commands += [(1, attendees_by_emails[email].id, {'state': state})]
else:
# Create new attendees
partner = self.env.user.partner_id if attendee.get('self') else self.env['res.partner'].find_or_create(attendee.get('email'))
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

partner = attendee[1]
else:
continue
attendee_commands += [(0, 0, {'state': state, 'partner_id': partner.id})]
partner_commands += [(4, partner.id)]
if attendee.get('displayName') and not partner.name:
partner.name = attendee.get('displayName')
if attendee[2].get('displayName') and not partner.name:
partner.name = attendee[2].get('displayName')
for odoo_attendee in attendees_by_emails.values():
# Remove old attendees
if tools.email_normalize(odoo_attendee.email) not in emails:
# Remove old attendees but only if it does not correspond to the current user.
email = tools.email_normalize(odoo_attendee.email)
if email not in emails and email != self.env.user.email:
attendee_commands += [(2, odoo_attendee.id)]
partner_commands += [(3, odoo_attendee.partner_id.id)]
return attendee_commands, partner_commands
Expand Down