Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WEB-1435] chore: conflict free issue description #6120

Open
wants to merge 33 commits into
base: preview
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
e781ab9
chore: new description binary endpoints
NarayanBavisetti Oct 24, 2024
cccc7b7
chore: conflict free issue description
aaryan610 Oct 24, 2024
d3b443e
chore: fix submitting status
aaryan610 Oct 25, 2024
a7d8bee
chore: update yjs utils
aaryan610 Oct 25, 2024
745714a
Merge branch 'preview' of https://github.com/makeplane/plane into cho…
aaryan610 Oct 25, 2024
baf1517
chore: handle component re-mounting
aaryan610 Oct 25, 2024
54ecea1
chore: update buffer response type
aaryan610 Oct 25, 2024
fcd06fc
chore: add try catch for issue description update
aaryan610 Oct 25, 2024
547fc86
chore: update buffer response type
aaryan610 Oct 25, 2024
7448c5b
chore: description binary in retrieve
NarayanBavisetti Nov 5, 2024
6275e2f
fix: merge conflicts resolved from preview
aaryan610 Nov 6, 2024
b3b1088
chore: update issue description hook
aaryan610 Nov 8, 2024
ccad0e7
chore: decode description binary
NarayanBavisetti Nov 8, 2024
389ee74
chore: migrations fixes and cleanup
sriramveeraghanta Nov 8, 2024
6667327
chore: migration fixes
NarayanBavisetti Nov 8, 2024
60d091e
fix: merge conflicts resolved from preview
aaryan610 Nov 8, 2024
6b437ee
fix: inbox issue description
aaryan610 Nov 9, 2024
a95143c
chore: move update operations to the issue store
aaryan610 Nov 9, 2024
ef76bd0
Merge branch 'preview' of github.com:makeplane/plane into chore/confl…
NarayanBavisetti Nov 12, 2024
e3ac9ef
fix: merge conflicts
NarayanBavisetti Nov 12, 2024
02e3c48
Merge branch 'chore/conflict-free-issue-description' of github.com:ma…
NarayanBavisetti Nov 12, 2024
e6787d8
chore: reverted the commit
NarayanBavisetti Nov 12, 2024
ea4d201
chore: removed the unwanted imports
NarayanBavisetti Nov 12, 2024
4d7d4b6
chore: remove unnecessary props
aaryan610 Nov 12, 2024
1fcc3ab
chore: remove unused services
aaryan610 Nov 12, 2024
3792699
chore: update live server error handling
aaryan610 Nov 12, 2024
11b0c82
Merge branch 'preview' of https://github.com/makeplane/plane into cho…
aaryan610 Nov 14, 2024
84246ea
Merge branch 'preview' of github.com:makeplane/plane into chore/confl…
NarayanBavisetti Nov 18, 2024
f409ee1
fix: merge conflicts resolved from preview
aaryan610 Nov 28, 2024
0b64cae
fix: merge conflicts resolved from preview
aaryan610 Nov 28, 2024
60b58ef
fix: live server post request
aaryan610 Dec 2, 2024
17f4806
fix: build errors
aaryan610 Dec 2, 2024
9e5b104
chore: remove yjs from the live server
aaryan610 Dec 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion apiserver/plane/app/serializers/draft.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,11 @@ class Meta:

class DraftIssueDetailSerializer(DraftIssueSerializer):
description_html = serializers.CharField()
description_binary = serializers.CharField()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider using Base64BinaryField for consistency and validation

The description_binary field is implemented as a plain CharField which could lead to:

  1. Inconsistency with other serializers that use Base64BinaryField
  2. Missing validation for base64-encoded data
  3. Potential security risks from unvalidated binary content

Consider using the same field type as other serializers:

-    description_binary = serializers.CharField()
+    description_binary = Base64BinaryField()

Also applies to: 271-274


class Meta(DraftIssueSerializer.Meta):
fields = DraftIssueSerializer.Meta.fields + ["description_html"]
fields = DraftIssueSerializer.Meta.fields + [
"description_html",
"description_binary",
]
read_only_fields = fields
25 changes: 24 additions & 1 deletion apiserver/plane/app/serializers/issue.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# Python imports
import base64

