From 4acaba96ba23c8c32c5705b2f6ae38707bcdcbf6 Mon Sep 17 00:00:00 2001 From: guru_sainath Date: Mon, 9 Sep 2024 20:22:59 +0530 Subject: [PATCH] [MOBIL-167] chore: handle status code in error messages, updated key in issue global search, and added project identifier in IssueType (#1075) * dev: updated project details * chore: udated version strawberry-graphql-django * chore: handled error messages and status code in error extensions * chore: resolved import error * chore: added project_identifier in issue_type * chore: project_identifier in issues global search * chore: handled error_extensions --- .../plane/graphql/permissions/project.py | 22 +++++++++++++-- .../plane/graphql/permissions/workspace.py | 21 +++++++++++++- apiserver/plane/graphql/queries/project.py | 4 +-- apiserver/plane/graphql/queries/search.py | 9 ++++-- apiserver/plane/graphql/types/issue.py | 7 ++++- apiserver/plane/graphql/types/workspace.py | 1 - apiserver/plane/graphql/utils/error_codes.py | 13 +++++++++ .../plane/graphql/utils/error_handler.py | 12 ++++++++ apiserver/plane/graphql/views.py | 28 ++++++++++++++++++- apiserver/requirements/base.txt | 2 +- 10 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 apiserver/plane/graphql/utils/error_codes.py create mode 100644 apiserver/plane/graphql/utils/error_handler.py diff --git a/apiserver/plane/graphql/permissions/project.py b/apiserver/plane/graphql/permissions/project.py index ac5ba0be55..48d7a0ad87 100644 --- a/apiserver/plane/graphql/permissions/project.py +++ b/apiserver/plane/graphql/permissions/project.py @@ -12,6 +12,7 @@ from strawberry.types import Info from strawberry.permission import BasePermission # Local Imports +from plane.graphql.utils.error_codes import ERROR_CODES from plane.db.models import ProjectMember # Permission Mappings @@ -23,6 +24,10 @@ Guest = 5 class IsAuthenticated(BasePermission): message = "User is not authenticated" + error_extensions = { + "code": "UNAUTHENTICATED", + "statusCode": ERROR_CODES["USER_NOT_AUTHENTICATED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: self.user = await sync_to_async(get_user, thread_sensitive=True)( @@ -33,11 +38,16 @@ class IsAuthenticated(BasePermission): class ProjectBasePermission(IsAuthenticated): message = "User does not have permission to access this project" + error_extensions = { + "code": "UNAUTHORIZED", + "statusCode": ERROR_CODES["USER_NOT_AUTHORIZED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: # First, check if the user is authenticated by calling the parent class's method if not await super().has_permission(source, info, **kwargs): self.message = IsAuthenticated.message + self.error_extensions = IsAuthenticated.error_extensions return False return await sync_to_async( @@ -52,13 +62,17 @@ class ProjectBasePermission(IsAuthenticated): class ProjectMemberPermission(IsAuthenticated): - message = "Only project admins or members can perform this action" + error_extensions = { + "code": "UNAUTHORIZED", + "statusCode": ERROR_CODES["USER_NOT_AUTHORIZED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: # First, check if the user is authenticated by calling the parent class's method if not await super().has_permission(source, info, **kwargs): self.message = IsAuthenticated.message + self.error_extensions = IsAuthenticated.error_extensions return False return await sync_to_async( @@ -74,13 +88,17 @@ class ProjectMemberPermission(IsAuthenticated): class ProjectAdminPermission(IsAuthenticated): - message = "Only admins can perform this action" + error_extensions = { + "code": "UNAUTHORIZED", + "statusCode": ERROR_CODES["USER_NOT_AUTHORIZED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: # First, check if the user is authenticated by calling the parent class's method if not await super().has_permission(source, info, **kwargs): self.message = IsAuthenticated.message + self.error_extensions = IsAuthenticated.error_extensions return False return await sync_to_async( diff --git a/apiserver/plane/graphql/permissions/workspace.py b/apiserver/plane/graphql/permissions/workspace.py index a751562666..14441b4c97 100644 --- a/apiserver/plane/graphql/permissions/workspace.py +++ b/apiserver/plane/graphql/permissions/workspace.py @@ -12,6 +12,7 @@ from strawberry.types import Info from strawberry.permission import BasePermission # Local Imports +from plane.graphql.utils.error_codes import ERROR_CODES from plane.db.models import WorkspaceMember # Permission Mappings @@ -23,6 +24,10 @@ Guest = 5 class IsAuthenticated(BasePermission): message = "User is not authenticated" + error_extensions = { + "code": "UNAUTHENTICATED", + "statusCode": ERROR_CODES["USER_NOT_AUTHENTICATED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: self.user = await sync_to_async(get_user, thread_sensitive=True)( @@ -33,11 +38,16 @@ class IsAuthenticated(BasePermission): class WorkspaceBasePermission(IsAuthenticated): message = "User does not have permission to access this workspace" + error_extensions = { + "code": "UNAUTHORIZED", + "statusCode": ERROR_CODES["USER_NOT_AUTHORIZED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: # First, check if the user is authenticated by calling the parent class's method if not await super().has_permission(source, info, **kwargs): self.message = IsAuthenticated.message + self.error_extensions = IsAuthenticated.error_extensions return False return await sync_to_async( @@ -51,13 +61,17 @@ class WorkspaceBasePermission(IsAuthenticated): class WorkspaceMemberPermission(IsAuthenticated): - message = "Workspace admins or members can perform this action" + error_extensions = { + "code": "UNAUTHORIZED", + "statusCode": ERROR_CODES["USER_NOT_AUTHORIZED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: # First, check if the user is authenticated by calling the parent class's method if not await super().has_permission(source, info, **kwargs): self.message = IsAuthenticated.message + self.error_extensions = IsAuthenticated.error_extensions return False return await sync_to_async( @@ -73,11 +87,16 @@ class WorkspaceMemberPermission(IsAuthenticated): class WorkspaceAdminPermission(IsAuthenticated): message = "Only workspace admins can perform this action" + error_extensions = { + "code": "UNAUTHORIZED", + "statusCode": ERROR_CODES["USER_NOT_AUTHORIZED"], + } async def has_permission(self, source: Any, info: Info, **kwargs) -> bool: # First, check if the user is authenticated by calling the parent class's method if not await super().has_permission(source, info, **kwargs): self.message = IsAuthenticated.message + self.error_extensions = IsAuthenticated.error_extensions return False return await sync_to_async( diff --git a/apiserver/plane/graphql/queries/project.py b/apiserver/plane/graphql/queries/project.py index e5f18bc32b..d53660e177 100644 --- a/apiserver/plane/graphql/queries/project.py +++ b/apiserver/plane/graphql/queries/project.py @@ -173,7 +173,7 @@ class ProjectQuery: .first() ) - project = await sync_to_async(get_project)() + project_detail = await sync_to_async(get_project)() # Background task to update recent visited project user_id = info.context.user.id @@ -185,7 +185,7 @@ class ProjectQuery: entity_identifier=project, ) - return project + return project_detail @strawberry.type diff --git a/apiserver/plane/graphql/queries/search.py b/apiserver/plane/graphql/queries/search.py index 381ad9c8b0..0e76fabd05 100644 --- a/apiserver/plane/graphql/queries/search.py +++ b/apiserver/plane/graphql/queries/search.py @@ -100,14 +100,19 @@ async def filter_issues( ) .distinct() .values( - "name", "id", "sequence_id", - "project__identifier", + "name", "project", + "project__identifier", ) ) )() + + for issue in issues: + issue["project_identifier"] = issue["project__identifier"] + del issue["project__identifier"] + return [IssueLiteType(**issue) for issue in issues] diff --git a/apiserver/plane/graphql/types/issue.py b/apiserver/plane/graphql/types/issue.py index 4b2b022b51..8fbe7ef216 100644 --- a/apiserver/plane/graphql/types/issue.py +++ b/apiserver/plane/graphql/types/issue.py @@ -68,6 +68,7 @@ class IssuesType: cycle: Optional[strawberry.ID] modules: Optional[list[strawberry.ID]] type: Optional[strawberry.ID] + project_identifier: Optional[str] @strawberry.field def state(self) -> int: @@ -130,6 +131,10 @@ class IssuesType: # Return the module IDs as strings return [str(module_id) for module_id in module_issues] + @strawberry.field + def project_identifier(self) -> Optional[str]: + return self.project.identifier + @strawberry_django.type(IssueUserProperty) class IssueUserPropertyType: @@ -236,7 +241,7 @@ class IssueLiteType: sequence_id: int workspace: strawberry.ID project: strawberry.ID - project__identifier: str + project_identifier: Optional[str] @strawberry.field def workspace(self) -> int: diff --git a/apiserver/plane/graphql/types/workspace.py b/apiserver/plane/graphql/types/workspace.py index e9cab264db..c3796e6013 100644 --- a/apiserver/plane/graphql/types/workspace.py +++ b/apiserver/plane/graphql/types/workspace.py @@ -1,5 +1,4 @@ # Third party imports -from asgiref.sync import sync_to_async from typing import Optional # Strawberry imports diff --git a/apiserver/plane/graphql/utils/error_codes.py b/apiserver/plane/graphql/utils/error_codes.py new file mode 100644 index 0000000000..4898d5cfb5 --- /dev/null +++ b/apiserver/plane/graphql/utils/error_codes.py @@ -0,0 +1,13 @@ +ERROR_CODES = { + # authentication error codes + "USER_NOT_AUTHENTICATED": 401, + "USER_NOT_AUTHORIZED": 403, + # issues + "INVALID_ARCHIVE_STATE_GROUP": 4091, + "INVALID_ISSUE_DATES": 4100, + "INVALID_ISSUE_START_DATE": 4101, + "INVALID_ISSUE_TARGET_DATE": 4102, + # pages + "PAGE_LOCKED": 4701, + "PAGE_ARCHIVED": 4702, +} diff --git a/apiserver/plane/graphql/utils/error_handler.py b/apiserver/plane/graphql/utils/error_handler.py new file mode 100644 index 0000000000..a90128cb39 --- /dev/null +++ b/apiserver/plane/graphql/utils/error_handler.py @@ -0,0 +1,12 @@ +# strawberry imports +from strawberry.exceptions import GraphQLError + + +class CustomGraphQLError(GraphQLError): + def __init__(self, message, code=None, field=None): + super().__init__(message) + self.code = code + self.field = field + + def extensions(self): + return {"code": self.code, "field": self.field} diff --git a/apiserver/plane/graphql/views.py b/apiserver/plane/graphql/views.py index 2375ea5bfd..6eee6de97a 100644 --- a/apiserver/plane/graphql/views.py +++ b/apiserver/plane/graphql/views.py @@ -1,11 +1,16 @@ # Django imports from django.contrib.auth import get_user +from typing import Any, Dict, Optional + # Third-Party Imports from asgiref.sync import sync_to_async # Strawberry imports -from strawberry.django.views import AsyncGraphQLView +from strawberry.django.views import AsyncGraphQLView +from strawberry.types import ExecutionResult +from strawberry.types.execution import ExecutionContext + class CustomGraphQLView(AsyncGraphQLView): async def get_context(self, request, response): @@ -16,3 +21,24 @@ class CustomGraphQLView(AsyncGraphQLView): context = await super().get_context(request, response) context.user = user return context + + async def process_result( + self, + request: Any, + result: ExecutionResult, + context: Optional[ExecutionContext] = None, + ) -> Dict[str, Any]: + processed_result = { + "data": result.data, + } + + if result.errors: + processed_result["errors"] = [ + { + "message": error.message, + "extensions": error.extensions or {}, + } + for error in result.errors + ] + + return processed_result diff --git a/apiserver/requirements/base.txt b/apiserver/requirements/base.txt index 399edbe273..3037eda188 100644 --- a/apiserver/requirements/base.txt +++ b/apiserver/requirements/base.txt @@ -69,4 +69,4 @@ python3-saml==1.16.0 # feature flag openfeature-sdk==0.7.0 # graphql -strawberry-graphql-django==0.47.0 +strawberry-graphql-django==0.47.2