Skip to content
Open
Show file tree
Hide file tree
Changes from 7 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
Copy Markdown

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
22 changes: 17 additions & 5 deletions src/components/tasks/card/TaskStatusEditMode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ 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';
Expand All @@ -14,6 +17,7 @@ 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,9 +37,11 @@ const TaskStatusEditMode = ({
task,
setEditedTaskDetails,
isDevMode,
isSelfTask,
}: Props) => {
const [saveStatus, setSaveStatus] = useState('');
const [updateTask] = useUpdateTaskMutation();
const [updateSelfTask] = useUpdateSelfTaskMutation();

const onChangeUpdateTaskStatus = ({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • can we update this function to use try catch to make this cleaner and easier to update? example:
const onChangeUpdateTaskStatus = async ({
        newStatus,
        newProgress,
    }: taskStatusUpdateHandleProp) => {
        setSaveStatus(PENDING);
        const payload: { status: string; percentCompleted?: number } = {
            status: newStatus,
        };

        if (newProgress !== undefined) {
            payload.percentCompleted = newProgress;
        }

        setEditedTaskDetails((prev: CardTaskDetails) => ({
            ...prev,
            ...payload,
        }));

        try{
            if (isDevMode && isSelfTask){
                await updateSelfTask({ id: task.id, task: payload })
            } else {
                await updateTask({ id: task.id, task: payload });
            }
        } catch(error){
            setSaveStatus(ERROR_STATUS);
        } finally {
            setTimeout(() => { 
                setSaveStatus('');
            }, 3000);
        }
    };
  • should we call setEditedTaskDetails after the API call is successful preventing any inconsistent states?
  • can we remove the setTimeout from the finally block?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Will update in next commit.

But not sure about removing setTimeout, will check and update that as well.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done

newStatus,
Expand All @@ -52,10 +58,16 @@ const TaskStatusEditMode = ({
...prev,
...payload,
}));
const response = updateTask({
id: task.id,
task: payload,
});
const response =
isDevMode && isSelfTask
? updateSelfTask({
id: task.id,
task: payload,
})
: updateTask({
id: task.id,
task: payload,
});

response
.unwrap()
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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: #041187;
Comment thread
Saitharun279 marked this conversation as resolved.
Outdated
padding: 0.5rem;
height: 2.5rem;
width: 10rem;
transition: 250ms ease-in-out;
}

.taskStatusUpdate:hover {
background-color: #041187;
Comment thread
Saitharun279 marked this conversation as resolved.
Outdated
color: #ffffff;
}

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