Skip to content

Commit d73686b

Browse files
authored
Merge branch 'main' into feat/add-max-reaction
2 parents ede3d48 + 4fe508b commit d73686b

13 files changed

Lines changed: 320 additions & 55 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to
1919
- ♿️(frontend) make doc search result labels uniquely identifiable #2212
2020
- ⬆️(backend) upgrade docspec to v3.0.x and adapt converter API #2220
2121
- ✨(backend) make forward auth request uri header configurable #2241
22+
- ♿️(frontend) fix sidebar resize handle for screen readers #2122
2223

2324
### Fixed
2425

@@ -29,6 +30,8 @@ and this project adheres to
2930
- 🐛(frontend) fix interlinking modal clipping #2213
3031
- 🛂(frontend) fix cannot manage member on small screen #2226
3132
- 🐛(backend) load jwks url when OIDC_RS_PRIVATE_KEY_STR is set
33+
- 🐛(backend) Prevent moving document to its own descendant or self #2208
34+
- 🐛(backend) return 400 when restoring a non-deleted document #2225
3235

3336
### Removed
3437

@@ -202,6 +205,7 @@ and this project adheres to
202205

203206
### Fixed
204207

208+
- 🐛(backend) enforce emoji validation for reactions #1965
205209
- 🐛(frontend) analytic feature flags problem #1953
206210
- 🐛(frontend) fix home collapsing panel #1954
207211
- 🐛(frontend) fix disabled color on icon Dropdown #1950

src/backend/core/api/serializers.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from django.utils.text import slugify
1414
from django.utils.translation import gettext_lazy as _
1515

16+
import emoji
1617
import magic
1718
from rest_framework import serializers
1819

@@ -875,6 +876,12 @@ class Meta:
875876
]
876877
read_only_fields = ["id", "created_at", "users"]
877878

879+
def validate_emoji(self, value):
880+
"""Ensure the reaction is a single emoji."""
881+
if not emoji.is_emoji(value):
882+
raise serializers.ValidationError("Reaction must be a single valid emoji.")
883+
return value
884+
878885

879886
class CommentSerializer(serializers.ModelSerializer):
880887
"""Serialize comments (nested under a thread) with reactions and abilities."""

src/backend/core/api/viewsets.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
from rest_framework import response as drf_response
4545
from rest_framework.permissions import AllowAny
4646
from rest_framework.views import APIView
47+
from treebeard.exceptions import InvalidMoveToDescendant
4748

4849
from core import authentication, choices, enums, models
4950
from core.api.filters import remove_accents
@@ -961,7 +962,13 @@ def move(self, request, *args, **kwargs):
961962
status=status.HTTP_400_BAD_REQUEST,
962963
)
963964

964-
document.move(target_document, pos=position)
965+
try:
966+
document.move(target_document, pos=position)
967+
except InvalidMoveToDescendant:
968+
return drf.response.Response(
969+
{"target_document_id": "Cannot move a document to its own descendant."},
970+
status=status.HTTP_400_BAD_REQUEST,
971+
)
965972