# Django imports
from django.utils import timezone
from django.core.validators import URLValidator
Expand Down Expand Up @@ -625,12 +628,32 @@ class Meta:
read_only_fields = fields


class Base64BinaryField(serializers.CharField):
def to_representation(self, value):
# Encode the binary data to base64 string for JSON response
if value:
return base64.b64encode(value).decode("utf-8")
return None

def to_internal_value(self, data):
# Decode the base64 string to binary data when saving
try:
return base64.b64decode(data)
except (TypeError, ValueError):
raise serializers.ValidationError("Invalid base64-encoded data")

Comment on lines +631 to +644
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add validation for binary data size and content.

The Base64BinaryField implementation needs additional safeguards:

  1. Maximum size validation to prevent memory issues
  2. Content type validation to ensure security
  3. Better error handling to distinguish between different error cases

Consider applying this enhancement:

 class Base64BinaryField(serializers.CharField):
+    def __init__(self, *args, **kwargs):
+        self.max_size = kwargs.pop('max_size', 10 * 1024 * 1024)  # 10MB default
+        super().__init__(*args, **kwargs)
+
     def to_representation(self, value):
         # Encode the binary data to base64 string for JSON response
         if value:
             return base64.b64encode(value).decode("utf-8")
         return None

     def to_internal_value(self, data):
         # Decode the base64 string to binary data when saving
         try:
+            if not data:
+                return None
             decoded = base64.b64decode(data)
+            if len(decoded) > self.max_size:
+                raise serializers.ValidationError(
+                    f"Binary data exceeds maximum size of {self.max_size} bytes"
+                )
             return decoded
         except TypeError:
-            raise serializers.ValidationError("Invalid base64-encoded data")
+            raise serializers.ValidationError("Data is not a valid string")
+        except ValueError:
+            raise serializers.ValidationError("Invalid base64 encoding")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
class Base64BinaryField(serializers.CharField):
def to_representation(self, value):
# Encode the binary data to base64 string for JSON response
if value:
return base64.b64encode(value).decode("utf-8")
return None
def to_internal_value(self, data):
# Decode the base64 string to binary data when saving
try:
return base64.b64decode(data)
except (TypeError, ValueError):
raise serializers.ValidationError("Invalid base64-encoded data")
class Base64BinaryField(serializers.CharField):
def __init__(self, *args, **kwargs):
self.max_size = kwargs.pop('max_size', 10 * 1024 * 1024) # 10MB default
super().__init__(*args, **kwargs)
def to_representation(self, value):
# Encode the binary data to base64 string for JSON response
if value:
return base64.b64encode(value).decode("utf-8")
return None
def to_internal_value(self, data):
# Decode the base64 string to binary data when saving
try:
if not data:
return None
decoded = base64.b64decode(data)
if len(decoded) > self.max_size:
raise serializers.ValidationError(
f"Binary data exceeds maximum size of {self.max_size} bytes"
)
return decoded
except TypeError:
raise serializers.ValidationError("Data is not a valid string")
except ValueError:
raise serializers.ValidationError("Invalid base64 encoding")


class IssueDetailSerializer(IssueSerializer):
description_html = serializers.CharField()
description_binary = Base64BinaryField()
is_subscribed = serializers.BooleanField(read_only=True)

class Meta(IssueSerializer.Meta):
fields = IssueSerializer.Meta.fields + ["description_html", "is_subscribed"]
fields = IssueSerializer.Meta.fields + [
"description_html",
"is_subscribed",
"description_binary",
]
read_only_fields = fields


