From 314d4494e58163d18c8047c6a092e213687761cf Mon Sep 17 00:00:00 2001 From: pablohashescobar Date: Thu, 1 Feb 2024 20:37:09 +0530 Subject: [PATCH 1/3] dev: error response for duplicate items created through external apis --- apiserver/plane/api/views/cycle.py | 37 +++++++++++++++++++++++++++ apiserver/plane/api/views/issue.py | 39 +++++++++++++++++++++++++++++ apiserver/plane/api/views/module.py | 37 +++++++++++++++++++++++++++ 3 files changed, 113 insertions(+) diff --git a/apiserver/plane/api/views/cycle.py b/apiserver/plane/api/views/cycle.py index c296bb11180..d70693648ff 100644 --- a/apiserver/plane/api/views/cycle.py +++ b/apiserver/plane/api/views/cycle.py @@ -243,6 +243,22 @@ def post(self, request, slug, project_id): ): serializer = CycleSerializer(data=request.data) if serializer.is_valid(): + if ( + request.data.get("external_id") + and request.data.get("external_source") + and Cycle.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Cycle with the same external id and external source already exists" + }, + status=status.HTTP_410_GONE, + ) serializer.save( project_id=project_id, owned_by=request.user, @@ -289,6 +305,27 @@ def patch(self, request, slug, project_id, pk): serializer = CycleSerializer(cycle, data=request.data, partial=True) if serializer.is_valid(): + if ( + request.data.get("external_id") + and (cycle.external_id != request.data.get("external_id")) + and request.data.get("external_source") + and ( + cycle.external_source + != request.data.get("external_source") + ) + and Cycle.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Cycle with the same external id and external source already exists" + }, + status=status.HTTP_410_GONE, + ) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/apiserver/plane/api/views/issue.py b/apiserver/plane/api/views/issue.py index e91f2a5f66f..a76a5357e52 100644 --- a/apiserver/plane/api/views/issue.py +++ b/apiserver/plane/api/views/issue.py @@ -220,6 +220,23 @@ def post(self, request, slug, project_id): ) if serializer.is_valid(): + if ( + request.data.get("external_id") + and request.data.get("external_source") + and Issue.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Issue with the same external id and external source already exists" + }, + status=status.HTTP_410_GONE, + ) + serializer.save() # Track the issue @@ -256,6 +273,28 @@ def patch(self, request, slug, project_id, pk=None): partial=True, ) if serializer.is_valid(): + if ( + str(request.data.get("external_id")) + and (issue.external_id != str(request.data.get("external_id"))) + and request.data.get("external_source") + and ( + issue.external_source + != request.data.get("external_source") + ) + and Issue.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Issue with the same external id and external source already exists" + }, + status=status.HTTP_410_GONE, + ) + serializer.save() issue_activity.delay( type="issue.activity.updated", diff --git a/apiserver/plane/api/views/module.py b/apiserver/plane/api/views/module.py index 1a9a21a3c19..c1cac5b5379 100644 --- a/apiserver/plane/api/views/module.py +++ b/apiserver/plane/api/views/module.py @@ -132,6 +132,22 @@ def post(self, request, slug, project_id): }, ) if serializer.is_valid(): + if ( + request.data.get("external_id") + and request.data.get("external_source") + and Module.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Module with the same external id and external source already exists" + }, + status=status.HTTP_410_GONE, + ) serializer.save() module = Module.objects.get(pk=serializer.data["id"]) serializer = ModuleSerializer(module) @@ -149,6 +165,27 @@ def patch(self, request, slug, project_id, pk): partial=True, ) if serializer.is_valid(): + if ( + request.data.get("external_id") + and (module.external_id != request.data.get("external_id")) + and request.data.get("external_source") + and ( + module.external_source + != request.data.get("external_source") + ) + and Module.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Module with the same external id and external source already exists" + }, + status=status.HTTP_410_GONE, + ) serializer.save() return Response(serializer.data, status=status.HTTP_201_CREATED) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) From 52c8195181dbbfa5665029f53f7841f3d60e63cd Mon Sep 17 00:00:00 2001 From: pablohashescobar Date: Thu, 1 Feb 2024 21:04:34 +0530 Subject: [PATCH 2/3] dev: return identifier and also add the validation for state --- apiserver/plane/api/views/cycle.py | 12 ++++++-- apiserver/plane/api/views/issue.py | 12 ++++++-- apiserver/plane/api/views/module.py | 12 ++++++-- apiserver/plane/api/views/state.py | 46 +++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/apiserver/plane/api/views/cycle.py b/apiserver/plane/api/views/cycle.py index d70693648ff..41280c19e91 100644 --- a/apiserver/plane/api/views/cycle.py +++ b/apiserver/plane/api/views/cycle.py @@ -253,9 +253,16 @@ def post(self, request, slug, project_id): external_id=request.data.get("external_id"), ).exists() ): + cycle = Cycle.objects.filter( + workspace__slug=slug, + project_id=project_id, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).first() return Response( { - "error": "Cycle with the same external id and external source already exists" + "error": "Cycle with the same external id and external source already exists", + "cycle": str(cycle.id), }, status=status.HTTP_410_GONE, ) @@ -322,7 +329,8 @@ def patch(self, request, slug, project_id, pk): ): return Response( { - "error": "Cycle with the same external id and external source already exists" + "error": "Cycle with the same external id and external source already exists", + "cycle_id": str(cycle.id), }, status=status.HTTP_410_GONE, ) diff --git a/apiserver/plane/api/views/issue.py b/apiserver/plane/api/views/issue.py index a76a5357e52..b475e0caf8d 100644 --- a/apiserver/plane/api/views/issue.py +++ b/apiserver/plane/api/views/issue.py @@ -230,9 +230,16 @@ def post(self, request, slug, project_id): external_id=request.data.get("external_id"), ).exists() ): + issue = Issue.objects.filter( + workspace__slug=slug, + project_id=project_id, + external_id=request.data.get("external_id"), + external_source=request.data.get("external_source"), + ).first() return Response( { - "error": "Issue with the same external id and external source already exists" + "error": "Issue with the same external id and external source already exists", + "issue_id": str(issue.id), }, status=status.HTTP_410_GONE, ) @@ -290,7 +297,8 @@ def patch(self, request, slug, project_id, pk=None): ): return Response( { - "error": "Issue with the same external id and external source already exists" + "error": "Issue with the same external id and external source already exists", + "issue_id": str(issue.id) }, status=status.HTTP_410_GONE, ) diff --git a/apiserver/plane/api/views/module.py b/apiserver/plane/api/views/module.py index c1cac5b5379..83e888d3c69 100644 --- a/apiserver/plane/api/views/module.py +++ b/apiserver/plane/api/views/module.py @@ -142,9 +142,16 @@ def post(self, request, slug, project_id): external_id=request.data.get("external_id"), ).exists() ): + module = Module.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).first() return Response( { - "error": "Module with the same external id and external source already exists" + "error": "Module with the same external id and external source already exists", + "module_id": str(module.id), }, status=status.HTTP_410_GONE, ) @@ -182,7 +189,8 @@ def patch(self, request, slug, project_id, pk): ): return Response( { - "error": "Module with the same external id and external source already exists" + "error": "Module with the same external id and external source already exists", + "module_id": str(module.id), }, status=status.HTTP_410_GONE, ) diff --git a/apiserver/plane/api/views/state.py b/apiserver/plane/api/views/state.py index f931c2ed264..4ac91903fbc 100644 --- a/apiserver/plane/api/views/state.py +++ b/apiserver/plane/api/views/state.py @@ -38,6 +38,30 @@ def post(self, request, slug, project_id): data=request.data, context={"project_id": project_id} ) if serializer.is_valid(): + if ( + request.data.get("external_id") + and request.data.get("external_source") + and State.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + state = State.objects.filter( + workspace__slug=slug, + project_id=project_id, + external_id=request.data.get("external_id"), + external_source=request.data.get("external_source"), + ).first() + return Response( + { + "error": "State with the same external id and external source already exists", + "state_id": str(state.id), + }, + status=status.HTTP_410_GONE, + ) + serializer.save(project_id=project_id) return Response(serializer.data, status=status.HTTP_200_OK) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -91,6 +115,28 @@ def patch(self, request, slug, project_id, state_id=None): ) serializer = StateSerializer(state, data=request.data, partial=True) if serializer.is_valid(): + if ( + str(request.data.get("external_id")) + and (state.external_id != str(request.data.get("external_id"))) + and request.data.get("external_source") + and ( + state.external_source + != request.data.get("external_source") + ) + and Issue.objects.filter( + project_id=project_id, + workspace__slug=slug, + external_source=request.data.get("external_source"), + external_id=request.data.get("external_id"), + ).exists() + ): + return Response( + { + "error": "Issue with the same external id and external source already exists", + "state_id": str(state.id), + }, + status=status.HTTP_410_GONE, + ) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) From a2eaed2aed9fac0ca8740f4fc63ac330927c66e1 Mon Sep 17 00:00:00 2001 From: pablohashescobar Date: Fri, 2 Feb 2024 15:55:25 +0530 Subject: [PATCH 3/3] fix: validation for external id and external source --- apiserver/plane/api/views/cycle.py | 11 +++-------- apiserver/plane/api/views/issue.py | 15 ++++++--------- apiserver/plane/api/views/module.py | 13 ++++--------- apiserver/plane/api/views/state.py | 15 +++++---------- 4 files changed, 18 insertions(+), 36 deletions(-) diff --git a/apiserver/plane/api/views/cycle.py b/apiserver/plane/api/views/cycle.py index 41280c19e91..7a9cfb1b5cb 100644 --- a/apiserver/plane/api/views/cycle.py +++ b/apiserver/plane/api/views/cycle.py @@ -264,7 +264,7 @@ def post(self, request, slug, project_id): "error": "Cycle with the same external id and external source already exists", "cycle": str(cycle.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save( project_id=project_id, @@ -315,15 +315,10 @@ def patch(self, request, slug, project_id, pk): if ( request.data.get("external_id") and (cycle.external_id != request.data.get("external_id")) - and request.data.get("external_source") - and ( - cycle.external_source - != request.data.get("external_source") - ) and Cycle.objects.filter( project_id=project_id, workspace__slug=slug, - external_source=request.data.get("external_source"), + external_source=request.data.get("external_source", cycle.external_source), external_id=request.data.get("external_id"), ).exists() ): @@ -332,7 +327,7 @@ def patch(self, request, slug, project_id, pk): "error": "Cycle with the same external id and external source already exists", "cycle_id": str(cycle.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK) diff --git a/apiserver/plane/api/views/issue.py b/apiserver/plane/api/views/issue.py index b475e0caf8d..530eef5bf42 100644 --- a/apiserver/plane/api/views/issue.py +++ b/apiserver/plane/api/views/issue.py @@ -241,7 +241,7 @@ def post(self, request, slug, project_id): "error": "Issue with the same external id and external source already exists", "issue_id": str(issue.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save() @@ -283,24 +283,19 @@ def patch(self, request, slug, project_id, pk=None): if ( str(request.data.get("external_id")) and (issue.external_id != str(request.data.get("external_id"))) - and request.data.get("external_source") - and ( - issue.external_source - != request.data.get("external_source") - ) and Issue.objects.filter( project_id=project_id, workspace__slug=slug, - external_source=request.data.get("external_source"), + external_source=request.data.get("external_source", issue.external_source), external_id=request.data.get("external_id"), ).exists() ): return Response( { "error": "Issue with the same external id and external source already exists", - "issue_id": str(issue.id) + "issue_id": str(issue.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save() @@ -310,6 +305,8 @@ def patch(self, request, slug, project_id, pk=None): actor_id=str(request.user.id), issue_id=str(pk), project_id=str(project_id), + external_id__isnull=False, + external_source__isnull=False, current_instance=current_instance, epoch=int(timezone.now().timestamp()), ) diff --git a/apiserver/plane/api/views/module.py b/apiserver/plane/api/views/module.py index 83e888d3c69..e2d59e126f8 100644 --- a/apiserver/plane/api/views/module.py +++ b/apiserver/plane/api/views/module.py @@ -153,7 +153,7 @@ def post(self, request, slug, project_id): "error": "Module with the same external id and external source already exists", "module_id": str(module.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save() module = Module.objects.get(pk=serializer.data["id"]) @@ -175,15 +175,10 @@ def patch(self, request, slug, project_id, pk): if ( request.data.get("external_id") and (module.external_id != request.data.get("external_id")) - and request.data.get("external_source") - and ( - module.external_source - != request.data.get("external_source") - ) and Module.objects.filter( project_id=project_id, workspace__slug=slug, - external_source=request.data.get("external_source"), + external_source=request.data.get("external_source", module.external_source), external_id=request.data.get("external_id"), ).exists() ): @@ -192,10 +187,10 @@ def patch(self, request, slug, project_id, pk): "error": "Module with the same external id and external source already exists", "module_id": str(module.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save() - return Response(serializer.data, status=status.HTTP_201_CREATED) + return Response(serializer.data, status=status.HTTP_200_OK) return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) def get(self, request, slug, project_id, pk=None): diff --git a/apiserver/plane/api/views/state.py b/apiserver/plane/api/views/state.py index 4ac91903fbc..88fb083f05e 100644 --- a/apiserver/plane/api/views/state.py +++ b/apiserver/plane/api/views/state.py @@ -59,7 +59,7 @@ def post(self, request, slug, project_id): "error": "State with the same external id and external source already exists", "state_id": str(state.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save(project_id=project_id) @@ -118,24 +118,19 @@ def patch(self, request, slug, project_id, state_id=None): if ( str(request.data.get("external_id")) and (state.external_id != str(request.data.get("external_id"))) - and request.data.get("external_source") - and ( - state.external_source - != request.data.get("external_source") - ) - and Issue.objects.filter( + and State.objects.filter( project_id=project_id, workspace__slug=slug, - external_source=request.data.get("external_source"), + external_source=request.data.get("external_source", state.external_source), external_id=request.data.get("external_id"), ).exists() ): return Response( { - "error": "Issue with the same external id and external source already exists", + "error": "State with the same external id and external source already exists", "state_id": str(state.id), }, - status=status.HTTP_410_GONE, + status=status.HTTP_409_CONFLICT, ) serializer.save() return Response(serializer.data, status=status.HTTP_200_OK)