966973
# Make sure we have at least one owner
967974
if (
@@ -989,7 +996,10 @@ def restore(self, request, *args, **kwargs):
989996
Restore a soft-deleted document if it was deleted less than x days ago.
990997
"""
991998
document = self.get_object()
992-
document.restore()
999+
try:
1000+
document.restore()
1001+
except RuntimeError as err:
1002+
raise drf.exceptions.ValidationError({"detail": str(err)}) from err
9931003

9941004
return drf_response.Response(
9951005
{"detail": "Document has been successfully restored."},
@@ -2268,7 +2278,7 @@ def cors_proxy(self, request, *args, **kwargs):
22682278
GET /api/v1.0/documents/<resource_id>/cors-proxy
22692279
Act like a proxy to fetch external resources and bypass CORS restrictions.
22702280
"""
2271-
url = request.query_params.get("url")
2281+
url = request.query_params.get("url", "").strip()
22722282
if not url:
22732283
return drf.response.Response(
22742284
{"detail": "Missing 'url' query parameter"},

src/backend/core/factories.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,10 @@ class ReactionFactory(factory.django.DjangoModelFactory):
231231

232232
class Meta:
233233
model = models.Reaction
234+
skip_postgeneration_save = True
234235

235236
comment = factory.SubFactory(CommentFactory)
236-
emoji = "test"
237+
emoji = factory.Faker("emoji")
237238

238239
@classmethod
239240
def generate_emojis(cls, n=10):

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

Lines changed: 70 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -644,11 +644,13 @@ def test_create_reaction_anonymous_user_public_document(link_role):
644644
document = factories.DocumentFactory(link_reach="public", link_role=link_role)
645645
thread = factories.ThreadFactory(document=document)
646646
comment = factories.CommentFactory(thread=thread)
647+
reaction = factories.ReactionFactory(comment=comment)
648+
647649
client = APIClient()
648650
response = client.post(
649651
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
650652
f"comments/{comment.id!s}/reactions/",
651-
{"emoji": "test"},
653+
{"emoji": reaction.emoji},
652654
)
653655
assert response.status_code == 401
654656

@@ -664,12 +666,14 @@ def test_create_reaction_authenticated_user_public_document():
664666
)
665667
thread = factories.ThreadFactory(document=document)
666668
comment = factories.CommentFactory(thread=thread)
669+
reaction = factories.ReactionFactory(comment=comment)
670+
667671
client = APIClient()
668672
client.force_login(user)
669673
response = client.post(
670674
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
671675
f"comments/{comment.id!s}/reactions/",
672-
{"emoji": "test"},
676+
{"emoji": reaction.emoji},
673677
)
674678
assert response.status_code == 403
675679

@@ -684,17 +688,19 @@ def test_create_reaction_authenticated_user_accessible_public_document():
684688
)
685689
thread = factories.ThreadFactory(document=document)
686690
comment = factories.CommentFactory(thread=thread)
691+
reaction = factories.ReactionFactory(comment=comment)
692+
687693
client = APIClient()
688694
client.force_login(user)
689695
response = client.post(
690696
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
691697
f"comments/{comment.id!s}/reactions/",
692-
{"emoji": "test"},
698+
{"emoji": reaction.emoji},
693699
)
694700
assert response.status_code == 201
695701

696702
assert models.Reaction.objects.filter(
697-
comment=comment, emoji="test", users__in=[user]
703+
comment=comment, emoji=reaction.emoji, users__in=[user]
698704
).exists()
699705

700706

@@ -709,12 +715,14 @@ def test_create_reaction_authenticated_user_connected_document_link_role_reader(
709715
)
710716
thread = factories.ThreadFactory(document=document)
711717
comment = factories.CommentFactory(thread=thread)
718+
reaction = factories.ReactionFactory(comment=comment)
719+
712720
client = APIClient()
713721
client.force_login(user)
714722
response = client.post(
715723
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
716724
f"comments/{comment.id!s}/reactions/",
717-
{"emoji": "test"},
725+
{"emoji": reaction.emoji},
718726
)
719727
assert response.status_code == 403
720728

@@ -737,17 +745,19 @@ def test_create_reaction_authenticated_user_connected_document(link_role):
737745
)
738746
thread = factories.ThreadFactory(document=document)
739747
comment = factories.CommentFactory(thread=thread)
748+
reaction = factories.ReactionFactory(comment=comment)
749+
740750
client = APIClient()
741751
client.force_login(user)
742752
response = client.post(
743753
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
744754
f"comments/{comment.id!s}/reactions/",
745-
{"emoji": "test"},
755+
{"emoji": reaction.emoji},
746756
)
747757
assert response.status_code == 201
748758

749759
assert models.Reaction.objects.filter(
750-
comment=comment, emoji="test", users__in=[user]
760+
comment=comment, emoji=reaction.emoji, users__in=[user]
751761
).exists()
752762

753763

@@ -760,12 +770,14 @@ def test_create_reaction_authenticated_user_restricted_accessible_document():
760770
document = factories.DocumentFactory(link_reach="restricted")
761771
thread = factories.ThreadFactory(document=document)
762772
comment = factories.CommentFactory(thread=thread)
773+
reaction = factories.ReactionFactory(comment=comment)
774+
763775
client = APIClient()
764776
client.force_login(user)
765777
response = client.post(
766778
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
767779
f"comments/{comment.id!s}/reactions/",
768-
{"emoji": "test"},
780+
{"emoji": reaction.emoji},
769781
)
770782
assert response.status_code == 403
771783

@@ -781,12 +793,14 @@ def test_create_reaction_authenticated_user_restricted_accessible_document_role_
781793
)
782794
thread = factories.ThreadFactory(document=document)
783795
comment = factories.CommentFactory(thread=thread)
796+
reaction = factories.ReactionFactory(comment=comment)
797+
784798
client = APIClient()
785799
client.force_login(user)
786800
response = client.post(
787801
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
788802
f"comments/{comment.id!s}/reactions/",
789-
{"emoji": "test"},
803+
{"emoji": reaction.emoji},
790804
)
791805
assert response.status_code == 403
792806

