-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Include accept / decline links in CalDAV invitation emails #9942
Conversation
I could help you for testing this feature. I have a lot of mail clients - mail servers - and scenario to test |
5f53787
to
346e205
Compare
Codecov Report
@@ Coverage Diff @@
## master #9942 +/- ##
=========================================
Coverage ? 51.94%
Complexity ? 26052
=========================================
Files ? 1668
Lines ? 96455
Branches ? 1290
=========================================
Hits ? 50103
Misses ? 46352
Partials ? 0
|
Oh no continuous-integration failed, does is it bad for the feature freeze ? |
346e205
to
9283c7b
Compare
We are struggling with 1 of the multiple thousand of tests we run on CI. This one is an acceptance test for the web UI where the started browser has some delays and thus the element was not rendered. A pretty hard to fix race condition. So it's fine, because it is a known limitation of this test suite. The remaining tests all succeeded so this is fine to be reviewed. |
I tried to test thoses changes but i got buged, Do i have to create a table myself ? Could you give us the syntaxe for adding oc_calendar_invitation_tokens ? EDIT : Alright, it was my fault, i dropped my database, deleted config.php, start a new install and i got my table. So now, testing day |
This is the right place :)
That's very odd, because it works for me: Anyway, the only HTML we inject is definitely valid: https://github.com/nextcloud/server/pull/9942/files#diff-d84119a7a4a15a279ced923237bd016bR503 So if anything, this is a bug in the E-Mail-Templating class and should be fixed separately.
That would be nice indeed, but also involves more much more work. This pull-request is already a vast enhancement, so I would just suggest to do smaller steps and implement the overview later on for Nextcloud 15.
This was there before adding those links as well and unfortunately I'm not aware of any way to get rid of those buttons. Your email client simply detects that this email contains an invitation and offers you to reply to that invitation by sending an email to the organizer. That mechanism is related to iMIP: https://tools.ietf.org/html/rfc6047 And yes, it doesn't change the events, because the user would have to manually import the ics files attached into their calendar. I was talking to @ChristophWurst the other day if we could integrate this with the mail app, so if you setup the mail app with your email account, it will automatically poll your email account in the background to check for new invitation responses and update the information in your calendar. Definitely an issue we are interested in solving, but not directly related to this pull-request. |
Some news. I got the email template bug on iphone with my Outlook.com address. With my imap address no graphical bug. |
@georgehrke Mind to rebase to get this finally in? |
Signed-off-by: Georg Ehrke <developer@georgehrke.com>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
9283c7b
to
f12922c
Compare
I took the libery of rebasing |
@MorrisJobke @rullzer Can we merge this? :) |
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.
Tested and works 👍
Only one minor nitpick, that could also be fixed later:
- accepting/declining from the mail created a second attendee with the
mailto:
prefix in the Nextcloud calendar app
I know what you mean. That's actually a bug in the calendar app. It's creating attendees with |
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.
The let do this 🎸 🚀 🍺
fixes #2338
Todo:
To test this:
Click Accept and wait till thunderbird refreshed the event from the server:
Click Decline and wait till thunderbird refreshed the event from the server:
Use more options to select Tentative:
We use common
X-
parameters to store number of guests and comment in event, sadly thunderbird doesn't support that, but we will build that into the calendar app.(If Thunderbird takes too long to refresh the event, you can try to move the event to a different way. Thunderbird will show you a warning about a sync conflict. Select to discard local changes and refresh event)