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

[16.0][IMP] resource_booking: Add support for mail_note subtype by default #129

Merged

Conversation

victoralmau
Copy link
Member

Add support for mail_note subtype by default

If we are creating the calendar event from resource booking, we want to notify only the partner_ids and not all followers (to avoid that each email is sent to all followers). Example: Resource booking with combination of several users. This happens only when the the subtype not is enabled by default in the instance.

Related to a3cd5a8

Please @pedrobaeza can you review it?

@Tecnativa TT49045

@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza added this to the 16.0 milestone Jun 18, 2024
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Isn't better to delay the subscription of the resources for avoiding both problems?

On the commit message, please change related to by complement of, as it's not the same.

@victoralmau
Copy link
Member Author

Isn't better to delay the subscription of the resources for avoiding both problems?

On the commit message, please change related to by complement of, as it's not the same.

Followers are added when creating the calendar.attendee (https://github.com/odoo/odoo/blob/16.0/addons/calendar/models/calendar_attendee.py#L69C19-L69C37), if you prefer we can do that approach (delete the existing followers of mail_note subtype from all attendee created before and add them later).

@pedrobaeza
Copy link
Member

What I still don't understand is why doing a manual calendar event with multiple attendees, this is not happening. Why can't we mimic what standard does there?

@victoralmau
Copy link
Member Author

Yes, it is possible for this to happen with a manual event.

Steps to reproduce it.

  • Activate the Notes subtype by default.
  • Create a calendar event with future date and add to: Admin + Demo + Azure
  • The invitation email from Azure will have been sent to Demo as well.

example

Do you think this is something that should be resolved in calendar?

resource_booking/models/calendar_event.py Outdated Show resolved Hide resolved
resource_booking/models/calendar_event.py Outdated Show resolved Hide resolved
@pedrobaeza
Copy link
Member

Let's keep it this way, as to get it upstream will be difficult, and mangle with followers will be difficult.

If we are creating the calendar event from resource booking, we want to
notify only the partner_ids and not all followers (to avoid that each email is
sent to all followers). Example: Resource booking with combination of several users.
This happens only when the the subtype not is enabled by default in the instance.

Related to OCA@a3cd5a8

TT49045
@victoralmau victoralmau force-pushed the 16.0-imp-resource_booking-TT49045-v2 branch from 22c8dba to 851dc9a Compare June 19, 2024 07:51
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Let's see if we finally tackle this:

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-129-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b071f82 into OCA:16.0 Jun 19, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

@pedrobaeza The merge process could not be finalized, because command git push origin 16.0-ocabot-merge-pr-129-by-pedrobaeza-bump-patch:16.0 failed with output:

To https://github.com/OCA/calendar
 ! [rejected]        16.0-ocabot-merge-pr-129-by-pedrobaeza-bump-patch -> 16.0 (fetch first)
error: failed to push some refs to 'https://***@github.com/OCA/calendar'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 16.0-imp-resource_booking-TT49045-v2 branch June 19, 2024 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants