Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions backend/apps/mentorship/api/internal/mutations/module.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
import strawberry
from django.core.exceptions import ObjectDoesNotExist, PermissionDenied, ValidationError
from django.db import transaction
from django.db.utils import IntegrityError
from django.utils import timezone
from graphql import GraphQLError

from apps.github.models import User as GithubUser
from apps.mentorship.api.internal.nodes.module import (
Expand Down Expand Up @@ -114,18 +116,29 @@ def create_module(self, info: strawberry.Info, input_data: CreateModuleInput) ->
program.ended_at,
)

module = Module.objects.create(
name=input_data.name,
description=input_data.description,
experience_level=input_data.experience_level.value,
started_at=started_at,
ended_at=ended_at,
domains=input_data.domains,
labels=input_data.labels,
tags=input_data.tags,
program=program,
project=project,
)
try:
with transaction.atomic():
module = Module.objects.create(
name=input_data.name,
description=input_data.description,
experience_level=input_data.experience_level.value,
started_at=started_at,
ended_at=ended_at,
domains=input_data.domains,
labels=input_data.labels,
tags=input_data.tags,
program=program,
project=project,
)
except IntegrityError as e:
error_message = str(e)
if "unique_module_key_in_program" in error_message:
msg = "This module name already exists in this program."
raise GraphQLError(
msg,
extensions={"code": "VALIDATION_ERROR", "field": "name"},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see you using 'VALIDATION_ERROR' anywhere right now 🤔

) from e
raise

if module.experience_level not in program.experience_levels:
program.experience_levels.append(module.experience_level)
Expand Down Expand Up @@ -392,7 +405,18 @@ def update_module(self, info: strawberry.Info, input_data: UpdateModuleInput) ->
mentors_to_set = resolve_mentors_from_logins(input_data.mentor_logins)
module.mentors.set(mentors_to_set)

module.save()
try:
with transaction.atomic():
module.save()
except IntegrityError as e:
error_message = str(e)
if "unique_module_key_in_program" in error_message:
msg = "This module name already exists in this program."
raise GraphQLError(
msg,
extensions={"code": "VALIDATION_ERROR", "field": "name"},
) from e
raise

if module.experience_level not in module.program.experience_levels:
module.program.experience_levels.append(module.experience_level)
Expand Down
136 changes: 131 additions & 5 deletions frontend/__tests__/unit/components/ModuleForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ describe('ModuleForm', () => {
expect(mockOnSubmit).not.toHaveBeenCalled()
})

