-
Notifications
You must be signed in to change notification settings - Fork 0
[ADD] calendar_shared_ics #1
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
base: 16.0
Are you sure you want to change the base?
Conversation
e8afe93 to
ff4a529
Compare
ff4a529 to
12286f4
Compare
|
@thomaspaulb @NL66278 so now this is incorporated in to portal wizard. Manager creates a calendard.shared.ics record, sets partner_id, generates token, url is generated. Manager sends mail to user, user clicks on button mail, user is directed to form view of the calendar record, where they can copy the link and configure their mail client. Let's agree on a final version, and then I'll finalize this with documentation and all |
| The calendar is protected by an access token embedded in the URL. | ||
| Anyone who obtains this URL can view the calendar. | ||
| Treat the link as a secret. | ||
| R No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This R is superfluous
|
|
||
| This module allows sharing an Odoo calendar as a **read-only ICS feed** that can be | ||
| subscribed to in external calendar clients such as **Outlook**, **Google Calendar**, | ||
| and **Thunderbird**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not loving the use of bold, can we limit it to, eg, only "Security note"
| share_url = fields.Char(compute="_compute_share_urls", readonly=True) | ||
|
|
||
| def _compute_access_url(self): | ||
| """Adjust to modufy access url""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modify
| base_url = self.get_base_url() | ||
| for cal in self: | ||
| cal._portal_ensure_token() | ||
| ics_url = f"{base_url}/calendar/shared/{cal.id}/ics?access_token={cal.access_token}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe simplify by using self.access_url + '?access_token=...' and depending on access_url as well? If this is not hallucinated then that should work properly
| """Rotate token (invalidate old subscription URLs).""" | ||
| for cal in self.sudo(): | ||
| cal.access_token = False | ||
| cal._portal_ensure_token() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken then someone could now, when he has the access token, also call this RPC method by which to reset the token to something else. Maybe as a precaution we could check for write access to the calendar, the access_token should not grant that.
|
Minor comments only, I think it remains to test this. Can you also open the OCA PR? You can mark it as Draft if you're still not sure about it fully |
| apply_partner_filter = fields.Boolean( | ||
| default=True, | ||
| help="If enabled, only events where this partner is an attendee are exported.", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When not using the wizard, it would be nice to have another checkbox here apply_user_filter which is on by default and which restricts the domain of the "partner_id" field to (portal) users only. Hell, maybe even a second checkbox "internal users", that also can be switched off if needed. Then it's easier to quickly find the user you are aiming for among a zyriad of portal users and partners, when you will mostly be looking for the internal people.
| """Return merged ICS content (bytes) for this feed.""" | ||
| self.ensure_one() | ||
| events = self._get_events_for_export() | ||
| files = events._get_ics_file() or {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the debug log of Thunderbird, at least one item that can't be processed correctly is one of the three items that contains:
RRULE:DTSTART:20220425T063000
RRULE:FREQ=WEEKLY;UNTIL=20220704T235959;BYDAY=MO
According to the JS log this is processed into a rule with a variable that's named DTSTART:20220425T063000 and which has a value of NULL, but more importantly, without a frequency, which causes the issue.
It looks to me as if my version of Thunderbird is expecting at most one line of RRULE per event, and that line should be written as RRULE:KEY=VALUE;KEY=VALUE;..., and that the multiple lines encountered here are not allowed.
Something that outputs this on nightly is: env['calendar.event'].browse(622200)._get_ics_file()[622200]
53d0f01 to
ae43446
Compare
No description provided.