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

AssignTask to rtk #699

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"plugins": ["react", "@typescript-eslint"],
"rules": {
"linebreak-style": ["error", "unix"],
"quotes": ["error", "single"],
"quotes": ["error", "single", { "avoidEscape": true }],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding this rule here?
We should follow one

Copy link
Author

Choose a reason for hiding this comment

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

image
if i doesn't add it to eslint, eslint is trying to escape character & prettier is trying to avoid escaping characters, resulting in git push failure when i try to push changes

Copy link
Contributor

@prakashchoudhary07 prakashchoudhary07 Jul 18, 2023

Choose a reason for hiding this comment

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

In that case, we should fix the prettier and eslint configurations, rather than just adding some rule to make precommits pass

Copy link
Author

Choose a reason for hiding this comment

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

In that case, we should fix the prettier and eslint configurations, rather than just adding some rule to make precommits pass

i searched a lot to fix that issue but not found any solution for it, so i reverted my code to use defalut lint rules

Copy link
Contributor

@bhtibrewal bhtibrewal Jul 19, 2023

Choose a reason for hiding this comment

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

if you face an issue that is not linked to your PR, please create a separate issue ticket and PR for it

"semi": ["error", "always"],
"react/react-in-jsx-scope": "off",
"no-mixed-spaces-and-tabs": "off"
Expand Down
32 changes: 0 additions & 32 deletions __mocks__/handlers/self.handler.js

This file was deleted.

40 changes: 40 additions & 0 deletions __mocks__/handlers/self.handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { rest } from 'msw';
const URL = process.env.NEXT_PUBLIC_BASE_URL;

export const userSelfData = {
id: 'aTOG168A86JXY5wVosJx',
github_display_name: 'Mahima Khandelwal',
github_id: 'Maheima',
roles: {
member: true,
archived: false,
super_user: false,
},
last_name: 'Khandelwal',
username: 'mahima',
incompleteUserDetails: false,
profileStatus: 'BLOCKED',
picture: {
publicId: 'profile/aTOG168A86JXY5wVosJx/fj2c46kpmpy3gi8tl63s',
url: 'https://res.cloudinary.com/realdevsquad/image/upload/v1674639637/profile/aTOG168A86JXY5wVosJx/fj2c46kpmpy3gi8tl63s.jpg'
},
status: 'active',
first_name: 'Mahima',
profileURL: 'https://mahima-profile-service.onrender.com'
Comment on lines +5 to +23
Copy link
Member

Choose a reason for hiding this comment

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

Please use dummy data for test

Copy link
Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Where?

Copy link
Author

@WYGIN WYGIN Jul 18, 2023

Choose a reason for hiding this comment

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

Where?

That data is not created by me i refactored it tor DRY them up but now i replaced my tests with duplicate data in path __ tests __/Unit/hooks/tasksApi.test.tsx so no more tests depending on that data in my tests, the file is renamed from a js file to ts file https://github.com/Real-Dev-Squad/website-status/pull/699/files#diff-6e68b838da6a1b199fcb4ef878d985dd3e29de628b6bbbcdaca7cf99de329d3e

};

export const selfHandlerFn = (status: number, response: object | null) => {
return rest.get(`${URL}/users/self`, (_, res, ctx) => {
return res(
ctx.delay(500),
ctx.status(status),
ctx.json(response)
);
});
};

const selfHandler = [
selfHandlerFn(200, userSelfData)
];

export default selfHandler;
61 changes: 58 additions & 3 deletions __mocks__/handlers/tasks.handler.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { tasks, PAGINATED_TASKS } from '../db/tasks';
import usersData from '../../__mocks__/db/users';
import { rest } from 'msw';
const URL = process.env.NEXT_PUBLIC_BASE_URL;

Expand All @@ -12,8 +13,62 @@ const taskHandlers = [
})
);
}),
rest.patch(`${URL}/tasks/:taskId`, (_, res, ctx) => {
return res(ctx.delay(5000), ctx.status(204));
rest.patch(`${URL}/tasks/:taskId`, async (req, res, ctx) => {
const { taskId } = req.params;
const { assignee } = await req.json();
const task = tasks.find(t => t.id === taskId);
const user = usersData.find(u => u.username === assignee);
if(!assignee) return res(
ctx.status(204),
ctx.delay(5000),
);
const currentUserResponse = await fetch(`${URL}/users/self`);
if(!currentUserResponse.ok) return res(
ctx.status(403),
ctx.delay(100),
ctx.json({
error: 'Forbidden',
message: 'You are restricted from performing this action'
}),
);
const currentUser = await currentUserResponse.json();
if(!(currentUser.roles.super_user || currentUser.roles.app_owner)) return res(
ctx.status(401),
ctx.delay(100),
ctx.json({
error: 'Unauthorized',
message: 'You are not authorized for this action.'
}),
);
if(!task) return res(
ctx.status(404),
ctx.delay(100),
ctx.json({
message: 'Task not found',
error: 'Not Found',
})
);
if(!user) return res(
ctx.status(404),
ctx.delay(100),
ctx.json({
message: 'User doesn\'t exist',
error: 'Not Found',
})
);
if(user.status !== 'idle') return res(
ctx.delay(100),
ctx.status(404),
ctx.json({
message: 'Task cannot be assigned to users with active or OOO status',
})
);
return res(
ctx.json({
message: 'Task assigned',
Id: taskId,
}),
);
}),
rest.post(`${URL}/tasks`, async (req, res, ctx) => {
const body = await req.json();
Expand Down Expand Up @@ -87,4 +142,4 @@ export const paginatedTasksHandler = [
)
);
}),
];
];
18 changes: 18 additions & 0 deletions __mocks__/handlers/users.handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,24 @@ const usersHandler = [
}
})
);
}),
rest.get(`${URL}/users/:userId`, (req, res, ctx) => {
const { userId } = req.params;
const user = usersData.find(user => user.username === userId);
if(!user)
return res(
ctx.delay(150),
ctx.status(404),
ctx.json({
message: 'User doesn\'t exist',
error: 'Not Found'
})
);
return res(
ctx.delay(150),
ctx.status(200),
ctx.json(user)
);
})
];

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import DragDropContextWrapper from '@/components/availability-panel/drag-drop-context';
import { tasks, TASK } from '../../../../../__mocks__/db/tasks';
import { renderWithProviders } from '../../../../../src/test-utils/renderWithProvider';

