Add attachments to events #4251

sazanof · 2022-06-01T09:14:44Z

Signed-off-by: Mikhail Sazanov m@sazanof.ru

I tried to implement attachment files in events. The parameter is described here https://datatracker.ietf.org/doc/html/rfc5545#section-3.8.1.1

Probably it will be a draft, although in fact everything works.

I decided to do this: there are several ways to add attachments:

From the Files application. Then a common link is formed (here I think to make it possible to provide a link not shared, but closed. but so far I have done so).
File upload. The file is uploaded to the root directory and a public link is formed to it

Probably here you can use a separate folder "Calendar". And in the future, the place for attachments may grow, but it is more convenient for users to download from a computer, rather than choose from "Files".

I also plan to delete the file itself when deleting the event (if the owner of the event does this action), if it is not needed in the repository.

Well, the loading indication (start - end) it should also be added.

Please tell me: where is the email notification template generated when an event notification arrives (I want to add attachments section)?

I would be grateful if you would take a look and give your comments. Thanks!=)

empty

with files

file options

ChristophWurst · 2022-06-01T09:18:56Z

This PR addresses #298

tcitworld · 2022-06-01T15:34:02Z

Please tell me: where is the email notification template generated when an event notification arrives (I want to add attachments section)?

This is handled by the server: https://github.com/nextcloud/server/blob/master/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php

codecov · 2022-06-01T15:36:23Z

Codecov Report

Base: 22.25% // Head: 21.51% // Decreases project coverage by -0.74% ⚠️

Coverage data is based on head (9d96445) compared to base (f00ce71).
Patch coverage: 1.47% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4251      +/-   ##
============================================
- Coverage     22.25%   21.51%   -0.74%     
- Complexity      334      338       +4     
============================================
  Files           227      231       +4     
  Lines         10789    11213     +424     
  Branches       2045     2138      +93     
============================================
+ Hits           2401     2413      +12     
- Misses         8388     8800     +412

