The nb_accesses cache TTL was very short, 30 seconds. That mean that the
user will hit the cache for a very short period and the cache is
probably not be hit. This is what we can see in the slow queries from
the pg_stat_statements table. The query to compute the nb_accesses is
executed a little bit less than the number of queries to list or
retrieve documents, meaning the cache is not used.
Comparing path with LEFT(value, LENGTH(path)) makes a sequential scan on
all the Document table, the more this table grow, the more the query
using it will be slow. We dediced instead to lookup on the path
extracting all ancestors path for a given document and then make a path
IN statement to use the index existing on the path column.
On the media_auth endpoint the first bottleneck we have is with
postgresql. We are looking for too much data and no index is used on the
attachments colum. When the lookup filter on the attachement columns, a
full scan is made on all the document table looking for each element in
the array, this operation is really expensive. To fix this we created a
GIN index on the attachments column. Also the readable_per_se lookup was
selecting too much data combined with the filter_descendants function.
We remove the usage of the filter_descendants, we choose to first fetch
all the paths where the attachment is found, this operation is fast
thanks to the new index, split all the paths in candidate paths and then
filter readable_per_se queryset with these paths. All these
modifications make the endpoint faster.
Updated the restore ability so that only directly deleted documents can
be restored. This prevents the restore action from being exposed for
child documents whose parent is deleted. This PR addresses #2127.
Owners could leave trashed documents via the leave endpoint, deleting
their `DocumentAccess` and all subtree access. When the last owner
leaves a document it creates an orphan as no owners remains in the doc.
Signed-off-by: BOUKERFA Mohamed El Amine <boukerfa.ma@gmail.com>
Deleting a user is not possible when it has created docs because the
on_delete on the Document class id RESTRICT and we don't want to change
it. We decided to have a specific workflow for correctly delete a user.
The document where the user is the sole owner must be deleted, the other
only the owner access must be deleted. For the remaining Documents where
the user is the creator, we set it to `null`, then the user can be
delete, remaining relations are deleted in cascade.
We want to allow users to leave a document where they have an access or
they have visited creating a link_trace. All subdocuments should also be
leaved at the same time.
To know if the user can leave a doc we have to check when computing the
abilities if a record is existing in the LinkTrace table. This is a N+1
query situation. To avoid it, we added an annotation in the
DocumentQueryset like we already do to annotate the user role.
There is one edge case where the annotation is made to soon, it is when the
user is visiting a document for the first time, the `get_object` add the
annotation and in the permission, we compute the abilities. The `leave`
property is False because the entry in the LinkTrace table is not made,
when the serializer ask for the abilities again, it is still False. So
in the `retrieve` method in the viewset we force the
`user_has_link_trace` to the correct value.
The mask feature has never been implemented by the frontend application
and it is not what we want to do. We want to implement a feature to
leave a document.
In the model method User::_handle_onboarding_documents_access we do not
allow using documents with restricted link_reach to be added as
onboarding documents. To check the real link_reach of the document, we
must use instead the computed_link_reach to be sure that a sub document
can also be used and compute its correct link_reach.
Admins/owners could overwrite another user's comment text and silently
take ownership of it.
Split write permissions: destroy stays broad (author + admin/owner) for
moderation.
Signed-off-by: Mohamed El Amine BOUKERFA <boukerfa.ma@gmail.com>
We have situation where the number of locks in the database can increase
dangerously creating deadlock situation. To remove this situation we
decided to change the strategy to manage document creation concurrency.
We decided to use a retry strategy, trying to create the document
multiple times while a usable path is found. To avoid having an
inifinite loop, we use a max_attempts counter configurable using the
setting TREEBEARD_PATH_COMPUTE_RETRY_MAX_ATTEMPTS
We want a dedicated endpoint to update a document content. Previously,
updating the content was made on the update action shared with all other
document's properties. When the title is updated, the response contains
the content, so a call to the s3 storage is made and we don't want this.
Isolating the content update will allow us in the next commit to remove
the content from the Document serializer.
The endpoint /api/v1.0/documents/{document_id}/content/ has been renamed
in /api/v1.0/documents/{document_id}/formatted-content/. formatted-content
seems more accurante and the content endpoint will be used for another
purpose more appropriated.
We want to lock the table just before the insert we want to protect is
made. In the case of the perform_create action in the Document viewset,
an http call is made after the lock and can take a very long time,
blocking for nothing the table.
In a past release we added a feature to create a sandbox document to a
newly created used. To create this sandbox document, we duplicate an
existing document and this duplicate is using the add_sibling method
with the "right" agument on this original document. Adding a sibling at
the right to a document involve moving right every root document created
after the original document, so the path of all this documents are
recalculated and changed. This can lead to the lost of some leaf in a
tree because to do this operation, multiple locks are created on the
database, creating lot of connection to the database and if the max
number connection to the database is reached or if the memory allocated
by the database is too hight, the database can close all connections
leading to inconsistent paths in the Document table.
## Purpose
integrate Find to Docs
## Proposal
- [x] ✨ add a `useSeachDocs` hook in charged of calling the search
endpoint.
- [x] ✨ add a optional `path` param to the `search` route. This param
represents the parent document path in case of a sub-documents
(descendants) search.
- [x] ⚡️return Indexer results directly without DB calls to retrieve the
Document objects. All informations necessary for display are indexed in
Find. We can skip the DB calls and improve performance.
- [x] ♻️ refactor react `DocSearchContent` components.
`DocSearchContent` and `DocSearchSubContent` are now merged a unique
component handling all search scenarios and relying on the unique
`search` route.
- [x] 🔥remove pagination logic in the Indexer. Removing the DB calls
also removes the DRF queryset object which handles the pagination. Also
we consider pagination not to be necessary for search v1.
- [x] 🔥remove the `document/<document_id>/descendants` route. This route
is not used anymore. The logic of finding the descendants are moved to
the internal `_list_descendants` method. This method is based on the
parent `path` instead of the parent `id` which has some consequence
about the user access management. Relying on the path prevents the use
of the `self.get_object()` method which used to handle the user access
logic.
- [x] ✨handle fallback logic on DRF based title search in case of
non-configured, badly configured or failing at run time indexer.
- [x] ✨handle language extension in `title` field. Find returns titles
with a language extension (ex: `{ title.fr: "rapport d'activité" }`
instead of `{ "title": "rapport d'activité" }`.
- [x] 🔧 add a `common.test` file to allow running the tests without
docker
- [x] ♻️ rename `SearchIndexer` -> `FindDocumentIndexer`. This class has
to do with Find in particular and the convention is more coherent with
`BaseDocumentIndexer`
- [x] ♻️ rename `SEARCH_INDEXER_URL` -> `INDEXING_URL` and
`SEARCH_INDEXER_QUERY_URL` -> `SEARCH_URL`. I found the original names
very confusing.
- [x] 🔧 update the environment variables to activate the
FindDocumentIndexer.
- [x] ✨automate the generation of encryption key during bootstrap.
OIDC_STORE_REFRESH_TOKEN_KEY is a mandatory secret key. We can not push
it on Github and we want any contributor to be able to run the app by
only running the `make bootstrap`. We chose to generate and wright it
into the `common.local` during bootstrap.
## External contributions
Thank you for your contribution! 🎉
Please ensure the following items are checked before submitting your
pull request:
- [x] I have read and followed the [contributing
guidelines](https://github.com/suitenumerique/docs/blob/main/CONTRIBUTING.md)
- [x] I have read and agreed to the [Code of
Conduct](https://github.com/suitenumerique/docs/blob/main/CODE_OF_CONDUCT.md)
- [x] I have signed off my commits with `git commit --signoff` (DCO
compliance)
- [x] I have signed my commits with my SSH or GPG key (`git commit -S`)
- [x] My commit messages follow the required format: `<gitmoji>(type)
title description`
- [x] I have added a changelog entry under `## [Unreleased]` section (if
noticeable change)
- [x] I have added corresponding tests for new features or bug fixes (if
applicable)
---------
Signed-off-by: charles <charles.englebert@protonmail.com>
When a user is created and a sandbox document should be created, we can
have a race condition on the document creation leading to an error for
the user. To avoid this we have to manage this part in a transaction and
locking the document table
When a user is created, we created accesses to a list of onboarding
documents. Doing this have side effect on the proximity search feature.
Instead of creating access, we should create link_reach
Backend part of #1796.
This changes allows to display an onboarding modal the first time that
the get_me() API view is called.
I originally tried to check if `User.last_login` was `None`, but it is
updated as soon as the user is logged, so I chose to create a flag on
the model.
Adds two methods to allow new users to start with some docs.
User._handle_onboarding_documents_access() gives READER access to
each document listed in settings.USER_ONBOARDING_DOCUMENTS.
User._duplicate_onboarding_sandbox_document() creates a local copy
of the sandbox document specified in
settings.USER_ONBOARDING_SANDBOX_DOCUMENT.
Add utm_source=docssharelink and utm_campaign={docId} query parameters
to shared document links (copy link and invitation emails).
Signed-off-by: Chaïb Martinez <chaibax@gmail.com>
Some identity providers might change the case, but in our products we
don't consider case variation to be consider as different email
addresses.
Next step would be to normalize the DB value of email to be lower-case.
For now, the reconciliation requests are imported through CSV in the
Django admin, which sends confirmation email to both addresses. When
both are checked, the actual reconciliation is processed, and all
user-related content is updated.
## Purpose
Fix#1616 // Replaces #1708
For now, the reconciliation requests are imported through CSV in the
Django admin, which sends confirmation email to both addresses. When
both are checked, the actual reconciliation is processed, and all
user-related content is updated.
## Proposal
- [x] New `UserReconciliationCsvImport` model to manage the import of
reconciliation requests through a task
(`user_reconciliation_csv_import_job`)
- [x] New `UserReconciliation` model to store the user reconciliation
requests themselves (a row = a `active_user`/`inactive_user` pair)
- [x] On save, a confirmation email is sent to the users
- [x] A `process_reconciliation` admin action process the action on the
requested entries, if both emails have been checked.
- [x] Bulk update the `DocumentAccess` items, while managing the case
where both users have access to the document (keeping the higher role)
- [x] Bulk update the `LinkTrace` items, while managing the case where
both users have link traces to the document
- [x] Bulk update the `DocumentFavorite` items, while managing the case
where both users have put the document in their favorites
- [x] Bulk update the comment system items (`Thread`, `Comment` and
`Reaction` items)
- [x] Bulk update the `is_active` status on both users
- [x] New `USER_RECONCILIATION_FORM_URL` env variable for the "make a
new request" URL in an email.
- [x] Write unit tests
- [x] Remove the unused `email_user()` method on `User`, replaced with
`send_email()` similar to the one on the `Document` model
## Demo page reconciliation success
<img width="1149" height="746" alt="image"
src="https://github.com/user-attachments/assets/09ba2b38-7af3-41fa-a64f-ce3c4fd8548d"
/>
---------
Co-authored-by: Anthony LC <anthony.le-courric@mail.numerique.gouv.fr>
Most of Docs app is configured thanks to environment
variables, except the url in the email that
was from the django site table.
Now we can set it with DJANGO_EMAIL_URL_APP
environment variable to have a better consistency.
We keep the previous way to avoid breaking
changes.
The template feature is removed.
Migration created to drop related tables.
Files modified:
- viewsets
- serializers
- models
- admin
- factories
- urls
- tests
- demo data
## Purpose
The email template is made with the idea that they link to a document.
This change allows to customize the label of the button (currently,
"Open") to allow for a different action verb. Additionally, the
'document_title' parameter is renamed to 'link_label' to reflect that it
can link to other things than documents.
## Proposal
- [x] Email template `template.mjml` updated as proposed
- [x] Method `send_email()` updated
- [x] Translations updated
Reduce the number of Find API calls by grouping all the latest changes
for indexation : send all the documents updated or deleted since the
triggering of the task.
Signed-off-by: Fabre Florian <ffabre@hybird.org>
New SEARCH_INDEXER_CLASS setting to define the indexer service class.
Raise ImpoperlyConfigured errors instead of RuntimeError in index service.
Signed-off-by: Fabre Florian <ffabre@hybird.org>
On document content or permission changes, start a celery job that will call the
indexation API of the app "Find".
Signed-off-by: Fabre Florian <ffabre@hybird.org>
In order to store the comments on a document, we created a new model
Comment. User is nullable because anonymous users can comment a Document
is this one is public with a link_role commentator.
Like in other abilities, we compute a set_role_to property on the
abilities. This set_role_to contains all the roles lower or equal than
the current user role. We rely on this propoerty to validate the accept
endpoint and it will be used by the front allpication to built the role
select list.
The user's short_name field length was set to 20. This is not enought
and we have some users who cannot register because of that. We changed
this length to a higher one, 100, like the full_name.
The abilities for a deleted document were too open. We want to restrict
them. Only the restore, retrieve and tree is allowed. The tree method
will need some modifications to get the right informations.
A user can be invited and no control is made on the email case. Then,
when a new user is created, we are looking if there are pending
invitation and the lookup used is case sensitive. We change it using
__iexact which is case insensitive.
This allows API users to process document content, enabling the
use of Docs as a headless CMS for instance, or any kind of document
processing. Fixes#1206.
An editor who created a subpages should be allowed to delete it.
We change the abilities to be coherent between the creation and the
deletion.
Fixes#1193
Once users have visited a document to which they have access,
they can't remove it from their list view anymore. Several
users reported that this is annoying because a document that
gets a lot of updates keeps popping up at the top of their list
view.
They want to be able to mask the document in a click. We propose
to add a "masked documents" section in the left side bar where the
masked documents can still be found.
With the soft delete feature, relying on the is_leaf method from the
treebeard is not accurate anymore. To determine if a node is a leaf, it
checks if the number of numchild is equal to 0. But a node can have soft
deleted children, then numchild is equal to 0, but it is not a leaf
because if we want to add a child we have to look for the last child to
compute a correct path. Otherwise we will have an error saying that the
path already exists.
We were returning the list of roles a user has on a document (direct
and inherited). Now that we introduced priority on roles, we are able
to determine what is the max role and return only this one.
This commit also changes the role that is returned for the restricted
reach: we now return None because the role is not relevant in this
case.
Manuel Raynaud