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: recurring transactions added- migrations, ui, api updated. Closes #5 #114

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
-- CreateTable
CREATE TABLE "RecurringTransactions" (
"groupId" VARCHAR(255) NOT NULL,
"expenseId" VARCHAR(255) NOT NULL,
"createNextAt" INT NOT NULL,
"lockId" VARCHAR(255) DEFAULT NULL,

CONSTRAINT "RecurringTransactions_pkey" PRIMARY KEY ("groupId", "expenseId")
);

CREATE INDEX "idx_recurring_transactions_group_expense_next_create" ON "RecurringTransactions" ("groupId", "expenseId", "createNextAt" DESC);
CREATE UNIQUE INDEX "idx_unq_recurring_transactions_lock_id" ON "RecurringTransactions" ("lockId");

-- AlterTable
ALTER TABLE "Expense" ADD COLUMN "recurringDays" TEXT NOT NULL DEFAULT 0;
11 changes: 11 additions & 0 deletions prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ model Expense {
createdAt DateTime @default(now())
documents ExpenseDocument[]
notes String?
recurringDays Int @default(0)
Copy link

Choose a reason for hiding this comment

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

Indentation/alignment does not match the lines above.

}

model ExpenseDocument {
Expand Down Expand Up @@ -80,3 +81,13 @@ model ExpensePaidFor {

@@id([expenseId, participantId])
}

model RecurringTransactions {
groupId String
expenseId String
createNextAt Int
lockId String? @unique

@@id([groupId, expenseId])
@@index([groupId, expenseId, createNextAt(sort: Desc)])
}
3 changes: 2 additions & 1 deletion src/app/groups/[groupId]/expenses/[expenseId]/edit/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ export default async function EditExpensePage({

async function deleteExpenseAction() {
'use server'
await deleteExpense(expenseId)
await deleteExpense(groupId, expenseId)
redirect(`/groups/${groupId}`)
}


return (
<Suspense>
<ExpenseForm
Expand Down
2 changes: 1 addition & 1 deletion src/app/groups/[groupId]/expenses/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export default async function GroupExpensesPage({
async function Expenses({ groupId }: { groupId: string }) {
const group = await cached.getGroup(groupId)
if (!group) notFound()
const expenses = await getGroupExpenses(group.id)
const expenses = (await getGroupExpenses(group.id))
Copy link

Choose a reason for hiding this comment

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

Unnecessary change.


return (
<ExpenseList
Expand Down
1 change: 0 additions & 1 deletion src/app/groups/recent-group-list-card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export function RecentGroupListCard({
})

const isStarred = state.starredGroups.includes(group.id)
const isArchived = state.archivedGroups.includes(group.id)
Copy link

Choose a reason for hiding this comment

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

Should this change still be part of this PR?


return (
<li key={group.id}>
Expand Down
38 changes: 38 additions & 0 deletions src/components/expense-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { match } from 'ts-pattern'
import { DeletePopup } from './delete-popup'
import { extractCategoryFromTitle } from './expense-form-actions'
import { Textarea } from './ui/textarea'
import { AsyncButton } from './async-button'
Copy link

Choose a reason for hiding this comment

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

This import isn't used.


export type Props = {
group: NonNullable<Awaited<ReturnType<typeof getGroup>>>
Expand Down Expand Up @@ -164,6 +165,10 @@ export function ExpenseForm({
return field?.value
}
const defaultSplittingOptions = getDefaultSplittingOptions(group)

const getRecurringField = (field?: { value: string }) => {
return field?.value
}
const form = useForm<ExpenseFormValues>({
resolver: zodResolver(expenseFormSchema),
defaultValues: expense
Expand All @@ -182,6 +187,7 @@ export function ExpenseForm({
isReimbursement: expense.isReimbursement,
documents: expense.documents,
notes: expense.notes ?? '',
recurringDays: String(expense.recurringDays),
Copy link

Choose a reason for hiding this comment

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

Why does this need to be a String here rather than a number?

Copy link
Author

@neonshobhit neonshobhit May 26, 2024

Choose a reason for hiding this comment

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

I started with Number, but I thought about this as well. How to store "1st of every month"?
Cron makes more sense. Please check here.
Most probably I will have to change every thing from days to cron - which I can add in v2 PR.

}
: searchParams.get('reimbursement')
? {
Expand All @@ -205,6 +211,7 @@ export function ExpenseForm({
saveDefaultSplittingOptions: false,
documents: [],
notes: '',
recurringDays: "0",
}
: {
title: searchParams.get('title') ?? '',
Expand Down Expand Up @@ -232,7 +239,9 @@ export function ExpenseForm({
]
: [],
notes: '',
recurringDays: "0",
},

})
const [isCategoryLoading, setCategoryLoading] = useState(false)

Expand All @@ -241,6 +250,7 @@ export function ExpenseForm({
return onSubmit(values)
}

const recurringDays = [{ "key": "Never","value": "0"}, { "key":"weekly", "value": "7"}, {"key": "fortnightly", "value": "14"}, {"key": "monthly", "value": "30"}, {"key": "bimonthly", "value": "60"}]
Copy link

Choose a reason for hiding this comment

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

This supports recurrences on an interval of days, but not other typical calendar occurrences like "on the first day of every month". I think it's fine for that to be version 1 of this feature, but rather than display "monthly" I think it should display "every 30 days" so that it doesn't lead to any surprises.

Copy link
Author

Choose a reason for hiding this comment

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

Yes. I'll add a fixes PR later to fix this issue.
Right now, changing to every 30 days.

return (
<Form {...form}>
<form onSubmit={form.handleSubmit(submit)}>
Expand Down Expand Up @@ -413,6 +423,34 @@ export function ExpenseForm({
<FormControl>
<Textarea className="text-base" {...field} />
</FormControl>
</FormItem>
)}
/>
<FormField
control={form.control}
name="recurringDays"
render={({ field }) => (
<FormItem className="sm:order-5">
<FormLabel>Recurring Days</FormLabel>
<Select
onValueChange={field.onChange}
defaultValue={getRecurringField(field)}
>
<SelectTrigger>
<SelectValue placeholder="Never" />
</SelectTrigger>
<SelectContent>
{recurringDays.map(({ key, value }) => (
<SelectItem key={key} value={value}>
{key}
</SelectItem>
))}
</SelectContent>
</Select>
<FormDescription>
Select recursive days.
Copy link

Choose a reason for hiding this comment

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

I would not use "recursive" here, or possibly even "recurrence". "Repeat every 30 days" seems the most natural English to me.

Copy link
Author

Choose a reason for hiding this comment

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

We can have options like 30 days, 7 days etc. Can you please suggest other title?
or
"Repeat after:" this seems okay?

Copy link

@rgov rgov May 26, 2024

Choose a reason for hiding this comment

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

The Apple Calendar app uses "Repeat: <never|every day|every week|...>" which seems like a decent way to display this and a set of choices.

</FormDescription>
<FormMessage />
</FormItem>
)}
/>
Expand Down
Loading