[WIP] misc fixes

CHANGELOG.md

@@ -165,7 +165,7 @@ and this project adheres to
 
 - 🛂(frontend) match email if no existing user matches the sub
 - 🐛(backend) gitlab oicd userinfo endpoint #232
-- 🛂(frontend) redirect to the OIDC when private doc and unauthentified #292
+- 🛂(frontend) redirect to the OIDC when private doc and unauthenticated #292
 - ♻️(backend) getting list of document versions available for a user #258
 - 🔧(backend) fix configuration to avoid different ssl warning #297
 - 🐛(frontend) fix editor break line not working #302
@@ -294,7 +294,7 @@ and this project adheres to
 - ⚡️(e2e) unique login between tests (#80)
 - ⚡️(CI) improve e2e job (#86)
 - ♻️(frontend) improve the error and message info ui (#93)
-- ✏️(frontend) change all occurences of pad to doc (#99)
+- ✏️(frontend) change all occurrences of pad to doc (#99)
 
 ## Fixed
 

docs/release.md

@@ -6,7 +6,7 @@ Whenever we are cooking a new release (e.g. `4.18.1`) we should follow a standar
    2.  Bump the release number for backend project, frontend projects, and Helm files:
 
        - for backend, update the version number by hand in `pyproject.toml`,
-       - for each projects (`src/frontend`, `src/frontend/apps/*`, `src/frontend/packages/*`, `src/mail`), run `yarn version --new-version --no-git-tag-version 4.18.1` in their directory. This will update their `package.json` for you,
+       - for each project (`src/frontend`, `src/frontend/apps/*`, `src/frontend/packages/*`, `src/mail`), run `yarn version --new-version --no-git-tag-version 4.18.1` in their directory. This will update their `package.json` for you,
        - for Helm, update Docker image tag in files located at `src/helm/env.d` for both `preprod` and `production` environments:
 
          ```yaml

src/backend/core/api/utils.py

@@ -13,7 +13,7 @@
 
 def generate_s3_authorization_headers(key):
     """
-    Generate authorization headers for an s3 object.
+    Generate authorization headers for a s3 object.
     These headers can be used as an alternative to signed urls with many benefits:
     - the urls of our files never expire and can be stored in our documents' content
     - we don't leak authorized urls that could be shared (file access can only be done

src/backend/core/api/viewsets.py

@@ -621,15 +621,15 @@ def attachment_upload(self, request, *args, **kwargs):
 
     def _authorize_subrequest(self, request, pattern):
         """
-        Shared method to authorize access based on the original URL of an Nginx subrequest
+        Shared method to authorize access based on the original URL of a Nginx subrequest
         and user permissions. Returns a dictionary of URL parameters if authorized.
 
         The original url is passed by nginx in the "HTTP_X_ORIGINAL_URL" header.
         See corresponding ingress configuration in Helm chart and read about the
         nginx.ingress.kubernetes.io/auth-url annotation to understand how the Nginx ingress
         is configured to do this.
 
-        Based on the original url and the logged in user, we must decide if we authorize Nginx
+        Based on the original url and the logged-in user, we must decide if we authorize Nginx
         to let this request go through (by returning a 200 code) or if we block it (by returning
         a 403 error). Note that we return 403 errors without any further details for security
         reasons.
@@ -697,7 +697,7 @@ def _authorize_subrequest(self, request, pattern):
     @drf.decorators.action(detail=False, methods=["get"], url_path="media-auth")
     def media_auth(self, request, *args, **kwargs):
         """
-        This view is used by an Nginx subrequest to control access to a document's
+        This view is used by a Nginx subrequest to control access to a document's
         attachment file.
 
         When we let the request go through, we compute authorization headers that will be added to
@@ -718,7 +718,7 @@ def media_auth(self, request, *args, **kwargs):
     @drf.decorators.action(detail=False, methods=["get"], url_path="collaboration-auth")
     def collaboration_auth(self, request, *args, **kwargs):
         """
-        This view is used by an Nginx subrequest to control access to a document's
+        This view is used by a Nginx subrequest to control access to a document's
         collaboration server.
         """
         _, user_abilities, user_id = self._authorize_subrequest(
@@ -834,7 +834,7 @@ class DocumentAccessViewSet(
     serializer_class = serializers.DocumentAccessSerializer
 
     def perform_create(self, serializer):
-        """Add a new access to the document and send an email to the new added user."""
+        """Add new access to the document and email the new added user."""
         access = serializer.save()
         language = self.request.headers.get("Content-Language", "en-us")
 
@@ -846,7 +846,7 @@ def perform_create(self, serializer):
         )
 
     def perform_update(self, serializer):
-        """Update an access to the document and notify the collaboration server."""
+        """Update access to the document and notify the collaboration server."""
         access = serializer.save()
 
         access_user_id = None
@@ -859,7 +859,7 @@ def perform_update(self, serializer):
         )
 
     def perform_destroy(self, instance):
-        """Delete an access to the document and notify the collaboration server."""
+        """Delete access to the document and notify the collaboration server."""
         instance.delete()
 
         # Notify collaboration server about the access removed
@@ -1098,7 +1098,7 @@ def get_queryset(self):
         return queryset
 
     def perform_create(self, serializer):
-        """Save invitation to a document then send an email to the invited user."""
+        """Save invitation to a document then email the invited user."""
         invitation = serializer.save()
 
         language = self.request.headers.get("Content-Language", "en-us")

src/backend/core/authentication/urls.py

@@ -2,7 +2,7 @@
 
 from django.urls import path
 
-from mozilla_django_oidc.urls import urlpatterns as mozzila_oidc_urls
+from mozilla_django_oidc.urls import urlpatterns as mozilla_oidc_urls
 
 from .views import OIDCLogoutCallbackView, OIDCLogoutView
 
@@ -14,5 +14,5 @@
         OIDCLogoutCallbackView.as_view(),
         name="oidc_logout_callback",
     ),
-    *mozzila_oidc_urls,
+    *mozilla_oidc_urls,
 ]

src/backend/core/factories.py

@@ -19,6 +19,7 @@ class UserFactory(factory.django.DjangoModelFactory):
 
     class Meta:
         model = models.User
+        skip_postgeneration_save = True
 
     sub = factory.Sequence(lambda n: f"user{n!s}")
     email = factory.Faker("email")
@@ -36,6 +37,8 @@ def with_owned_document(self, create, extracted, **kwargs):
         if create and (extracted is True):
             UserDocumentAccessFactory(user=self, role="owner")
 
+        self.save()
+
     @factory.post_generation
     def with_owned_template(self, create, extracted, **kwargs):
         """
@@ -45,6 +48,8 @@ def with_owned_template(self, create, extracted, **kwargs):
         if create and (extracted is True):
             UserTemplateAccessFactory(user=self, role="owner")
 
+        self.save()
+
 
 class DocumentFactory(factory.django.DjangoModelFactory):
     """A factory to create documents"""

src/backend/core/migrations/0001_initial.py

@@ -1,7 +1,5 @@
 # Generated by Django 5.0.3 on 2024-05-28 20:29
 
-import django.contrib.auth.models
-import django.core.validators
 import django.db.models.deletion
 import timezone_field.fields
 import uuid
@@ -145,7 +143,7 @@ class Migration(migrations.Migration):
         ),
         migrations.AddConstraint(
             model_name='documentaccess',
-            constraint=models.CheckConstraint(check=models.Q(models.Q(('team', ''), ('user__isnull', False)), models.Q(('team__gt', ''), ('user__isnull', True)), _connector='OR'), name='check_document_access_either_user_or_team', violation_error_message='Either user or team must be set, not both.'),
+            constraint=models.CheckConstraint(condition=models.Q(models.Q(('team', ''), ('user__isnull', False)), models.Q(('team__gt', ''), ('user__isnull', True)), _connector='OR'), name='check_document_access_either_user_or_team', violation_error_message='Either user or team must be set, not both.'),
         ),
         migrations.AddConstraint(
             model_name='invitation',
@@ -161,6 +159,6 @@ class Migration(migrations.Migration):
         ),
         migrations.AddConstraint(
             model_name='templateaccess',
-            constraint=models.CheckConstraint(check=models.Q(models.Q(('team', ''), ('user__isnull', False)), models.Q(('team__gt', ''), ('user__isnull', True)), _connector='OR'), name='check_template_access_either_user_or_team', violation_error_message='Either user or team must be set, not both.'),
+            constraint=models.CheckConstraint(condition=models.Q(models.Q(('team', ''), ('user__isnull', False)), models.Q(('team__gt', ''), ('user__isnull', True)), _connector='OR'), name='check_template_access_either_user_or_team', violation_error_message='Either user or team must be set, not both.'),
         ),
     ]

src/backend/core/models.py

@@ -155,7 +155,7 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin):
     email = models.EmailField(_("identity email address"), blank=True, null=True)
 
     # Unlike the "email" field which stores the email coming from the OIDC token, this field
-    # stores the email used by staff users to login to the admin site
+    # stores the email used by staff users to log in to the admin site
     admin_email = models.EmailField(
         _("admin email address"), unique=True, blank=True, null=True
     )
@@ -199,6 +199,7 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin):
 
     class Meta:
         db_table = "impress_user"
+        ordering = ("-created_at",)
         verbose_name = _("user")
         verbose_name_plural = _("users")
 
@@ -695,7 +696,7 @@ class Meta:
                 violation_error_message=_("This team is already in this document."),
             ),
             models.CheckConstraint(
-                check=models.Q(user__isnull=False, team="")
+                condition=models.Q(user__isnull=False, team="")
                 | models.Q(user__isnull=True, team__gt=""),
                 name="check_document_access_either_user_or_team",
                 violation_error_message=_("Either user or team must be set, not both."),
@@ -760,7 +761,7 @@ def generate_pdf(self, body_html, metadata):
         """
         document_html = weasyprint.HTML(
             string=DjangoTemplate(self.code).render(
-                Context({"body": html.format_html(body_html), **metadata})
+                Context({"body": html.format_html("{}", body_html), **metadata})
             )
         )
         css = weasyprint.CSS(
@@ -779,7 +780,7 @@ def generate_word(self, body_html, metadata):
         Generate and return a docx document wrapped around the current template
         """
         template_string = DjangoTemplate(self.code).render(
-            Context({"body": html.format_html(body_html), **metadata})
+            Context({"body": html.format_html("{}", body_html), **metadata})
         )
 
         html_string = f"""
@@ -797,7 +798,6 @@ def generate_word(self, body_html, metadata):
         """
 
         reference_docx = "core/static/reference.docx"
-        output = BytesIO()
 
         # Convert the HTML to a temporary docx file
         with tempfile.NamedTemporaryFile(suffix=".docx", prefix="docx_") as tmp_file:
@@ -884,7 +884,7 @@ class Meta:
                 violation_error_message=_("This team is already in this template."),
             ),
             models.CheckConstraint(
-                check=models.Q(user__isnull=False, team="")
+                condition=models.Q(user__isnull=False, team="")
                 | models.Q(user__isnull=True, team__gt=""),
                 name="check_template_access_either_user_or_team",
                 violation_error_message=_("Either user or team must be set, not both."),

src/backend/core/services/collaboration_services.py

@@ -17,7 +17,7 @@ def __init__(self):
     def reset_connections(self, room, user_id=None):
         """
         Reset connections of a room in the collaboration server.
-        Reseting a connection means that the user will be disconnected and will
+        Resetting a connection means that the user will be disconnected and will
         have to reconnect to the collaboration server, with updated rights.
         """
         endpoint = "reset-connections"

src/backend/core/tests/authentication/test_backends.py

@@ -341,7 +341,7 @@ def test_authentication_getter_existing_disabled_user_via_email(
     django_assert_num_queries, monkeypatch
 ):
     """
-    If an existing user does not matches the sub but matches the email and is disabled,
+    If an existing user does not match the sub but matches the email and is disabled,
     an error should be raised and a user should not be created.
     """
 

src/backend/core/tests/documents/test_api_document_accesses.py

@@ -698,7 +698,7 @@ def test_api_document_accesses_delete_administrators_except_owners(
     mock_reset_connections,  # pylint: disable=redefined-outer-name
 ):
     """
-    Users who are administrators in a document should be allowed to delete an access
+    Users who are administrators in a document should be allowed to delete access
     from the document provided it is not ownership.
     """
     user = factories.UserFactory()

src/backend/core/tests/documents/test_api_document_versions.py

@@ -285,7 +285,7 @@ def test_api_document_versions_retrieve_authenticated_related(via, mock_user_tea
     assert response.status_code == 404
 
     # Create a new version should not make it available to the user because
-    # only the current version is available to the user but it is excluded
+    # only the current version is available to the user, but it is excluded
     # from the list
     document.content = "new content 1"
     document.save()

src/backend/core/tests/documents/test_api_documents_ai_transform.py

@@ -134,7 +134,7 @@ def test_api_documents_ai_transform_authenticated_forbidden(reach, role):
 @patch("openai.resources.chat.completions.Completions.create")
 def test_api_documents_ai_transform_authenticated_success(mock_create, reach, role):
     """
-    Autenticated who are not related to a document should be able to request AI transform
+    Authenticated who are not related to a document should be able to request AI transform
     if the link reach and role permit it.
     """
     user = factories.UserFactory()

src/backend/core/tests/documents/test_api_documents_ai_translate.py

@@ -154,7 +154,7 @@ def test_api_documents_ai_translate_authenticated_forbidden(reach, role):
 @patch("openai.resources.chat.completions.Completions.create")
 def test_api_documents_ai_translate_authenticated_success(mock_create, reach, role):
     """
-    Autenticated who are not related to a document should be able to request AI translate
+    Authenticated who are not related to a document should be able to request AI translate
     if the link reach and role permit it.
     """
     user = factories.UserFactory()

src/backend/core/tests/documents/test_api_documents_attachment_upload.py

@@ -111,7 +111,7 @@ def test_api_documents_attachment_upload_authenticated_forbidden(reach, role):
 )
 def test_api_documents_attachment_upload_authenticated_success(reach, role):
     """
-    Autenticated who are not related to a document should be able to upload a file
+    Authenticated who are not related to a document should be able to upload a file
     if the link reach and role permit it.
     """
     user = factories.UserFactory()
@@ -225,7 +225,7 @@ def test_api_documents_attachment_upload_invalid(client):
 
 
 def test_api_documents_attachment_upload_size_limit_exceeded(settings):
-    """The uploaded file should not exceeed the maximum size in settings."""
+    """The uploaded file should not exceed the maximum size in settings."""
     settings.DOCUMENT_IMAGE_MAX_SIZE = 1048576  # 1 MB for test
 
     user = factories.UserFactory()

src/backend/core/tests/documents/test_api_documents_media_auth.py

@@ -160,7 +160,7 @@ def test_api_documents_media_auth_authenticated_restricted():
 @pytest.mark.parametrize("via", VIA)
 def test_api_documents_media_auth_related(via, mock_user_teams):
     """
-    Users who have a specific access to a document, whatever the role, should be able to
+    Users who have specific access to a document, whatever the role, should be able to
     retrieve related attachments.
     """
     user = factories.UserFactory()

src/backend/core/tests/templates/test_api_template_accesses.py

@@ -647,7 +647,7 @@ def test_api_template_accesses_delete_administrators_except_owners(
     via, mock_user_teams
 ):
     """
-    Users who are administrators in a template should be allowed to delete an access
+    Users who are administrators in a template should be allowed to delete access
     from the template provided it is not ownership.
     """
     user = factories.UserFactory()

src/backend/core/tests/test_models_documents.py

@@ -84,7 +84,7 @@ def test_models_documents_file_key():
 def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role):
     """
     Check abilities returned for a document giving insufficient roles to link holders
-    i.e anonymous users or authenticated users who have no specific role on the document.
+    i.e. anonymous users or authenticated users who have no specific role on the document.
     """
     document = factories.DocumentFactory(link_reach=reach, link_role=role)
     user = factories.UserFactory() if is_authenticated else AnonymousUser()
@@ -121,7 +121,7 @@ def test_models_documents_get_abilities_forbidden(is_authenticated, reach, role)
 def test_models_documents_get_abilities_reader(is_authenticated, reach):
     """
     Check abilities returned for a document giving reader role to link holders
-    i.e anonymous users or authenticated users who have no specific role on the document.
+    i.e. anonymous users or authenticated users who have no specific role on the document.
     """
     document = factories.DocumentFactory(link_reach=reach, link_role="reader")
     user = factories.UserFactory() if is_authenticated else AnonymousUser()
@@ -158,7 +158,7 @@ def test_models_documents_get_abilities_reader(is_authenticated, reach):
 def test_models_documents_get_abilities_editor(is_authenticated, reach):
     """
     Check abilities returned for a document giving editor role to link holders
-    i.e anonymous users or authenticated users who have no specific role on the document.
+    i.e. anonymous users or authenticated users who have no specific role on the document.
     """
     document = factories.DocumentFactory(link_reach=reach, link_role="editor")
     user = factories.UserFactory() if is_authenticated else AnonymousUser()
@@ -449,7 +449,7 @@ def test_models_documents__email_invitation__success():
 
 def test_models_documents__email_invitation__success_fr():
     """
-    The email invitation is sent successfully in french.
+    The email invitation is sent successfully in French.
     """
     document = factories.DocumentFactory()
 

src/backend/core/tests/test_models_users.py

@@ -27,7 +27,7 @@ def test_models_users_id_unique():
 
 
 def test_models_users_send_mail_main_existing():
-    """The "email_user' method should send mail to the user's email address."""
+    """The 'email_user' method should send mail to the user's email address."""
     user = factories.UserFactory()
 
     with mock.patch("django.core.mail.send_mail") as mock_send:
@@ -37,7 +37,7 @@ def test_models_users_send_mail_main_existing():
 
 
 def test_models_users_send_mail_main_missing():
-    """The "email_user' method should fail if the user has no email address."""
+    """The 'email_user' method should fail if the user has no email address."""
     user = factories.UserFactory(email=None)
 
     with pytest.raises(ValueError) as excinfo:

src/backend/core/tests/test_services_ai_services.py

@@ -1,5 +1,5 @@
 """
-Test ai API endpoints in the impress core app.
+Test AI API endpoints in the impress core app.
 """
 
 import json

src/backend/demo/management/commands/createsuperuser.py

@@ -15,7 +15,7 @@ def add_arguments(self, parser):
         """Define required arguments "email" and "password"."""
         parser.add_argument(
             "--email",
-            help=("Email for the user."),
+            help="Email for the user.",
         )
         parser.add_argument(
             "--password",