Feature/crud user #35

binhtt13122000 · 2021-08-23T10:56:45Z

Create Pages for CRUD user and related components.
Link demo: https://drive.google.com/file/d/1eCwRU8OM5I3Pjcb9gL6odnwSqWAGHmLK/view?usp=sharing
Some feature: Update, change status, create, search by email, paging

zKirinz

Cool ^^

zKirinz · 2021-08-24T10:31:20Z

src/components/UserCard/index.tsx

+                <BoxBase pt={2} width={1} display="flex" justifyContent="flex-end">
+                    <BoxBase mr={1}>
+                        <ButtonBase
+                            onClick={() => userProps.onUpdate(userProps.userId)}


I think you need to test this behavior so as to get full coverage, right?

now, test coverages 100%, thank for your help!

zKirinz · 2021-08-24T10:33:14Z

src/pages/UpSertUser/UpSertPage/index.tsx

+interface IUpSertPageProps {
+    userProps?: IUser;
+    isUpdate: boolean;
+    sendDatatoServer: (user: IUser) => void;


I think it should be "sendDataToServer"

zKirinz · 2021-08-24T10:58:20Z

src/pages/UpSertUser/UpSertPage/index.tsx

+                        }}
+                        src={user.avatar || anonymousAvatarLink}
+                        style={avatarStyle}
+                    />


I think we should use AvatarBase component instead of img tag. AvatarBase already has a BoxBase outside of it. You can easily custom it to fit your need.

ok I will do it to reuse

zKirinz

Great work! But it will be better to have a record of live demo and the stories, don't you think?

binh1298 · 2021-08-29T04:00:32Z

src/components/Alerts/AlertBase/index.tsx

