File Comments DAV Resource

[blizzz]

Contributes to #16328

• Comments WebDAV resource (own endpoint) Comments DAV implementation #21664
• Base url will be: remote.php/dav/comments/${enityType}/${entityId} Comments DAV implementation #21664
• Controller/Service has a public method to read number of comments for a specific file Comments DAV implementation #21664
• ^ also starting a specific timestamp Comments DAV implementation #21664
• PUT to write comments Comments DAV implementation #21664 POST
• Provide an Interface, so a storage may replace the core implementation we should always go via CommentsManager, which can be replaced. No hard references to \OC\Comment$foo Comments DAV implementation #21664
• as additional piece of information, also return the display name of the comments' actor Comments DAV implementation #21664

@nickvergessen @blizzz @DeepDiver1975 @jancborchardt

[DeepDiver1975]

Base url will be: remote.php/dav/comments/${enityType}/${entityId}

In contrary to all other resources this one will not reflect the user in the url because there is no user specific information.
The entityId has to hold potential user specific information.

[DeepDiver1975]

Provides a possibility to mark a comment as read (user-specific)

the data model currently does not reflect this - and this would require a user specific dav resource

[nickvergessen]

We decided to not do that, but do the read tracking in the timeline feature?

[blizzz]

Yes – back then i misunderstood that we want to keep the possibility open – will adjust.

[DeepDiver1975]

We decided to not do that, but do the read tracking in the timeline feature?

but the information about what is read and what not has to be stored user specific - even if it is only timestamp

[nickvergessen]

Yes but it's part of #20259

[DeepDiver1975]

Yes but it's part of #20259

ah - got it - missed that part

[blizzz]

I see this as my next step. I was having a look on how @PVince81 is dealing with #20786. These are my (live written) thoughts, input welcome.

We said we like to have the base URL: remote.php/dav/comments/${enityType}/${entityId}

Since we also got the "verb" field, I wonder whether we should append it there as well, being $verb. However, this would require to touch interfaces again, since this was not in mind when designing them. Since we only have the comments verb now anyway, it's not a problem. Granted that in a query the desired verb can be passed in a PROPFIND request, that's absolutely not a problem.

I understand we need a CommentsCollection which holds some typed collections as reflected by ${entityType}, which so far only will be a Comments/FileCollection. Since we will not support MKCOL here (available collections are rather static and present codewise), in future we like to have this extensible. I don't see it in the scope for 9.0, however.

On the other hand, our CommentsManager is made to support different types, which makes it unnecessary overhead. Having $(entityType) in the URL is a challenge, because they are essentially a string in the database – if present. They cannot be created using MKCOL because they are comment-specific, but we cannot leave them out either, since a single ID in the URL is not specific enough.

If from a DAV persepective it is OK to enlist just the types that are already known (i.e. present in the database in our implementation) but create nodes on unknown types, we win. I confess, the thought alone is ugly. I wonder whether the thought to create Nodes in comments/ and have the final destination returned is any better? Anyway, both attempts can be misused to pollute the Comments table with wild types. The only way around is to have a way to register possible types. Any new app or extension would do this either on install (requires another table) or on runtime (requires an API method at the CommentsManager). Latter sounds to be the best approach for me. It's not much else than a protected property, prefilled with 'comments' and a registerType() method on the manger's side (and its interface).

A comment is represented by a Node, thus CommentNode. It's generic, due to our data structure, and can be used for any entity type. As in @PVince81's system tags approach it will utilize the CommentsManager() to do whatever it is supposed to do: reading, modifying, deleting comments.

I understand the Plugin is technically some kind of specific controller.

That's my picture now how it is going to look like

[PVince81]

If from a DAV persepective it is OK to enlist just the types that are already known (i.e. present in the database in our implementation)

Yes. Let's hard-code the known types for now like I did for system tags. In the future we can provide a mechanism for apps to provide their own additional types. No need for MKCOL 😄

CommentNode might not be enough, you'll also need a CommentMappingNode to represent the mappings inside the relations endpoint. See how I did it for system tags.

[PVince81]

Regarding the verb, I think it could be an attribute "oc:verb" in the CommentMappingNode in the relations endpoint.

[PVince81]

Ok, when looking at your data structure it seems that the comment itself is always the relationship.
There are no individual comments detached from the relationships.

So you'll only have a single endpoint remote.php/dav/comments/${entityType}/${entityId}/${commentId} like you said above.

Adding new comments has to be a POST on the collection remote.php/dav/comments/${entityType}/${entityId}/ with a JSON object, because you don't know the id beforehand. Then you'll get a "Location: " header with the URL containing the newly generated comment id.

[PVince81]

Regarding replies I also wonder whether we want a tree structure remote.php/dav/comments/${entityType}/${entityId}/${commentId}/${replyId} where the comments are actually collections themselves. This way to get a comment and its tree, do a PROPFIND with Depth: 2 on the ${entityId}

