Skip to content
Open
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
135 changes: 126 additions & 9 deletions apps/frontend/src/components/launches/repeat.component.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
'use client';

import { FC, useMemo, useState } from 'react';
import { FC, useMemo, useRef, useState } from 'react';
import { Select } from '@gitroom/react/form/select';
import { useT } from '@gitroom/react/translation/get.transation.service.client';
import { useClickOutside } from '@mantine/hooks';
import { isUSCitizen } from '@gitroom/frontend/components/launches/helpers/isuscitizen.utils';
import clsx from 'clsx';
import { RepeatIcon, DropdownArrowIcon } from '@gitroom/frontend/components/ui/icons';

const CUSTOM_REPEAT_VALUE = -1;

const UNIT_OPTIONS = [
{ value: 1, label: 'Day(s)' },
{ value: 7, label: 'Week(s)' },
{ value: 30, label: 'Month(s)' },
];

const getList = (t: (key: string, fallback: string) => string) => [
{
value: 1,
Expand Down Expand Up @@ -44,11 +53,16 @@ const getList = (t: (key: string, fallback: string) => string) => [
value: 30,
label: t('month', 'Month'),
},
{
value: CUSTOM_REPEAT_VALUE,
label: t('custom', 'Custom...')
},
{
value: null,
label: t('cancel', 'Cancel'),
},
];

export const RepeatComponent: FC<{
repeat: number | null;
onChange: (newVal: number) => void;
Expand All @@ -57,6 +71,10 @@ export const RepeatComponent: FC<{
const t = useT();
const list = getList(t);
const [isOpen, setIsOpen] = useState(false);
const [showCustom, setShowCustom] = useState<boolean>(false);
const [customAmount, setCustomAmount] = useState(1);
const [customUnit, setCustomUnit] = useState(1); // multiplier: 1=day, 7=week, 30=month
const inputRef = useRef<HTMLInputElement>(null);

const ref = useClickOutside(() => {
if (!isOpen) {
Comment on lines 79 to 80

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: When the custom repeat panel is open, clicking outside the dropdown fails to reset the showCustom state, causing the custom panel to appear on the next open.
Severity: MEDIUM

Suggested Fix

Add setShowCustom(false) to the callback function within the useClickOutside hook to ensure the component's state is fully reset when the dropdown is closed by clicking outside.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/frontend/src/components/launches/repeat.component.tsx#L79-L80

Potential issue: The `useClickOutside` hook at line 79 correctly closes the dropdown by
setting `isOpen` to `false`. However, if the custom date panel is open (`showCustom` is
`true`), the hook does not reset `showCustom` to `false`. Consequently, the next time
the user opens the dropdown, they are incorrectly shown the custom panel again instead
of the initial list of preset options. This happens when a user opens the custom panel
and then decides to dismiss the entire dropdown by clicking away.

Expand All @@ -69,9 +87,44 @@ export const RepeatComponent: FC<{
if (!repeat) {
return '';
}
return list.find((p) => p.value === repeat)?.label;
const presetLabel = list.find((p) => p.value === repeat)?.label;
if (presetLabel) {
return presetLabel;
}
// Custom value: expressed as weeks or months for better readability when possible
if (repeat % 7 === 0) {
return `${repeat / 7} Week(s)`;
}
if (repeat % 30 === 0) {
return `${repeat / 30} Month(s)`;
}
Comment on lines +95 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The everyLabel function incorrectly prioritizes weeks over months. An interval like 210 days (7 months) will be displayed as '30 Week(s)' instead of '7 Month(s)'.
Severity: LOW

Suggested Fix

In the everyLabel function, reorder the conditional checks to evaluate divisibility by 30 (months) before checking for divisibility by 7 (weeks). This ensures that intervals representing whole months are displayed as months, not weeks.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/frontend/src/components/launches/repeat.component.tsx#L95-L100

Potential issue: The label generation logic in the `everyLabel` function incorrectly
prioritizes weeks over months due to the order of divisibility checks. When a repeat
interval is a multiple of both 7 (for weeks) and 30 (for months), such as 210 days, the
check for divisibility by 7 is performed first. For example, if a user selects a custom
repeat of 7 months, it is stored as 210 days. The condition `210 % 7 === 0` evaluates to
true, causing the function to return '30 Week(s)' instead of the user-intended '7
Month(s)', which would have been returned by the subsequent `210 % 30 === 0` check. This
leads to a mismatch between the user's selection and the displayed label.

Did we get this right? 👍 / 👎 to inform future reviews.

return `${repeat} Day(s)`;
}, [repeat, list]);

const handleItemClick = (value: number | null) => {
if (value === CUSTOM_REPEAT_VALUE) {
setShowCustom(true);
// To directly focus on input after custom panel opens for better UX
setTimeout(() => {
inputRef.current?.focus();
}, 50);
return;
}
props.onChange(Number(value));
setIsOpen(false);
setShowCustom(false);
}

const handleCustomApply = () => {
if (!customAmount || customAmount < 1) {
return;
}
const totalDays = customAmount * customUnit;
Comment on lines +118 to +122

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The custom repeat interval lacks upper-bound validation on both the frontend and backend, allowing for impractically long scheduling delays.
Severity: MEDIUM

Suggested Fix

Add a validation check in handleCustomApply in repeat.component.tsx to ensure customAmount does not exceed the intended maximum (e.g., 999). Additionally, implement server-side validation in the backend service or repository layer to reject values for inter that are outside the acceptable range before saving to the database.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/frontend/src/components/launches/repeat.component.tsx#L118-L122

Potential issue: The custom repeat interval for post scheduling lacks upper-bound
validation. While the frontend input has an HTML `max` attribute set to 999, the
`handleCustomApply` function does not enforce this limit, allowing a user to manually
enter a much larger number. This value is passed to the backend and saved directly to
the database without any server-side validation. The scheduling workflow then uses this
large number to calculate the next post time, which can result in an impractically long
delay (e.g., years or decades), effectively breaking the repeat functionality for that
post.

Also affects:

  • apps/backend/src/modules/posts/posts.repository.ts:557

props.onChange(Number(totalDays));
setIsOpen(false);
setShowCustom(false);
}

return (
<div
ref={ref}
Expand All @@ -97,19 +150,83 @@ export const RepeatComponent: FC<{
</div>
</div>
{isOpen && (
<div className="z-[300] absolute start-0 bottom-[100%] w-[240px] bg-newBgColorInner p-[12px] menu-shadow -translate-y-[10px] flex flex-col">
{list.map((p) => (
<div className="z-[300] absolute start-0 bottom-[100%] w-[340px] bg-newBgColorInner p-[12px] menu-shadow -translate-y-[10px] flex flex-col">
{!showCustom && list.map((p) => (
<div
onClick={() => {
props.onChange(Number(p.value));
setIsOpen(false);
}}
onClick={() => handleItemClick(p.value as number | null)}
key={p.label}
className="h-[40px] py-[8px] px-[20px] -mx-[12px] hover:bg-newBgColor"
className={clsx('h-[40px] py-[8px] px-[20px] -mx-[12px] hover:bg-newBgColor', p.value === CUSTOM_REPEAT_VALUE && 'text-[#612BD3] font-[700]')}
>
{p.label}
</div>
))}

{/* Custom Repeat Panel */}
{showCustom && (
<div className="flex flex-col gap-[10px]">
<div className="text-[14px] font-[600] mb-[2px]">
{t('custom_repeat', 'Custom Repeat Interval')}
</div>

{/* Amount + Unit row */}
<div className="flex gap-[8px] items-center">
<input
ref={inputRef}
type="number"
min={1}
max={999}
value={customAmount}
onChange={(e) => {
setCustomAmount(Number(e.target.value));
}}
className="w-[70px] h-[36px] rounded-[6px] border bg-newBgColor text-center text-[15px] font-[600] focus:outline-none focus:border-[#612BD3]"
/>
Comment on lines +173 to +183

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The custom repeat amount input allows decimal values, which are sent to the backend. The backend expects an integer for intervalInDays and will throw an error.
Severity: HIGH

Suggested Fix

Add the step={1} attribute to the <input type="number"> to restrict input to whole numbers. Additionally, consider adding a Math.floor() or Math.round() call in handleCustomApply before calling props.onChange to ensure only integers are propagated, providing a more robust defense.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: apps/frontend/src/components/launches/repeat.component.tsx#L173-L183

Potential issue: The `<input type="number">` for the custom repeat amount lacks a
`step={1}` attribute, allowing users to enter decimal values like "1.5". The frontend
validation (`!customAmount || customAmount < 1`) does not prevent this. The calculated
total days (e.g., `1.5 * 7 = 10.5`) is sent as the `inter` parameter to the backend. The
backend attempts to save this float value into the `intervalInDays` database column,
which is defined as an integer (`Int?`). This type mismatch will cause a Prisma/database
error, preventing the operation from completing.

<div className="flex gap-[4px]">
{UNIT_OPTIONS.map((u) => (
<button
key={u.value}
type="button"
onClick={() => setCustomUnit(u.value)}
className={clsx(
'h-[36px] px-[10px] rounded-[6px] text-[12px] font-[600] border transition-colors',
customUnit === u.value
? 'bg-[#612BD3] border-[#612BD3] text-white'
: 'border-newTextColor/20 hover:border-[#612BD3] hover:text-[#612BD3]',
)}
>
{u.label}
</button>
))}
</div>
</div>

{/* Preview */}
{customAmount && (
<div className="text-[12px] text-newTextColor/50">
Every {customAmount} {UNIT_OPTIONS.find(u => u.value === customUnit)?.label} = {customAmount * customUnit} day(s)
</div>
)}

{/* Actions */}
<div className="flex gap-[8px] mt-[4px]">
<button
type="button"
onClick={() => setShowCustom(false)}
className="flex-1 h-[34px] rounded-[6px] border border-newTextColor/20 text-[13px] font-[600] hover:bg-newBgColor"
>
{t('back', 'Back')}
</button>
<button
type="button"
onClick={handleCustomApply}
disabled={!customAmount || customAmount < 1}
className="flex-1 h-[34px] rounded-[6px] bg-[#612BD3] text-white text-[13px] font-[600] disabled:opacity-40 hover:bg-[#4f22a8] transition-colors"
>
{t('apply', 'Apply')}
</button>
</div>
</div>
)}
</div>
)}
</div>
Expand Down