export const urlPattern =
/(https:\/\/www\.|http:\/\/www\.|https:\/\/|http:\/\/)?[a-zA-Z]{2,}(\.[a-zA-Z]{2,})(\.[a-zA-Z]{2,})?\/[a-zA-Z0-9]{2,}|((https:\/\/www\.|http:\/\/www\.|https:\/\/|http:\/\/)?[a-zA-Z]{2,}(\.[a-zA-Z]{2,})(\.[a-zA-Z]{2,})?)|(https:\/\/www\.|http:\/\/www\.|https:\/\/|http:\/\/)?[a-zA-Z0-9]{2,}\.[a-zA-Z0-9]{2,}\.[a-zA-Z0-9]{2,}(\.[a-zA-Z0-9]{2,})?/g;
Copy link
Contributor

Choose a reason for hiding this comment

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

What pattern are we matching here?

Copy link
Author

@WYGIN WYGIN Jul 19, 2023

Choose a reason for hiding this comment

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

What pattern are we matching here?

i changed that pattern with a simple relative url pattern please check it

export const idleMemberFallbackText = 'No idle members found';
export const tasksFallbackText = 'No task found';

describe('DrogDropContextWrapper Component', () => {
describe('should show placeholder', () => {
test('when there is no task & members', () => {
const { queryAllByAltText, getByText } = renderWithProviders(
<DragDropContextWrapper
idleMembers={[]}
unAssignedTasks={[]}
refreshData={null}
/>
);
getByText(idleMemberFallbackText);
getByText(tasksFallbackText);
const ghostNodes = queryAllByAltText('ghost');
expect(ghostNodes).toHaveLength(2);
expect(ghostNodes[0]).toHaveAttribute(
'src',
expect.stringMatching(urlPattern)
);
expect(ghostNodes[1]).toHaveAttribute(
'src',
expect.stringMatching(urlPattern)
);
});

test('when there is no task', () => {
const { getByAltText, getByText } = renderWithProviders(
<DragDropContextWrapper
idleMembers={['Akash Shukla']}
unAssignedTasks={[]}
refreshData={null}
/>
);
expect(getByText(tasksFallbackText)).toBeVisible();
expect(getByAltText('ghost')).toHaveAttribute(
'src',
expect.stringMatching(urlPattern)
);
});

test('when there is no idleMemeber', () => {
const { getByAltText, getByText } = renderWithProviders(
<DragDropContextWrapper
idleMembers={[]}
unAssignedTasks={tasks}
refreshData={null}
/>
);
expect(getByText(idleMemberFallbackText)).toBeVisible();
expect(getByAltText('ghost')).toHaveAttribute(
'src',
expect.stringMatching(urlPattern)
);
});
});

test('should render all the tasks & members as expected', () => {
const { queryAllByText, getByText } = renderWithProviders(
<DragDropContextWrapper
idleMembers={['Akash Shukla']}
unAssignedTasks={tasks}
refreshData={null}
/>
);
const unAssignedTaskNodes = queryAllByText(TASK.title);
expect(unAssignedTaskNodes).toHaveLength(10);
expect(unAssignedTaskNodes[0]).toBeVisible();
expect(unAssignedTaskNodes[6]).toBeVisible();
getByText('Akash Shukla');
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't understand why is this written at the end.

Copy link
Author

@WYGIN WYGIN Jul 19, 2023

Choose a reason for hiding this comment

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

didn't understand why is this written at the end.

That line is to test if the member is in document or not. It will throw an error if the member is not in document or if that same member is in document for more than one time, resulting in test failure AFAIK

I would also add toBeVisible in my next commit so that it assures that component is rendering member

});

test('should have tasks & members items draggable', () => {
const { container } = renderWithProviders(
<DragDropContextWrapper
idleMembers={['Akash Shukla']}
unAssignedTasks={tasks}
refreshData={null}
/>
);
const taskList = container.querySelectorAll('.memberCard');
expect(taskList[0].getAttribute('draggable')).toBeTruthy();
});
});
Loading
Loading