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
48 changes: 47 additions & 1 deletion __tests__/Unit/Components/Tasks/Card.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const DEFAULT_PROPS = {
startedOn: '1618790400',
isNoteworthy: true,
title: 'test 1 for drag and drop',
purpose: 'string',
purpose: 'to test purpose',
percentCompleted: 0,
endsOn: 1618790400,
status: COMPLETED,
Expand All @@ -54,9 +54,55 @@ const DEFAULT_PROPS = {
onContentChange: jest.fn(),
};

describe("Task card, self task's purpose and status", () => {
it('renders the card with title and status', () => {
renderWithRouter(
<Provider store={store()}>
<Card {...DEFAULT_PROPS} />
</Provider>,
{
query: { dev: 'true' },
}
);
expect(
screen.getByText('test 1 for drag and drop')
).toBeInTheDocument();
expect(screen.getByText('Done')).toBeInTheDocument();
});

it('displays the purpose if provided', () => {
renderWithRouter(
<Provider store={store()}>
<Card {...DEFAULT_PROPS} />
</Provider>,
{
query: { dev: 'true' },
}
);
expect(screen.getByText('to test purpose')).toBeInTheDocument();
});

it('does not display the purpose if not provided', () => {
const PROPS_WITHOUT_PURPOSE = {
...DEFAULT_PROPS,
content: { ...DEFAULT_PROPS.content, purpose: '' },
};
renderWithRouter(
<Provider store={store()}>
<Card {...PROPS_WITHOUT_PURPOSE} />
</Provider>,
{
query: { dev: 'true' },
}
);
expect(screen.queryByText('to test purpose')).not.toBeInTheDocument();
});
});

jest.mock('@/hooks/useUserData', () => {
return () => ({
data: {
username: 'ankur',
roles: {
admin: true,
super_user: true,
Expand Down
23 changes: 23 additions & 0 deletions __tests__/Unit/Components/Tasks/TaskDropDown.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,29 @@ describe('TaskDropDown', () => {
expect(onChange).toHaveBeenCalledTimes(0);
expect(screen.getByTestId('task-status')).toHaveValue(oldStatus);
});

it('should render cardPurposeAndStatusFont for label and taskStatusUpdate for select when isDevMode is true', () => {
const oldProgress = 100;
const oldStatus = BACKEND_TASK_STATUS.NEEDS_REVIEW;
const TASK_STATUS_UPDATE = 'taskStatusUpdate';
const CARD_PURPOSE_STATUS_FONT = 'cardPurposeAndStatusFont';

render(
<TaskDropDown
isDevMode={true}
oldProgress={oldProgress}
oldStatus={oldStatus}
onChange={onChange}
/>
);
expect(screen.getByTestId('task-status')).toHaveClass(
TASK_STATUS_UPDATE
);
expect(screen.getByTestId('task-status-label')).toHaveClass(
CARD_PURPOSE_STATUS_FONT
);
});

it('should not show any model info on change of status from in progress to backlog', () => {
const oldProgress = 80;
const oldStatus = BACKEND_TASK_STATUS.IN_PROGRESS;
Expand Down
23 changes: 23 additions & 0 deletions __tests__/Unit/Components/Tasks/TaskStatusEditMode.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ const BLOCKED_TASK = {

describe('TaskStatusEditMode', () => {
let updateTaskSpy: any;
let updateSelfTaskSpy: any;
beforeEach(() => {
updateTaskSpy = jest.spyOn(tasksApi, 'useUpdateTaskMutation');
updateSelfTaskSpy = jest.spyOn(tasksApi, 'useUpdateSelfTaskMutation');
});

afterEach(() => {
Expand Down Expand Up @@ -173,6 +175,27 @@ describe('TaskStatusEditMode', () => {
expect(screen.getByTestId('error')).toBeInTheDocument();
});
});

it('change task status from BLOCKED to IN_PROGRESS when and isDevMode are true', async () => {
const setEditedTaskDetails = jest.fn();

renderWithRouter(
<Provider store={store()}>
<TaskStatusEditMode
task={BLOCKED_TASK}
setEditedTaskDetails={setEditedTaskDetails}
isDevMode={true}
isSelfTask={true}
/>
</Provider>
);

const statusSelect = screen.getByLabelText('Status:');
expect(statusSelect).toHaveValue('BLOCKED');

fireEvent.change(statusSelect, { target: { value: 'IN_PROGRESS' } });
expect(statusSelect).toHaveValue('IN_PROGRESS');
});
Comment on lines +179 to +198
Copy link

Choose a reason for hiding this comment

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

This test verifies the UI state changes but doesn't confirm the actual API call functionality. Consider adding an assertion to verify that updateSelfTaskMutation is called with the correct parameters:

expect(updateSelfTaskSpy).toHaveBeenCalledWith({ 
  id: BLOCKED_TASK.id, 
  task: expect.objectContaining({ status: 'IN_PROGRESS' }) 
});

This would ensure the status update is properly propagated to the API layer, not just reflected in the UI component.

Suggested change
it('change task status from BLOCKED to IN_PROGRESS when and isDevMode are true', async () => {
const setEditedTaskDetails = jest.fn();
renderWithRouter(
<Provider store={store()}>
<TaskStatusEditMode
task={BLOCKED_TASK}
setEditedTaskDetails={setEditedTaskDetails}
isDevMode={true}
isSelfTask={true}
/>
</Provider>
);
const statusSelect = screen.getByLabelText('Status:');
expect(statusSelect).toHaveValue('BLOCKED');
fireEvent.change(statusSelect, { target: { value: 'IN_PROGRESS' } });
expect(statusSelect).toHaveValue('IN_PROGRESS');
});
it('change task status from BLOCKED to IN_PROGRESS when isSelfTask and isDevMode are true', async () => {
const setEditedTaskDetails = jest.fn();
const updateSelfTaskSpy = jest.fn().mockResolvedValue({});
jest.spyOn(taskHooks, 'useUpdateSelfTaskMutation').mockReturnValue([updateSelfTaskSpy, { isLoading: false }]);
renderWithRouter(
<Provider store={store()}>
<TaskStatusEditMode
task={BLOCKED_TASK}
setEditedTaskDetails={setEditedTaskDetails}
isDevMode={true}
isSelfTask={true}
/>
</Provider>
);
const statusSelect = screen.getByLabelText('Status:');
expect(statusSelect).toHaveValue('BLOCKED');
fireEvent.change(statusSelect, { target: { value: 'IN_PROGRESS' } });
expect(statusSelect).toHaveValue('IN_PROGRESS');
expect(updateSelfTaskSpy).toHaveBeenCalledWith({
id: BLOCKED_TASK.id,
task: expect.objectContaining({ status: 'IN_PROGRESS' })
});
});

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

});

describe('test beautifyStatus function', () => {
Expand Down
35 changes: 35 additions & 0 deletions src/components/tasks/TaskDropDown.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useState } from 'react';
import { BACKEND_TASK_STATUS } from '@/constants/task-status';
import { beautifyStatus } from './card/TaskStatusEditMode';
import styles from '@/components/tasks/card/card.module.scss';

import { MSG_ON_0_PROGRESS, MSG_ON_100_PROGRESS } from '@/constants/constants';
import TaskDropDownModel from './TaskDropDownModel';
Expand Down Expand Up @@ -102,6 +103,40 @@ export default function TaskDropDown({
onChange(payload);
setMessage('');
};
if (isDevMode) {
return (
<>
<label
className={styles.cardPurposeAndStatusFont}
data-testid="task-status-label"
>
Status:
<select
className={styles.taskStatusUpdate}
data-testid="task-status"
name="status"
onChange={handleChange}
value={newStatus}
>
{taskStatus.map(([name, status]) => (
<option
data-testid={`task-status-${name}`}
key={status}
value={status}
>
{beautifyStatus(name, isDevMode)}
</option>
))}
</select>
</label>
<TaskDropDownModel
message={message}
resetProgressAndStatus={resetProgressAndStatus}
handleProceed={handleProceed}
/>
</>
);
}
return (
<>
<label>
Expand Down
35 changes: 31 additions & 4 deletions src/components/tasks/card/TaskStatusEditMode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,24 @@ import task, {
import { useState } from 'react';
import styles from '@/components/tasks/card/card.module.scss';
import { PENDING, SAVED, ERROR_STATUS } from '../constants';
import { useUpdateTaskMutation } from '@/app/services/tasksApi';
import {
useUpdateTaskMutation,
useUpdateSelfTaskMutation,
} from '@/app/services/tasksApi';
import { StatusIndicator } from './StatusIndicator';
import TaskDropDown from '../TaskDropDown';
import { TASK_STATUS_MAPING } from '@/constants/constants';
import {
STATUS_UPDATE_ERROR,
STATUS_UPDATE_SUCCESSFUL,
TASK_STATUS_MAPING,
} from '@/constants/constants';
import { toast, ToastTypes } from '@/helperFunctions/toast';

type Props = {
task: task;
setEditedTaskDetails: React.Dispatch<React.SetStateAction<CardTaskDetails>>;
isDevMode?: boolean;
isSelfTask?: boolean;
};

// TODO: remove this after fixing the card beautify status
Expand All @@ -33,11 +42,14 @@ const TaskStatusEditMode = ({
task,
setEditedTaskDetails,
isDevMode,
isSelfTask,
}: Props) => {
const [saveStatus, setSaveStatus] = useState('');
const [updateTask] = useUpdateTaskMutation();
const [updateSelfTask] = useUpdateSelfTaskMutation();
const { SUCCESS, ERROR } = ToastTypes;

const onChangeUpdateTaskStatus = ({
const onChangeUpdateTaskStatus = async ({
newStatus,
newProgress,
}: taskStatusUpdateHandleProp) => {
Expand All @@ -48,6 +60,21 @@ const TaskStatusEditMode = ({
if (newProgress !== undefined) {
payload.percentCompleted = newProgress;
}

try {
if (isDevMode && isSelfTask) {
await updateSelfTask({ id: task.id, task: payload }).unwrap();
}
toast(SUCCESS, STATUS_UPDATE_SUCCESSFUL);
Comment on lines +64 to +68
Copy link

Choose a reason for hiding this comment

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

The success toast is displayed unconditionally, regardless of whether updateSelfTask was executed or successful. Consider moving the toast inside the if block after the await call to ensure it only appears when the operation actually succeeds:

try {
    if (isDevMode && isSelfTask) {
        await updateSelfTask({ id: task.id, task: payload }).unwrap();
        toast(SUCCESS, STATUS_UPDATE_SUCCESSFUL);
    }
}

This way, the success message will only be shown when the task update operation is both attempted and completed successfully.

Suggested change
try {
if (isDevMode && isSelfTask) {
await updateSelfTask({ id: task.id, task: payload }).unwrap();
}
toast(SUCCESS, STATUS_UPDATE_SUCCESSFUL);
try {
if (isDevMode && isSelfTask) {
await updateSelfTask({ id: task.id, task: payload }).unwrap();
toast(SUCCESS, STATUS_UPDATE_SUCCESSFUL);
}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

} catch (error) {
const errorMessage =
error && typeof error === 'object' && 'data' in error
? (error.data as { message?: string }).message ||
STATUS_UPDATE_ERROR
: STATUS_UPDATE_ERROR;
toast(ERROR, errorMessage);
}

setEditedTaskDetails((prev: CardTaskDetails) => ({
...prev,
...payload,
Expand Down Expand Up @@ -80,7 +107,7 @@ const TaskStatusEditMode = ({
oldProgress={task.percentCompleted}
onChange={onChangeUpdateTaskStatus}
/>
<StatusIndicator status={saveStatus} />
{!isDevMode && <StatusIndicator status={saveStatus} />}
</div>
);
};
Expand Down
37 changes: 37 additions & 0 deletions src/components/tasks/card/card.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@
color: #aeaeae;
}

.cardPurposeAndStatusFont {
font-size: 1.1rem;
color: #555;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a predefined color from variables.scss here?

Copy link
Author

Choose a reason for hiding this comment

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

All the colors that I used are not present in variables.css. I have taken them from my site.
should i add them in variables.scss ?

}
.cardPurposeText {
padding: 8px;
color: #aeaeae;
font-size: 1rem;
}

.cardStatusFont {
font-size: 1.3rem;
font-weight: 500;
Expand Down Expand Up @@ -247,10 +257,37 @@
justify-content: space-between;
}

.taskStatusDateAndPurposeContainer {
display: grid;
align-items: baseline;
grid-template-columns: 2fr 3fr;
gap: 2rem;
grid-auto-flow: column;
margin-bottom: 1rem;
}

.taskStatusEditMode {
margin-top: 0.8rem;
}

.taskStatusUpdate {
border: 1px solid #000000b3;
Copy link
Contributor

Choose a reason for hiding this comment

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

  • are there existing css variables we can use instead of hardcoding the color?

Copy link
Author

Choose a reason for hiding this comment

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

Didn't find any existing css variables for this one.

border-radius: 4px;
font-size: inherit;
font-weight: bolder;
background-color: transparent;
color: $theme-primary-alt;
padding: 0.5rem;
height: 2.5rem;
width: 10rem;
transition: 250ms ease-in-out;
}

.taskStatusUpdate:hover {
background-color: $theme-primary-alt;
color: #ffffff;
}

.taskTagLevelWrapper {
display: flex;
padding-top: 0.5rem;
Expand Down
Loading