Allow organizing documents in a tree structure #516
Conversation
We choose to use Django-treebeard for its quality, performance and stability. Adding tree structure to documents is as simple as inheriting from the MP_Node class.
The test_api_documents_list file was getting too long. We can extract tests on filters and ordering.
Now that we have a tree structure, we should only include parents of a visible subtree in list results.
A document should inherit the access rights a user has on any of its ancestors.
This information is useful for the frontend to display the document tree structure and is cheap to expose.
This endpoint is nested under a document's detail endpoint.
We add a POST method to the existing children endpoint.
Including the content field in the list view is not efficient as we need to query the object storage to retrieve it. We want to display an excerpt of the content on the list view so we should store it in database. We let the frontend compute it and save it for us in the new "excerpt" field because we are not supposed to have access to the content (E2EE feature coming)
sampaccoud
changed the title
There was a problem hiding this comment.
That is nice!!
It is a bit hard to review from a frontend side perspective without too much context, but it looks what we need to do our tree structure.
For me, if you put back abilities, we could merge it because except that, it does not seems to impact negatively the frontend; then we will iterate when we will start to integrate the tree on the frontend.
| @@ -59,7 +59,6 @@ def test_api_documents_list_format(): | |||
| assert len(results) == 1 | |||
| assert results[0] == { | |||
| "id": str(document.id), | |||
| "content": document.content, | |||
There was a problem hiding this comment.
excert is not a value that we should get in the document_list ?
| @@ -326,6 +326,7 @@ class Document(MP_Node, BaseModel): | |||
| """Pad document carrying the content.""" | |||
|
|
|||
| title = models.CharField(_("title"), max_length=255, null=True, blank=True) | |||
| excerpt = models.TextField(_("excerpt"), max_length=300, null=True, blank=True) | |||
There was a problem hiding this comment.
Ok, so it is the front setting this var during saving as well I guess.
| @@ -165,7 +164,6 @@ class Meta: | |||
| ] | |||
| read_only_fields = [ | |||
| "id", | |||
| "abilities", | |||
There was a problem hiding this comment.
It will break the frontend if you remove it. We use the abilities in the data grid, to know if the current user can delete or not a document.
We use it as well to compute the current role of the user on the document (not sure we still do it with the new design though).
| queue.push( | ||
| models.Document( | ||
| depth=1, |
There was a problem hiding this comment.
Is it possible to add some children docs, with different depth as well ?
| "is_favorite", | ||
| "link_role", | ||
| "link_reach", | ||
| "nb_accesses", | ||
| "numchild", | ||
| "path", |
There was a problem hiding this comment.
What is the purpose of path ?
| def test_api_documents_children_list_authenticated_related_direct(): | ||
| """ | ||
| Authenticated users should be allowed to retrieve the children of a document | ||
| to which they are directly related whatever the role. | ||
| """ |
There was a problem hiding this comment.
I am wondering:
- I am a member of a child document
- I am not a member of a parent document
Do I have the right to see the parent documents ?
If not, will this child document appear in the endpoint GET /api/v1.0/documents ?
Purpose
We want to allow nesting documents in a tree structure.
Proposal