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

refactor: use feedback string column #87

Merged
merged 3 commits into from
Nov 5, 2024
Merged
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
6 changes: 4 additions & 2 deletions apps/client/src/app/[groupId]/[questionId]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { useUser } from '@account-kit/react'
import { Loader } from 'client/c/Loader'
import { withAuth } from 'client/components/withAuth'
import { useQuestionStats } from 'client/h/useQuestionStats'
import { trpc } from 'client/l/trpc'
import { useEffect } from 'react'

Expand All @@ -12,6 +13,7 @@ const QuestionDetails = ({ params: { questionId: questionIdStr } }: { params: {
const { data: question, isLoading, refetch } = trpc.questions.find.useQuery({ questionId }, {
select: ({ data }) => data,
})
const { data: { no, yes } } = useQuestionStats({ questionId })

useEffect(() => {
refetch()
Expand All @@ -21,8 +23,8 @@ const QuestionDetails = ({ params: { questionId: questionIdStr } }: { params: {
return (
<div>
<h1 className='text-2xl'>{question.title}</h1>
<p>yes: {question.yes}</p>
<p>no: {question.no}</p>
<p>yes: {yes}</p>
<p>no: {no}</p>
{user?.email === question.author && (
<button
type='button'
Expand Down
2 changes: 1 addition & 1 deletion apps/client/src/components/CreateQuestionForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const CreateQuestionForm: FC<CreateQuestionFormProps> = ({ onClose }) =>
defaultValues: { title: '' },
onSubmit: ({ value: { title } }) => {
if (user?.email === undefined) throw new Error('User not found')
createQuestion({ author: user.email, groupId: clientConfig.bandada.pseGroupId, title })
createQuestion({ author: user.email, groupId: clientConfig.bandada.pseGroupId, title, type: 'boolean' })
},
validatorAdapter: zodValidator(),
validators: { onChange: CreateQuestionDto.pick({ title: true }) },
Expand Down
18 changes: 5 additions & 13 deletions apps/client/src/components/QuestionCard/YN/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { YNQuestionStatus } from 'client/c/QuestionCard/YN/YNQuestionStatus'
import { useSendFeedback } from 'client/h/useSendFeedback'
import { useQuestionStats } from 'client/hooks/useQuestionStats'
import { ThumbsDown, ThumbsUp } from 'lucide-react'
import Link from 'next/link'
import type { FC } from 'react'
Expand All @@ -10,18 +11,9 @@ export const YNQuestionCard: FC<Question> = ({
group_id: groupId,
title,
active,
yes,
no,
}) => {
// Use the custom hook for sending feedback
const {
sendFeedback,
isSending,
errors,
} = useSendFeedback({
groupId,
questionId,
})
const { data: { no, yes } } = useQuestionStats({ questionId })
const { sendFeedback, isSending, errors } = useSendFeedback({ groupId, questionId })

if (errors.length > 0)
return errors.map(({ message, type }) => <p key={message} className='text-red -text-sm'>{`${type}: ${message}`}</p>)
Expand All @@ -45,7 +37,7 @@ export const YNQuestionCard: FC<Question> = ({
className='mr-2'
type='button'
onClick={() => {
sendFeedback(true)
sendFeedback('true')
}}
disabled={isSending}
>
Expand All @@ -55,7 +47,7 @@ export const YNQuestionCard: FC<Question> = ({
type='button'
disabled={isSending}
onClick={() => {
sendFeedback(false)
sendFeedback('false')
}}
>
{no} <ThumbsDown className='inline-block' size={20} />
Expand Down
9 changes: 9 additions & 0 deletions apps/client/src/hooks/useQuestionStats.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { trpc } from 'client/l/trpc'
import type { QuestionStatsDto } from 'server/questions/dto'

export const useQuestionStats = ({ questionId }: QuestionStatsDto) => {
// TODO: handle free text questions
return trpc.questions.stats.useQuery({ questionId, type: 'boolean' }, {
initialData: { no: 0, yes: 0 },
})
}
6 changes: 4 additions & 2 deletions apps/client/src/hooks/useSendFeedback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { useIdentity } from 'client/h/useIdentity'
import { trpc } from 'client/l/trpc'
import { useCallback, useMemo, useState } from 'react'
import type { SendFeedbackDto } from 'server/feedbacks/dto'
import { toBytes } from 'viem'

enum ErrorType {
ExportGroupError = 'ExportGroupError',
SendError = 'SendError',
Expand Down Expand Up @@ -33,11 +35,11 @@ export const useSendFeedback = (
return errors
}, [exportGroupError, generateProofError, sendError])

const sendFeedback = useCallback((feedback: boolean) => {
const sendFeedback = useCallback((feedback: string) => {
if (nodes === undefined || isNodesLoading || identity.isNone() || feedback === null) return
const group = Group.import(nodes)
// use questionId in scope as they are unique within the app (postgres questions table primary key)
generateProof(identity.get(), group, BigInt(feedback), `${questionId}`, 16)
generateProof(identity.get(), group, toBytes(feedback), questionId, 16)
.then((proof) => {
send({ groupId, feedback, proof, questionId })
})
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/feedbacks/dto/create-feedback.dto.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { z } from 'zod'

export const CreateFeedbackDto = z.object({
feedback: z.boolean(),
feedback: z.string().min(1, { message: 'Feedback cannot be empty' }),
questionId: z.number().positive(),
})

Expand Down
22 changes: 19 additions & 3 deletions apps/server/src/feedbacks/dto/send-feedback.dto.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import type { SemaphoreProof } from '@semaphore-protocol/core'
import type { OptionsType, QuestionType } from 'server/questions/dto'
import { z } from 'zod'

export const SendFeedbackDto = z.object({
groupId: z.string(),
feedback: z.boolean(),
questionId: z.number(),
feedback: z.string().min(1, { message: 'Feedback cannot be empty' }),
groupId: z.string().min(1, { message: 'Group ID cannot be empty' }),
questionId: z.number().positive(),
proof: z.object({
merkleTreeDepth: z.number().int().min(16).max(32),
merkleTreeRoot: z.string(),
Expand All @@ -16,3 +17,18 @@ export const SendFeedbackDto = z.object({
})

export type SendFeedbackDto = Omit<z.infer<typeof SendFeedbackDto>, 'proof'> & { proof: SemaphoreProof }

export const DynamicFeedbackSchema = (type: QuestionType, options?: OptionsType) => {
switch (type) {
case 'boolean':
return z.enum(['yes', 'no'])
case 'number':
return z.number().int().min(0)
case 'option':
if (options === null || options === undefined || options.length === 0)
throw new Error('Options are required for option type')
return z.enum(options as [string, ...string[]])
case 'text':
return z.string().min(1)
}
}
18 changes: 14 additions & 4 deletions apps/server/src/feedbacks/feedbacks.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Injectable } from '@nestjs/common'
import { verifyProof } from '@semaphore-protocol/core'
import { CreateFeedbackDto, SendFeedbackDto } from 'server/feedbacks/dto'
import { type CreateFeedbackDto, DynamicFeedbackSchema, type SendFeedbackDto } from 'server/feedbacks/dto'
import { NullifiersService } from 'server/nullifiers/nullifiers.service'
import { QuestionsService } from 'server/questions/questions.service'
import { RootsService } from 'server/roots/roots.service'
Expand All @@ -19,10 +19,20 @@ export class FeedbacksService {
return this.supabase.from('feedbacks').insert({ feedback, question_id })
}

// TODO: handle errors, abstract in smaller steps?
// some of the validation could happen at the router layer
// but due to the dynamic nature of the validation
// (constraints between feedback and question types that aren't enforced at the db layer)
// it is better to keep it here for better separation of concerns and maintainability
// the router keeps doing only static input validation
async send({ groupId, feedback, proof, questionId }: SendFeedbackDto) {
if (!await this.questions.isInactive({ questionId }))
throw new Error('Question is inactive, you cannot send feedback anymore')
const { data: question } = await this.questions.find({ questionId })
if (question === null) throw new Error('Question does not exist')

const { active, type, options } = question
if (!active) throw new Error('Question is inactive, you cannot send feedback anymore')

const feedbackSchema = DynamicFeedbackSchema(type, options)
feedbackSchema.parse(feedback)

if (await this.nullifiers.isAlreadyUsed({ nullifier: proof.nullifier })) throw new Error('Nullifier already used')
if (await this.roots.isNotLatestAndHasExpired({ groupId, root: proof.merkleTreeRoot }))
Expand Down
10 changes: 10 additions & 0 deletions apps/server/src/questions/dto/create-question.dto.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import type { Database, Tables } from 'server/supabase/supabase.types'
import { z } from 'zod'

export type QuestionType = Database['public']['Enums']['question_type']
const questionTypes = ['boolean', 'number', 'option', 'text'] as const
export const questionTypeSchema = z.enum(questionTypes, {
message: 'Invalid question type, must be one of: boolean, number, option, text',
})

export type OptionsType = Tables<'questions'>['options']

export const CreateQuestionDto = z.object({
author: z.string().email(),
groupId: z.string().min(1, { message: 'Group ID cannot be empty' }),
title: z.string().min(10, { message: 'Title must be at least 10 characters long' }).includes('?', {
message: 'Title must include a question mark',
}),
type: questionTypeSchema,
})

export type CreateQuestionDto = z.infer<typeof CreateQuestionDto>
1 change: 1 addition & 0 deletions apps/server/src/questions/dto/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from './create-question.dto'
export * from './find-all-questions.dto'
export * from './find-question.dto'
export * from './question-stats.dto'
export * from './toggle-question.dto'
5 changes: 5 additions & 0 deletions apps/server/src/questions/dto/question-stats.dto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { z } from 'zod'
import { questionTypeSchema } from './create-question.dto'

export const QuestionStatsDto = z.object({ questionId: z.number().positive(), type: questionTypeSchema.optional() })
export type QuestionStatsDto = z.infer<typeof QuestionStatsDto>
9 changes: 8 additions & 1 deletion apps/server/src/questions/questions.router.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { Injectable } from '@nestjs/common'
import { on } from 'node:events'
import { CreateQuestionDto, FindAllQuestionsDto, FindQuestionDto, ToggleQuestionDto } from 'server/questions/dto'
import {
CreateQuestionDto,
FindAllQuestionsDto,
FindQuestionDto,
QuestionStatsDto,
ToggleQuestionDto,
} from 'server/questions/dto'
import { Question } from 'server/questions/entities'
import { QuestionsService } from 'server/questions/questions.service'
import { TrpcService } from 'server/trpc/trpc.service'
Expand Down Expand Up @@ -32,6 +38,7 @@ export class QuestionsRouter {
yield payload as { type: 'INSERT' | 'UPDATE'; data: Question }
}
}),
stats: this.trpc.procedure.input(QuestionStatsDto).query(async ({ input }) => this.questions.stats(input)),
toggle: this.trpc.procedure.input(ToggleQuestionDto).mutation(async ({ input }) => this.questions.toggle(input)),
})
}
23 changes: 21 additions & 2 deletions apps/server/src/questions/questions.service.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injectable, OnModuleInit } from '@nestjs/common'
import type { CreateQuestionDto, FindAllQuestionsDto, FindQuestionDto, ToggleQuestionDto } from 'server/questions/dto'
import { SupabaseService } from 'server/supabase/supabase.service'
import { QuestionStatsDto } from './dto/question-stats.dto'

@Injectable()
export class QuestionsService implements OnModuleInit {
Expand All @@ -12,8 +13,8 @@ export class QuestionsService implements OnModuleInit {
this.supabase.subscribe(this.resource)
}

async create({ author, groupId: group_id, title }: CreateQuestionDto) {
return this.supabase.from(this.resource).insert({ author, group_id, title })
async create({ author, groupId: group_id, title, type }: CreateQuestionDto) {
return this.supabase.from(this.resource).insert({ author, group_id, title, type })
}

async find({ questionId }: FindQuestionDto) {
Expand All @@ -34,6 +35,24 @@ export class QuestionsService implements OnModuleInit {
return data.active
}

async stats({ questionId, type }: QuestionStatsDto) {
if (type === undefined) {
const { data: question } = await this.find({ questionId })
if (question === null) throw new Error('question not found')
type = question.type
}

switch (type) {
case 'boolean': {
const { data } = await this.supabase.rpc('count_boolean_feedbacks', { question_id: questionId })
return { no: data?.no ?? 0, yes: data?.yes ?? 0 }
}
default:
// TODO support text question type
throw new Error('Unsupported question type')
}
}

async toggle({ active, questionId }: ToggleQuestionDto) {
return this.supabase.from(this.resource).update({ active }).eq('id', questionId)
}
Expand Down
1 change: 1 addition & 0 deletions apps/server/src/supabase/supabase.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export class SupabaseService implements OnModuleDestroy {
private readonly supabase = createClient<Database>(serverConfig.supabase.url, serverConfig.supabase.anonKey)

from = this.supabase.from.bind(this.supabase)
rpc = this.supabase.rpc.bind(this.supabase)

onModuleDestroy() {
this.supabase.removeAllChannels()
Expand Down
33 changes: 21 additions & 12 deletions apps/server/src/supabase/supabase.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,19 @@ export type Database = {
feedbacks: {
Row: {
created_at: string
feedback: boolean
feedback: string
id: number
question_id: number
}
Insert: {
created_at?: string
feedback: boolean
feedback: string
id?: never
question_id: number
}
Update: {
created_at?: string
feedback?: boolean
feedback?: string
id?: never
question_id?: number
}
Expand Down Expand Up @@ -106,29 +106,29 @@ export type Database = {
created_at: string
group_id: string
id: number
no: number
options: string[] | null
title: string
yes: number
type: Database["public"]["Enums"]["question_type"]
}
Insert: {
active?: boolean
author: string
created_at?: string
group_id: string
id?: never
no?: number
options?: string[] | null
title: string
yes?: number
type: Database["public"]["Enums"]["question_type"]
}
Update: {
active?: boolean
author?: string
created_at?: string
group_id?: string
id?: never
no?: number
options?: string[] | null
title?: string
yes?: number
type?: Database["public"]["Enums"]["question_type"]
}
Relationships: []
}
Expand Down Expand Up @@ -158,13 +158,22 @@ export type Database = {
[_ in never]: never
}
Functions: {
[_ in never]: never
count_boolean_feedbacks: {
Args: {
question_id: number
}
Returns: Database["public"]["CompositeTypes"]["boolean_feedbacks_count"]
}
}
Enums: {
[_ in never]: never
boolean_feedback: "yes" | "no"
question_type: "boolean" | "text" | "number" | "option"
}
CompositeTypes: {
[_ in never]: never
boolean_feedbacks_count: {
yes: number | null
no: number | null
}
}
}
}
Expand Down
Loading