@@ -806,28 +820,72 @@ def test_create_reaction_authenticated_user_restricted_accessible_document_role_
806820
document = factories.DocumentFactory(link_reach="restricted", users=[(user, role)])
807821
thread = factories.ThreadFactory(document=document)
808822
comment = factories.CommentFactory(thread=thread)
823+
reaction = factories.ReactionFactory(comment=comment)
824+
809825
client = APIClient()
810826
client.force_login(user)
811827
response = client.post(
812828
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
813829
f"comments/{comment.id!s}/reactions/",
814-
{"emoji": "test"},
830+
{"emoji": reaction.emoji},
815831
)
816832
assert response.status_code == 201
817833

818834
assert models.Reaction.objects.filter(
819-
comment=comment, emoji="test", users__in=[user]
835+
comment=comment, emoji=reaction.emoji, users__in=[user]
820836
).exists()
821837

822838
response = client.post(
823839
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
824840
f"comments/{comment.id!s}/reactions/",
825-
{"emoji": "test"},
841+
{"emoji": reaction.emoji},
826842
)
827843
assert response.status_code == 400
828844
assert response.json() == {"user_already_reacted": True}
829845

830846

847+
def test_create_reaction_invalid_emoji():
848+
"""Users should not be able to submit non-emojis as reactions."""
849+
user = factories.UserFactory()
850+
document = factories.DocumentFactory(
851+
link_reach="restricted", users=[(user, models.RoleChoices.COMMENTER)]
852+
)
853+
thread = factories.ThreadFactory(document=document)
854+
comment = factories.CommentFactory(thread=thread)
855+
856+
client = APIClient()
857+
client.force_login(user)
858+
859+
response = client.post(
860+
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
861+
f"comments/{comment.id!s}/reactions/",
862+
{"emoji": "test"},
863+
)
864+
assert response.status_code == 400
865+
assert "Reaction must be a single valid emoji." in str(response.json())
866+
867+
868+
def test_create_reaction_multiple_emojis():
869+
"""Users should not be able to submit multiple emojis as a single reaction."""
870+
user = factories.UserFactory()
871+
document = factories.DocumentFactory(
872+
link_reach="restricted", users=[(user, models.RoleChoices.COMMENTER)]
873+
)
874+
thread = factories.ThreadFactory(document=document)
875+
comment = factories.CommentFactory(thread=thread)
876+
877+
client = APIClient()
878+
client.force_login(user)
879+
880+
response = client.post(
881+
f"/api/v1.0/documents/{document.id!s}/threads/{thread.id!s}/"
882+
f"comments/{comment.id!s}/reactions/",
883+
{"emoji": "🐛🐛"},
884+
)
885+
assert response.status_code == 400
886+
assert "Reaction must be a single valid emoji." in str(response.json())
887+
888+
831889
# Delete reaction
832890

833891

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,31 @@ def test_api_docs_cors_proxy_valid_url(mock_getaddrinfo):
5555
assert response.streaming_content
5656

5757

58+
@unittest.mock.patch("core.api.viewsets.socket.getaddrinfo")
59+
@responses.activate
60+
def test_api_docs_cors_proxy_url_with_surrounding_whitespace(mock_getaddrinfo):
61+
"""
62+
URLs with leading or trailing whitespace must still be proxied successfully,
63+
otherwise images whose `src` has stray whitespace are missing from the PDF export.
64+
"""
65+
document = factories.DocumentFactory(link_reach="public")
66+
67+
# Mock DNS resolution to return a public IP address
68+
mock_getaddrinfo.return_value = [
69+
(socket.AF_INET, socket.SOCK_STREAM, 0, "", ("8.8.8.8", 0))
70+
]
71+
72+
client = APIClient()
73+
url_to_fetch = "https://external-url.com/assets/logo-gouv.png"
74+
responses.get(url_to_fetch, body=b"", status=200, content_type="image/png")
75+
response = client.get(
76+
f"/api/v1.0/documents/{document.id!s}/cors-proxy/?url= {url_to_fetch} "
77+
)
78+
assert response.status_code == 200
79+
assert response.headers["Content-Type"] == "image/png"
80+
assert response.streaming_content
81+
82+
5883
def test_api_docs_cors_proxy_without_url_query_string():
5984
"""Test the CORS proxy API for documents without a URL query string."""
6085
document = factories.DocumentFactory(link_reach="public")

0 commit comments

Comments
 (0)