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
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ export const AddTaskdialog = ({
setNewTask({ ...newTask, project: value });
}
}}
data-testid="project-select"
>
<SelectTrigger
onKeyDown={(e) => {
Expand All @@ -284,7 +285,6 @@ export const AddTaskdialog = ({
}}
ref={(element) => (inputRefs.current.project = element)}
id="project"
data-testid="project-select"
>
<SelectValue
placeholder={
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/components/HomeComponents/Tasks/TaskDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ export const TaskDialog = ({
editedPriority: value,
})
}
data-testid="priority-select"
>
<SelectTrigger className="flex-grow mr-2">
<SelectValue placeholder="Select priority" />
Expand Down Expand Up @@ -1096,11 +1097,9 @@ export const TaskDialog = ({
onSaveProject(task, project);
}
}}
data-testid="project-select"
>
<SelectTrigger
id="project"
data-testid="project-select"
>
<SelectTrigger id="project">
<SelectValue
placeholder={
uniqueProjects.length
Expand Down Expand Up @@ -1374,6 +1373,7 @@ export const TaskDialog = ({
onValueChange={(value) =>
onUpdateState({ editedRecur: value })
}
data-testid="recur-select"
>
<SelectTrigger className="flex-grow">
<SelectValue placeholder="Select recurrence" />
Expand Down
221 changes: 217 additions & 4 deletions frontend/src/components/HomeComponents/Tasks/__tests__/Tasks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ jest.mock('@/components/ui/multi-select', () => ({

jest.mock('@/components/ui/select', () => {
return {
Select: ({ children, onValueChange, value }: any) => {
Select: ({ children, onValueChange, value, ...props }: any) => {
return (
<select
data-testid="project-select"
Copy link
Collaborator

Choose a reason for hiding this comment

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

these data-testid related changes are not really desired.
should be reverted

{...props}
value={value}
onChange={(e) => onValueChange?.(e.target.value)}
>
Expand Down Expand Up @@ -182,6 +182,12 @@ jest.mock('../TaskSkeleton', () => {

global.fetch = jest.fn().mockResolvedValue({ ok: true });

global.ResizeObserver = class ResizeObserver {
observe() {}
unobserve() {}
disconnect() {}
};

describe('Tasks Component', () => {
const localStorageMock = (() => {
let store: { [key: string]: string } = {};
Expand Down Expand Up @@ -1229,7 +1235,7 @@ describe('Tasks Component', () => {
const editButton = within(priorityRow).getByLabelText('edit');
fireEvent.click(editButton);

const select = within(priorityRow).getByTestId('project-select');
const select = within(priorityRow).getByTestId('priority-select');
fireEvent.change(select, { target: { value: 'H' } });

const saveButton = screen.getByLabelText('save');
Expand Down Expand Up @@ -1339,7 +1345,7 @@ describe('Tasks Component', () => {
const editButton = within(recurRow).getByLabelText('edit');
fireEvent.click(editButton);

const select = within(recurRow).getByTestId('project-select');
const select = within(recurRow).getByTestId('recur-select');
fireEvent.change(select, { target: { value: 'weekly' } });

const saveButton = screen.getByLabelText('save');
Expand Down Expand Up @@ -1737,4 +1743,211 @@ describe('Tasks Component', () => {
expect(task1Row).toBeInTheDocument();
});
});

describe('Recur Editing', () => {
test('does not save when recur is set to "none"', async () => {
render(<Tasks {...mockProps} />);

await screen.findByText('Task 12');
fireEvent.click(screen.getByText('Task 12'));

expect(await screen.findByText('Recur:')).toBeInTheDocument();

const recurLabel = screen.getByText('Recur:');
const recurRow = recurLabel.closest('tr') as HTMLElement;
const editButton = within(recurRow).getByLabelText('edit');

fireEvent.click(editButton);

const select = within(recurRow).getByTestId('recur-select');
fireEvent.change(select, { target: { value: 'none' } });

const saveButton = screen.getByLabelText('save');
fireEvent.click(saveButton);

const hooks = require('../hooks');
expect(hooks.editTaskOnBackend).not.toHaveBeenCalled();
});

test('saves recur when a valid value is selected', async () => {
render(<Tasks {...mockProps} />);

await screen.findByText('Task 12');
fireEvent.click(screen.getByText('Task 12'));

expect(await screen.findByText('Recur:')).toBeInTheDocument();

const recurLabel = screen.getByText('Recur:');
const recurRow = recurLabel.closest('tr') as HTMLElement;
const editButton = within(recurRow).getByLabelText('edit');

fireEvent.click(editButton);

const select = within(recurRow).getByTestId('recur-select');
fireEvent.change(select, { target: { value: 'daily' } });

const saveButton = screen.getByLabelText('save');
fireEvent.click(saveButton);

const hooks = require('../hooks');
expect(hooks.editTaskOnBackend).toHaveBeenCalledWith(
expect.objectContaining({ recur: 'daily' })
);
});

test('does not save when recur is empty string', async () => {
render(<Tasks {...mockProps} />);

await screen.findByText('Task 12');
fireEvent.click(screen.getByText('Task 12'));

expect(await screen.findByText('Recur:')).toBeInTheDocument();

const recurLabel = screen.getByText('Recur:');
const recurRow = recurLabel.closest('tr') as HTMLElement;
const editButton = within(recurRow).getByLabelText('edit');
fireEvent.click(editButton);

const select = within(recurRow).getByTestId('recur-select');
fireEvent.change(select, { target: { value: '' } });
const saveButton = within(recurRow).getByLabelText('save');
fireEvent.click(saveButton);

const hooks = require('../hooks');
expect(hooks.editTaskOnBackend).not.toHaveBeenCalled();
});
});

describe('Priority Editing', () => {
test('saving priority calls modifyTaskOnBackend with correct value', async () => {
render(<Tasks {...mockProps} />);

await screen.findByText('Task 12');
fireEvent.click(screen.getByText('Task 12'));

expect(await screen.findByText('Priority:')).toBeInTheDocument();

const priorityLabel = screen.getByText('Priority:');
const priorityRow = priorityLabel.closest('tr') as HTMLElement;
const editButton = within(priorityRow).getByLabelText('edit');
fireEvent.click(editButton);

const select = within(priorityRow).getByTestId('priority-select');
fireEvent.change(select, { target: { value: 'H' } });

const saveButton = screen.getByLabelText('save');
fireEvent.click(saveButton);

await waitFor(() => {
const hooks = require('../hooks');
expect(hooks.modifyTaskOnBackend).toHaveBeenCalledWith(
expect.objectContaining({ priority: 'H' })
);
});
});

test('saving "NONE" priority sends empty string to backend', async () => {
render(<Tasks {...mockProps} />);

await screen.findByText('Task 12');
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have too many tests, some are vague as well, and it will be even more hard to distinguish later.
Do move blocks like these to separate functions

await screen.findByText('Task 12');
fireEvent.click(screen.getByText('Task 12'));
expect(await screen.findByText('Priority:')).toBeInTheDocument();

Copy link
Contributor Author

@ShivaGupta-14 ShivaGupta-14 Jan 24, 2026

Choose a reason for hiding this comment

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

got it! is this good structure, should i proceed with this

frontend/src/components/HomeComponents/Tasks/
├── __tests__/
   ├── setup.ts       ->  mocks
   ├── helpers.ts     ->  shared utilities
   ├── Tasks.test.tsx
   ├── Tasks.keyboard.test.tsx
   ├── Tasks.priority.test.tsx
   ├── Tasks.recur.test.tsx
   ├── Tasks.reports.test.tsx
   ├── Tasks.annotations.test.tsx

I'll extract repeated code (like opening task dialog) into helpers.ts.

  • Also, could you point out which tests you found vague? I'd like to improve those

fireEvent.click(screen.getByText('Task 12'));

expect(await screen.findByText('Priority:')).toBeInTheDocument();

const priorityLabel = screen.getByText('Priority:');
const priorityRow = priorityLabel.closest('tr') as HTMLElement;
const editButton = within(priorityRow).getByLabelText('edit');
fireEvent.click(editButton);

const select = within(priorityRow).getByTestId('priority-select');
fireEvent.change(select, { target: { value: 'NONE' } });

const saveButton = screen.getByLabelText('save');
fireEvent.click(saveButton);

await waitFor(() => {
const hooks = require('../hooks');
expect(hooks.modifyTaskOnBackend).toHaveBeenCalledWith(
expect.objectContaining({
priority: '',
})
);
});
});

test('shows error toast when priority save fails', async () => {
const { toast } = require('react-toastify');
const hooks = require('../hooks');
Copy link
Collaborator

Choose a reason for hiding this comment

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

imo just move this to the top, its being reused a lot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, i will


hooks.modifyTaskOnBackend.mockRejectedValueOnce(
new Error('Network error')
);

render(<Tasks {...mockProps} />);
await screen.findByText('Task 12');

fireEvent.click(screen.getByText('Task 12'));

await waitFor(() => {
expect(screen.getByText('Priority:')).toBeInTheDocument();
});

const priorityLabel = screen.getByText('Priority:');
const priorityRow = priorityLabel.closest('tr') as HTMLElement;

const editButton = within(priorityRow).getByLabelText('edit');
fireEvent.click(editButton);

const select = within(priorityRow).getByTestId('priority-select');
fireEvent.change(select, { target: { value: 'H' } });

const saveButton = within(priorityRow).getByLabelText('save');
fireEvent.click(saveButton);

await waitFor(() => {
expect(toast.error).toHaveBeenCalledWith(
expect.stringContaining('Failed to update priority')
);
});
});
});

describe('Reports Toggle', () => {
test('clicking "Show Reports" button switches view from tasks to reports', async () => {
render(<Tasks {...mockProps} />);

await screen.findByText('Task 1');

expect(screen.getByText('Here are')).toBeInTheDocument();

const toggleBtn = screen.getByRole('button', { name: /show reports/i });
fireEvent.click(toggleBtn);

expect(
screen.getByRole('button', { name: /show tasks/i })
).toBeInTheDocument();
});

test('clicking "Show Tasks" returns to task list view', async () => {
render(<Tasks {...mockProps} />);
await screen.findByText('Task 1');

fireEvent.click(screen.getByRole('button', { name: /show reports/i }));
fireEvent.click(screen.getByRole('button', { name: /show tasks/i }));

expect(screen.getByText('Task 1')).toBeInTheDocument();
});

test('hotkeys are disabled when reports view is shown', async () => {
render(<Tasks {...mockProps} />);
await screen.findByText('Task 1');

fireEvent.click(screen.getByRole('button', { name: /show reports/i }));
fireEvent.keyDown(window, { key: 'a' });

expect(
screen.queryByText(/fill in the details below/i)
).not.toBeInTheDocument();
});
});
});
Loading