added changes for edit and delete guides #28

ArikLeikin · 2024-08-28T13:59:47Z

No description provided.

nitzanto · 2024-08-28T18:28:22Z

src/app/(admin)/new_dashboard/shared/Card/index.tsx

Why was this file changed? This could affect a lot of pages in the app.

nitzanto · 2024-08-28T18:28:43Z

src/app/(admin)/new_dashboard/guides/hooks/guideClientHooks.ts

+export function useAddReview() {
+  return useMutation({
+    mutationFn: (review: CreateReviewDto) => guideClient.addReview(review),
+  });
+}


Didnt ori already have something for this?

@JosephOri

I do have exactly the same hook at reviewHooks.ts in the very same folder

nitzanto · 2024-08-28T18:30:01Z

src/app/(admin)/new_dashboard/guides/components/Editor/index.tsx

Why did we get rid of react hook form here? we were using it for validation

@JosephOri @roiy0987 please confirm

nitzanto · 2024-08-28T18:31:13Z

src/app/(admin)/new_dashboard/guides/components/DeleteEditButtons/index.tsx

+"use client";
+
+import React from "react";
+import { Box, IconButton } from "@mui/material";
+import ModeEditOutlineOutlinedIcon from "@mui/icons-material/ModeEditOutlineOutlined";
+import DeleteOutlinedIcon from "@mui/icons-material/DeleteOutlined";
+import { useDeleteGuide } from "../../hooks/guideClientHooks";
+import { useParams, useRouter } from "next/navigation";
+
+const DeleteEditButtons: React.FC = () => {
+  const router = useRouter();
+  const params = useParams();
+
+  const id = params?.id ? Number(params.id) : 0;
+  const deleteMutation = useDeleteGuide(id);
+
+  const handleDelete = () => {
+    deleteMutation.mutate();
+  };
+
+  const handleEdit = () => {
+    router.push(`/new_dashboard/guides/edit/${id}`);
+  };
+
+  return (
+    <Box
+      sx={{
+        display: "flex",
+        gap: 0,
+      }}
+    >
+      <IconButton aria-label="edit" onClick={handleEdit}>
+        <ModeEditOutlineOutlinedIcon />
+      </IconButton>
+      <IconButton aria-label="delete" onClick={handleDelete}>
+        <DeleteOutlinedIcon />
+      </IconButton>
+    </Box>
+  );
+};
+
+export default DeleteEditButtons;


Too specific. What if I want to add more buttons? You can just call it ButtonsGroup or something else.

Also make sure to add a confirmation dialog here when the user wants to delete the guide. Because he might do so by mistake and there's no check here.

nitzanto · 2024-08-28T18:31:54Z

src/api/types/Guide.ts

  reviews: z.array(reviewSchema).optional(),
 });

-export type Guide = z.infer<typeof guideSchema>;
-
-export type CreateGuideRequest = {
-  title: string;
-  contentHTML: string;
+export type UpdateGuidePayload = {
+  id: number;
+  guide: {
+    title: string;
+    contentHTML: string;
+  };
 };
+
+export type GuideRequestUpdate = Partial<Guide>;
+
+export type Guide = z.infer<typeof guideSchema>;


We were using zod for validation.

nitzanto · 2024-08-28T18:32:04Z

src/api/guide.client.ts

+  async addReview(review: CreateReviewDto) {
+    return await this.post<Review>("reviews", review);
+  }


@JosephOri didnt you already create this?

I was indeed created exactly this at review.client.ts

nitzanto · 2024-08-28T18:32:43Z

src/app/(admin)/new_dashboard/guides/hooks/guideClientHooks.ts

+    onError: () => {
+      alert("Failed to delete guide. Please try again.");
+    },


Why is there an alert here? We should display an alert with mui. There are lot of components that can be used to display this

nitzanto · 2024-08-28T18:33:02Z

src/app/(admin)/new_dashboard/guides/hooks/guideClientHooks.ts

+    onError: (error) => {
+      console.log(error);
+
+      alert("Failed to update guide. Please try again.");
+    },


You can use a mui toast to display the error or an alert component.

JosephOri · 2024-08-28T18:27:32Z

src/api/guide.client.ts

    return await this.get<Guide>("guides/" + guideId);
  }

-  async createGuide(createGuideRequest: CreateGuideRequest) {


why delete create guide function?

JosephOri · 2024-08-28T18:30:16Z

src/api/types/Guide.ts

-export type CreateGuideRequest = {
-  title: string;
-  contentHTML: string;
+export type UpdateGuidePayload = {


delete the update guide payload, we dont need it cause it can be partial type of Guide, as you did here

JosephOri · 2024-08-28T18:31:31Z

src/api/types/Guide.ts

+  };
 };
+
+export type GuideRequestUpdate = Partial<Guide>;


why use UpdateGuidePayload and not this const?

JosephOri · 2024-08-28T18:35:58Z

src/app/(admin)/new_dashboard/guides/components/Editor/index.tsx


 interface Props {
  initialTitle?: string;
+  initialIssue?: string;


what is initial issue for? now the issue will be changed to an array of categories, better to delete all the issue logic from here, when we merge the rest of the pr's all of this logic will be redundent, and we have written existing functions to handle the issue adding.

JosephOri · 2024-08-28T18:39:04Z

src/app/(admin)/new_dashboard/guides/components/Editor/index.tsx

    "align",
  ];

+  const handleTitleChange = (event: ChangeEvent<HTMLInputElement>) => {


better use a form approach like in the create guide page

JosephOri · 2024-08-28T18:45:32Z

src/app/(admin)/new_dashboard/guides/components/Editor/index.tsx

this component is basically almost a copy of the create guide page, it is better if you will use the create guide page with different props to determine if its an edit page of create page, you can extract the code in the create guide page into a component and use it with different props
https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

JosephOri · 2024-08-28T18:48:47Z

src/app/(admin)/new_dashboard/guides/hooks/guideClientHooks.ts

+export function useAddReview() {
+  return useMutation({
+    mutationFn: (review: CreateReviewDto) => guideClient.addReview(review),
+  });
+}


I do have exactly the same hook at reviewHooks.ts in the very same folder

JosephOri · 2024-08-28T18:50:58Z

src/app/(admin)/new_dashboard/shared/Card/index.tsx

  children?: JSX.Element;
  middlecontent?: string | JSX.Element;
  fullHeight?: boolean;
+  buttons?: JSX.Element;


if it supposed to be a react element name it with a capital letter, and give more explanatory name

added changes for edit and delete guides

95e9da3

ArikLeikin requested review from JosephOri, nitzanto and roiy0987 August 28, 2024 13:59

nitzanto requested changes Aug 28, 2024

View reviewed changes

JosephOri requested changes Aug 28, 2024

View reviewed changes

added changes for edit and delete guides #28

Are you sure you want to change the base?

added changes for edit and delete guides #28

Uh oh!

Conversation

ArikLeikin commented Aug 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JosephOri Aug 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JosephOri Aug 28, 2024 •

edited

Loading