From 55beb10b9f0c70dec4df74fb5f9be4ef7af266da Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sat, 27 Dec 2025 21:32:47 +0530 Subject: [PATCH 01/29] Module removal and editing improvements --- .../api/internal/mutations/module.py | 61 ++++++ frontend/src/components/EntityActions.tsx | 186 ++++++++++++++---- frontend/src/types/__generated__/graphql.ts | 7 + .../__generated__/EntityActions.generated.ts | 13 ++ 4 files changed, 233 insertions(+), 34 deletions(-) create mode 100644 frontend/types/__generated__/EntityActions.generated.ts diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index f83ab80668..19ad0e91ac 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -399,3 +399,64 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> module.program.save(update_fields=["experience_levels"]) return module + + @strawberry.mutation(permission_classes=[IsAuthenticated]) + @transaction.atomic + def delete_module( + self, + info: strawberry.Info, + program_key: str, + module_key: str, + ) -> str: + """Delete a mentorship module. User must be an admin of the program.""" + user = info.context.request.user + + try: + module = Module.objects.select_related("program").get( + key=module_key, program__key=program_key + ) + except Module.DoesNotExist as e: + msg = "Module not found." + raise ObjectDoesNotExist(msg) from e + + try: + admin_as_mentor = Mentor.objects.get(nest_user=user) + except Mentor.DoesNotExist as err: + msg = "Only mentors can delete modules." + logger.warning( + "User '%s' is not a mentor and cannot delete modules.", + user.username, + exc_info=True, + ) + raise PermissionDenied(msg) from err + + if not module.program.admins.filter(id=admin_as_mentor.id).exists(): + raise PermissionDenied + + program = module.program + module_name = module.name + + # Clean up experience levels if this module is the only one using it + experience_level_to_remove = module.experience_level + if ( + experience_level_to_remove in program.experience_levels + and not Module.objects.filter( + program=program, experience_level=experience_level_to_remove + ) + .exclude(id=module.id) + .exists() + ): + program.experience_levels.remove(experience_level_to_remove) + program.save(update_fields=["experience_levels"]) + + # Delete the module + module.delete() + + logger.info( + "User '%s' deleted module '%s' from program '%s'.", + user.username, + module_name, + program_key, + ) + + return f"Module '{module_name}' has been deleted successfully." diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index 8183d0f453..09986a57c2 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -1,10 +1,26 @@ 'use client' +import { gql } from '@apollo/client' +import { useMutation } from '@apollo/client/react' +import { Button } from '@heroui/button' +import { Modal, ModalContent, ModalHeader, ModalBody, ModalFooter } from '@heroui/modal' +import { addToast } from '@heroui/toast' import { useRouter } from 'next/navigation' import type React from 'react' import { useState, useRef, useEffect } from 'react' import { FaEllipsisV } from 'react-icons/fa' import { ProgramStatusEnum } from 'types/__generated__/graphql' +import { GetProgramAndModulesDocument } from 'types/__generated__/programsQueries.generated' + +interface DeleteModuleResponse { + deleteModule: boolean +} + +const DELETE_MODULE_MUTATION = gql` + mutation DeleteModule($programKey: String!, $moduleKey: String!) { + deleteModule(programKey: $programKey, moduleKey: $moduleKey) + } +` interface EntityActionsProps { type: 'program' | 'module' @@ -23,8 +39,12 @@ const EntityActions: React.FC = ({ }) => { const router = useRouter() const [dropdownOpen, setDropdownOpen] = useState(false) + const [deleteModalOpen, setDeleteModalOpen] = useState(false) + const [isDeleting, setIsDeleting] = useState(false) const dropdownRef = useRef(null) + const [deleteModule] = useMutation(DELETE_MODULE_MUTATION) + const handleAction = (actionKey: string) => { switch (actionKey) { case 'edit_program': @@ -43,6 +63,9 @@ const EntityActions: React.FC = ({ router.push(`/my/mentorship/programs/${programKey}/modules/${moduleKey}/issues`) } break + case 'delete_module': + setDeleteModalOpen(true) + break case 'publish': setStatus?.(ProgramStatusEnum.Published) break @@ -56,6 +79,66 @@ const EntityActions: React.FC = ({ setDropdownOpen(false) } + const handleDeleteConfirm = async () => { + if (!moduleKey) return + + setIsDeleting(true) + + try { + const result = await deleteModule({ + variables: { programKey, moduleKey }, + + update(cache) { + const existing = cache.readQuery({ + query: GetProgramAndModulesDocument, + variables: { programKey }, + }) + + if (!existing || !existing.getProgramModules) { + throw new Error('Program modules not found in cache') + } + + cache.writeQuery({ + query: GetProgramAndModulesDocument, + variables: { programKey }, + data: { + ...existing, + getProgramModules: existing.getProgramModules.filter( + (module) => module.key !== moduleKey + ), + }, + }) + }, + }) + + if (!result?.data || typeof result.data !== 'object' || !('deleteModule' in result.data)) { + throw new Error('Delete mutation failed on server') + } + + addToast({ + title: 'Success', + description: 'Module has been deleted successfully.', + color: 'success', + }) + + setDeleteModalOpen(false) + router.push(`/my/mentorship/programs/${programKey}`) + } catch (error) { + const description = + error instanceof Error && error.message.includes('Permission') + ? 'You do not have permission to delete this module.' + : 'Failed to delete module. Please try again.' + + addToast({ + title: 'Error', + description, + color: 'danger', + }) + } finally { + setIsDeleting(false) + } + } + const options = type === 'program' ? [ @@ -72,6 +155,7 @@ const EntityActions: React.FC = ({ : [ { key: 'edit_module', label: 'Edit' }, { key: 'view_issues', label: 'View Issues' }, + { key: 'delete_module', label: 'Delete', className: 'text-red-500' }, ] useEffect(() => { @@ -94,42 +178,76 @@ const EntityActions: React.FC = ({ } return ( -
- - {dropdownOpen && ( -
- {options.map((option) => { - const handleMenuItemClick = (e: React.MouseEvent) => { - e.preventDefault() - e.stopPropagation() - handleAction(option.key) - } - - return ( - + {dropdownOpen && ( +
+ {options.map((option) => { + const handleMenuItemClick = (e: React.MouseEvent) => { + e.preventDefault() + e.stopPropagation() + handleAction(option.key) + } + + return ( + + ) + })} +
+ )} +
+ + {type === 'module' && ( + setDeleteModalOpen(false)}> + + Delete Module + +

Are you sure you want to delete this module? This action cannot be undone.

+
+ + + - ) - })} -
+ Delete + + + + )} - + ) } diff --git a/frontend/src/types/__generated__/graphql.ts b/frontend/src/types/__generated__/graphql.ts index 7a879917c3..a9e09ed4e5 100644 --- a/frontend/src/types/__generated__/graphql.ts +++ b/frontend/src/types/__generated__/graphql.ts @@ -364,6 +364,7 @@ export type Mutation = { createApiKey: CreateApiKeyResult; createModule: ModuleNode; createProgram: ProgramNode; + deleteModule: Scalars['String']['output']; githubAuth: GitHubAuthResult; logoutUser: LogoutResult; revokeApiKey: RevokeApiKeyResult; @@ -406,6 +407,12 @@ export type MutationCreateProgramArgs = { }; +export type MutationDeleteModuleArgs = { + moduleKey: Scalars['String']['input']; + programKey: Scalars['String']['input']; +}; + + export type MutationGithubAuthArgs = { accessToken: Scalars['String']['input']; }; diff --git a/frontend/types/__generated__/EntityActions.generated.ts b/frontend/types/__generated__/EntityActions.generated.ts new file mode 100644 index 0000000000..28f4e997ba --- /dev/null +++ b/frontend/types/__generated__/EntityActions.generated.ts @@ -0,0 +1,13 @@ +import * as Types from '../../src/types/__generated__/graphql'; + +import { TypedDocumentNode as DocumentNode } from '@graphql-typed-document-node/core'; +export type DeleteModuleMutationVariables = Types.Exact<{ + programKey: Types.Scalars['String']['input']; + moduleKey: Types.Scalars['String']['input']; +}>; + + +export type DeleteModuleMutation = { deleteModule: string }; + + +export const DeleteModuleDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"DeleteModule"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"deleteModule"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"programKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}}},{"kind":"Argument","name":{"kind":"Name","value":"moduleKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}}}]}]}}]} as unknown as DocumentNode; \ No newline at end of file From e867b1e566df0553afdab7b0c530402a226ce917 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sat, 27 Dec 2025 23:42:55 +0530 Subject: [PATCH 02/29] fixed coderabbit review --- .../api/internal/mutations/module.py | 5 ++- frontend/src/components/CardDetailsPage.tsx | 2 +- frontend/src/components/EntityActions.tsx | 42 +++++++------------ frontend/src/components/SingleModuleCard.tsx | 2 +- .../src/graphql/mutations/deleteModule.ts | 7 ++++ 5 files changed, 29 insertions(+), 29 deletions(-) create mode 100644 frontend/src/graphql/mutations/deleteModule.ts diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index 19ad0e91ac..f523bf0be1 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -342,7 +342,10 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> ) raise PermissionDenied(msg) from err - if not module.program.admins.filter(id=creator_as_mentor.id).exists(): + is_program_admin = module.program.admins.filter(id=creator_as_mentor.id).exists() + is_module_mentor = module.mentors.filter(id=creator_as_mentor.id).exists() + + if not (is_program_admin or is_module_mentor): raise PermissionDenied started_at, ended_at = _validate_module_dates( diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index fae64ca9ac..7fe304ce54 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -103,7 +103,7 @@ const DetailsCard = ({ accessLevel === 'admin' && admins?.some( (admin) => admin.login === ((data as ExtendedSession)?.user?.login as string) - ) && } + ) && } {!isActive && } {isArchived && type === 'repository' && } {IS_PROJECT_HEALTH_ENABLED && type === 'project' && healthMetricsData.length > 0 && ( diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index 09986a57c2..548b03217f 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -1,6 +1,5 @@ 'use client' -import { gql } from '@apollo/client' import { useMutation } from '@apollo/client/react' import { Button } from '@heroui/button' import { Modal, ModalContent, ModalHeader, ModalBody, ModalFooter } from '@heroui/modal' @@ -9,25 +8,17 @@ import { useRouter } from 'next/navigation' import type React from 'react' import { useState, useRef, useEffect } from 'react' import { FaEllipsisV } from 'react-icons/fa' +import { DELETE_MODULE_MUTATION } from '../graphql/mutations/deleteModule' import { ProgramStatusEnum } from 'types/__generated__/graphql' import { GetProgramAndModulesDocument } from 'types/__generated__/programsQueries.generated' -interface DeleteModuleResponse { - deleteModule: boolean -} - -const DELETE_MODULE_MUTATION = gql` - mutation DeleteModule($programKey: String!, $moduleKey: String!) { - deleteModule(programKey: $programKey, moduleKey: $moduleKey) - } -` - interface EntityActionsProps { type: 'program' | 'module' programKey: string moduleKey?: string status?: string setStatus?: (newStatus: string) => void + isAdmin?: boolean } const EntityActions: React.FC = ({ @@ -36,6 +27,7 @@ const EntityActions: React.FC = ({ moduleKey, status, setStatus, + isAdmin = false, }) => { const router = useRouter() const [dropdownOpen, setDropdownOpen] = useState(false) @@ -94,24 +86,22 @@ const EntityActions: React.FC = ({ variables: { programKey }, }) - if (!existing || !existing.getProgramModules) { - throw new Error('Program modules not found in cache') + if (existing?.getProgramModules) { + cache.writeQuery({ + query: GetProgramAndModulesDocument, + variables: { programKey }, + data: { + ...existing, + getProgramModules: existing.getProgramModules.filter( + (module) => module.key !== moduleKey + ), + }, + }) } - - cache.writeQuery({ - query: GetProgramAndModulesDocument, - variables: { programKey }, - data: { - ...existing, - getProgramModules: existing.getProgramModules.filter( - (module) => module.key !== moduleKey - ), - }, - }) }, }) - if (!result?.data || typeof result.data !== 'object' || !('deleteModule' in result.data)) { + if (!result?.data || typeof result.data !== 'object' || !result.data.deleteModule) { throw new Error('Delete mutation failed on server') } @@ -155,7 +145,7 @@ const EntityActions: React.FC = ({ : [ { key: 'edit_module', label: 'Edit' }, { key: 'view_issues', label: 'View Issues' }, - { key: 'delete_module', label: 'Delete', className: 'text-red-500' }, + ...(isAdmin ? [{ key: 'delete_module', label: 'Delete', className: 'text-red-500' }] : []), ] useEffect(() => { diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index 1de2743282..9cdb503da6 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -62,7 +62,7 @@ const SingleModuleCard: React.FC = ({ module, accessLevel - {isAdmin && } + {isAdmin && } {/* Description */} diff --git a/frontend/src/graphql/mutations/deleteModule.ts b/frontend/src/graphql/mutations/deleteModule.ts new file mode 100644 index 0000000000..dca4756ef8 --- /dev/null +++ b/frontend/src/graphql/mutations/deleteModule.ts @@ -0,0 +1,7 @@ +import { gql } from '@apollo/client' + +export const DELETE_MODULE_MUTATION = gql` + mutation DeleteModule($programKey: String!, $moduleKey: String!) { + deleteModule(programKey: $programKey, moduleKey: $moduleKey) + } +` From 18b1b2057e5a406e8431b633777b6005cdce927a Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sun, 28 Dec 2025 00:26:20 +0530 Subject: [PATCH 03/29] feat: implement module deletion and editing with proper permissions - Add delete_module mutation in backend with permission checks - Update update_module to allow both program admins and module mentors - Implement delete functionality in EntityActions component - Add DELETE_MODULE_MUTATION with inline gql definition - Pass isAdmin prop to EntityActions for delete button visibility - Fix mutation result validation to check actual value - Add Apollo Client mock in jest.setup.ts for tests - All tests passing except unrelated CreateModule timeout --- frontend/jest.setup.ts | 12 ++++++++++++ frontend/src/components/CardDetailsPage.tsx | 9 ++++++++- frontend/src/components/EntityActions.tsx | 16 ++++++++++++++-- frontend/src/components/SingleModuleCard.tsx | 9 ++++++++- frontend/src/graphql/mutations/deleteModule.ts | 7 ------- .../__generated__/EntityActions.generated.ts | 2 +- 6 files changed, 43 insertions(+), 12 deletions(-) delete mode 100644 frontend/src/graphql/mutations/deleteModule.ts diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index dd410b6e98..dace2c727e 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -128,3 +128,15 @@ jest.mock('ics', () => { createEvent: jest.fn(), } }) + +jest.mock('@apollo/client/react', () => { + const actual = jest.requireActual('@apollo/client/react') + return { + ...actual, + useMutation: jest.fn(() => [ + jest.fn().mockResolvedValue({ data: { deleteModule: true } }), + { data: null, loading: false, error: null, called: false }, + ]), + } +}) + diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index 7fe304ce54..acc85e75b8 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -103,7 +103,14 @@ const DetailsCard = ({ accessLevel === 'admin' && admins?.some( (admin) => admin.login === ((data as ExtendedSession)?.user?.login as string) - ) && } + ) && ( + + )} {!isActive && } {isArchived && type === 'repository' && } {IS_PROJECT_HEALTH_ENABLED && type === 'project' && healthMetricsData.length > 0 && ( diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index 548b03217f..ba94634edf 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -1,5 +1,6 @@ 'use client' +import { gql } from '@apollo/client' import { useMutation } from '@apollo/client/react' import { Button } from '@heroui/button' import { Modal, ModalContent, ModalHeader, ModalBody, ModalFooter } from '@heroui/modal' @@ -8,10 +9,19 @@ import { useRouter } from 'next/navigation' import type React from 'react' import { useState, useRef, useEffect } from 'react' import { FaEllipsisV } from 'react-icons/fa' -import { DELETE_MODULE_MUTATION } from '../graphql/mutations/deleteModule' import { ProgramStatusEnum } from 'types/__generated__/graphql' import { GetProgramAndModulesDocument } from 'types/__generated__/programsQueries.generated' +const DELETE_MODULE_MUTATION = gql` + mutation DeleteModule($programKey: String!, $moduleKey: String!) { + deleteModule(programKey: $programKey, moduleKey: $moduleKey) + } +` + +type DeleteModuleResponse = { + deleteModule: boolean +} + interface EntityActionsProps { type: 'program' | 'module' programKey: string @@ -145,7 +155,9 @@ const EntityActions: React.FC = ({ : [ { key: 'edit_module', label: 'Edit' }, { key: 'view_issues', label: 'View Issues' }, - ...(isAdmin ? [{ key: 'delete_module', label: 'Delete', className: 'text-red-500' }] : []), + ...(isAdmin + ? [{ key: 'delete_module', label: 'Delete', className: 'text-red-500' }] + : []), ] useEffect(() => { diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index 9cdb503da6..b9b66542a9 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -62,7 +62,14 @@ const SingleModuleCard: React.FC = ({ module, accessLevel - {isAdmin && } + {isAdmin && ( + + )} {/* Description */} diff --git a/frontend/src/graphql/mutations/deleteModule.ts b/frontend/src/graphql/mutations/deleteModule.ts deleted file mode 100644 index dca4756ef8..0000000000 --- a/frontend/src/graphql/mutations/deleteModule.ts +++ /dev/null @@ -1,7 +0,0 @@ -import { gql } from '@apollo/client' - -export const DELETE_MODULE_MUTATION = gql` - mutation DeleteModule($programKey: String!, $moduleKey: String!) { - deleteModule(programKey: $programKey, moduleKey: $moduleKey) - } -` diff --git a/frontend/types/__generated__/EntityActions.generated.ts b/frontend/types/__generated__/EntityActions.generated.ts index 28f4e997ba..749f193925 100644 --- a/frontend/types/__generated__/EntityActions.generated.ts +++ b/frontend/types/__generated__/EntityActions.generated.ts @@ -10,4 +10,4 @@ export type DeleteModuleMutationVariables = Types.Exact<{ export type DeleteModuleMutation = { deleteModule: string }; -export const DeleteModuleDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"DeleteModule"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"deleteModule"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"programKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}}},{"kind":"Argument","name":{"kind":"Name","value":"moduleKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}}}]}]}}]} as unknown as DocumentNode; \ No newline at end of file +export const DeleteModuleDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"DeleteModule"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"deleteModule"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"programKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}}},{"kind":"Argument","name":{"kind":"Name","value":"moduleKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}}}]}]}}]} as unknown as DocumentNode; From 8aec659153fc4204b2843ac690e3411d35718ec0 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sun, 28 Dec 2025 01:17:14 +0530 Subject: [PATCH 04/29] fixed check error --- .../unit/pages/CreateModule.test.tsx | 54 ++++++++++--------- frontend/jest.setup.ts | 1 - 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/frontend/__tests__/unit/pages/CreateModule.test.tsx b/frontend/__tests__/unit/pages/CreateModule.test.tsx index 79279a8f59..458d7751ed 100644 --- a/frontend/__tests__/unit/pages/CreateModule.test.tsx +++ b/frontend/__tests__/unit/pages/CreateModule.test.tsx @@ -49,35 +49,37 @@ describe('CreateModulePage', () => { jest.clearAllMocks() }) - it('submits the form and navigates to programs page', async () => { - const user = userEvent.setup() - - ;(useSession as jest.Mock).mockReturnValue({ - data: { user: { login: 'admin-user' } }, - status: 'authenticated', - }) - ;(useQuery as unknown as jest.Mock).mockReturnValue({ - data: { - getProgram: { - admins: [{ login: 'admin-user' }], - }, - }, - loading: false, - }) - ;(useMutation as unknown as jest.Mock).mockReturnValue([ - mockCreateModule.mockResolvedValue({ + it( + 'submits the form and navigates to programs page', + async () => { + const user = userEvent.setup() + + ;(useSession as jest.Mock).mockReturnValue({ + data: { user: { login: 'admin-user' } }, + status: 'authenticated', + }) + ;(useQuery as unknown as jest.Mock).mockReturnValue({ data: { - createModule: { - key: 'my-test-module', + getProgram: { + admins: [{ login: 'admin-user' }], }, }, - }), - { loading: false }, - ]) + loading: false, + }) + ;(useMutation as unknown as jest.Mock).mockReturnValue([ + mockCreateModule.mockResolvedValue({ + data: { + createModule: { + key: 'my-test-module', + }, + }, + }), + { loading: false }, + ]) - render() + render() - // Fill all inputs + // Fill all inputs await user.type(screen.getByLabelText('Name'), 'My Test Module') await user.type(screen.getByLabelText(/Description/i), 'This is a test module') await user.type(screen.getByLabelText(/Start Date/i), '2025-07-15') @@ -121,5 +123,7 @@ describe('CreateModulePage', () => { expect(mockCreateModule).toHaveBeenCalled() expect(mockPush).toHaveBeenCalledWith('/my/mentorship/programs/test-program') }) - }) + }, + 10000 + ) }) diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index dace2c727e..0ecec0e7c6 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -139,4 +139,3 @@ jest.mock('@apollo/client/react', () => { ]), } }) - From c525845d6bc4cfe269aabc9dd78dec46d3761215 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sun, 28 Dec 2025 01:27:24 +0530 Subject: [PATCH 05/29] fixed coderabbit review --- frontend/jest.setup.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index 0ecec0e7c6..5bfbb8feeb 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -131,11 +131,14 @@ jest.mock('ics', () => { jest.mock('@apollo/client/react', () => { const actual = jest.requireActual('@apollo/client/react') + const mockUseMutation = jest.fn(() => [ + jest.fn().mockResolvedValue({ data: { deleteModule: true } }), + { data: null, loading: false, error: null, called: false }, + ]) + return { ...actual, - useMutation: jest.fn(() => [ - jest.fn().mockResolvedValue({ data: { deleteModule: true } }), - { data: null, loading: false, error: null, called: false }, - ]), + useMutation: mockUseMutation, + __mockUseMutation: mockUseMutation, } }) From 454d961d6710fafddcda62cc3cff060abdf5edfe Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Mon, 29 Dec 2025 15:49:49 +0530 Subject: [PATCH 06/29] added functionaility to edit module by mentor --- .../api/internal/mutations/module.py | 64 +++++++++++++++---- .../modules/[moduleKey]/edit/page.tsx | 40 ++++++++++-- frontend/src/components/CardDetailsPage.tsx | 19 ++++-- frontend/src/components/EntityActions.tsx | 13 ++-- frontend/src/components/SingleModuleCard.tsx | 9 ++- 5 files changed, 115 insertions(+), 30 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index f523bf0be1..c63af1279d 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -320,7 +320,15 @@ def clear_task_deadline( @strawberry.mutation(permission_classes=[IsAuthenticated]) @transaction.atomic def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> ModuleNode: - """Update an existing mentorship module. User must be an admin of the program.""" + """Update an existing mentorship module. + + User must either be: + - An admin of the program, or + - A mentor explicitly assigned to this module + + Admins can edit any field and manage mentor assignments. + Module mentors can edit module details but cannot modify mentor assignments. + """ user = info.context.request.user try: @@ -332,21 +340,37 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> raise ObjectDoesNotExist(msg) from e try: - creator_as_mentor = Mentor.objects.get(nest_user=user) - except Mentor.DoesNotExist as err: - msg = "Only mentors can edit modules." + editor_as_mentor = Mentor.objects.get(nest_user=user) + except Mentor.DoesNotExist: + try: + github_user = user.github_user + editor_as_mentor, _ = Mentor.objects.get_or_create( + github_user=github_user, + defaults={"nest_user": user} + ) + except Exception as err: + msg = f"User '{user.username}' is not registered as a mentor. Only mentors can edit modules." + logger.warning( + "Failed to find or create mentor for user '%s' (ID: %s): %s", + user.username, + user.id, + str(err), + exc_info=True, + ) + raise PermissionDenied(msg) from err + + is_program_admin = module.program.admins.filter(id=editor_as_mentor.id).exists() + is_module_mentor = module.mentors.filter(id=editor_as_mentor.id).exists() + + if not (is_program_admin or is_module_mentor): + msg = "You do not have permission to edit this module. Only program admins and module mentors can edit modules." logger.warning( - "User '%s' is not a mentor and cannot edit modules.", + "Unauthorized edit attempt: User '%s' is neither a program admin nor a module mentor for module '%s'.", user.username, + module.name, exc_info=True, ) - raise PermissionDenied(msg) from err - - is_program_admin = module.program.admins.filter(id=creator_as_mentor.id).exists() - is_module_mentor = module.mentors.filter(id=creator_as_mentor.id).exists() - - if not (is_program_admin or is_module_mentor): - raise PermissionDenied + raise PermissionDenied(msg) started_at, ended_at = _validate_module_dates( input_data.started_at, @@ -379,6 +403,15 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> raise ObjectDoesNotExist(msg) from err if input_data.mentor_logins is not None: + if not is_program_admin: + msg = "Only program admins can modify mentor assignments." + logger.warning( + "Unauthorized mentor assignment attempt: Non-admin mentor '%s' tried to modify mentors for module '%s'.", + user.username, + module.name, + exc_info=True, + ) + raise PermissionDenied(msg) mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins) module.mentors.set(mentors_to_set) @@ -401,6 +434,13 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> module.program.save(update_fields=["experience_levels"]) + logger.info( + "User '%s' successfully updated module '%s' in program '%s'.", + user.username, + module.name, + module.program.key, + ) + return module @strawberry.mutation(permission_classes=[IsAuthenticated]) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index cd9c85450f..d7bfde1d0f 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -56,13 +56,18 @@ const EditModulePage = () => { (admin: { login: string }) => admin.login === currentUserLogin ) - if (isAdmin) { + // Check if user is a module mentor + const isMentor = data.getModule.mentors?.some( + (mentor: { login: string }) => mentor.login === currentUserLogin + ) + + if (isAdmin || isMentor) { setAccessStatus('allowed') } else { setAccessStatus('denied') addToast({ title: 'Access Denied', - description: 'Only program admins can edit modules.', + description: 'Only program admins and module mentors can edit this module.', color: 'danger', variant: 'solid', timeout: 4000, @@ -95,14 +100,18 @@ const EditModulePage = () => { if (!formData) return try { - const input = { + const currentUserLogin = (sessionData as ExtendedSession)?.user?.login + const isAdmin = data?.getProgram?.admins?.some( + (admin: { login: string }) => admin.login === currentUserLogin + ) + + const input: any = { description: formData.description, domains: parseCommaSeparated(formData.domains), endedAt: formData.endedAt || null, experienceLevel: formData.experienceLevel as ExperienceLevelEnum, key: moduleKey, labels: parseCommaSeparated(formData.labels), - mentorLogins: parseCommaSeparated(formData.mentorLogins), name: formData.name, programKey: programKey, projectId: formData.projectId, @@ -111,6 +120,11 @@ const EditModulePage = () => { tags: parseCommaSeparated(formData.tags), } + // Only include mentorLogins if user is an admin + if (isAdmin) { + input.mentorLogins = parseCommaSeparated(formData.mentorLogins) + } + const result = await updateModule({ variables: { input } }) const updatedModuleKey = result.data?.updateModule?.key || moduleKey @@ -123,7 +137,23 @@ const EditModulePage = () => { }) router.push(`/my/mentorship/programs/${programKey}/modules/${updatedModuleKey}`) } catch (err) { - handleAppError(err) + let errorMessage = 'Failed to update module. Please try again.' + + if (err instanceof Error) { + if (err.message.includes('Permission') || err.message.includes('not have permission')) { + errorMessage = 'You do not have permission to edit this module. Only program admins and assigned mentors can edit modules.' + } else if (err.message.includes('mentor')) { + errorMessage = err.message + } + } + + addToast({ + title: 'Error', + description: errorMessage, + color: 'danger', + variant: 'solid', + timeout: 4000, + }) } } diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index acc85e75b8..325a2fc0e9 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -99,18 +99,23 @@ const DetailsCard = ({ setStatus={setStatus} /> )} - {type === 'module' && - accessLevel === 'admin' && - admins?.some( - (admin) => admin.login === ((data as ExtendedSession)?.user?.login as string) - ) && ( + {type === 'module' && (() => { + const currentUserLogin = ((data as ExtendedSession)?.user?.login as string) + const isAdmin = accessLevel === 'admin' && admins?.some( + (admin) => admin.login === currentUserLogin + ) + const isMentor = mentors?.some( + (mentor) => mentor.login === currentUserLogin + ) + return (isAdmin || isMentor) ? ( - )} + ) : null + })()} {!isActive && } {isArchived && type === 'repository' && } {IS_PROJECT_HEALTH_ENABLED && type === 'project' && healthMetricsData.length > 0 && ( diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index ba94634edf..6b750b6265 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -124,10 +124,15 @@ const EntityActions: React.FC = ({ setDeleteModalOpen(false) router.push(`/my/mentorship/programs/${programKey}`) } catch (error) { - const description = - error instanceof Error && error.message.includes('Permission') - ? 'You do not have permission to delete this module.' - : 'Failed to delete module. Please try again.' + let description = 'Failed to delete module. Please try again.' + + if (error instanceof Error) { + if (error.message.includes('Permission') || error.message.includes('not have permission')) { + description = 'You do not have permission to delete this module. Only program admins can delete modules.' + } else if (error.message.includes('Unauthorized')) { + description = 'Unauthorized: You must be a program admin to delete modules.' + } + } addToast({ title: 'Error', diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index b9b66542a9..08ec6bc81d 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -26,9 +26,13 @@ const SingleModuleCard: React.FC = ({ module, accessLevel const { data } = useSession() const pathname = usePathname() + const currentUserLogin = ((data as ExtendedSession)?.user?.login as string) + const isAdmin = accessLevel === 'admin' && - admins?.some((admin) => admin.login === ((data as ExtendedSession)?.user?.login as string)) + admins?.some((admin) => admin.login === currentUserLogin) + + const isMentor = module.mentors?.some((mentor) => mentor.login === currentUserLogin) // Extract programKey from pathname (e.g., /my/mentorship/programs/[programKey]) const programKey = pathname?.split('/programs/')[1]?.split('/')[0] || '' @@ -50,6 +54,7 @@ const SingleModuleCard: React.FC = ({ module, accessLevel target="_blank" rel="noopener noreferrer" className="flex-1" + data-testid="module-link" >

= ({ module, accessLevel - {isAdmin && ( + {(isAdmin || isMentor) && ( Date: Mon, 29 Dec 2025 17:32:04 +0530 Subject: [PATCH 07/29] fixed sonar and coderabbit review --- .../api/internal/mutations/module.py | 24 +++++---- .../unit/pages/CreateModule.test.tsx | 54 +++++++++---------- .../__tests__/unit/pages/EditModule.test.tsx | 2 +- frontend/jest.setup.ts | 2 +- .../modules/[moduleKey]/edit/page.tsx | 11 ++-- frontend/src/components/CardDetailsPage.tsx | 33 ++++++------ frontend/src/components/EntityActions.tsx | 3 +- frontend/src/components/SingleModuleCard.tsx | 7 ++- .../__generated__/EntityActions.generated.ts | 2 +- 9 files changed, 69 insertions(+), 69 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index c63af1279d..dfa84fc69c 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -345,11 +345,13 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> try: github_user = user.github_user editor_as_mentor, _ = Mentor.objects.get_or_create( - github_user=github_user, - defaults={"nest_user": user} + github_user=github_user, defaults={"nest_user": user} ) except Exception as err: - msg = f"User '{user.username}' is not registered as a mentor. Only mentors can edit modules." + msg = ( + f"User '{user.username}' is not registered as a mentor. " + "Only mentors can edit modules." + ) logger.warning( "Failed to find or create mentor for user '%s' (ID: %s): %s", user.username, @@ -363,12 +365,15 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> is_module_mentor = module.mentors.filter(id=editor_as_mentor.id).exists() if not (is_program_admin or is_module_mentor): - msg = "You do not have permission to edit this module. Only program admins and module mentors can edit modules." + msg = ( + "You do not have permission to edit this module. " + "Only program admins and module mentors can edit modules." + ) logger.warning( - "Unauthorized edit attempt: User '%s' is neither a program admin nor a module mentor for module '%s'.", + "Unauthorized edit attempt: User '%s' is neither a program admin " + "nor a module mentor for module '%s'.", user.username, module.name, - exc_info=True, ) raise PermissionDenied(msg) @@ -406,10 +411,10 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> if not is_program_admin: msg = "Only program admins can modify mentor assignments." logger.warning( - "Unauthorized mentor assignment attempt: Non-admin mentor '%s' tried to modify mentors for module '%s'.", + "Unauthorized mentor assignment attempt: Non-admin mentor '%s' " + "tried to modify mentors for module '%s'.", user.username, module.name, - exc_info=True, ) raise PermissionDenied(msg) mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins) @@ -474,7 +479,8 @@ def delete_module( raise PermissionDenied(msg) from err if not module.program.admins.filter(id=admin_as_mentor.id).exists(): - raise PermissionDenied + msg = "Only program admins can delete modules." + raise PermissionDenied(msg) program = module.program module_name = module.name diff --git a/frontend/__tests__/unit/pages/CreateModule.test.tsx b/frontend/__tests__/unit/pages/CreateModule.test.tsx index 458d7751ed..2e9bca18f6 100644 --- a/frontend/__tests__/unit/pages/CreateModule.test.tsx +++ b/frontend/__tests__/unit/pages/CreateModule.test.tsx @@ -49,37 +49,35 @@ describe('CreateModulePage', () => { jest.clearAllMocks() }) - it( - 'submits the form and navigates to programs page', - async () => { - const user = userEvent.setup() - - ;(useSession as jest.Mock).mockReturnValue({ - data: { user: { login: 'admin-user' } }, - status: 'authenticated', - }) - ;(useQuery as unknown as jest.Mock).mockReturnValue({ + it('submits the form and navigates to programs page', async () => { + const user = userEvent.setup() + + ;(useSession as jest.Mock).mockReturnValue({ + data: { user: { login: 'admin-user' } }, + status: 'authenticated', + }) + ;(useQuery as unknown as jest.Mock).mockReturnValue({ + data: { + getProgram: { + admins: [{ login: 'admin-user' }], + }, + }, + loading: false, + }) + ;(useMutation as unknown as jest.Mock).mockReturnValue([ + mockCreateModule.mockResolvedValue({ data: { - getProgram: { - admins: [{ login: 'admin-user' }], + createModule: { + key: 'my-test-module', }, }, - loading: false, - }) - ;(useMutation as unknown as jest.Mock).mockReturnValue([ - mockCreateModule.mockResolvedValue({ - data: { - createModule: { - key: 'my-test-module', - }, - }, - }), - { loading: false }, - ]) + }), + { loading: false }, + ]) - render() + render() - // Fill all inputs + // Fill all inputs await user.type(screen.getByLabelText('Name'), 'My Test Module') await user.type(screen.getByLabelText(/Description/i), 'This is a test module') await user.type(screen.getByLabelText(/Start Date/i), '2025-07-15') @@ -123,7 +121,5 @@ describe('CreateModulePage', () => { expect(mockCreateModule).toHaveBeenCalled() expect(mockPush).toHaveBeenCalledWith('/my/mentorship/programs/test-program') }) - }, - 10000 - ) + }, 10000) }) diff --git a/frontend/__tests__/unit/pages/EditModule.test.tsx b/frontend/__tests__/unit/pages/EditModule.test.tsx index 4b8deef346..f9a0b60fa9 100644 --- a/frontend/__tests__/unit/pages/EditModule.test.tsx +++ b/frontend/__tests__/unit/pages/EditModule.test.tsx @@ -144,7 +144,7 @@ describe('EditModulePage', () => { await waitFor(() => { expect(addToast).toHaveBeenCalledWith({ title: 'Access Denied', - description: 'Only program admins can edit modules.', + description: 'Only program admins and module mentors can edit this module.', color: 'danger', variant: 'solid', timeout: 4000, diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index 5bfbb8feeb..c0233e24bc 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -135,7 +135,7 @@ jest.mock('@apollo/client/react', () => { jest.fn().mockResolvedValue({ data: { deleteModule: true } }), { data: null, loading: false, error: null, called: false }, ]) - + return { ...actual, useMutation: mockUseMutation, diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index d7bfde1d0f..c7afc82937 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -5,8 +5,8 @@ import { addToast } from '@heroui/toast' import { useParams, useRouter } from 'next/navigation' import { useSession } from 'next-auth/react' import React, { useEffect, useState } from 'react' -import { ErrorDisplay, handleAppError } from 'app/global-error' -import { ExperienceLevelEnum } from 'types/__generated__/graphql' +import { ErrorDisplay } from 'app/global-error' +import { ExperienceLevelEnum, type UpdateModuleInput } from 'types/__generated__/graphql' import { UpdateModuleDocument } from 'types/__generated__/moduleMutations.generated' import { GetProgramAdminsAndModulesDocument } from 'types/__generated__/moduleQueries.generated' import type { ExtendedSession } from 'types/auth' @@ -56,7 +56,6 @@ const EditModulePage = () => { (admin: { login: string }) => admin.login === currentUserLogin ) - // Check if user is a module mentor const isMentor = data.getModule.mentors?.some( (mentor: { login: string }) => mentor.login === currentUserLogin ) @@ -105,7 +104,7 @@ const EditModulePage = () => { (admin: { login: string }) => admin.login === currentUserLogin ) - const input: any = { + const input: UpdateModuleInput = { description: formData.description, domains: parseCommaSeparated(formData.domains), endedAt: formData.endedAt || null, @@ -120,7 +119,6 @@ const EditModulePage = () => { tags: parseCommaSeparated(formData.tags), } - // Only include mentorLogins if user is an admin if (isAdmin) { input.mentorLogins = parseCommaSeparated(formData.mentorLogins) } @@ -141,7 +139,8 @@ const EditModulePage = () => { if (err instanceof Error) { if (err.message.includes('Permission') || err.message.includes('not have permission')) { - errorMessage = 'You do not have permission to edit this module. Only program admins and assigned mentors can edit modules.' + errorMessage = + 'You do not have permission to edit this module. Only program admins and assigned mentors can edit modules.' } else if (err.message.includes('mentor')) { errorMessage = err.message } diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index 325a2fc0e9..28dbea4cf6 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -99,23 +99,22 @@ const DetailsCard = ({ setStatus={setStatus} /> )} - {type === 'module' && (() => { - const currentUserLogin = ((data as ExtendedSession)?.user?.login as string) - const isAdmin = accessLevel === 'admin' && admins?.some( - (admin) => admin.login === currentUserLogin - ) - const isMentor = mentors?.some( - (mentor) => mentor.login === currentUserLogin - ) - return (isAdmin || isMentor) ? ( - - ) : null - })()} + {type === 'module' && + (() => { + const currentUserLogin = (data as ExtendedSession)?.user?.login + const isAdmin = + accessLevel === 'admin' && + admins?.some((admin) => admin.login === currentUserLogin) + const isMentor = mentors?.some((mentor) => mentor.login === currentUserLogin) + return isAdmin || isMentor ? ( + + ) : null + })()} {!isActive && } {isArchived && type === 'repository' && } {IS_PROJECT_HEALTH_ENABLED && type === 'project' && healthMetricsData.length > 0 && ( diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index 6b750b6265..2a111ef11d 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -128,7 +128,8 @@ const EntityActions: React.FC = ({ if (error instanceof Error) { if (error.message.includes('Permission') || error.message.includes('not have permission')) { - description = 'You do not have permission to delete this module. Only program admins can delete modules.' + description = + 'You do not have permission to delete this module. Only program admins can delete modules.' } else if (error.message.includes('Unauthorized')) { description = 'Unauthorized: You must be a program admin to delete modules.' } diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index 08ec6bc81d..775861d09c 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -6,7 +6,7 @@ import { usePathname } from 'next/navigation' import { useSession } from 'next-auth/react' import React from 'react' import { HiUserGroup } from 'react-icons/hi' -import { ExtendedSession } from 'types/auth' +import type { ExtendedSession } from 'types/auth' import type { Module } from 'types/mentorship' import { formatDate } from 'utils/dateFormatter' import EntityActions from 'components/EntityActions' @@ -26,11 +26,10 @@ const SingleModuleCard: React.FC = ({ module, accessLevel const { data } = useSession() const pathname = usePathname() - const currentUserLogin = ((data as ExtendedSession)?.user?.login as string) + const currentUserLogin = (data as ExtendedSession)?.user?.login const isAdmin = - accessLevel === 'admin' && - admins?.some((admin) => admin.login === currentUserLogin) + accessLevel === 'admin' && admins?.some((admin) => admin.login === currentUserLogin) const isMentor = module.mentors?.some((mentor) => mentor.login === currentUserLogin) diff --git a/frontend/types/__generated__/EntityActions.generated.ts b/frontend/types/__generated__/EntityActions.generated.ts index 749f193925..28f4e997ba 100644 --- a/frontend/types/__generated__/EntityActions.generated.ts +++ b/frontend/types/__generated__/EntityActions.generated.ts @@ -10,4 +10,4 @@ export type DeleteModuleMutationVariables = Types.Exact<{ export type DeleteModuleMutation = { deleteModule: string }; -export const DeleteModuleDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"DeleteModule"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"deleteModule"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"programKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}}},{"kind":"Argument","name":{"kind":"Name","value":"moduleKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}}}]}]}}]} as unknown as DocumentNode; +export const DeleteModuleDocument = {"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"mutation","name":{"kind":"Name","value":"DeleteModule"},"variableDefinitions":[{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}},{"kind":"VariableDefinition","variable":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}},"type":{"kind":"NonNullType","type":{"kind":"NamedType","name":{"kind":"Name","value":"String"}}}}],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"deleteModule"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"programKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"programKey"}}},{"kind":"Argument","name":{"kind":"Name","value":"moduleKey"},"value":{"kind":"Variable","name":{"kind":"Name","value":"moduleKey"}}}]}]}}]} as unknown as DocumentNode; \ No newline at end of file From e7540f2ee5a75f827dfc7d38f5506e7bdda5efc3 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Mon, 29 Dec 2025 21:48:11 +0530 Subject: [PATCH 08/29] Added no sonar --- .../programs/[programKey]/modules/[moduleKey]/edit/page.tsx | 2 ++ frontend/src/components/CardDetailsPage.tsx | 1 + frontend/src/components/SingleModuleCard.tsx | 1 + 3 files changed, 4 insertions(+) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index c7afc82937..df8aa3933f 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -51,6 +51,7 @@ const EditModulePage = () => { return } + // NOSONAR - NextAuth callback adds login property to session at runtime const currentUserLogin = (sessionData as ExtendedSession)?.user?.login const isAdmin = data.getProgram.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin @@ -99,6 +100,7 @@ const EditModulePage = () => { if (!formData) return try { + // NOSONAR - NextAuth callback adds login property to session at runtime const currentUserLogin = (sessionData as ExtendedSession)?.user?.login const isAdmin = data?.getProgram?.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index 28dbea4cf6..6c5f8ca0de 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -101,6 +101,7 @@ const DetailsCard = ({ )} {type === 'module' && (() => { + // NOSONAR - NextAuth callback adds login property to session at runtime const currentUserLogin = (data as ExtendedSession)?.user?.login const isAdmin = accessLevel === 'admin' && diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index 775861d09c..b30b4839fb 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -26,6 +26,7 @@ const SingleModuleCard: React.FC = ({ module, accessLevel const { data } = useSession() const pathname = usePathname() + // NOSONAR - NextAuth callback adds login property to session at runtime const currentUserLogin = (data as ExtendedSession)?.user?.login const isAdmin = From 44b51e94b1e2bc312f1b8fe987b8c5bf6337bfc4 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Wed, 31 Dec 2025 13:21:58 +0530 Subject: [PATCH 09/29] fixed nosoanr --- .../programs/[programKey]/modules/[moduleKey]/edit/page.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index df8aa3933f..66c00bc246 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -51,7 +51,7 @@ const EditModulePage = () => { return } - // NOSONAR - NextAuth callback adds login property to session at runtime + // NOSONAR (typescript:S1525) - The 'login' property is added to the session object at runtime by NextAuth configuration callbacks. This is a safe type assertion as ExtendedSession explicitly defines the login property. const currentUserLogin = (sessionData as ExtendedSession)?.user?.login const isAdmin = data.getProgram.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin @@ -100,7 +100,7 @@ const EditModulePage = () => { if (!formData) return try { - // NOSONAR - NextAuth callback adds login property to session at runtime + // NOSONAR (typescript:S1525) - The 'login' property is added to the session object at runtime by NextAuth configuration callbacks. This is a safe type assertion as ExtendedSession explicitly defines the login property. const currentUserLogin = (sessionData as ExtendedSession)?.user?.login const isAdmin = data?.getProgram?.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin From f729e3db51a73945293a9a2d443707e6ad496f46 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Wed, 31 Dec 2025 13:51:41 +0530 Subject: [PATCH 10/29] updated nosonar comment --- .../programs/[programKey]/modules/[moduleKey]/edit/page.tsx | 4 +++- frontend/src/components/SingleModuleCard.tsx | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index 66c00bc246..c6cd5b2754 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -100,7 +100,9 @@ const EditModulePage = () => { if (!formData) return try { - // NOSONAR (typescript:S1525) - The 'login' property is added to the session object at runtime by NextAuth configuration callbacks. This is a safe type assertion as ExtendedSession explicitly defines the login property. + // NOSONAR (typescript:S1525) + // `login` is dynamically injected by NextAuth callbacks at runtime. + // The ExtendedSession type explicitly models this augmentation, so this assertion is intentional and safe. const currentUserLogin = (sessionData as ExtendedSession)?.user?.login const isAdmin = data?.getProgram?.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index b30b4839fb..c2329df0cd 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -26,7 +26,9 @@ const SingleModuleCard: React.FC = ({ module, accessLevel const { data } = useSession() const pathname = usePathname() - // NOSONAR - NextAuth callback adds login property to session at runtime + // NOSONAR (typescript:S1525) + // `login` is injected into the session object by NextAuth callbacks at runtime. + // This runtime augmentation is intentionally modeled by ExtendedSession, making the assertion safe. const currentUserLogin = (data as ExtendedSession)?.user?.login const isAdmin = From 3b7162d26eec706f136e7851693bb9baef986852 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Wed, 31 Dec 2025 16:02:18 +0530 Subject: [PATCH 11/29] update nosonar warning --- frontend/jest.setup.ts | 4 ++-- frontend/src/components/SingleModuleCard.tsx | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index c0233e24bc..825a0e6620 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -66,8 +66,8 @@ jest.mock('next/navigation', () => { } }) -if (!global.structuredClone) { - global.structuredClone = (val) => JSON.parse(JSON.stringify(val)) +if (!globalThis.structuredClone) { + globalThis.structuredClone = (val) => structuredClone(val) } beforeAll(() => { diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index c2329df0cd..bf3becfbe0 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -26,9 +26,7 @@ const SingleModuleCard: React.FC = ({ module, accessLevel const { data } = useSession() const pathname = usePathname() - // NOSONAR (typescript:S1525) - // `login` is injected into the session object by NextAuth callbacks at runtime. - // This runtime augmentation is intentionally modeled by ExtendedSession, making the assertion safe. + // NOSONAR Type assertion needed: ExtendedSession defines the `login` property that NextAuth adds at runtime const currentUserLogin = (data as ExtendedSession)?.user?.login const isAdmin = From 968b9dad37adf34acd5b5862f37dfcef925b982e Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Wed, 31 Dec 2025 16:16:10 +0530 Subject: [PATCH 12/29] fixed coderabbit review --- frontend/jest.setup.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index 825a0e6620..5296a85deb 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -66,10 +66,6 @@ jest.mock('next/navigation', () => { } }) -if (!globalThis.structuredClone) { - globalThis.structuredClone = (val) => structuredClone(val) -} - beforeAll(() => { if (typeof globalThis !== 'undefined') { jest.spyOn(globalThis, 'requestAnimationFrame').mockImplementation((cb) => { From 3d13803c5a1b33c974c2a10bcdb548ba9334d90d Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 9 Jan 2026 13:43:24 +0530 Subject: [PATCH 13/29] Fixed coderabbit review --- .../api/internal/mutations/module.py | 31 +++++++------------ 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index c02a025265..38393035e5 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -1,5 +1,6 @@ """GraphQL mutations for mentorship modules in the mentorship app.""" +import contextlib import logging from datetime import datetime @@ -341,27 +342,17 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> except Module.DoesNotExist as e: raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) from e - try: + editor_as_mentor = None + with contextlib.suppress(Mentor.DoesNotExist): editor_as_mentor = Mentor.objects.get(nest_user=user) - except Mentor.DoesNotExist: - try: - github_user = user.github_user - editor_as_mentor, _ = Mentor.objects.get_or_create( - github_user=github_user, defaults={"nest_user": user} - ) - except Exception as err: - msg = ( - f"User '{user.username}' is not registered as a mentor. " - "Only mentors can edit modules." - ) - logger.warning( - "Failed to find or create mentor for user '%s' (ID: %s): %s", - user.username, - user.id, - str(err), - exc_info=True, - ) - raise PermissionDenied(msg) from err + + if editor_as_mentor is None: + with contextlib.suppress(AttributeError, Mentor.DoesNotExist): + editor_as_mentor = Mentor.objects.get(github_user=user.github_user) + + if editor_as_mentor is None: + msg = "Only mentors can edit modules." + raise PermissionDenied(msg) is_program_admin = module.program.admins.filter(id=editor_as_mentor.id).exists() is_module_mentor = module.mentors.filter(id=editor_as_mentor.id).exists() From 9292706c6401f363db20f60f7d3eb69a000809fb Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 9 Jan 2026 14:37:11 +0530 Subject: [PATCH 14/29] Resolve coderabbit review --- .../api/internal/mutations/module.py | 47 ++++++++++++------- frontend/jest.setup.ts | 3 +- .../modules/[moduleKey]/edit/page.tsx | 2 - 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index 38393035e5..e7060c9970 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -402,16 +402,27 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> if input_data.mentor_logins is not None: if not is_program_admin: - msg = "Only program admins can modify mentor assignments." - logger.warning( - "Unauthorized mentor assignment attempt: Non-admin mentor '%s' " - "tried to modify mentors for module '%s'.", - user.username, - module.name, - ) - raise PermissionDenied(msg) - mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins) - module.mentors.set(mentors_to_set) + current_logins = { + login.lower() + for login in module.mentors.values_list("github_user__login", flat=True) + } + requested_logins = {login.lower() for login in input_data.mentor_logins} + + if requested_logins != current_logins: + msg = "Only program admins can modify mentor assignments." + logger.warning( + "Unauthorized mentor assignment attempt: Non-admin mentor '%s' " + "tried to modify mentors for module '%s'.", + user.username, + module.name, + ) + raise PermissionDenied(msg) + # Mentor list unchanged; skip the update + input_data.mentor_logins = None + + if input_data.mentor_logins is not None: + mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins) + module.mentors.set(mentors_to_set) module.save() @@ -457,19 +468,23 @@ def delete_module( key=module_key, program__key=program_key ) except Module.DoesNotExist as e: - msg = "Module not found." - raise ObjectDoesNotExist(msg) from e + raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) from e - try: + admin_as_mentor = None + with contextlib.suppress(Mentor.DoesNotExist): admin_as_mentor = Mentor.objects.get(nest_user=user) - except Mentor.DoesNotExist as err: + + if admin_as_mentor is None: + with contextlib.suppress(AttributeError, Mentor.DoesNotExist): + admin_as_mentor = Mentor.objects.get(github_user=user.github_user) + + if admin_as_mentor is None: msg = "Only mentors can delete modules." logger.warning( "User '%s' is not a mentor and cannot delete modules.", user.username, - exc_info=True, ) - raise PermissionDenied(msg) from err + raise PermissionDenied(msg) if not module.program.admins.filter(id=admin_as_mentor.id).exists(): msg = "Only program admins can delete modules." diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index 5296a85deb..bcdabd074b 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -128,13 +128,12 @@ jest.mock('ics', () => { jest.mock('@apollo/client/react', () => { const actual = jest.requireActual('@apollo/client/react') const mockUseMutation = jest.fn(() => [ - jest.fn().mockResolvedValue({ data: { deleteModule: true } }), + jest.fn().mockResolvedValue({ data: {} }), { data: null, loading: false, error: null, called: false }, ]) return { ...actual, useMutation: mockUseMutation, - __mockUseMutation: mockUseMutation, } }) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index c6cd5b2754..356ea4ef58 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -145,8 +145,6 @@ const EditModulePage = () => { if (err.message.includes('Permission') || err.message.includes('not have permission')) { errorMessage = 'You do not have permission to edit this module. Only program admins and assigned mentors can edit modules.' - } else if (err.message.includes('mentor')) { - errorMessage = err.message } } From dca555f4de1122f15358a8d4c2eee3bed9e1f3da Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 9 Jan 2026 14:58:09 +0530 Subject: [PATCH 15/29] fix code --- backend/apps/mentorship/api/internal/mutations/module.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index e7060c9970..bea4bde853 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -346,8 +346,8 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> with contextlib.suppress(Mentor.DoesNotExist): editor_as_mentor = Mentor.objects.get(nest_user=user) - if editor_as_mentor is None: - with contextlib.suppress(AttributeError, Mentor.DoesNotExist): + if editor_as_mentor is None and hasattr(user, "github_user"): + with contextlib.suppress(Mentor.DoesNotExist): editor_as_mentor = Mentor.objects.get(github_user=user.github_user) if editor_as_mentor is None: @@ -474,8 +474,8 @@ def delete_module( with contextlib.suppress(Mentor.DoesNotExist): admin_as_mentor = Mentor.objects.get(nest_user=user) - if admin_as_mentor is None: - with contextlib.suppress(AttributeError, Mentor.DoesNotExist): + if admin_as_mentor is None and hasattr(user, "github_user"): + with contextlib.suppress(Mentor.DoesNotExist): admin_as_mentor = Mentor.objects.get(github_user=user.github_user) if admin_as_mentor is None: From 123391451c1344d815231693ceabce0e5e1e3818 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 9 Jan 2026 15:08:32 +0530 Subject: [PATCH 16/29] fixed coderabbit comment --- .../mentorship/api/internal/mutations/module.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index bea4bde853..e4c4ae31e6 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -81,13 +81,21 @@ def create_module(self, info: strawberry.Info, input_data: CreateModuleInput) -> try: program = Program.objects.get(key=input_data.program_key) project = Project.objects.get(id=input_data.project_id) - creator_as_mentor = Mentor.objects.get(nest_user=user) except (Program.DoesNotExist, Project.DoesNotExist) as e: msg = f"{e.__class__.__name__} matching query does not exist." raise ObjectDoesNotExist(msg) from e - except Mentor.DoesNotExist as e: + + creator_as_mentor = None + with contextlib.suppress(Mentor.DoesNotExist): + creator_as_mentor = Mentor.objects.get(nest_user=user) + + if creator_as_mentor is None and hasattr(user, "github_user"): + with contextlib.suppress(Mentor.DoesNotExist): + creator_as_mentor = Mentor.objects.get(github_user=user.github_user) + + if creator_as_mentor is None: msg = "Only mentors can create modules." - raise PermissionDenied(msg) from e + raise PermissionDenied(msg) if not program.admins.filter(id=creator_as_mentor.id).exists(): raise PermissionDenied From 4b79b2412563ce523302f26f606637d57ad6cdc2 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 9 Jan 2026 15:55:40 +0530 Subject: [PATCH 17/29] fixed --- .../api/internal/mutations/module.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index e4c4ae31e6..f4d332e6c0 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -150,7 +150,7 @@ def assign_issue_to_user( .first() ) if module is None: - raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) mentor = Mentor.objects.filter(nest_user=user).first() if mentor is None: @@ -160,11 +160,11 @@ def assign_issue_to_user( gh_user = GithubUser.objects.filter(login=user_login).first() if gh_user is None: - raise ObjectDoesNotExist(msg="Assignee not found.") + raise ObjectDoesNotExist("Assignee not found.") issue = module.issues.filter(number=issue_number).first() if issue is None: - raise ObjectDoesNotExist(msg=ISSUE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(ISSUE_NOT_FOUND_MSG) issue.assignees.add(gh_user) @@ -192,7 +192,7 @@ def unassign_issue_from_user( .first() ) if module is None: - raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) mentor = Mentor.objects.filter(nest_user=user).first() if mentor is None: @@ -202,11 +202,11 @@ def unassign_issue_from_user( gh_user = GithubUser.objects.filter(login=user_login).first() if gh_user is None: - raise ObjectDoesNotExist(msg="Assignee not found.") + raise ObjectDoesNotExist("Assignee not found.") issue = module.issues.filter(number=issue_number).first() if issue is None: - raise ObjectDoesNotExist(msg=f"Issue {issue_number} not found in this module.") + raise ObjectDoesNotExist(f"Issue {issue_number} not found in this module.") issue.assignees.remove(gh_user) @@ -232,7 +232,7 @@ def set_task_deadline( .first() ) if module is None: - raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) mentor = Mentor.objects.filter(nest_user=user).first() if mentor is None: @@ -247,7 +247,7 @@ def set_task_deadline( .first() ) if issue is None: - raise ObjectDoesNotExist(msg=ISSUE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(ISSUE_NOT_FOUND_MSG) assignees = issue.assignees.all() if not assignees.exists(): @@ -294,7 +294,7 @@ def clear_task_deadline( .first() ) if module is None: - raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) mentor = Mentor.objects.filter(nest_user=user).first() if mentor is None: @@ -309,7 +309,7 @@ def clear_task_deadline( .first() ) if issue is None: - raise ObjectDoesNotExist(msg=ISSUE_NOT_FOUND_MSG) + raise ObjectDoesNotExist(ISSUE_NOT_FOUND_MSG) assignees = issue.assignees.all() if not assignees.exists(): @@ -348,7 +348,7 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> key=input_data.key, program__key=input_data.program_key ) except Module.DoesNotExist as e: - raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) from e + raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) from e editor_as_mentor = None with contextlib.suppress(Mentor.DoesNotExist): @@ -476,7 +476,7 @@ def delete_module( key=module_key, program__key=program_key ) except Module.DoesNotExist as e: - raise ObjectDoesNotExist(msg=MODULE_NOT_FOUND_MSG) from e + raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) from e admin_as_mentor = None with contextlib.suppress(Mentor.DoesNotExist): From 0a330f66ea8d405d88c629fafaa401b087d848a1 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 9 Jan 2026 16:13:41 +0530 Subject: [PATCH 18/29] fixed check command fail --- backend/apps/mentorship/api/internal/mutations/module.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index f4d332e6c0..c81e0cadcf 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -160,7 +160,8 @@ def assign_issue_to_user( gh_user = GithubUser.objects.filter(login=user_login).first() if gh_user is None: - raise ObjectDoesNotExist("Assignee not found.") + msg = "Assignee not found." + raise ObjectDoesNotExist(msg) issue = module.issues.filter(number=issue_number).first() if issue is None: @@ -202,11 +203,13 @@ def unassign_issue_from_user( gh_user = GithubUser.objects.filter(login=user_login).first() if gh_user is None: - raise ObjectDoesNotExist("Assignee not found.") + msg = "Assignee not found." + raise ObjectDoesNotExist(msg) issue = module.issues.filter(number=issue_number).first() if issue is None: - raise ObjectDoesNotExist(f"Issue {issue_number} not found in this module.") + msg = f"Issue {issue_number} not found in this module." + raise ObjectDoesNotExist(msg) issue.assignees.remove(gh_user) From d2afc0640d817f140afa08f8dc278ab5cf9ba449 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Thu, 15 Jan 2026 21:29:33 +0530 Subject: [PATCH 19/29] fixed sonarqube warning --- frontend/src/components/CardDetailsPage.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index 8158522bcb..821d732135 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -131,8 +131,7 @@ const DetailsCard = ({ )} {type === 'module' && (() => { - // NOSONAR - NextAuth callback adds login property to session at runtime - const currentUserLogin = (data as ExtendedSession)?.user?.login + const currentUserLogin = session?.user?.login const isAdmin = accessLevel === 'admin' && admins?.some((admin) => admin.login === currentUserLogin) From 04e794c88de7012eed450d18e75e644c9ea2cf0b Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 16 Jan 2026 01:33:12 +0530 Subject: [PATCH 20/29] Fixed sonarqube warning --- .../[programKey]/modules/[moduleKey]/edit/page.tsx | 13 ++++++------- frontend/src/components/SingleModuleCard.tsx | 5 ++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index 356ea4ef58..a0718b3460 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -19,7 +19,10 @@ import ModuleForm from 'components/ModuleForm' const EditModulePage = () => { const { programKey, moduleKey } = useParams<{ programKey: string; moduleKey: string }>() const router = useRouter() - const { data: sessionData, status: sessionStatus } = useSession() + const { data: sessionData, status: sessionStatus } = useSession() as { + data: ExtendedSession | null + status: string + } const [formData, setFormData] = useState(null) const [accessStatus, setAccessStatus] = useState<'checking' | 'allowed' | 'denied'>('checking') @@ -51,8 +54,7 @@ const EditModulePage = () => { return } - // NOSONAR (typescript:S1525) - The 'login' property is added to the session object at runtime by NextAuth configuration callbacks. This is a safe type assertion as ExtendedSession explicitly defines the login property. - const currentUserLogin = (sessionData as ExtendedSession)?.user?.login + const currentUserLogin = sessionData?.user?.login const isAdmin = data.getProgram.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin ) @@ -100,10 +102,7 @@ const EditModulePage = () => { if (!formData) return try { - // NOSONAR (typescript:S1525) - // `login` is dynamically injected by NextAuth callbacks at runtime. - // The ExtendedSession type explicitly models this augmentation, so this assertion is intentional and safe. - const currentUserLogin = (sessionData as ExtendedSession)?.user?.login + const currentUserLogin = sessionData?.user?.login const isAdmin = data?.getProgram?.admins?.some( (admin: { login: string }) => admin.login === currentUserLogin ) diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index bf3becfbe0..e3fd447e94 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -23,11 +23,10 @@ interface SingleModuleCardProps { } const SingleModuleCard: React.FC = ({ module, accessLevel, admins }) => { - const { data } = useSession() + const { data: sessionData } = useSession() as { data: ExtendedSession | null } const pathname = usePathname() - // NOSONAR Type assertion needed: ExtendedSession defines the `login` property that NextAuth adds at runtime - const currentUserLogin = (data as ExtendedSession)?.user?.login + const currentUserLogin = sessionData?.user?.login const isAdmin = accessLevel === 'admin' && admins?.some((admin) => admin.login === currentUserLogin) From 851203b30839f478f90a16da73e88fad1e6e2917 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 16 Jan 2026 12:50:21 +0530 Subject: [PATCH 21/29] Remove view Issues from mentor --- .../unit/components/EntityActions.test.tsx | 20 ++++++++++++++++--- frontend/src/components/EntityActions.tsx | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/frontend/__tests__/unit/components/EntityActions.test.tsx b/frontend/__tests__/unit/components/EntityActions.test.tsx index d94d430dcd..7d64c09ddc 100644 --- a/frontend/__tests__/unit/components/EntityActions.test.tsx +++ b/frontend/__tests__/unit/components/EntityActions.test.tsx @@ -82,7 +82,14 @@ describe('EntityActions', () => { describe('Module Actions - View Issues', () => { it('navigates to view issues page when View Issues is clicked with moduleKey', () => { - render() + render( + + ) const button = screen.getByRole('button', { name: /Module actions menu/ }) fireEvent.click(button) @@ -95,7 +102,7 @@ describe('EntityActions', () => { }) it('does not navigate when moduleKey is missing for view issues action', () => { - render() + render() const button = screen.getByRole('button', { name: /Module actions menu/ }) fireEvent.click(button) @@ -106,7 +113,14 @@ describe('EntityActions', () => { }) it('closes dropdown after clicking View Issues', () => { - render() + render( + + ) const button = screen.getByRole('button', { name: /Module actions menu/ }) fireEvent.click(button) expect(button).toHaveAttribute('aria-expanded', 'true') diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index a1b04f6099..daed5994bc 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -160,7 +160,7 @@ const EntityActions: React.FC = ({ ] : [ { key: 'edit_module', label: 'Edit' }, - { key: 'view_issues', label: 'View Issues' }, + ...(isAdmin ? [{ key: 'view_issues', label: 'View Issues' }] : []), ...(isAdmin ? [{ key: 'delete_module', label: 'Delete', className: 'text-red-500' }] : []), From 3cf76b3c7334dfb1ccaf9efc6652f4ced3747a9d Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sat, 24 Jan 2026 21:53:11 +0530 Subject: [PATCH 22/29] fixed merge conflict --- .../programs/[programKey]/modules/[moduleKey]/edit/page.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index 8717cd83ba..beec778a23 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -127,7 +127,6 @@ const EditModulePage = () => { input.mentorLogins = parseCommaSeparated(formData.mentorLogins) } - const result = await updateModule({ variables: { input } }) const result = await updateModule({ awaitRefetchQueries: true, refetchQueries: [{ query: GetProgramAndModulesDocument, variables: { programKey } }], From 0e63158d57c6fe3cbbdc89c550e213c6d783e68c Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sat, 24 Jan 2026 22:25:42 +0530 Subject: [PATCH 23/29] fixed sonarqube issue --- frontend/jest.setup.ts | 2 ++ frontend/src/components/SingleModuleCard.tsx | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/jest.setup.ts b/frontend/jest.setup.ts index 95000363b3..0a8782f3c4 100644 --- a/frontend/jest.setup.ts +++ b/frontend/jest.setup.ts @@ -171,3 +171,5 @@ jest.mock('@apollo/client/react', () => { useMutation: mockUseMutation, } }) + +expect.extend(toHaveNoViolations) diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index 4bb665249f..1b8eadaa9f 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -10,7 +10,6 @@ import React, { useState } from 'react' import { FaFolderOpen } from 'react-icons/fa' import { HiUserGroup } from 'react-icons/hi' import type { ExtendedSession } from 'types/auth' -import { ExtendedSession } from 'types/auth' import type { Contributor } from 'types/contributor' import type { Module } from 'types/mentorship' import { formatDate } from 'utils/dateFormatter' From 3da7d252d94a0adfed646285cc7cd966d63f9424 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sat, 24 Jan 2026 23:58:22 +0530 Subject: [PATCH 24/29] fixed code rabbit review --- backend/apps/mentorship/api/internal/mutations/module.py | 7 +++++++ .../[programKey]/modules/[moduleKey]/edit/page.tsx | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index 9d37753add..553185ee74 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -532,6 +532,13 @@ def delete_module( # Delete the module module.delete() + + def _invalidate(): + invalidate_module_cache(module_key, program_key) + + invalidate_program_cache(program_key) + + transaction.on_commit(_invalidate) logger.info( "User '%s' deleted module '%s' from program '%s'.", diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index beec778a23..b0a3758ba0 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -6,7 +6,7 @@ import { useParams, useRouter } from 'next/navigation' import { useSession } from 'next-auth/react' import React, { useEffect, useState } from 'react' import { ErrorDisplay } from 'app/global-error' -import { ExperienceLevelEnum, type UpdateModuleInput } from 'types/__generated__/graphql' +import { ExperienceLevelEnum, UpdateModuleInput } from 'types/__generated__/graphql' import { UpdateModuleDocument } from 'types/__generated__/moduleMutations.generated' import { GetProgramAdminsAndModulesDocument } from 'types/__generated__/moduleQueries.generated' import { GetProgramAndModulesDocument } from 'types/__generated__/programsQueries.generated' From b12ad1ec513c8b725db48ecf0f6a3af6f92a4a60 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sun, 25 Jan 2026 00:08:24 +0530 Subject: [PATCH 25/29] fixed check --- .../api/internal/mutations/module.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index 553185ee74..4cbdb30410 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -358,6 +358,11 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> except Module.DoesNotExist as e: raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) from e + # Check if user is a program admin or module mentor + is_program_admin = False + is_module_mentor = False + + # Try to find the Mentor object for this user editor_as_mentor = None with contextlib.suppress(Mentor.DoesNotExist): editor_as_mentor = Mentor.objects.get(nest_user=user) @@ -366,12 +371,10 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> with contextlib.suppress(Mentor.DoesNotExist): editor_as_mentor = Mentor.objects.get(github_user=user.github_user) - if editor_as_mentor is None: - msg = "Only mentors can edit modules." - raise PermissionDenied(msg) - - is_program_admin = module.program.admins.filter(id=editor_as_mentor.id).exists() - is_module_mentor = module.mentors.filter(id=editor_as_mentor.id).exists() + # Check permissions if we found a Mentor object + if editor_as_mentor is not None: + is_program_admin = module.program.admins.filter(id=editor_as_mentor.id).exists() + is_module_mentor = module.mentors.filter(id=editor_as_mentor.id).exists() if not (is_program_admin or is_module_mentor): msg = ( @@ -532,10 +535,10 @@ def delete_module( # Delete the module module.delete() - + def _invalidate(): - invalidate_module_cache(module_key, program_key) - + invalidate_module_cache(module_key, program_key) + invalidate_program_cache(program_key) transaction.on_commit(_invalidate) From 0992bab1827968f7ae0d515ce78c4467db929e16 Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Sun, 25 Jan 2026 00:15:34 +0530 Subject: [PATCH 26/29] fixed coderabbit review --- backend/apps/mentorship/api/internal/mutations/module.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index 4cbdb30410..d76fb2ac88 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -100,7 +100,7 @@ def create_module(self, info: strawberry.Info, input_data: CreateModuleInput) -> raise PermissionDenied(msg) if not program.admins.filter(id=creator_as_mentor.id).exists(): - raise PermissionDenied + raise PermissionDenied("Only program admins can create modules.") started_at, ended_at = _validate_module_dates( input_data.started_at, @@ -538,8 +538,7 @@ def delete_module( def _invalidate(): invalidate_module_cache(module_key, program_key) - - invalidate_program_cache(program_key) + invalidate_program_cache(program_key) transaction.on_commit(_invalidate) From 03fdca349f050aa1fe05536560fa9b30ea1d1cfe Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 20 Feb 2026 22:37:03 +0530 Subject: [PATCH 27/29] fixed merge conflict error and updated mentor logic --- .pre-commit-config.yaml | 1 + .../api/internal/mutations/module.py | 115 ++------ .../unit/components/CardDetailsPage.test.tsx | 4 +- .../unit/components/EntityActions.test.tsx | 278 ++++++++++++++++++ .../modules/[moduleKey]/edit/page.tsx | 2 +- frontend/src/components/CardDetailsPage.tsx | 3 +- frontend/src/components/EntityActions.tsx | 52 +--- 7 files changed, 317 insertions(+), 138 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5740d9019c..a350daaed4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -60,6 +60,7 @@ repos: args: - --fix files: \.md$ + language_version: 22.13.0 - repo: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt rev: 0.2.3 diff --git a/backend/apps/mentorship/api/internal/mutations/module.py b/backend/apps/mentorship/api/internal/mutations/module.py index 5694e08aae..97e9ecc94f 100644 --- a/backend/apps/mentorship/api/internal/mutations/module.py +++ b/backend/apps/mentorship/api/internal/mutations/module.py @@ -1,6 +1,5 @@ """GraphQL mutations for mentorship modules in the mentorship app.""" -import contextlib import logging from datetime import datetime @@ -48,6 +47,20 @@ def resolve_mentors_from_logins(logins: list[str]) -> set[Mentor]: return mentors +def _is_mentor_of_module(user, module) -> bool: + """Check if the given user is a mentor for the module. + + Runs a fallback check against github_user if the mentor hasn't linked + their nest_user profile yet. + """ + if Mentor.objects.filter(nest_user=user, modules=module).exists(): + return True + return ( + hasattr(user, "github_user") + and Mentor.objects.filter(github_user=user.github_user, modules=module).exists() + ) + + def _validate_module_dates(started_at, ended_at, program_started_at, program_ended_at) -> tuple: """Validate and normalize module start/end dates against program constraints.""" if started_at is None or ended_at is None: @@ -91,21 +104,6 @@ def create_module(self, info: strawberry.Info, input_data: CreateModuleInput) -> msg = f"{e.__class__.__name__} matching query does not exist." raise ObjectDoesNotExist(msg) from e - creator_as_mentor = None - with contextlib.suppress(Mentor.DoesNotExist): - creator_as_mentor = Mentor.objects.get(nest_user=user) - - if creator_as_mentor is None and hasattr(user, "github_user"): - with contextlib.suppress(Mentor.DoesNotExist): - creator_as_mentor = Mentor.objects.get(github_user=user.github_user) - - if creator_as_mentor is None: - msg = "Only mentors can create modules." - raise PermissionDenied(msg) - - if not program.admins.filter(id=creator_as_mentor.id).exists(): - raise PermissionDenied("Only program admins can create modules.") - if not program.admins.filter(nest_user=user).exists(): raise PermissionDenied @@ -160,7 +158,7 @@ def assign_issue_to_user( if module is None: raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) - if not Mentor.objects.filter(nest_user=user, modules=module).exists(): + if not _is_mentor_of_module(user, module): raise PermissionDenied(NOT_MENTOR_ASSIGN_MSG) gh_user = GithubUser.objects.filter(login=user_login).first() @@ -199,7 +197,7 @@ def unassign_issue_from_user( if module is None: raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) - if not Mentor.objects.filter(nest_user=user, modules=module).exists(): + if not _is_mentor_of_module(user, module): raise PermissionDenied(NOT_MENTOR_UNASSIGN_MSG) gh_user = GithubUser.objects.filter(login=user_login).first() @@ -240,7 +238,7 @@ def set_task_deadline( if module is None: raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) - if not Mentor.objects.filter(nest_user=user, modules=module).exists(): + if not _is_mentor_of_module(user, module): raise PermissionDenied(NOT_MENTOR_SET_DEADLINE_MSG) issue = ( @@ -302,7 +300,7 @@ def clear_task_deadline( if module is None: raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) - if not Mentor.objects.filter(nest_user=user, modules=module).exists(): + if not _is_mentor_of_module(user, module): raise PermissionDenied(NOT_MENTOR_CLEAR_DEADLINE_MSG) issue = ( @@ -341,8 +339,7 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> - An admin of the program, or - A mentor explicitly assigned to this module - Admins can edit any field and manage mentor assignments. - Module mentors can edit module details but cannot modify mentor assignments. + Admins and module mentors can edit any field and manage mentor assignments. """ user = info.context.request.user @@ -356,7 +353,7 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) from e is_admin = module.program.admins.filter(nest_user=user).exists() - is_mentor = Mentor.objects.filter(nest_user=user, modules=module).exists() + is_mentor = _is_mentor_of_module(user, module) if not (is_admin or is_mentor): msg = "Only admins of the program or mentors of this module can edit modules." raise PermissionDenied(msg) @@ -392,28 +389,8 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> raise ObjectDoesNotExist(msg) from err if input_data.mentor_logins is not None: - if not is_program_admin: - current_logins = { - login.lower() - for login in module.mentors.values_list("github_user__login", flat=True) - } - requested_logins = {login.lower() for login in input_data.mentor_logins} - - if requested_logins != current_logins: - msg = "Only program admins can modify mentor assignments." - logger.warning( - "Unauthorized mentor assignment attempt: Non-admin mentor '%s' " - "tried to modify mentors for module '%s'.", - user.username, - module.name, - ) - raise PermissionDenied(msg) - # Mentor list unchanged; skip the update - input_data.mentor_logins = None - - if input_data.mentor_logins is not None: - mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins) - module.mentors.set(mentors_to_set) + mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins) + module.mentors.set(mentors_to_set) module.save() @@ -434,21 +411,6 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) -> module.program.save(update_fields=["experience_levels"]) - logger.info( - "User '%s' successfully updated module '%s' in program '%s'.", - user.username, - module.name, - module.program.key, - ) - program_key = module.program.key - - def _invalidate(): - invalidate_module_cache(old_module_key, program_key) - if module.key != old_module_key: - invalidate_module_cache(module.key, program_key) - - transaction.on_commit(_invalidate) - return module @strawberry.mutation(permission_classes=[IsAuthenticated]) @@ -469,30 +431,13 @@ def delete_module( except Module.DoesNotExist as e: raise ObjectDoesNotExist(MODULE_NOT_FOUND_MSG) from e - admin_as_mentor = None - with contextlib.suppress(Mentor.DoesNotExist): - admin_as_mentor = Mentor.objects.get(nest_user=user) - - if admin_as_mentor is None and hasattr(user, "github_user"): - with contextlib.suppress(Mentor.DoesNotExist): - admin_as_mentor = Mentor.objects.get(github_user=user.github_user) - - if admin_as_mentor is None: - msg = "Only mentors can delete modules." - logger.warning( - "User '%s' is not a mentor and cannot delete modules.", - user.username, - ) - raise PermissionDenied(msg) - - if not module.program.admins.filter(id=admin_as_mentor.id).exists(): + if not module.program.admins.filter(nest_user=user).exists(): msg = "Only program admins can delete modules." raise PermissionDenied(msg) program = module.program module_name = module.name - # Clean up experience levels if this module is the only one using it experience_level_to_remove = module.experience_level if ( experience_level_to_remove in program.experience_levels @@ -505,20 +450,6 @@ def delete_module( program.experience_levels.remove(experience_level_to_remove) program.save(update_fields=["experience_levels"]) - # Delete the module module.delete() - def _invalidate(): - invalidate_module_cache(module_key, program_key) - invalidate_program_cache(program_key) - - transaction.on_commit(_invalidate) - - logger.info( - "User '%s' deleted module '%s' from program '%s'.", - user.username, - module_name, - program_key, - ) - return f"Module '{module_name}' has been deleted successfully." diff --git a/frontend/__tests__/unit/components/CardDetailsPage.test.tsx b/frontend/__tests__/unit/components/CardDetailsPage.test.tsx index 1293412174..a8d4d59734 100644 --- a/frontend/__tests__/unit/components/CardDetailsPage.test.tsx +++ b/frontend/__tests__/unit/components/CardDetailsPage.test.tsx @@ -445,6 +445,7 @@ jest.mock('components/EntityActions', () => ({ moduleKey, status: _status, setStatus: _setStatus, + isAdmin, ...props }: { type: string @@ -452,9 +453,10 @@ jest.mock('components/EntityActions', () => ({ moduleKey?: string status?: string setStatus?: (status: string) => void + isAdmin?: boolean [key: string]: unknown }) => ( -
+
EntityActions: type={type}, programKey={programKey}, moduleKey={moduleKey}
), diff --git a/frontend/__tests__/unit/components/EntityActions.test.tsx b/frontend/__tests__/unit/components/EntityActions.test.tsx index 8fce768c57..0e9676934c 100644 --- a/frontend/__tests__/unit/components/EntityActions.test.tsx +++ b/frontend/__tests__/unit/components/EntityActions.test.tsx @@ -1,3 +1,5 @@ +import { useMutation } from '@apollo/client/react' +import { addToast } from '@heroui/toast' import { render, screen, fireEvent, waitFor } from '@testing-library/react' import { useRouter } from 'next/navigation' import { ProgramStatusEnum } from 'types/__generated__/graphql' @@ -9,12 +11,21 @@ jest.mock('next/navigation', () => ({ useRouter: jest.fn(), })) +jest.mock('@apollo/client/react', () => ({ + useMutation: jest.fn(), +})) + +jest.mock('@heroui/toast', () => ({ + addToast: jest.fn(), +})) + describe('EntityActions', () => { beforeEach(() => { jest.clearAllMocks() ;(useRouter as jest.Mock).mockReturnValue({ push: mockPush, }) + ;(useMutation as unknown as jest.Mock).mockReturnValue([jest.fn()]) }) describe('Program Actions - Create Module', () => { @@ -752,4 +763,271 @@ describe('EntityActions', () => { expect(button).toHaveAttribute('aria-expanded', 'false') }) }) + + describe('Module Actions - Delete Module', () => { + let mockDeleteMutation: jest.Mock + + beforeEach(() => { + mockDeleteMutation = jest.fn() + ;(useMutation as unknown as jest.Mock).mockReturnValue([mockDeleteMutation]) + }) + + it('opens delete modal when Delete is clicked', () => { + render( + + ) + const button = screen.getByRole('button', { name: /Module actions menu/ }) + fireEvent.click(button) + + const deleteButton = screen.getByText('Delete') + fireEvent.click(deleteButton) + + expect(screen.getByText('Delete Module')).toBeInTheDocument() + expect(screen.getByText(/Are you sure you want to delete this module/)).toBeInTheDocument() + }) + + it('closes delete modal when Cancel is clicked', async () => { + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + expect(screen.getByText('Delete Module')).toBeInTheDocument() + fireEvent.click(screen.getByRole('button', { name: /Cancel/i })) + + await waitFor(() => { + expect(screen.queryByText('Delete Module')).not.toBeInTheDocument() + }) + }) + + it('closes delete modal when Modal close button is clicked (onClose)', async () => { + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + expect(screen.getByText('Delete Module')).toBeInTheDocument() + + const closeButton = screen.getByRole('button', { name: /Close/i }) + fireEvent.click(closeButton) + + await waitFor(() => { + expect(screen.queryByText('Delete Module')).not.toBeInTheDocument() + }) + }) + + it('does not do anything if moduleKey is missing on submit', async () => { + render() + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + const confirmButton = screen.getByRole('button', { name: 'Delete' }) + fireEvent.click(confirmButton) + + expect(mockDeleteMutation).not.toHaveBeenCalled() + }) + + it('handles successful module deletion', async () => { + mockDeleteMutation.mockResolvedValueOnce({ data: { deleteModule: true } }) + + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + const confirmButton = screen.getByRole('button', { name: 'Delete' }) + fireEvent.click(confirmButton) + + await waitFor(() => { + expect(mockDeleteMutation).toHaveBeenCalledWith({ + variables: { programKey: 'test-program', moduleKey: 'test-module' }, + update: expect.any(Function), + }) + }) + + // We should check that addToast was called with success + expect(addToast).toHaveBeenCalledWith({ + title: 'Success', + description: 'Module has been deleted successfully.', + color: 'success', + }) + + expect(mockPush).toHaveBeenCalledWith('/my/mentorship/programs/test-program') + }) + + it('calls update cache function when delete is successful', async () => { + // Create a mock cache object with readQuery and writeQuery + const mockCache = { + readQuery: jest.fn().mockReturnValue({ + getProgramModules: [{ key: 'test-module' }, { key: 'other-module' }], + }), + writeQuery: jest.fn(), + } + + mockDeleteMutation.mockImplementationOnce(({ update }) => { + // execute update cache function directly to test its internals + // eslint-disable-next-line @typescript-eslint/no-explicit-any + if (update) update(mockCache as any) + return Promise.resolve({ data: { deleteModule: true } }) + }) + + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(mockCache.readQuery).toHaveBeenCalled() + expect(mockCache.writeQuery).toHaveBeenCalledWith( + expect.objectContaining({ + data: { + getProgramModules: [{ key: 'other-module' }], + }, + }) + ) + }) + }) + + it('handles mutation structural failure', async () => { + mockDeleteMutation.mockResolvedValueOnce({ data: { deleteModule: false } }) + + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Error', + color: 'danger', + description: 'Failed to delete module. Please try again.', + }) + ) + }) + }) + + it('handles Permission Denied error from server', async () => { + mockDeleteMutation.mockRejectedValueOnce( + new Error('Permission denied: You do not have permission') + ) + + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Error', + color: 'danger', + description: + 'You do not have permission to delete this module. Only program admins can delete modules.', + }) + ) + }) + }) + + it('handles Unauthorized error from server', async () => { + mockDeleteMutation.mockRejectedValueOnce(new Error('Unauthorized request')) + + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Error', + color: 'danger', + description: 'Unauthorized: You must be a program admin to delete modules.', + }) + ) + }) + }) + + it('handles a generic network error from server', async () => { + mockDeleteMutation.mockRejectedValueOnce(new Error('Network disconnected')) + + render( + + ) + fireEvent.click(screen.getByRole('button', { name: /Module actions menu/ })) + fireEvent.click(screen.getByText('Delete')) + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => { + expect(addToast).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Error', + color: 'danger', + description: 'Failed to delete module. Please try again.', + }) + ) + }) + }) + }) }) diff --git a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx index da2b2b816d..4767957ce4 100644 --- a/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx +++ b/frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx @@ -5,7 +5,7 @@ import { addToast } from '@heroui/toast' import { useParams, useRouter } from 'next/navigation' import { useSession } from 'next-auth/react' import React, { useEffect, useState } from 'react' -import { ErrorDisplay, handleAppError } from 'app/global-error' +import { ErrorDisplay } from 'app/global-error' import { ExperienceLevelEnum } from 'types/__generated__/graphql' import type { UpdateModuleInput } from 'types/__generated__/graphql' import { UpdateModuleDocument } from 'types/__generated__/moduleMutations.generated' diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index c7eb6fcedb..72e9ae4587 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -154,6 +154,7 @@ const DetailsCard = ({ )} {type === 'module' && (() => { + if (!programKey || !entityKey) return null const currentUserLogin = session?.user?.login const isAdmin = accessLevel === 'admin' && @@ -164,7 +165,7 @@ const DetailsCard = ({ type="module" programKey={programKey} moduleKey={entityKey} - isAdmin={isAdmin} + isAdmin={isAdmin ? true : undefined} /> ) : null })()} diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index 828853c467..ea6357a235 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -230,11 +230,10 @@ const EntityActions: React.FC = ({ } return ( - //--------------------need to verify locally-------------- <>
- {dropdownOpen && ( -
- {options.map((option, index) => { - const handleMenuItemClick = (e: React.MouseEvent) => { - e.preventDefault() - e.stopPropagation() - handleAction(option.key) - setFocusIndex(-1) - } - - return ( - From 95ece1c12060cb1ea1409d069983e193fad8f62f Mon Sep 17 00:00:00 2001 From: Anurag Yadav Date: Fri, 20 Feb 2026 22:56:31 +0530 Subject: [PATCH 28/29] fix review --- .pre-commit-config.yaml | 1 - .../unit/components/EntityActions.test.tsx | 17 ++++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a350daaed4..5740d9019c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -60,7 +60,6 @@ repos: args: - --fix files: \.md$ - language_version: 22.13.0 - repo: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt rev: 0.2.3 diff --git a/frontend/__tests__/unit/components/EntityActions.test.tsx b/frontend/__tests__/unit/components/EntityActions.test.tsx index 0e9676934c..8cb46bf144 100644 --- a/frontend/__tests__/unit/components/EntityActions.test.tsx +++ b/frontend/__tests__/unit/components/EntityActions.test.tsx @@ -866,20 +866,16 @@ describe('EntityActions', () => { variables: { programKey: 'test-program', moduleKey: 'test-module' }, update: expect.any(Function), }) + expect(addToast).toHaveBeenCalledWith({ + title: 'Success', + description: 'Module has been deleted successfully.', + color: 'success', + }) + expect(mockPush).toHaveBeenCalledWith('/my/mentorship/programs/test-program') }) - - // We should check that addToast was called with success - expect(addToast).toHaveBeenCalledWith({ - title: 'Success', - description: 'Module has been deleted successfully.', - color: 'success', - }) - - expect(mockPush).toHaveBeenCalledWith('/my/mentorship/programs/test-program') }) it('calls update cache function when delete is successful', async () => { - // Create a mock cache object with readQuery and writeQuery const mockCache = { readQuery: jest.fn().mockReturnValue({ getProgramModules: [{ key: 'test-module' }, { key: 'other-module' }], @@ -888,7 +884,6 @@ describe('EntityActions', () => { } mockDeleteMutation.mockImplementationOnce(({ update }) => { - // execute update cache function directly to test its internals // eslint-disable-next-line @typescript-eslint/no-explicit-any if (update) update(mockCache as any) return Promise.resolve({ data: { deleteModule: true } }) From 7be0b5fd3a970e877e0403966e49fd14162786fe Mon Sep 17 00:00:00 2001 From: Kate Date: Sat, 21 Feb 2026 17:51:52 -0800 Subject: [PATCH 29/29] Update mentors permissions to view module issues --- .../unit/components/CardDetailsPage.test.tsx | 2 + frontend/graphql-codegen.ts | 55 ++++++++++++------- frontend/src/components/CardDetailsPage.tsx | 1 + frontend/src/components/EntityActions.tsx | 4 +- frontend/src/components/SingleModuleCard.tsx | 1 + .../__generated__/EntityActions.generated.ts | 2 +- 6 files changed, 42 insertions(+), 23 deletions(-) rename frontend/{ => src}/types/__generated__/EntityActions.generated.ts (95%) diff --git a/frontend/__tests__/unit/components/CardDetailsPage.test.tsx b/frontend/__tests__/unit/components/CardDetailsPage.test.tsx index a8d4d59734..834749cfa1 100644 --- a/frontend/__tests__/unit/components/CardDetailsPage.test.tsx +++ b/frontend/__tests__/unit/components/CardDetailsPage.test.tsx @@ -446,6 +446,7 @@ jest.mock('components/EntityActions', () => ({ status: _status, setStatus: _setStatus, isAdmin, + isMentor: _isMentor, ...props }: { type: string @@ -454,6 +455,7 @@ jest.mock('components/EntityActions', () => ({ status?: string setStatus?: (status: string) => void isAdmin?: boolean + isMentor?: boolean [key: string]: unknown }) => (
diff --git a/frontend/graphql-codegen.ts b/frontend/graphql-codegen.ts index 39f3762cbc..39fad299b7 100644 --- a/frontend/graphql-codegen.ts +++ b/frontend/graphql-codegen.ts @@ -20,32 +20,45 @@ if (!response.ok) { } const csrfToken = (await response.json()).csrftoken +const sharedOperationConfig = { + config: { + avoidOptionals: { + // Use `null` for nullable fields instead of optionals + field: true, + // Allow nullable input fields to remain unspecified + inputValue: false, + }, + defaultScalarType: 'any', + // Apollo Client always includes `__typename` fields + nonOptionalTypename: true, + // Apollo Client doesn't add the `__typename` field to root types so + // don't generate a type for the `__typename` for root operation types. + skipTypeNameForRoot: true, + }, + plugins: ['typescript-operations', 'typed-document-node'], + preset: 'near-operation-file', + presetConfig: { + baseTypesPath: './types/__generated__/graphql.ts', + }, +} + const config: CodegenConfig = { - documents: ['src/**/*.{ts,tsx}', '!src/types/__generated__/**'], generates: { './src/': { - config: { - avoidOptionals: { - // Use `null` for nullable fields instead of optionals - field: true, - // Allow nullable input fields to remain unspecified - inputValue: false, - }, - defaultScalarType: 'any', - // Apollo Client always includes `__typename` fields - nonOptionalTypename: true, - // Apollo Client doesn't add the `__typename` field to root types so - // don't generate a type for the `__typename` for root operation types. - skipTypeNameForRoot: true, + ...sharedOperationConfig, + documents: ['src/**/*.{ts,tsx}', '!src/types/__generated__/**', '!src/server/**'], + presetConfig: { + ...sharedOperationConfig.presetConfig, + folder: '../types/__generated__', }, - // Order of plugins matter - plugins: ['typescript-operations', 'typed-document-node'], - preset: 'near-operation-file', + }, + './src/server/': { + ...sharedOperationConfig, + documents: ['src/server/**/*.ts'], presetConfig: { - // This should be the file generated by the "typescript" plugin above, - // relative to the directory specified for this configuration - baseTypesPath: './types/__generated__/graphql.ts', - // Relative to the source files + ...sharedOperationConfig.presetConfig, + // Resolved from baseOutputDir ./src/server/, so this points to src/types/__generated__/graphql.ts + baseTypesPath: '../types/__generated__/graphql.ts', folder: '../../types/__generated__', }, }, diff --git a/frontend/src/components/CardDetailsPage.tsx b/frontend/src/components/CardDetailsPage.tsx index 72e9ae4587..16fc425c56 100644 --- a/frontend/src/components/CardDetailsPage.tsx +++ b/frontend/src/components/CardDetailsPage.tsx @@ -166,6 +166,7 @@ const DetailsCard = ({ programKey={programKey} moduleKey={entityKey} isAdmin={isAdmin ? true : undefined} + isMentor={isMentor ? true : undefined} /> ) : null })()} diff --git a/frontend/src/components/EntityActions.tsx b/frontend/src/components/EntityActions.tsx index ea6357a235..204f3f1199 100644 --- a/frontend/src/components/EntityActions.tsx +++ b/frontend/src/components/EntityActions.tsx @@ -29,6 +29,7 @@ interface EntityActionsProps { status?: string setStatus?: (newStatus: ProgramStatusEnum) => void | Promise isAdmin?: boolean + isMentor?: boolean } const EntityActions: React.FC = ({ @@ -38,6 +39,7 @@ const EntityActions: React.FC = ({ status, setStatus, isAdmin = false, + isMentor = false, }) => { const router = useRouter() const [dropdownOpen, setDropdownOpen] = useState(false) @@ -163,7 +165,7 @@ const EntityActions: React.FC = ({ ] : [ { key: 'edit_module', label: 'Edit' }, - ...(isAdmin ? [{ key: 'view_issues', label: 'View Issues' }] : []), + ...(isAdmin || isMentor ? [{ key: 'view_issues', label: 'View Issues' }] : []), ...(isAdmin ? [{ key: 'delete_module', label: 'Delete', className: 'text-red-500' }] : []), diff --git a/frontend/src/components/SingleModuleCard.tsx b/frontend/src/components/SingleModuleCard.tsx index 1b8eadaa9f..e16d7b74ab 100644 --- a/frontend/src/components/SingleModuleCard.tsx +++ b/frontend/src/components/SingleModuleCard.tsx @@ -129,6 +129,7 @@ const SingleModuleCard: React.FC = ({ module, accessLevel programKey={programKey} moduleKey={module.key} isAdmin={isAdmin} + isMentor={isMentor} /> )}
diff --git a/frontend/types/__generated__/EntityActions.generated.ts b/frontend/src/types/__generated__/EntityActions.generated.ts similarity index 95% rename from frontend/types/__generated__/EntityActions.generated.ts rename to frontend/src/types/__generated__/EntityActions.generated.ts index 28f4e997ba..3387fa7afd 100644 --- a/frontend/types/__generated__/EntityActions.generated.ts +++ b/frontend/src/types/__generated__/EntityActions.generated.ts @@ -1,4 +1,4 @@ -import * as Types from '../../src/types/__generated__/graphql'; +import * as Types from './graphql'; import { TypedDocumentNode as DocumentNode } from '@graphql-typed-document-node/core'; export type DeleteModuleMutationVariables = Types.Exact<{