[PVince81]

Maybe let's make a checklist of all operations that the GUI will need to be able to manage tags, and then write down how the Webdav operation will look like, like I did in #20786 (comment)

[blizzz]

Regarding the verb, I think it could be an attribute "oc:verb" in the CommentMappingNode in the relations endpoint.

I don't really see what that Mapping is for, but I'll look again.

[blizzz]

Regarding replies I also wonder whether we want a tree structure remote.php/dav/comments/${entityType}/${entityId}/${commentId}/${replyId} where the comments are actually collections themselves. This way to get a comment and its tree, do a PROPFIND with Depth: 2 on the ${entityId}

Following this theory in a possible feature this would be an endless URL, as replies could have replies. In the current scenario it would work. Thinking as how a file structure would be, then you are right, and remote.php/dav/comments/${entityType}/${entityId}/${commentId}/${replyId}/${yetAnotherReplyId}/${YetOneMoreAnotherReplyId} would be totally valid.

[blizzz]

Maybe let's make a checklist of all operations that the GUI will need to be able to manage tags, and then write down how the Webdav operation will look like, like I did in #20786 (comment)

API spec

URL root is always remote.php/dav/comments/

State	Function	Action	URL	Remark
🏁	create comment	POST	…/${entityType}/${entityId}/	in future maybe also this url with several IDs appended (== replies)
⌛	update comment	PROPPATCH	…/${entityType}/${entityId}/ ${commentId}
⌛	delete comment	DELETE	…/${entityType}/${entityId}/ ${commentId}
⌛	list comments	PROPFIND	…/${entityType}/${entityId}	The Comments Manager returns back the whole comments. Filter restrictions (limit, offset, datetime) may be provided per request header.

[blizzz]

If from a DAV persepective it is OK to enlist just the types that are already known (i.e. present in the database in our implementation)

Yes. Let's hard-code the known types for now like I did for system tags. In the future we can provide a mechanism for apps to provide their own additional types. No need for MKCOL 😄

I don't see a MKCOL showing up neither in the short nor long future :)

It's not much else than a protected property, prefilled with 'comments' and a registerType() method on the manger's side (and its interface).

→ 'comment' will be hardcoded there of course and for now we can leave out the registerType() method, but it will be easy to add it.

[PVince81]

Regarding "list comments", will it then return a flat list ? And some comments will have a "oc:parentid" attribute to make it possible for the clients to attach them to the matching parent ?

Or as said above we could use multi-level comments (sub-collections), but I'd limit it to one-level. Didn't we say we wanted to limit to one level anyway?

[blizzz]

Regarding "list comments", will it then return a flat list ? And some comments will have a "oc:parentid" attribute to make it possible for the clients to attach them to the matching parent ?

Yes. For now, we decided to do flat-list only anyway (see #20267).

Or as said above we could use multi-level comments (sub-collections), but I'd limit it to one-level. Didn't we say we wanted to limit to one level anyway?

If we decide to go to 1-level in the future, that's how we would change it to.

[PVince81]

Sounds good!

[nickvergessen]

Just throwing it in here, replies could also be:

remote.php/dav/comments/comment/${commentId}/${replyId}
remote.php/dav/comments/comment/${replyId}/${yetAnotherReplyId}
remote.php/dav/comments/comment/${yetAnotherReplyId}/${YetOneMoreAnotherReplyId}

Because that describes, what it is? a comment on a comment?

So entityType = comment and entityId = comment you want to reply to

[blizzz]

Just throwing it in here, replies could also be:

remote.php/dav/comments/comment/${commentId}/${replyId}
remote.php/dav/comments/comment/${replyId}/${yetAnotherReplyId}
remote.php/dav/comments/comment/${yetAnotherReplyId}/${YetOneMoreAnotherReplyId}

Because that describes, what it is? a comment on a comment?

So entityType = comment and entityId = comment you want to reply to

Unsure about the necessity of a tree structure in the URL. If we go for a tree structure, we could just either return all the direct children of commentId or all, and there is no need for a replyId, because you could request just the children of any comment, be it on root level or somewhere deeper in the tree.

Anyway, it's not critical to decide this now, but when (if) we need it.

[PVince81]

The "comment on a comment" approach sounds good instead of a full tree.

[blizzz]

Removed Provide an Interface, so a storage may replace the core implementation requirement as this is already realized in the CommentsManager. The DAV part should be generic.

In agreement with @PVince81

[PVince81]

@blizzz for compliance, we should add your new report type to the list of supported reports set (OPTIONS method). To do so, your plugin must override this method https://github.com/fruux/sabre-dav/blob/master/lib/DAV/ServerPlugin.php#L83

Reopen or new ticket ?

[blizzz]

@PVince81 look here #21932

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.