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

Feature/ Add "series" to link related articles #1202

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 24 additions & 1 deletion app/(editor)/create/[[...paramsArr]]/_client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { uploadFile } from "@/utils/s3helpers";
import { getUploadUrl } from "@/app/actions/getUploadUrl";
import EditorNav from "./navigation";
import { type Session } from "next-auth";
import { FormDataSchema } from "@/schema/post";

const Create = ({ session }: { session: Session | null }) => {
const params = useParams();
Expand Down Expand Up @@ -161,6 +162,7 @@ const Create = ({ session }: { session: Session | null }) => {
Sentry.captureException(error);
},
});

const {
mutate: create,
data: createData,
Expand Down Expand Up @@ -212,11 +214,17 @@ const Create = ({ session }: { session: Session | null }) => {

const getFormData = () => {
const data = getValues();

const sanitizedSeriesName = FormDataSchema.parse({
seriesName: data.seriesName,
});

Comment on lines +217 to +221
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling for series name sanitization.

The current implementation has two potential issues:

  1. Missing error handling for the Zod schema parsing
  2. Using spread operator could accidentally override other fields if FormDataSchema includes more fields

Consider this safer implementation:

-    const sanitizedSeriesName = FormDataSchema.parse({
-      seriesName: data.seriesName,
-    });
-
+    let sanitizedSeriesName;
+    try {
+      const result = FormDataSchema.parse({
+        seriesName: data.seriesName,
+      });
+      sanitizedSeriesName = result.seriesName;
+    } catch (error) {
+      toast.error("Invalid series name format");
+      return;
+    }
+
     const formData = {
       ...data,
       tags,
       canonicalUrl: data.canonicalUrl || undefined,
       excerpt: data.excerpt || removeMarkdown(data.body, {}).substring(0, 155),
-      ...sanitizedSeriesName
+      seriesName: sanitizedSeriesName
     };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sanitizedSeriesName = FormDataSchema.parse({
seriesName: data.seriesName,
});
let sanitizedSeriesName;
try {
const result = FormDataSchema.parse({
seriesName: data.seriesName,
});
sanitizedSeriesName = result.seriesName;
} catch (error) {
toast.error("Invalid series name format");
return;
}
const formData = {
...data,
tags,
canonicalUrl: data.canonicalUrl || undefined,
excerpt: data.excerpt || removeMarkdown(data.body, {}).substring(0, 155),
seriesName: sanitizedSeriesName
};

const formData = {
...data,
tags,
canonicalUrl: data.canonicalUrl || undefined,
excerpt: data.excerpt || removeMarkdown(data.body, {}).substring(0, 155),
...sanitizedSeriesName
};
return formData;
};
Expand All @@ -229,6 +237,7 @@ const Create = ({ session }: { session: Session | null }) => {
await create({ ...formData });
} else {
await save({ ...formData, id: postId });

setSavedTime(
new Date().toLocaleString(undefined, {
dateStyle: "medium",
Expand Down Expand Up @@ -564,10 +573,24 @@ const Create = ({ session }: { session: Session | null }) => {
{copied ? "Copied" : "Copy Link"}
</div>
</button>
<p className="mt-2 text-sm text-neutral-600 dark:text-neutral-400">
<p className="mt-2 mb-2 text-sm text-neutral-600 dark:text-neutral-400">
Share this link with others to preview your
draft. Anyone with the link can view your draft.
</p>

<label htmlFor="seriesName">
Series Name
</label>
<input
id="seriesName"
type="text"
placeholder="The name of my series"
defaultValue={data?.series?.name || ""}
{...register("seriesName")}
/>
<p className="mt-2 text-sm text-neutral-600 dark:text-neutral-400">
This text is case-sensitive so make sure you type it exactly as you did in previous articles to ensure they are connected
</p>
Comment on lines +581 to +593
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation for series name.

While the UI implementation is good, consider adding validation to ensure consistent series names.

Consider adding validation:

 <input
   id="seriesName"
   type="text"
   placeholder="The name of my series"
   defaultValue={data?.series?.name || ""}
+  pattern="^[a-zA-Z0-9\s-]+$"
+  title="Series name can only contain letters, numbers, spaces, and hyphens"
   {...register("seriesName", {
+    validate: {
+      format: (value) => 
+        !value || /^[a-zA-Z0-9\s-]+$/.test(value) || 
+        "Series name can only contain letters, numbers, spaces, and hyphens"
+    }
+  })}
 />
+{errors.seriesName && (
+  <p className="mt-1 text-sm text-red-600">
+    {errors.seriesName.message}
+  </p>
+)}

Committable suggestion was skipped due to low confidence.

</DisclosurePanel>
</>
)}
Expand Down
1 change: 1 addition & 0 deletions drizzle/0000_initial_schema.sql
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, sorry for the major delay. I've been crazy busy. A few issues are still here. You can't do migrations in older migrations or they'll never run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ohh.... Sure,
I will push them into a new migration file

Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ CREATE TABLE IF NOT EXISTS "Membership" (
"createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL
);
--> statement-breakpoint

CREATE UNIQUE INDEX IF NOT EXISTS "Post_id_key" ON "Post" ("id");--> statement-breakpoint
CREATE UNIQUE INDEX IF NOT EXISTS "Post_slug_key" ON "Post" ("slug");--> statement-breakpoint
CREATE UNIQUE INDEX IF NOT EXISTS "PostTag_tagId_postId_key" ON "PostTag" ("tagId","postId");--> statement-breakpoint
Expand Down
17 changes: 17 additions & 0 deletions drizzle/0011_add_series.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--> statement-breakpoint
CREATE TABLE IF NOT EXISTS "Series" (
"id" SERIAL PRIMARY KEY,
"name" TEXT NOT NULL,
"userId" text NOT NULL,
"createdAt" TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL,
"updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL
);
Comment on lines +2 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add necessary indexes and constraints to the Series table

Consider the following improvements for better performance and data integrity:

  1. Add a composite unique constraint on (name, userId) to prevent duplicate series names per user
  2. Add an index on userId for faster user-specific queries
  3. Add an index on createdAt for chronological ordering
  4. Add a length limit to the name field

Apply this modification:

 CREATE TABLE IF NOT EXISTS "Series" (
   "id" SERIAL PRIMARY KEY,
-  "name" TEXT NOT NULL,
+  "name" VARCHAR(255) NOT NULL,
   "userId" text NOT NULL,
   "createdAt" TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL,
   "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL
 );
+CREATE UNIQUE INDEX "series_name_userid_unique_idx" ON "Series"("name", "userId");
+CREATE INDEX "series_userid_idx" ON "Series"("userId");
+CREATE INDEX "series_createdat_idx" ON "Series"("createdAt");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CREATE TABLE IF NOT EXISTS "Series" (
"id" SERIAL PRIMARY KEY,
"name" TEXT NOT NULL,
"userId" text NOT NULL,
"createdAt" TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL,
"updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL
);
CREATE TABLE IF NOT EXISTS "Series" (
"id" SERIAL PRIMARY KEY,
"name" VARCHAR(255) NOT NULL,
"userId" text NOT NULL,
"createdAt" TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL,
"updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL
);
CREATE UNIQUE INDEX "series_name_userid_unique_idx" ON "Series"("name", "userId");
CREATE INDEX "series_userid_idx" ON "Series"("userId");
CREATE INDEX "series_createdat_idx" ON "Series"("createdAt");

-->statement-breakpoint

ALTER TABLE "Post" ADD COLUMN "seriesId" INTEGER;
--> statement-breakpoint
DO $$ BEGIN
ALTER TABLE "Post" ADD CONSTRAINT "Post_seriesId_fkey" FOREIGN KEY ("seriesId") REFERENCES "public"."Series" ("id") ON DELETE SET NULL;
EXCEPTION
WHEN duplicate_object THEN null;
END $$
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions schema/post.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ export const SavePostSchema = z.object({
canonicalUrl: z.optional(z.string().trim().url()),
tags: z.string().array().max(5).optional(),
published: z.string().datetime().optional(),
seriesName: z.string()
.trim()
.min(1, "Series name cannot be empty")
.max(50, "Series name is too long")
.regex(/^[\w\s-]+$/, "Series name can only contain letters, numbers, spaces, and hyphens")
.optional()
});

export const PublishPostSchema = z.object({
Expand Down Expand Up @@ -66,6 +72,10 @@ export const GetPostsSchema = z.object({
tag: z.string().nullish(),
});

export const FormDataSchema = z.object({
seriesName: z.string().trim().optional()
})

export type SavePostInput = z.TypeOf<typeof SavePostSchema>;
export type ConfirmPostInput = z.TypeOf<typeof ConfirmPostSchema>;

Expand Down
2 changes: 1 addition & 1 deletion server/api/router/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const appRouter = createTRPCRouter({
notification: notificationRouter,
admin: adminRouter,
report: reportRouter,
tag: tagRouter,
tag: tagRouter
});

// export type definition of API
Expand Down
Loading
Loading