Skip to content

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

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.

Sign up for GitHub

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

Open
Saitharun279 wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
96602a6
feat: #1263 Added code changes for purpose field in task card
Saitharun279 Jan 8, 2025
8f34209
feat: Real-Dev-Squad#1263 | Added status field in Task Card
Saitharun279 Jan 13, 2025
c790c99
feat | issue#1263 prettier-fix
Saitharun279 Jan 16, 2025
045a8d6
feat | issue#1263 Added feature flag
Saitharun279 Jan 16, 2025
d615a30
Merge branch 'develop' into feature/issue#1263-purpose_status
Saitharun279 Jan 16, 2025
2088552
Merge branch 'develop' into feature/issue#1263-purpose_status
Saitharun279 Jan 17, 2025
90f78e4
feat:issue#1263 | Added unit tests
Saitharun279 Jan 18, 2025
a30aebe
feat/#1263 | Used existing css variables
Saitharun279 Jan 18, 2025
f89f1a8
feat/#1263 | Removed percentage check and renamed variable
Saitharun279 Jan 18, 2025
de52c89
feat:#1262 | Modifed task status update to use try catch and toast me…
Saitharun279 Jan 18, 2025
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
32 changes: 32 additions & 0 deletions src/components/tasks/TaskDropDown.tsx
View file
Open in desktop
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,37 @@ export default function TaskDropDown({
onChange(payload);
setMessage('');
};
if (isDevMode) {
return (
<>
<label className={styles.cardPurposeAndStatusFont}>
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
View file
Open in desktop
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

@yesyash yesyash Jan 18, 2025

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

@Saitharun279 Saitharun279 Jan 18, 2025

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

@Saitharun279 Saitharun279 Jan 18, 2025

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
View file
Open in desktop
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

@AnujChhikara AnujChhikara Jan 18, 2025

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

@Saitharun279 Saitharun279 Jan 18, 2025

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

@yesyash yesyash Jan 18, 2025

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

@Saitharun279 Saitharun279 Jan 18, 2025

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
95 changes: 75 additions & 20 deletions src/components/tasks/card/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
import { useGetUsersByUsernameQuery } from '@/app/services/usersApi';
import { ConditionalLinkWrapper } from './ConditionalLinkWrapper';
import useUserData from '@/hooks/useUserData';
import { useGetUserQuery } from '@/app/services/userApi';
import { isTaskDetailsPageLinkEnabled } from '@/constants/FeatureFlags';
import { useUpdateTaskMutation } from '@/app/services/tasksApi';
import ProgressIndicator from './progressContainer/ProgressIndicator';
Expand Down Expand Up @@ -75,6 +76,8 @@ const Card: FC<CardProps> = ({

const { isUserAuthorized } = useUserData();

const { data: userData } = useGetUserQuery();

const [showEditButton, setShowEditButton] = useState(false);

const [keyLongPressed] = useKeyLongPressed();
Expand Down Expand Up @@ -266,6 +269,10 @@ const Card: FC<CardProps> = ({
setIsEditMode(true);
};
const isEditable = shouldEdit && isUserAuthorized && isEditMode;
const isSelfTask = editedTaskDetails.assignee === userData?.username;
const verifiedTask =
Copy link
Copy Markdown
Contributor

@yesyash yesyash Jan 18, 2025

Choose a reason for hiding this comment

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

  • should this be named isTaskCompleted as we're doing a check on the percentage this task has be completed?

Copy link
Copy Markdown
Author

@Saitharun279 Saitharun279 Jan 18, 2025

Choose a reason for hiding this comment

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

  1. The dropdown to update task staus should only be shown for non verified tasks, this variable verifiedTask is to check if task status is verified.
  2. The check for 100 percent completion can be removed, as the status updation API allows update to Verified when percentage is 100.
    I will remove this percentage check in next commit.

I think the variable can be renamed to isVerifiedTask.

Copy link
Copy Markdown
Author

@Saitharun279 Saitharun279 Jan 18, 2025

Choose a reason for hiding this comment

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

Removed 100 percentage check and modified variable name to isVerifiedTask

editedTaskDetails.status === VERIFIED &&
editedTaskDetails.percentCompleted === 100;

const getFormattedClosedAtDate = () => {
const closedAt = cardDetails?.github?.issue?.closedAt;
Expand Down Expand Up @@ -543,7 +550,13 @@ const Card: FC<CardProps> = ({
</div>
)}
</div>
<div className={styles.taskStatusAndDateContainer}>
<div
className={
isDevMode
? styles.taskStatusDateAndPurposeContainer
: styles.taskStatusAndDateContainer
}
>
<div className={styles.dateInfo}>
<div className={styles.dateSection}>
<p className={styles.cardSpecialFont}>
Expand All @@ -569,26 +582,45 @@ const Card: FC<CardProps> = ({
: `Started ${getStartedAgo()}`}
</span>
</div>
{/*card purpose*/}
{isDevMode && isSelfTask && editedTaskDetails.purpose && (
<div>
<b className={styles.cardPurposeAndStatusFont}>
Purpose:{' '}
</b>
<span className={styles.cardPurposeText}>
{editedTaskDetails.purpose}
Copy link
Copy Markdown
Contributor

@AnujChhikara AnujChhikara Jan 18, 2025

Choose a reason for hiding this comment

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

do we have purpose field when we create the TCR from status site?

Copy link
Copy Markdown
Author

@Saitharun279 Saitharun279 Jan 18, 2025

Choose a reason for hiding this comment

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

I just checked, it is not present in the TCR.
I think it needs to be added.

</span>
</div>
)}

{/* EDIT task status */}
<div className={styles.taskStatusEditMode}>
{isEditable ? (
<TaskStatusEditMode
task={editedTaskDetails}
setEditedTaskDetails={setEditedTaskDetails}
isDevMode={isDevMode}
/>
) : (
<div className={styles.statusContainer} style={{}}>
<p className={styles.cardSpecialFont}>Status:</p>
<p
data-testid="task-status"
className={styles.statusText}
>
{beautifyStatus(cardDetails.status, isDevMode)}
</p>
</div>
)}
</div>
{!isDevMode && (
<div className={styles.taskStatusEditMode}>
{isEditable ? (
<TaskStatusEditMode
task={editedTaskDetails}
setEditedTaskDetails={setEditedTaskDetails}
isDevMode={isDevMode}
/>
) : (
<div className={styles.statusContainer} style={{}}>
<p className={styles.cardSpecialFont}>
Status:
</p>
<p
data-testid="task-status"
className={styles.statusText}
>
{beautifyStatus(
cardDetails.status,
isDevMode
)}
</p>
</div>
)}
</div>
)}
</div>

<div className={styles.contributor}>
Expand Down Expand Up @@ -630,6 +662,29 @@ const Card: FC<CardProps> = ({
{showAssignButton() && <AssigneeButton />}
</div>

{/* EDIT task status */}
{isDevMode && (
<div className={styles.taskStatusEditMode}>
{isEditable || (!verifiedTask && isSelfTask) ? (
<TaskStatusEditMode
task={editedTaskDetails}
setEditedTaskDetails={setEditedTaskDetails}
isDevMode={isDevMode}
isSelfTask={isSelfTask}
/>
) : (
<div className={styles.statusContainer} style={{}}>
<p className={styles.cardSpecialFont}>Status:</p>
<p
data-testid="task-status"
className={styles.statusText}
>
{beautifyStatus(cardDetails.status, isDevMode)}
</p>
</div>
)}
</div>
)}
<div className={styles.cardItems}>
<div
className={`${styles.taskTagLevelWrapper} ${
Expand Down