it('calls onSubmit when all fields are valid', () => {
it('calls onSubmit when all fields are valid', async () => {
const validFormData = {
...defaultFormData,
name: 'Valid Module Name',
Expand All @@ -481,11 +481,15 @@ describe('ModuleForm', () => {
renderModuleForm({ formData: validFormData })

const form = document.querySelector('form')
if (form) {
fireEvent.submit(form)
}
await act(async () => {
if (form) {
fireEvent.submit(form)
}
})

expect(mockOnSubmit).toHaveBeenCalled()
await waitFor(() => {
expect(mockOnSubmit).toHaveBeenCalled()
})
})

it('sets all fields as touched on submit', () => {
Expand Down Expand Up @@ -523,6 +527,128 @@ describe('ModuleForm', () => {
expect(screen.getByTestId('submit-button')).not.toBeDisabled()
})
})

describe('Mutation Error Display (validationErrors prop)', () => {
const validFormData = {
...defaultFormData,
name: 'Test Module',
description: 'A valid description that is long enough',
startedAt: '2024-01-01',
endedAt: '2024-12-31',
projectId: 'project-123',
projectName: 'My Project',
experienceLevel: 'BEGINNER',
}

it('displays mutation error for name field after submission', () => {
const { rerender } = render(
<ModuleForm
formData={validFormData}
setFormData={mockSetFormData}
onSubmit={mockOnSubmit}
loading={false}
title="Create Module"
/>
)

// Submit first to mark fields as touched
const form = document.querySelector('form')
fireEvent.submit(form!)

// Rerender with mutation errors (simulates page catching backend error)
rerender(
<ModuleForm
formData={validFormData}
setFormData={mockSetFormData}
onSubmit={mockOnSubmit}
loading={false}
title="Create Module"
validationErrors={{
name: 'This module name already exists in this program.',
}}
/>
)

expect(screen.getByTestId('module-name-error')).toHaveTextContent(
'This module name already exists in this program.'
)
})

it('does not display mutation error when validationErrors is empty', () => {
renderModuleForm({
formData: validFormData,
validationErrors: {},
})

expect(screen.queryByTestId('module-name-error')).not.toBeInTheDocument()
})

it('allows resubmission even when validationErrors.name is set', async () => {
const { rerender } = render(
<ModuleForm
formData={validFormData}
setFormData={mockSetFormData}
onSubmit={mockOnSubmit}
loading={false}
title="Create Module"
/>
)

// First submit to mark fields as touched
const form = document.querySelector('form')
fireEvent.submit(form!)
expect(mockOnSubmit).toHaveBeenCalledTimes(1)
mockOnSubmit.mockClear()

// Rerender with mutation errors
rerender(
<ModuleForm
formData={validFormData}
setFormData={mockSetFormData}
onSubmit={mockOnSubmit}
loading={false}
title="Create Module"
validationErrors={{
name: 'This module name already exists in this program.',
}}
/>
)

// Second submit should still call onSubmit so parent can clear errors and retry
fireEvent.submit(form!)

await waitFor(() => {
expect(mockOnSubmit).toHaveBeenCalledTimes(1)
})
})

it('allows submission when validationErrors has no name error', async () => {
renderModuleForm({
formData: validFormData,
validationErrors: {},
})

const form = document.querySelector('form')
fireEvent.submit(form!)

await waitFor(() => {
expect(mockOnSubmit).toHaveBeenCalled()
})
})

it('allows submission when validationErrors is undefined', async () => {
renderModuleForm({
formData: validFormData,
})

const form = document.querySelector('form')
fireEvent.submit(form!)

await waitFor(() => {
expect(mockOnSubmit).toHaveBeenCalled()
})
})
})
})

describe('ProjectSelector', () => {
Expand Down
21 changes: 8 additions & 13 deletions frontend/__tests__/unit/pages/CreateModule.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ describe('CreateModulePage', () => {
expect(moduleForm).toBeInTheDocument()
})
})
it('handles form submission error gracefully', async () => {
it('handles form submission error by displaying inline error', async () => {
const user = userEvent.setup({ delay: null })

;(useSession as jest.Mock).mockReturnValue({
Expand Down Expand Up @@ -300,16 +300,14 @@ describe('CreateModulePage', () => {

await waitFor(() => {
expect(mockCreateModuleError).toHaveBeenCalled()
expect(addToast).toHaveBeenCalledWith(
expect.objectContaining({
title: 'Creation Failed',
description: 'Network error',
color: 'danger',
})
// Error is displayed inline via validationErrors, not via addToast
expect(addToast).not.toHaveBeenCalledWith(
expect.objectContaining({ title: 'Creation Failed' })
)
})
})
it('handles non-Error submission failure', async () => {
it('handles non-Error submission failure via handleAppError', async () => {
const { handleAppError } = jest.requireMock('app/global-error')
const user = userEvent.setup({ delay: null })

;(useSession as jest.Mock).mockReturnValue({
Expand Down Expand Up @@ -353,11 +351,8 @@ describe('CreateModulePage', () => {

await waitFor(() => {
expect(mockCreateModuleError).toHaveBeenCalled()
expect(addToast).toHaveBeenCalledWith(
expect.objectContaining({
description: 'Something went wrong while creating the module.',
})
)
// Non-Error values fall through to handleAppError
expect(handleAppError).toHaveBeenCalledWith('String error')
})
}, 10000)
})
Loading