Expand Down
10 changes: 10 additions & 0 deletions apiserver/plane/app/urls/intake.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,14 @@
),
name="inbox-issue",
),
path(
"workspaces/<str:slug>/projects/<uuid:project_id>/inbox-issues/<uuid:pk>/description/",
IntakeIssueViewSet.as_view(
{
"get": "retrieve_description",
"post": "update_description",
}
),
name="inbox-issue-description",
),
]
19 changes: 19 additions & 0 deletions apiserver/plane/app/urls/issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@
),
name="project-issue",
),
path(
"workspaces/<str:slug>/projects/<uuid:project_id>/issues/<uuid:pk>/description/",
IssueViewSet.as_view(
{
"get": "retrieve_description",
"post": "update_description",
}
),
name="project-issue-description",
),
path(
"workspaces/<str:slug>/projects/<uuid:project_id>/issue-labels/",
LabelViewSet.as_view({"get": "list", "post": "create"}),
Expand Down Expand Up @@ -233,6 +243,15 @@
),
name="project-issue-archive-unarchive",
),
path(
"workspaces/<str:slug>/projects/<uuid:project_id>/archived-issues/<uuid:pk>/description/",
IssueArchiveViewSet.as_view(
{
"get": "retrieve_description",
}
),
name="archive-issue-description",
),
## End Issue Archives
## Issue Relation
path(
Expand Down
10 changes: 10 additions & 0 deletions apiserver/plane/app/urls/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@
),
name="workspace-drafts-issues",
),
path(
"workspaces/<str:slug>/draft-issues/<uuid:pk>/description/",
WorkspaceDraftIssueViewSet.as_view(
{
"get": "retrieve_description",
"post": "update_description",
}
),
name="workspace-drafts-issues",
),
path(
"workspaces/<str:slug>/draft-to-issue/<uuid:draft_id>/",
WorkspaceDraftIssueViewSet.as_view({"post": "create_draft_to_issue"}),
Expand Down
84 changes: 84 additions & 0 deletions apiserver/plane/app/views/intake/base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Python imports
import json
import requests
import base64

# Django import
from django.utils import timezone
Expand All @@ -9,6 +11,9 @@
from django.contrib.postgres.fields import ArrayField
from django.db.models import Value, UUIDField
from django.db.models.functions import Coalesce
from django.http import StreamingHttpResponse
from django.conf import settings


# Third party imports
from rest_framework import status
Expand Down Expand Up @@ -579,3 +584,82 @@ def destroy(self, request, slug, project_id, pk):

intake_issue.delete()
return Response(status=status.HTTP_204_NO_CONTENT)

@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
def retrieve_description(self, request, slug, project_id, pk):
issue = Issue.objects.filter(
pk=pk, workspace__slug=slug, project_id=project_id
).first()
if issue is None:
return Response(
{"error": "Issue not found"},
status=404,
)
binary_data = issue.description_binary

def stream_data():
if binary_data:
yield binary_data
else:
yield b""

response = StreamingHttpResponse(
stream_data(), content_type="application/octet-stream"
)
response["Content-Disposition"] = (
'attachment; filename="issue_description.bin"'
)
return response

@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
def update_description(self, request, slug, project_id, pk):
issue = Issue.objects.get(
workspace__slug=slug, project_id=project_id, pk=pk
)
base64_description = issue.description_binary
# convert to base64 string
if base64_description:
base64_description = base64.b64encode(base64_description).decode(
"utf-8"
)
data = {
"original_document": base64_description,
"updates": request.data.get("description_binary"),
}
base_url = f"{settings.LIVE_BASE_URL}/resolve-document-conflicts/"
try:
response = requests.post(base_url, json=data, headers=None)
except requests.RequestException:
return Response(
{"error": "Failed to connect to the external service"},
status=status.HTTP_502_BAD_GATEWAY,
)

if response.status_code == 200:
issue.description = response.json().get(
"description", issue.description
)
issue.description_html = response.json().get("description_html")
response_description_binary = response.json().get(
"description_binary"
)
issue.description_binary = base64.b64decode(
response_description_binary
)
issue.save()

def stream_data():
if issue.description_binary:
yield issue.description_binary
else:
yield b""

response = StreamingHttpResponse(
stream_data(), content_type="application/octet-stream"
)
response["Content-Disposition"] = (
'attachment; filename="issue_description.bin"'
)
return response

return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)
28 changes: 28 additions & 0 deletions apiserver/plane/app/views/issue/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from django.utils import timezone
from django.utils.decorators import method_decorator
from django.views.decorators.gzip import gzip_page
from django.http import StreamingHttpResponse


