-
Notifications
You must be signed in to change notification settings - Fork 0
added changes for edit and delete guides #28
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import { CreateReviewDto } from "@/app/(admin)/new_dashboard/guides/dto/CreateReviewDto"; | ||
| import { BaseClient } from "./base.client"; | ||
| import { CreateGuideRequest, Guide } from "./types/Guide"; | ||
| import { Guide, GuideRequestUpdate } from "./types/Guide"; | ||
| import { Review } from "./types/Review"; | ||
|
|
||
| export class GuideClient extends BaseClient { | ||
|
|
@@ -12,7 +12,15 @@ export class GuideClient extends BaseClient { | |
| return await this.get<Guide>("guides/" + guideId); | ||
| } | ||
|
|
||
| async createGuide(createGuideRequest: CreateGuideRequest) { | ||
| return await this.post<Guide>("guides", createGuideRequest); | ||
| async deleteGuide(guideId: number) { | ||
| return await this.delete<Guide>("guides/" + guideId); | ||
| } | ||
|
|
||
| async addReview(review: CreateReviewDto) { | ||
| return await this.post<Review>("reviews", review); | ||
| } | ||
|
Comment on lines
+19
to
+21
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JosephOri didnt you already create this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was indeed created exactly this at |
||
|
|
||
| async updateGuide(id: number, guide: GuideRequestUpdate) { | ||
| return await this.patch<Guide>("guides/" + id, guide); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,9 +12,14 @@ const guideSchema = z.object({ | |
| reviews: z.array(reviewSchema).optional(), | ||
| }); | ||
|
|
||
| export type Guide = z.infer<typeof guideSchema>; | ||
|
|
||
| export type CreateGuideRequest = { | ||
| title: string; | ||
| contentHTML: string; | ||
| export type UpdateGuidePayload = { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. delete the update guide payload, we dont need it cause it can be partial type of Guide, as you did here |
||
| id: number; | ||
| guide: { | ||
| title: string; | ||
| contentHTML: string; | ||
| }; | ||
| }; | ||
|
|
||
| export type GuideRequestUpdate = Partial<Guide>; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why use |
||
|
|
||
| export type Guide = z.infer<typeof guideSchema>; | ||
|
Comment on lines
12
to
+25
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We were using zod for validation. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| "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; | ||
|
Comment on lines
+1
to
+42
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did we get rid of react hook form here? we were using it for validation @JosephOri @roiy0987 please confirm
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,26 @@ | ||
| import dynamic from "next/dynamic"; | ||
| import { useState } from "react"; | ||
| import { ChangeEvent, useState } from "react"; | ||
| import "react-quill/dist/quill.snow.css"; | ||
| import { Box, Button, TextField } from "@mui/material"; | ||
| import SaveAsIcon from "@mui/icons-material/SaveAs"; | ||
| import { UseFormRegister, UseFormSetValue } from "react-hook-form"; | ||
| import { CreateGuideRequest } from "@/api/types/Guide"; | ||
|
|
||
| const DynamicReactQuill = dynamic(() => import("react-quill")); | ||
|
|
||
| interface Props { | ||
| initialTitle?: string; | ||
| initialIssue?: string; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| initialContent?: string; | ||
| onSave: () => void; | ||
| register: UseFormRegister<CreateGuideRequest>; | ||
| error: Error | null; | ||
| setValue: UseFormSetValue<CreateGuideRequest>; | ||
| onSave: (title: string, content: string) => void; | ||
| } | ||
|
|
||
| const GuideEditor = ({ | ||
| initialContent = "", | ||
| initialTitle = "", | ||
| initialContent = "", | ||
| onSave, | ||
| register, | ||
| error, | ||
| setValue, | ||
| }: Props) => { | ||
| const [guideTitle, setGuideTitle] = useState<string>(initialTitle); | ||
| const [contentHTML, setContent] = useState<string>(initialContent); | ||
|
|
||
| const modules = { | ||
| toolbar: [ | ||
| [{ header: [1, 2, 3, 4, 5, 6, false] }], | ||
|
|
@@ -55,32 +51,35 @@ const GuideEditor = ({ | |
| "align", | ||
| ]; | ||
|
|
||
| const handleTitleChange = (event: ChangeEvent<HTMLInputElement>) => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. better use a form approach like in the create guide page |
||
| setGuideTitle(event.target.value); | ||
| }; | ||
|
|
||
| return ( | ||
| <> | ||
| <TextField | ||
| {...register("title")} | ||
| label="Title" | ||
| variant="outlined" | ||
| fullWidth | ||
| defaultValue={initialTitle} | ||
| onChange={(e) => setValue("title", e.target.value)} | ||
| sx={{ mb: 2 }} | ||
| value={guideTitle} | ||
| onChange={handleTitleChange} | ||
| /> | ||
| <TextField label="Issue" variant="outlined" fullWidth sx={{ mb: 2 }} /> | ||
| <DynamicReactQuill | ||
| theme="snow" | ||
| modules={modules} | ||
| formats={formats} | ||
| value={contentHTML} | ||
| style={{ height: "570px" }} | ||
| defaultValue={initialContent} // Initial content for the editor | ||
| onChange={(content) => setValue("contentHTML", content)} | ||
| onChange={(content) => setContent(content)} | ||
| /> | ||
| <input type="hidden" {...register("contentHTML")} /> | ||
| <Box display="flex" flexDirection="column" alignItems="flex-end" mt={6}> | ||
| <Button | ||
| variant="contained" | ||
| color="primary" | ||
| startIcon={<SaveAsIcon />} | ||
| onClick={() => onSave()} | ||
| onClick={() => onSave(guideTitle, contentHTML)} | ||
| > | ||
| Save Guide | ||
| </Button> | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,66 @@ | ||
| "use client"; | ||
| import PageContainer from "@/components/PageContainer"; | ||
| import DashboardCard from "../../../shared/Card"; | ||
| import { Box } from "@mui/material"; | ||
| import { Alert, Box, CircularProgress, Typography } from "@mui/material"; | ||
| import GuideEditor from "../../components/Editor"; | ||
| import { useUpdateGuide } from "./hooks/useUpdateGuide"; | ||
| import { useGuide, useUpdateGuide } from "../../hooks/guideClientHooks"; | ||
| import { useParams, useRouter } from "next/navigation"; | ||
|
|
||
| const Page = () => { | ||
| const { handleSave } = useUpdateGuide(); | ||
|
|
||
| return ( | ||
| <PageContainer title="Edit Guide"> | ||
| <Box | ||
| sx={{ | ||
| height: "calc(100vh - 30px)", | ||
| }} | ||
| > | ||
| <DashboardCard title="Edit Guide" fullHeight> | ||
| <GuideEditor onSave={handleSave} /> | ||
| </DashboardCard> | ||
| </Box> | ||
| </PageContainer> | ||
| ); | ||
| const handleSave = useUpdateGuide(); | ||
| const params = useParams(); | ||
| const router = useRouter(); | ||
|
|
||
| const id = params?.id ? Number(params.id) : 0; | ||
|
|
||
| const handleOnSave = (id: number, title: string, contentHTML: string) => { | ||
| handleSave.mutate({ id, guide: { title, contentHTML } }); | ||
| }; | ||
|
|
||
| const { | ||
| data: response, | ||
| isLoading, | ||
| isError, | ||
| error, | ||
| isSuccess, | ||
| } = useGuide(id ?? 0); | ||
|
|
||
| if (isLoading) { | ||
| return <CircularProgress />; | ||
| } | ||
|
|
||
| if (isError) { | ||
| return <Alert severity="error">{error.message}</Alert>; | ||
| } | ||
|
|
||
| if (isSuccess && "data" in response) { | ||
| const guide = response.data; | ||
|
|
||
| const creatorAndDateInfo = `Created by ${ | ||
| guide.creator?.username | ||
| } on ${new Date(guide.createdAt).toLocaleDateString()}`; | ||
|
|
||
| if (!guide.title) { | ||
| return <Typography>Guide not found</Typography>; | ||
| } | ||
| return ( | ||
| <PageContainer title="Edit Guide"> | ||
| <Box | ||
| sx={{ | ||
| height: "calc(100vh - 30px)", | ||
| }} | ||
| > | ||
| <DashboardCard title="Edit Guide" fullHeight> | ||
| <GuideEditor | ||
| onSave={(title, content) => handleOnSave(id, title, content)} | ||
| initialTitle={guide.title} | ||
| initialContent={guide.contentHTML} | ||
| /> | ||
| </DashboardCard> | ||
| </Box> | ||
| </PageContainer> | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| export default Page; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { GuideClient } from "@/api/guide.client"; | ||
| import { useQuery, useMutation } from "@tanstack/react-query"; | ||
| import { CreateReviewDto } from "../dto/CreateReviewDto"; | ||
| import { | ||
| Guide, | ||
| GuideRequestUpdate, | ||
| UpdateGuidePayload, | ||
| } from "@/api/types/Guide"; | ||
| import { useRouter } from "next/navigation"; | ||
|
|
||
| const guideClient = new GuideClient(); | ||
|
|
||
| export function useAllGuides() { | ||
| return useQuery({ | ||
| queryKey: ["guides"], | ||
| queryFn: () => guideClient.getAllGuides(), | ||
| }); | ||
| } | ||
|
|
||
| export function useGuide(guideId: number) { | ||
| return useQuery({ | ||
| queryKey: ["guide", guideId], | ||
| queryFn: () => guideClient.getGuide(guideId), | ||
| }); | ||
| } | ||
|
|
||
| export function useDeleteGuide(guideId: number) { | ||
| const router = useRouter(); | ||
| return useMutation({ | ||
| mutationFn: () => guideClient.deleteGuide(guideId), | ||
| onSuccess: () => { | ||
| router.push("/new_dashboard/guides"); | ||
| }, | ||
| onError: () => { | ||
| alert("Failed to delete guide. Please try again."); | ||
| }, | ||
|
Comment on lines
+34
to
+36
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| }); | ||
| } | ||
|
|
||
| export const useUpdateGuide = () => { | ||
| const router = useRouter(); | ||
| return useMutation({ | ||
| mutationFn: async (payload: UpdateGuidePayload) => { | ||
| const { id, guide } = payload; | ||
| return guideClient.updateGuide(id, guide); | ||
| }, | ||
| onSuccess: (data, variables) => { | ||
| const { id } = variables; | ||
| router.push(`/new_dashboard/guides/${id}`); | ||
| }, | ||
| onError: (error) => { | ||
| console.log(error); | ||
|
|
||
| alert("Failed to update guide. Please try again."); | ||
| }, | ||
|
Comment on lines
+51
to
+55
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use a mui toast to display the error or an alert component. |
||
| }); | ||
| }; | ||
|
|
||
| export function useAddReview() { | ||
| return useMutation({ | ||
| mutationFn: (review: CreateReviewDto) => guideClient.addReview(review), | ||
| }); | ||
| } | ||
|
Comment on lines
+59
to
+63
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didnt ori already have something for this?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do have exactly the same hook at |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why delete create guide function?