-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[WEB-1435] dev: conflict free issue descriptions #5912
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
Changes from all commits
e781ab9
cccc7b7
d3b443e
a7d8bee
745714a
baf1517
54ecea1
fcd06fc
547fc86
7448c5b
6275e2f
b3b1088
ccad0e7
389ee74
6667327
60d091e
6b437ee
a95143c
ef76bd0
e3ac9ef
02e3c48
e6787d8
ea4d201
4d7d4b6
1fcc3ab
3792699
11b0c82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 | ||||||||||||||||||
|
|
@@ -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 | ||||||||||||||||||
|
|
@@ -40,7 +45,6 @@ | |||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class IntakeViewSet(BaseViewSet): | ||||||||||||||||||
|
|
||||||||||||||||||
| serializer_class = IntakeSerializer | ||||||||||||||||||
| model = Intake | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -89,7 +93,6 @@ def destroy(self, request, slug, project_id, pk): | |||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class IntakeIssueViewSet(BaseViewSet): | ||||||||||||||||||
|
|
||||||||||||||||||
| serializer_class = IntakeIssueSerializer | ||||||||||||||||||
| model = IntakeIssue | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
@@ -640,3 +643,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 | ||||||||||||||||||
|
|
||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
| @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 | ||||||||||||||||||
| ) | ||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
| 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: | ||||||||||||||||||
|
Comment on lines
+688
to
+691
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout to prevent hanging connections The Apply this diff: - response = requests.post(base_url, json=data, headers=None)
+ response = requests.post(base_url, json=data, headers=None, timeout=10)📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: Codacy Static Code Analysis[warning] 690-690: apiserver/plane/app/views/intake/base.py#L690 [warning] 690-690: apiserver/plane/app/views/intake/base.py#L690 |
||||||||||||||||||
| 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) | ||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -725,6 +729,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() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+734
to
+736
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Use Replace the filter().first() pattern with get_object_or_404 for cleaner error handling. Apply this diff: - 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,
- )
+ issue = get_object_or_404(
+ Issue.issue_objects,
+ pk=pk,
+ workspace__slug=slug,
+ project_id=project_id
+ )📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def update_description(self, request, slug, project_id, pk): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue = Issue.issue_objects.get( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workspace__slug=slug, project_id=project_id, pk=pk | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base64_description = issue.description_binary | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # convert to base64 string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if base64_description: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base64_description = base64.b64encode(base64_description).decode( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "utf-8" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| data = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "original_document": base64_description, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "updates": request.data.get("description_binary"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+772
to
+779
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add timeout and improve error handling for external service call. The request to the external service needs a timeout and better error handling. Apply this diff: base_url = f"{settings.LIVE_BASE_URL}/resolve-document-conflicts/"
try:
- response = requests.post(base_url, json=data, headers=None)
+ response = requests.post(
+ base_url,
+ json=data,
+ headers=None,
+ timeout=5
+ )
except requests.RequestException:
return Response(
{"error": "Failed to connect to the external service"},
status=status.HTTP_502_BAD_GATEWAY,
)
+ except requests.Timeout:
+ return Response(
+ {"error": "Request to external service timed out"},
+ status=status.HTTP_504_GATEWAY_TIMEOUT,
+ )📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: Codacy Static Code Analysis[warning] 774-774: apiserver/plane/app/views/issue/base.py#L774 [warning] 774-774: apiserver/plane/app/views/issue/base.py#L774 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue.save() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def stream_data(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if issue.description_binary: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| yield issue.description_binary | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| yield b"" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
NarayanBavisetti marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.