-const AlertBase = (props: AlertBaseProps) => {
-    return <MaterialAlert {...props} />;
-};
+const AlertBase = React.forwardRef(function AlertBase(props: AlertBaseProps, ref) {


Please add generic here

binh1298 · 2021-08-29T04:08:37Z

src/components/Images/ImageBase/index.tsx

+        sizeWidth = null,
+        sizeHeight = null,


I think we should set a default size here

Or you can force these props in the interface

I will ignore they in the interface

src/components/Images/ImageBase/index.tsx

binh1298 · 2021-08-29T04:19:04Z

src/components/Slide/SlideBase/index.tsx

+export interface SlideBaseProps extends MaterialSlideProps {}
+
+const SlideBase = React.forwardRef((props: SlideBaseProps, ref) => {
+    return <MaterialSlide ref={ref} {...props} mountOnEnter unmountOnExit />;


I think we should give other devs ways to customize mount behavior

binh1298 · 2021-08-29T04:19:11Z

src/components/Slide/SlideBase/index.tsx

+
+export interface SlideBaseProps extends MaterialSlideProps {}
+
+const SlideBase = React.forwardRef((props: SlideBaseProps, ref) => {


Please add generic here

I choose generic for Ref is unknown because it is set Ref in the root source.

binh1298 · 2021-08-29T04:20:15Z

src/components/UserCard/__test__/UserCard.test.tsx

+        render(<UserCard {...props} />);
+        const UserCardButtons = screen.getByTestId("UserCard__buttons").querySelectorAll("button");
+        const UpdateButton = UserCardButtons[0];
+        fireEvent.click(UpdateButton!);


Are you sure you don't need to await this?

I think await is not need, because in this code, click is set updatedId = userId. This action is not needed waiting for time to test so await is not need in this test

binh1298 · 2021-08-29T04:21:13Z

src/components/UserCard/index.tsx

+
+export const anonymousAvatarLink =
+    "https://cdn1.vectorstock.com/i/thumb-large/22/05/male-profile-picture-vector-1862205.jpg";
+const UserCard = (userProps: IUserProps) => {


I think we should put user's exclusive components in /pages/user or something

@kien123456k hello please help me check again

binh1298 · 2021-08-29T04:22:13Z

src/pages/UpSertUser/InsertPage/index.tsx

+
+    const sendDataToServer = (user: IUser) => {
+        mutate({
+            name: user.name || "",


I think we should validate instead of sending ""

src/pages/Users/index.tsx

binh1298 · 2021-08-29T04:24:42Z

src/pages/Users/hooks/useGetUsers.ts

+
+const useGetUser = (limit: number = 1, offset: number = 0, _ilike: string = "%%") => {
+    const queryClient = useQueryClient();
+    const result = useQuery(["GetUsersWithPagingQuery", { limit, offset, _ilike }], async () => {


Suggested change

const result = useQuery(["GetUsersWithPagingQuery", { limit, offset, _ilike }], async () => {

const result = useQuery(["GetUsersWithPagingQuery", limit, offset, _ilike ], async () => {

I will change it

src/pages/UpSertUser/hooks/useInsertUser.ts

binh1298 · 2021-08-29T04:25:59Z

src/pages/UpSertUser/UpdatePage/index.tsx

+
+    const sendDataToServer = (user: IUser) => {
+        mutate({
+            name: user.name || "",


Please validate instead of sending blank to db

…ic to needed component

…nent

… DrawerBase, DrawerTriggerButton, ImageBase, ListItemButtonBase and ListBase

…s after they are refactored

…routes

…r generated interface

…story

…serCard test

binh1298 · 2022-02-12T04:23:41Z

src/components/Buttons/ButtonBase/index.tsx

+            sx={{
+                borderRadius: "calc(1rem + 6px)",
+            }}


Why do we need this?

It is from the old code of @thienduc12311 to make the styling like in Reply design.

binh1298 · 2022-02-12T07:46:32Z

src/pages/Users/hooks/useUpdateUser.ts

+    options?: UseMutationOptions<UpdateUserMutation, GraphQLErrorType, UpdateUserMutationVariables>
+) => {
+    return useMutation<UpdateUserMutation, UpdateUserMutationVariables>({
+        queryKey: ["UpdateUser"],


this query key is missing something, do you know what it is?

binh1298 · 2022-02-12T07:50:13Z

src/components/Slide/SlideBase/index.tsx

+    unmountOnExit?: boolean;
+    mountOnEnter?: boolean;


I'm not sure if it's necessary to add this?

Okay, I removed it.

binh1298 · 2022-02-12T07:54:32Z

src/components/ButtonGroups/ButtonGroupBase/__test__/ButtonGroupBase.test.tsx

+describe("<ButtonGroupBase />", () => {
+    let props: ButtonGroupBaseProps;
+
+    it("should exist children with primary color", () => {


You're not testing for primary color here but your test says so

Okay, my bad.

binh1298 · 2022-02-12T07:54:48Z

src/common/types.ts

@@ -11,3 +12,11 @@ export type QueryInfo = {
    preloadedQuery: PreloadedQuery<OperationType>;
    queryObject: any;
 };
+
+export interface IUser {


Not sure why this is necessary?

binh1298 · 2022-02-12T07:55:41Z

src/pages/UpSertUser/UpSertPage/index.tsx

+                    </TypographyBase>
+                </BoxBase>
+            </BoxBase>
+            <BoxBase width={1} mt={5} display="flex" justifyContent="center" alignItems="center">


I think you should consider use grid for this one right?

binhtt13122000 added 7 commits August 20, 2021 09:42

fix: create hook to implement react query

b66cf30

create UserCard component and it's test

2013c09

do upsert page for users

e195d63

generate query and variable graphql

5787b69

update feature for user to view avatar in add/edit page

fa414b4

set type for onError of image

fb21b10

change loader to progress component

3ffb562

binhtt13122000 added the ready for review PR is ready to be reviewed label Aug 23, 2021

zKirinz reviewed Aug 24, 2021

View reviewed changes

binhtt13122000 added 3 commits August 27, 2021 08:45

add transition for snackbar to fix bug and rewrite test for UserCard

7108d54

add component image and it's test

37676cd

implement image for upsert screen

257c707

zKirinz reviewed Aug 27, 2021

View reviewed changes

binh1298 reviewed Aug 29, 2021

View reviewed changes

src/components/Images/ImageBase/index.tsx Show resolved Hide resolved

binh1298 reviewed Aug 29, 2021

View reviewed changes

src/pages/Users/index.tsx Outdated Show resolved Hide resolved

binh1298 reviewed Aug 29, 2021

View reviewed changes

src/pages/UpSertUser/hooks/useInsertUser.ts Outdated Show resolved Hide resolved

binh1298 reviewed Aug 29, 2021

View reviewed changes

binh1298 added require adjustment PR needs adjustments and removed ready for review PR is ready to be reviewed labels Aug 29, 2021

binhtt13122000 and others added 3 commits August 29, 2021 20:58

resolve comment in pull request: UserCard, logic ofgraphql, add gener…

9fd308d

…ic to needed component

add type for props in foward ref

7bfb910

setup props for Slide Base

0392810

zKirinz added 20 commits January 5, 2022 19:12

update muiv4 references to mui5 references

7f30810

fix using wrong BoxBase component path /Boxs instead of /Boxes

36461fa

use styled API of MuiV5 instead of style attribute on ImageBase compo…

5095b57

…nent

change styled MUIv5 API to sx props MUIV5 in AppBarBase, DividerBase,…

0c7a8cc

… DrawerBase, DrawerTriggerButton, ImageBase, ListItemButtonBase and ListBase

create CarcActionsBase component and its stories, tests

c3435f0

replace MUI components with base components in UserCard

203bbc3

refactor and rename useGetUserByID to useGetUserDetail hook

c891a4b

refactor and rename useGetUsers to useGetUsersList hook

3f36adf

refactor useInsertUser and useUpdateUser hook

235f6bf

update InsertPage, UpdatePage, UpsertPage and Users pages to fit hook…

8c4be85

…s after they are refactored

update graphql generated and move all user relative pages to private …

10a538e

…routes

change Scalars['uuid'] to IUser['userId'] type in UserCard component

eb1761f

refactor IUser interface from UserCard component to common/types

bd63390

change entity attributes in User relative pages from primitive to Use…

c03868c

…r generated interface

create ButtonGroupBase component and its tests

75d8480

change ButtonBase style to sx props and remove comment in ButtonBase …

1058fa3

…story

refactor ButtonsGroup component of User pages to PairButton component

99dd072

move PairButton out of ButtonGroups folder

0a9636e

refactor UserCard and Content components of User pages

d15bbe0

fix AlertBase, AppBarBase, ButtonGroupBase, DrawerTriggerButton and U…

dedd9cc

…serCard test

zKirinz added ready for review PR is ready to be reviewed and removed require adjustment PR needs adjustments labels Jan 17, 2022

binh1298 reviewed Feb 12, 2022

View reviewed changes

binh1298 added require adjustment PR needs adjustments and removed ready for review PR is ready to be reviewed labels Feb 12, 2022


		export interface SlideBaseProps extends MaterialSlideProps {}

		const SlideBase = React.forwardRef((props: SlideBaseProps, ref) => {

	const result = useQuery(["GetUsersWithPagingQuery", { limit, offset, _ilike }], async () => {
	const result = useQuery(["GetUsersWithPagingQuery", limit, offset, _ilike ], async () => {

Feature/crud user #35

Are you sure you want to change the base?

Feature/crud user #35

Conversation

binhtt13122000 commented Aug 23, 2021 • edited by zKirinz Loading

zKirinz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zKirinz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binhtt13122000 Aug 29, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binhtt13122000 commented Aug 23, 2021 •

edited by zKirinz

Loading

binhtt13122000 Aug 29, 2021 •

edited

Loading