Flag	Coverage Δ
javascript	`13.66% <1.47%> (-0.57%)`	⬇️
php	`63.72% <ø> (-0.35%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/AppNavigation/Settings.vue	`0.00% <0.00%> (ø)`
...pNavigation/Settings/SettingsAttachmentsFolder.vue	`0.00% <0.00%> (ø)`
.../components/Editor/Attachments/AttachmentsList.vue	`0.00% <0.00%> (ø)`
src/services/attachmentService.js	`0.00% <0.00%> (ø)`
src/store/calendarObjectInstance.js	`0.00% <0.00%> (ø)`
src/views/Calendar.vue	`0.00% <0.00%> (ø)`
src/views/EditSidebar.vue	`0.00% <0.00%> (ø)`
src/store/settings.js	`88.49% <22.22%> (-5.80%)`	⬇️
src/models/event.js	`57.14% <50.00%> (-0.27%)`	⬇️
src/models/attachment.js	`66.66% <66.66%> (ø)`
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

tcitworld · 2022-06-05T15:28:48Z

src/services/attachmentService.js

+			}))
+		}
+		else {
+			const url = `/remote.php/dav/files/${dav.userId}/${file.name}`


There's the brand new nextcloud-upload package from @skjnldsv that might be a better fit: https://github.com/nextcloud/nextcloud-upload.

Yes, clearly.

Yes, clearly.

Maybe a stupid question, sorry. npm doesn't have it. How to properly add this dependence?

It's not released yet :)
Nor documented btw. We can discuss what are your needs to make sure it covers them!

I still need to cover the Upload/Choose menu you have in your screenshots. Other than that, seems to be good :)

@skjnldsv Well, since we are not in a hurry, we can discuss =).

I would like to be able to upload to a specific folder + have a callback to provide shared access or something else. Maybe a subscription to some events (loading before, progress, after) ...
By buttons - you need a universal button that can be entered into any standard NC component. Ex.: ListItem,ActionButton, Button and so on.

It would also be cool to be able to set the template of the uploaded file (before uploading, at the moment and after).

If you make the component universal, then in the future consider using it to upload not only to DAV, but also when writing a letter in the mail, for example...

It's been published.

tcitworld · 2022-06-05T15:30:00Z

src/store/calendarObjectInstance.js

+	addAttachmentByLocalFile(state, { calendarObjectInstance, file }){
+
+		const attachment = AttachmentProperty.fromLink(file.url)
+		// hot-fix needed temporary, becase calendar-js has no fileName get-setter


Can you open a PR there? https://github.com/nextcloud/calendar-js

src/components/Editor/Attachments/AttachmentsList.vue

tcitworld · 2022-06-05T15:56:54Z

I also plan to delete the file itself when deleting the event (if the owner of the event does this action), if it is not needed in the repository.

Note : this will only affect events deleted through the web UI.

In the future we need to implement CalDAV-Managed Attachments (RFC 8607) sabre-io/dav#837 and let the server handle everything instead. We have time though, I'm not aware of any client implementing this (at least not Evolution, KOrganizer or even Thunderbird - this should interest @devvv4ever btw)

tcitworld · 2022-06-06T10:25:43Z

I decided to do this: there are several ways to add attachments:

From the Files application. Then a common link is formed (here I think to make it possible to provide a link not shared, but closed. but so far I have done so).

File upload. The file is uploaded to the root directory and a public link is formed to it

Probably here you can use a separate folder "Calendar". And in the future, the place for attachments may grow, but it is more convenient for users to download from a computer, rather than choose from "Files".

I'm not fond of uploading directly to the user root folder, it might get crowded real quick.

One possibility would be to handle this in the app backend (it would be irrelevant once RFC 8607 would be implemented in dav server - see above) and put files in user's appdata. However the users wouldn't have access to files apart from accessing them directly from the event.

In the case of Talk the app creates a Talk folder in the user root to put attachments. Is that a pattern we should reproduce here with a Calendar folder ? Sounds the best to me. Opinions @nextcloud/designers @nickvergessen

Later and with the help of file tracking for RFC 8607 we can improve these files discoverability just like Talk has now with a « all files attached to events in this calendar » section.

tcitworld · 2022-06-06T10:26:47Z

@sazanof Tests have to be updated for the new attachments property.

sazanof · 2022-06-06T12:59:45Z

@sazanof Tests have to be updated for the new attachments property.

Tell me, is it enough to add a parameter or do I need to write a full test? If it's a complete test, then I ask for help, since I've never written tests and don't understand how to write them (It's a matter of time and effort, I don't argue). =(
I tried to write my own block for checking (by analogy with other tests) it()
This error occurs if I add my parameters to

expect(getDefaultEvent Object()).to Equal({
....
attachments: [
	'ATTACHMENT1',
	'ATTACHMENT2',
	'ATTACHMENT3',
	'ATTACHMENT4',
	]
})

https://gist.github.com/sazanof/ecb2aaa8c760f93e74d2952294ba09ba

When checking, I get an error. I can't understand why it occurs...

 ● Test suite failed to run

    TypeError: Converting circular structure to JSON
        --> --> --> --> --> --> --> --> starting at object with constructor 'CalendarComponent'
        --- property '_root' closes the circle
        at stringify (<anonymous>)

nimishavijay · 2022-06-06T13:27:29Z

In the case of Talk the app creates a Talk folder in the user root to put attachments. Is that a pattern we should reproduce here with a Calendar folder ? Sounds the best to me.

Agreed, "Calendar" folder sounds good. We could also create subfolders for each calendar if possible, I think that would make it more organised.

devvv4ever · 2022-06-06T13:56:37Z

@tcitworld Thank you for thinking on us here too :) We would definitely love to implement attachment sync in DAVx⁵! Managed Attachments would be a really really cool feature also on Android. The problem here is that the Android Calendar storage does not offer attachments storage capabilities. So we'd need to find other apps that can provide an interface for it. We need a collaboration with others then, too (etar, acalendar+, jtx board, etc) because DAVx⁵ is not a calendar app itself so that people can also access their attachments then on Android.

nickvergessen · 2022-06-07T09:43:47Z

I think it shouldn't be in the root anymore. I think Deck and etc. use an Attachment/ root folder

nimishavijay · 2022-06-08T08:53:15Z

Dunno if the frontend is ready for reviewing but I have some design feedback that could be considered, feel free to incorporate it :) The general flow looks really great! The only feedback is regarding wording and icons.

In the "empty" screenshot:

The empty content icon could be the create-new-folder MDI icon or just the folder MDI icon
Color of the empty content icon should be var(color-text-maxcontrast)
"Choose" --> "Add from Files"
Upload icon --> file-upload MDI icon

In the "with files" screenshot:

Instead of a 3dot menu icon that opens a menu with "Choose" and "Upload" we could make it just another list item with an add icon and text "New attachment"
If it is possible to show a preview thumbnail we could do that
If it is possible to display a filetype icon we could do that
Currently the subline shows the type of file it is, but that is already understood from the extension in the name of the file. Instead we could show the size of the file (for eg. 300 KB)
"Choose" --> "Add from Files"
Upload icon --> file-upload MDI icon

In the "file options" screenshot:

Not sure if this is already the case but clicking on the file should open it in the viewer if possible. If not it could be downloaded.
Hence we would not need the "View file" option in the action menu. It could be changed to "Download". Icon stays the same
"Delete file" --> "Delete"
Delete icon --> delete MDI icon

Super nice work! cc @jancborchardt

sazanof · 2022-06-08T09:06:45Z

@nimishavijay Hello!

Thank you for the detailed answer and your wishes. I will definitely take them into account when updating the design.

Regarding the preview for attachments: I have already implemented this, I just closed the discussion, and this branch is no longer shown. My joint.=) Here is a link to this comment

#4251 (comment)

file options actions, in fact, will not be needed if you open a file by clicking on its name and add only one delete button to the right of it.

But what if we need to introduce a document preview function? Preview in the modal window, download and delete. Of course it won't be soon, and then you can fix the design, but still.

Thanks again for your comments!

jancborchardt · 2022-06-08T09:35:39Z

Good stuff! Agree with @nimishavijay's feedback, just 2 points/questions:

To keep keeping it simple, we can put the attachment section on the bottom of "Details" instead of adding a separate "Attachments" section. Details is visible by default.
As to where the Files are stored: Regardless of how we do it, it would mean that only 1 copy exists and collaboration is possible, right? Or will files uploaded from the cloud be duplicated?

Georg wrote back in the issue (not sure if still current): #298 (comment)

Instead of writing the attachment into the actual calendar data I would suggest to upload it to the nextcloud and just link there.

nimishavijay · 2022-06-08T10:28:45Z

Just checked Google on how attachments are handled and they are uploaded into the root directory of your Google Drive, and the attachment basically links to that file. If you email another person the the event, they are able to see the attachment and if they click on it, it opens the file in Drive.

cc @jancborchardt

sazanof · 2022-06-08T15:49:29Z

@nimishavijay, @jancborchardt
I tried to take into account all your comments and added a little of my own. What came out of it - you can see in the screenshots and a4e764c.

no attachments

with attachments

What do you say?

jancborchardt

@sazanof really cool! :) Some additional feedback:

"Upload from PC" → better "Upload from device" since people might be on tablet or mobile
The preview needs some border radius (like in Files), var(--border-radius)
The subline is too technical and too much information, it can just be cut

miaulalala · 2023-01-03T07:28:28Z

Another few notes from reading RFC 8607:

Changing attachments should trigger scheduling messages to attendees https://datatracker.ietf.org/doc/html/rfc8607#section-3.12.6

This should be doable by hooking into the sabre event lifecycle methinks.

MANAGED-ID occurrences must be removed when exporting and importing data https://datatracker.ietf.org/doc/html/rfc8607#section-3.12.7

I think the export should be ok, we sanitise the data a lot. Of course testing is needed.
The import might work or it might not - \Sabre\VObject\Component\VEvent::getValidationRules is pretty strict, had this in another ticket where the (valid) ENCODING param threw.

Signed-off-by: Mikhail Sazanov <m@sazanof.ru>

st3iny

I tested it again and it works really great now! I fixed the share creation as it wasn't using the attachment folder.

There is a bug with the attachment folder picker and I'm currently researching what is causing it.

All feedback has been addressed.

st3iny

Tested and works!

Please squash all commits (including mine) into a single commit and then we are ready to merge.

Signed-off-by: Mikhail Sazanov <m@sazanof.ru>

sazanof · 2023-02-15T15:48:14Z

Hello, @st3iny! Thank you for review =)
Hello everyone! So... drum roll...

Tested and works!

Please squash all commits (including mine) into a single commit and then we are ready to merge.

szaimen · 2023-02-15T16:58:02Z

🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉

sazanof · 2023-02-15T17:21:19Z

*** Sounds of a cork flying out of a champagne bottle ***

ChristophWurst · 2023-03-23T07:56:21Z

@sazanof https://youtu.be/CeKNPSsK-kI?t=524 🙏

Add the attachment feature to the readme nextcloud#4251 Signed-off-by: strobeflash <68517177+strobeflash@users.noreply.github.com>

Add the attachment feature to the readme #4251 Signed-off-by: strobeflash <68517177+strobeflash@users.noreply.github.com>

sazanof force-pushed the enh/attachements-to-events branch from 72e07ca to 2786aab Compare June 1, 2022 10:45

szaimen requested review from st3iny and raimund-schluessler June 3, 2022 10:54

szaimen added 3. to review Waiting for reviews enhancement New feature request labels Jun 3, 2022

szaimen added this to the v3.4.0 milestone Jun 3, 2022

tcitworld reviewed Jun 5, 2022

View reviewed changes

sazanof requested a review from tcitworld June 6, 2022 09:48

sazanof force-pushed the enh/attachements-to-events branch from 505b2ba to 43ff211 Compare June 6, 2022 13:01

sazanof force-pushed the enh/attachements-to-events branch from 1f9598e to 75fab86 Compare June 8, 2022 16:37

szaimen requested review from nimishavijay and jancborchardt June 10, 2022 10:12

sazanof requested a review from skjnldsv June 10, 2022 13:01

ChristophWurst removed this from the v3.4.0 milestone Jun 13, 2022

jancborchardt previously requested changes Jun 16, 2022

View reviewed changes

sazanof requested review from szaimen and tcitworld and removed request for szaimen November 25, 2022 06:50

miaulalala mentioned this pull request Jan 17, 2023

Attachments for events #298

Closed

sazanof force-pushed the enh/attachements-to-events branch from 2fb2e8a to e087fab Compare January 25, 2023 19:13

sazanof pushed a commit to sazanof/nextcloud_calendar that referenced this pull request Jan 25, 2023

Fixup. Review nextcloud#4251 (review)

771b273

Signed-off-by: Mikhail Sazanov <m@sazanof.ru>

sazanof pushed a commit to sazanof/nextcloud_calendar that referenced this pull request Jan 25, 2023

Fix linter issues nextcloud#4251 (comment)

b99bd53

Signed-off-by: Mikhail Sazanov <m@sazanof.ru>

sazanof requested review from st3iny and removed request for tcitworld January 25, 2023 19:14

st3iny reviewed Feb 15, 2023

View reviewed changes

st3iny added this to the v4.3.0 milestone Feb 15, 2023

st3iny approved these changes Feb 15, 2023

View reviewed changes

Add attachments to events

9d96445

Signed-off-by: Mikhail Sazanov <m@sazanof.ru>

sazanof force-pushed the enh/attachements-to-events branch from 2da21de to 9d96445 Compare February 15, 2023 15:46

st3iny added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 15, 2023

st3iny merged commit f97e970 into nextcloud:main Feb 15, 2023

tcitworld mentioned this pull request Feb 21, 2023

Implement CalDAV-Managed Attachments (RFC 8607) #5001

Open

miaulalala mentioned this pull request Mar 16, 2023

Document new calendar attachments nextcloud/documentation#9911

Closed

ChristophWurst mentioned this pull request Mar 20, 2023

4.3.0-rc2 creates a folder 'Calendar' in the user's home (in /apps/files) each time the web interfaces is accessed #5073

Closed

strobeflash added a commit to strobeflash/calendar that referenced this pull request Mar 30, 2023

Update README.md

35af25e

Add the attachment feature to the readme nextcloud#4251 Signed-off-by: strobeflash <68517177+strobeflash@users.noreply.github.com>

strobeflash mentioned this pull request Mar 30, 2023

Add event attachments to readme feature list #5119

Closed

kesselb pushed a commit that referenced this pull request Nov 27, 2024

chore: Add event attachments to README.md

56ee073

Add the attachment feature to the readme #4251 Signed-off-by: strobeflash <68517177+strobeflash@users.noreply.github.com>

kesselb mentioned this pull request Nov 29, 2024

Event attachments: Enable directory selection #6534

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add attachments to events #4251

Add attachments to events #4251

sazanof commented Jun 1, 2022 •

edited by st3iny

Loading

ChristophWurst commented Jun 1, 2022

tcitworld commented Jun 1, 2022

codecov bot commented Jun 1, 2022 •

edited

Loading

tcitworld Jun 5, 2022

skjnldsv Jun 7, 2022

sazanof Jun 7, 2022 •

edited

Loading

skjnldsv Jun 7, 2022

sazanof Jun 8, 2022

tcitworld Aug 15, 2022

tcitworld Jun 5, 2022

tcitworld commented Jun 5, 2022

tcitworld commented Jun 6, 2022 •

edited

Loading

tcitworld commented Jun 6, 2022

sazanof commented Jun 6, 2022 •

edited

Loading

nimishavijay commented Jun 6, 2022

devvv4ever commented Jun 6, 2022

nickvergessen commented Jun 7, 2022

nimishavijay commented Jun 8, 2022

sazanof commented Jun 8, 2022

jancborchardt commented Jun 8, 2022 •

edited

Loading

nimishavijay commented Jun 8, 2022

sazanof commented Jun 8, 2022

jancborchardt left a comment •

edited by st3iny

Loading

miaulalala commented Jan 3, 2023

st3iny left a comment

st3iny left a comment

sazanof commented Feb 15, 2023

szaimen commented Feb 15, 2023

sazanof commented Feb 15, 2023 •

edited

Loading

ChristophWurst commented Mar 23, 2023

Add attachments to events #4251

Add attachments to events #4251

Conversation

sazanof commented Jun 1, 2022 • edited by st3iny Loading

ChristophWurst commented Jun 1, 2022

tcitworld commented Jun 1, 2022

codecov bot commented Jun 1, 2022 • edited Loading

Codecov Report

tcitworld Jun 5, 2022

Choose a reason for hiding this comment

skjnldsv Jun 7, 2022

Choose a reason for hiding this comment

sazanof Jun 7, 2022 • edited Loading

Choose a reason for hiding this comment

skjnldsv Jun 7, 2022

Choose a reason for hiding this comment

sazanof Jun 8, 2022

Choose a reason for hiding this comment

tcitworld Aug 15, 2022

Choose a reason for hiding this comment

tcitworld Jun 5, 2022

Choose a reason for hiding this comment

tcitworld commented Jun 5, 2022

tcitworld commented Jun 6, 2022 • edited Loading

tcitworld commented Jun 6, 2022

sazanof commented Jun 6, 2022 • edited Loading

nimishavijay commented Jun 6, 2022

devvv4ever commented Jun 6, 2022

nickvergessen commented Jun 7, 2022

nimishavijay commented Jun 8, 2022

sazanof commented Jun 8, 2022

jancborchardt commented Jun 8, 2022 • edited Loading

nimishavijay commented Jun 8, 2022

sazanof commented Jun 8, 2022

jancborchardt left a comment • edited by st3iny Loading

Choose a reason for hiding this comment

miaulalala commented Jan 3, 2023

st3iny left a comment

Choose a reason for hiding this comment

st3iny left a comment

Choose a reason for hiding this comment

sazanof commented Feb 15, 2023

szaimen commented Feb 15, 2023

sazanof commented Feb 15, 2023 • edited Loading

ChristophWurst commented Mar 23, 2023

sazanof commented Jun 1, 2022 •

edited by st3iny

Loading

codecov bot commented Jun 1, 2022 •

edited

Loading

sazanof Jun 7, 2022 •

edited

Loading

tcitworld commented Jun 6, 2022 •

edited

Loading

sazanof commented Jun 6, 2022 •

edited

Loading

jancborchardt commented Jun 8, 2022 •

edited

Loading

jancborchardt left a comment •

edited by st3iny

Loading

sazanof commented Feb 15, 2023 •

edited

Loading