# Third Party imports
from rest_framework import status
Expand Down Expand Up @@ -294,6 +296,32 @@ def unarchive(self, request, slug, project_id, pk=None):

return Response(status=status.HTTP_204_NO_CONTENT)

@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
def retrieve_description(self, request, slug, project_id, pk):
issue = Issue.objects.filter(
pk=pk, workspace__slug=slug, project_id=project_id
).first()
if issue is None:
return Response(
{"error": "Issue not found"},
status=404,
)
binary_data = issue.description_binary

def stream_data():
if binary_data:
yield binary_data
else:
yield b""

response = StreamingHttpResponse(
stream_data(), content_type="application/octet-stream"
)
response["Content-Disposition"] = (
'attachment; filename="issue_description.bin"'
)
return response


class BulkArchiveIssuesEndpoint(BaseAPIView):
permission_classes = [ProjectEntityPermission]
Expand Down
82 changes: 82 additions & 0 deletions apiserver/plane/app/views/issue/base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Python imports
import json
import requests
import base64

# Django imports
from django.contrib.postgres.aggregates import ArrayAgg
Expand All @@ -20,8 +22,10 @@
)
from django.db.models.functions import Coalesce
from django.utils import timezone
from django.http import StreamingHttpResponse
from django.utils.decorators import method_decorator
from django.views.decorators.gzip import gzip_page
from django.conf import settings

# Third Party imports
from rest_framework import status
Expand Down Expand Up @@ -674,6 +678,84 @@ def destroy(self, request, slug, project_id, pk=None):
)
return Response(status=status.HTTP_204_NO_CONTENT)

@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
def retrieve_description(self, request, slug, project_id, pk):
issue = Issue.issue_objects.filter(
pk=pk, workspace__slug=slug, project_id=project_id
).first()
if issue is None:
return Response(
{"error": "Issue not found"},
status=404,
)
binary_data = issue.description_binary

def stream_data():
if binary_data:
yield binary_data
else:
yield b""

response = StreamingHttpResponse(
stream_data(), content_type="application/octet-stream"
)
response["Content-Disposition"] = (
'attachment; filename="issue_description.bin"'
)
return response

def update_description(self, request, slug, project_id, pk):
issue = Issue.issue_objects.get(
workspace__slug=slug, project_id=project_id, pk=pk
)
base64_description = issue.description_binary
# convert to base64 string
if base64_description:
base64_description = base64.b64encode(base64_description).decode(
"utf-8"
)
data = {
"original_document": base64_description,
"updates": request.data.get("description_binary"),
}
base_url = f"{settings.LIVE_BASE_URL}/resolve-document-conflicts/"
try:
response = requests.post(base_url, json=data, headers=None)
except requests.RequestException:
return Response(
{"error": "Failed to connect to the external service"},
status=status.HTTP_502_BAD_GATEWAY,
)

if response.status_code == 200:
issue.description = response.json().get(
"description", issue.description
)
issue.description_html = response.json().get("description_html")
response_description_binary = response.json().get(
"description_binary"
)
issue.description_binary = base64.b64decode(
response_description_binary
)
issue.save()

def stream_data():
if issue.description_binary:
yield issue.description_binary
else:
yield b""

response = StreamingHttpResponse(
stream_data(), content_type="application/octet-stream"
)
response["Content-Disposition"] = (
'attachment; filename="issue_description.bin"'
)
return response

return Response(status=status.HTTP_500_INTERNAL_SERVER_ERROR)


class IssueUserDisplayPropertyEndpoint(BaseAPIView):
@allow_permission([ROLE.ADMIN, ROLE.MEMBER, ROLE.GUEST])
Expand Down
Loading
Loading