Send out mail when recurring meeting template completed #17492
Conversation
oliverguenther
force-pushed
the
feat/meeting-ics-improvements
branch
from
0d62ea4 to
9921781
Compare
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
1 similar comment
|
Caution The provided work package version does not match the core version Details:
Please make sure that:
|
|
The version warning is correct, but as the feature is developed behind a feature flag, it can already be merged in 15.2 |
| service.generate_series | ||
| end | ||
|
|
||
| result | ||
| .on_failure { |call| render_500(message: call.message) } | ||
| .on_success do |call| | ||
| send_data call.result, filename: filename_for_content_disposition("#{@recurring_meeting.title}.ics") |
There was a problem hiding this comment.
just a question: when we generate an ical for a single occurence, should we also include that info in the filename?
There was a problem hiding this comment.
Yes good point, I'll add that
There was a problem hiding this comment.
I wonder what the title for the occurrence would be: Series title - <Date of occurrence>? That might pose some problems on some OS if the format is slash-separated.
There was a problem hiding this comment.
I think in export filenames, etc we should always use an iso8601 date with dashes
| @@ -67,7 +67,7 @@ def add_attendees(event, meeting) | |||
| end | |||
|
|
|||
| def ical_uid(suffix) | |||
| "#{Setting.app_title}-#{Setting.host_name}-#{suffix}".dasherize | |||
| Digest::SHA256.hexdigest "#{Setting.app_title}-#{Setting.host_name}-#{suffix}" | |||
There was a problem hiding this comment.
This will change the identifier of all existing meetings. Have we checked what the calendars do in this case or if it will duplicate calendar entries?
There was a problem hiding this comment.
They would indeed change. But recurring meetings already introduced the app_title and host_name, which weren't present before and could result in collisions. So I fear that we have to break it once. Alternatively, we could store the uid as a database column and backfill it with the old values. WDYT?
There was a problem hiding this comment.
I think it will be fine, we should maybe just validate that people will not end up with duplicated entries. But as the "old" entry is not part of the feed anymore, I think it will be ok
| @@ -105,6 +118,7 @@ def occurrence_event(schedule_start_time, meeting) # rubocop:disable Metrics/Abc | |||
| e.dtend = ical_datetime meeting.end_time, tzid | |||
| e.url = url_helpers.project_meeting_url(meeting.project, meeting) | |||
| e.location = meeting.location.presence | |||
| e.sequence = 2 | |||
There was a problem hiding this comment.
According to the specs the sequence should be increased every time the calendar event is updated, right? We do have a lock_version on the meeting. Should we maybe use that as the sequence number?
Ticket
https://community.openproject.org/projects/meetings-stream/work_packages/60200/activity?query_id=5640