Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(1263): Add purpose & status to task card, based on task card in website-my #1319

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
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');
});
});

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
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
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
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
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: #041187;
Saitharun279 marked this conversation as resolved.
Show resolved Hide resolved
padding: 0.5rem;
height: 2.5rem;
width: 10rem;
transition: 250ms ease-in-out;
}

.taskStatusUpdate:hover {
background-color: #041187;
Saitharun279 marked this conversation as resolved.
Show resolved Hide resolved
color: #ffffff;
}

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