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 16 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 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
19 changes: 18 additions & 1 deletion app/(editor)/create/[[...paramsArr]]/_client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ const Create = ({ session }: { session: Session | null }) => {
Sentry.captureException(error);
},
});

const {
mutate: create,
data: createData,
Expand Down Expand Up @@ -217,6 +218,7 @@ const Create = ({ session }: { session: Session | null }) => {
tags,
canonicalUrl: data.canonicalUrl || undefined,
excerpt: data.excerpt || removeMarkdown(data.body, {}).substring(0, 155),
seriesName: data.seriesName || undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

We should santize this (using trim) on Zod.

};
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 validation and sanitization for series name.

The series name is currently accepted without any validation or sanitization. Since series names need to match exactly for articles to be connected (as mentioned in the UI help text), consider adding validation to prevent issues with whitespace, special characters, or case sensitivity.

 const getFormData = () => {
   const data = getValues();
+  const sanitizedSeriesName = data.seriesName?.trim();
   const formData = {
     ...data,
     tags,
     canonicalUrl: data.canonicalUrl || undefined,
     excerpt: data.excerpt || removeMarkdown(data.body, {}).substring(0, 155),
-    seriesName: data.seriesName || undefined
+    seriesName: sanitizedSeriesName || undefined
   };
   return formData;
 };

Committable suggestion skipped: line range outside the PR's diff.

return formData;
};
Expand All @@ -229,6 +231,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 +567,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
18 changes: 17 additions & 1 deletion 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 @@ -21,7 +21,8 @@ CREATE TABLE IF NOT EXISTS "Post" (
"updatedAt" timestamp(3) with time zone NOT NULL,
"slug" text NOT NULL,
"userId" text NOT NULL,
"showComments" boolean DEFAULT true NOT NULL
"showComments" boolean DEFAULT true NOT NULL,
"seriesId" INTEGER
);
--> statement-breakpoint
CREATE TABLE IF NOT EXISTS "PostTag" (
Expand Down Expand Up @@ -183,6 +184,15 @@ CREATE TABLE IF NOT EXISTS "Membership" (
"createdAt" timestamp(3) with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL
);
--> 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
);
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

Consider enhancing the Series table with additional columns and indexes.

While the basic structure is correct, consider these improvements for better functionality and performance:

  1. Add an ordering column for chronological arrangement of posts within a series
  2. Add a unique index to prevent duplicate series names per user
  3. Consider adding a soft delete column for better data management
 CREATE TABLE IF NOT EXISTS "Series" (
   "id" SERIAL PRIMARY KEY,
   "name" TEXT NOT NULL,
   "userId" text NOT NULL,
+  "orderColumn" INTEGER DEFAULT 0 NOT NULL,
+  "isDeleted" BOOLEAN DEFAULT FALSE NOT NULL,
   "createdAt" TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL,
   "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL
 );
+CREATE UNIQUE INDEX IF NOT EXISTS "Series_name_userId_key" ON "Series" ("name", "userId") WHERE "isDeleted" = FALSE;
📝 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" TEXT NOT NULL,
"userId" text NOT NULL,
"orderColumn" INTEGER DEFAULT 0 NOT NULL,
"isDeleted" BOOLEAN DEFAULT FALSE NOT NULL,
"createdAt" TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP NOT NULL,
"updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL
);
CREATE UNIQUE INDEX IF NOT EXISTS "Series_name_userId_key" ON "Series" ("name", "userId") WHERE "isDeleted" = FALSE;

--> 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 All @@ -208,6 +218,12 @@ EXCEPTION
WHEN duplicate_object THEN null;
END $$;
--> 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 $$
--> statement-breakpoint
DO $$ BEGIN
ALTER TABLE "Post" ADD CONSTRAINT "Post_userId_User_id_fk" FOREIGN KEY ("userId") REFERENCES "public"."User"("id") ON DELETE cascade ON UPDATE cascade;
EXCEPTION
Expand Down
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.

6 changes: 6 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
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