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

Generic data model and Supabase data service #125

Closed
wants to merge 2 commits into from
Closed
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
13 changes: 13 additions & 0 deletions src/models/GenericModel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// GenericModel class
export default abstract class GenericModel {
// Abstract property for table name
static TABLE_NAME: string;

static DATABASE_MAP: Object;
Comment on lines +3 to +6
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

Improve type safety of DATABASE_MAP.

The Object type is too broad and unsafe. Consider using a more specific type that properly represents the database field mappings.

-    static DATABASE_MAP: Object;
+    static DATABASE_MAP: Record<string, string>;

Also, consider making these properties abstract to enforce implementation:

-    static TABLE_NAME: string;
+    abstract static TABLE_NAME: string;
-    static DATABASE_MAP: Record<string, string>;
+    abstract static DATABASE_MAP: Record<string, string>;
📝 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
// Abstract property for table name
static TABLE_NAME: string;
static DATABASE_MAP: Object;
// Abstract property for table name
abstract static TABLE_NAME: string;
abstract static DATABASE_MAP: Record<string, string>;
🧰 Tools
🪛 Biome (1.9.4)

[error] 6-6: Don't use 'Object' as a type.

Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.

(lint/complexity/noBannedTypes)

🪛 eslint (1.23.1)

[error] 6-6: Don't use Object as a type. The Object type actually means "any non-nullish value", so it is marginally better than unknown.

  • If you want a type meaning "any object", you probably want object instead.
  • If you want a type meaning "any value", you probably want unknown instead.
  • If you really want a type meaning "any non-nullish value", you probably want NonNullable<unknown> instead.

(@typescript-eslint/ban-types)


// Abstract method for parsing JSON
static parseJson(json: any): GenericModel {
throw new Error("parseJson method not implemented.");
return json
}
Comment on lines +8 to +12
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

Improve type safety and implementation of parseJson.

Several issues need to be addressed:

  1. Using any type defeats TypeScript's type safety
  2. The return statement after throw is unreachable
  3. The method should use generics to ensure type-safe implementations
-    static parseJson(json: any): GenericModel {
-        throw new Error("parseJson method not implemented.");
-        return json
-    }
+    abstract static parseJson<T extends GenericModel>(
+        json: Record<string, unknown>
+    ): T;

This change:

  • Makes the method abstract to enforce implementation
  • Uses generics to ensure implementing classes return their specific type
  • Uses Record<string, unknown> for better type safety of JSON input
📝 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
// Abstract method for parsing JSON
static parseJson(json: any): GenericModel {
throw new Error("parseJson method not implemented.");
return json
}
// Abstract method for parsing JSON
abstract static parseJson<T extends GenericModel>(
json: Record<string, unknown>
): T;
🧰 Tools
🪛 Biome (1.9.4)

[error] 11-11: This code is unreachable

... because this statement will throw an exception beforehand

(lint/correctness/noUnreachable)

🪛 eslint (1.23.1)

[error] 9-9: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

}
36 changes: 36 additions & 0 deletions src/models/User.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import GenericModel from "./GenericModel"

export default class User extends GenericModel {

static TABLE_NAME = "members"
static DATABASE_MAP: Object = {
id: "uuid",
username: "username",
email: "fk_email",
identity: "fk_identity",
avatar: "avatar",
}
Comment on lines +5 to +12
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

Improve type safety and complete database mappings

  1. Replace the generic Object type with a proper type definition for better type safety.
  2. Several instance properties are missing from DATABASE_MAP.

Apply this diff to improve type safety and add missing mappings:

-    static DATABASE_MAP: Object = {
+    static DATABASE_MAP: Record<string, string> = {
         id:         "uuid",
         username:   "username",
         email:      "fk_email",
         identity:   "fk_identity",
         avatar:     "avatar",
+        phone:      "phone",
+        profileBackground: "profile_background",
+        joinedAt:   "joined_at",
+        department: "department",
+        grade:      "grade",
+        bio:        "bio"
     }

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

🧰 Tools
🪛 Biome (1.9.4)

[error] 6-6: Don't use 'Object' as a type.

Prefer explicitly define the object shape. This type means "any non-nullable value", which is slightly better than 'unknown', but it's still a broad type.

(lint/complexity/noBannedTypes)

🪛 eslint (1.23.1)

[error] 6-6: Don't use Object as a type. The Object type actually means "any non-nullish value", so it is marginally better than unknown.

  • If you want a type meaning "any object", you probably want object instead.
  • If you want a type meaning "any value", you probably want unknown instead.
  • If you really want a type meaning "any non-nullish value", you probably want NonNullable<unknown> instead.

(@typescript-eslint/ban-types)


public id: String = ""
public username: String = ""
public email: String = ""
public phone: String = ""
public avatar: String = ""
public profileBackground: String = ""
public joinedAt: Date = new Date()
public identity: String = ""
public department: String = ""
public grade: String = ""
public bio: String = ""
Comment on lines +14 to +24
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

Use lowercase primitive types

Replace uppercase String with lowercase string for better TypeScript conventions.

Apply this diff to fix the type declarations:

-    public id:                  String  = ""
-    public username:            String  = ""
-    public email:               String  = ""
-    public phone:               String  = ""
-    public avatar:              String  = ""
-    public profileBackground:   String  = ""
+    public id:                  string  = ""
+    public username:            string  = ""
+    public email:              string  = ""
+    public phone:              string  = ""
+    public avatar:             string  = ""
+    public profileBackground:  string  = ""
     public joinedAt:            Date    = new Date()
-    public identity:            String  = ""
-    public department:          String  = ""
-    public grade:               String  = ""
-    public bio:                 String  = ""
+    public identity:           string  = ""
+    public department:         string  = ""
+    public grade:              string  = ""
+    public bio:                string  = ""
📝 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
public id: String = ""
public username: String = ""
public email: String = ""
public phone: String = ""
public avatar: String = ""
public profileBackground: String = ""
public joinedAt: Date = new Date()
public identity: String = ""
public department: String = ""
public grade: String = ""
public bio: String = ""
public id: string = ""
public username: string = ""
public email: string = ""
public phone: string = ""
public avatar: string = ""
public profileBackground: string = ""
public joinedAt: Date = new Date()
public identity: string = ""
public department: string = ""
public grade: string = ""
public bio: string = ""
🧰 Tools
🪛 Biome (1.9.4)

[error] 14-14: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 15-15: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 16-16: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 17-17: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 18-18: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 19-19: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 21-21: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 22-22: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 23-23: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)


[error] 24-24: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

🪛 eslint (1.23.1)

[error] 14-14: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 15-15: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 16-16: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 17-17: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 18-18: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 19-19: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 21-21: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 22-22: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 23-23: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 24-24: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)



static parseJson(json: any) : User {
const user = new User()

for (const [userProperty, jsonField] of Object.entries(this.DATABASE_MAP))
if (json[jsonField] !== undefined)
(user as any)[userProperty] = json[jsonField];

return user
}
Comment on lines +27 to +35
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

Improve type safety and error handling in parseJson

  1. The method uses any type which reduces type safety
  2. Using this in static context could be confusing
  3. Missing input validation for required fields

Consider applying this improvement:

-    static parseJson(json: any) : User {
+    static parseJson(json: Record<string, unknown>) : User {
         const user = new User()
 
-        for (const [userProperty, jsonField] of Object.entries(this.DATABASE_MAP))
+        for (const [userProperty, jsonField] of Object.entries(User.DATABASE_MAP)) {
             if (json[jsonField] !== undefined)
-                (user as any)[userProperty] = json[jsonField];
+                (user as Record<string, unknown>)[userProperty] = json[jsonField];
+        }
+
+        // Validate required fields
+        if (!user.id || !user.email) {
+            throw new Error('Missing required fields: id and email are mandatory');
+        }
 
         return user
     }
📝 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
static parseJson(json: any) : User {
const user = new User()
for (const [userProperty, jsonField] of Object.entries(this.DATABASE_MAP))
if (json[jsonField] !== undefined)
(user as any)[userProperty] = json[jsonField];
return user
}
static parseJson(json: Record<string, unknown>) : User {
const user = new User()
for (const [userProperty, jsonField] of Object.entries(User.DATABASE_MAP)) {
if (json[jsonField] !== undefined)
(user as Record<string, unknown>)[userProperty] = json[jsonField];
}
// Validate required fields
if (!user.id || !user.email) {
throw new Error('Missing required fields: id and email are mandatory');
}
return user
}
🧰 Tools
🪛 Biome (1.9.4)

[error] 30-30: Using this in a static context can be confusing.

this refers to the class.
Unsafe fix: Use the class name instead.

(lint/complexity/noThisInStatic)

🪛 eslint (1.23.1)

[error] 27-27: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)


[error] 32-32: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

}
25 changes: 25 additions & 0 deletions src/routeTree.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Route as LoginImport } from './routes/login'
import { Route as IndexImport } from './routes/index'
import { Route as SalesIndexImport } from './routes/sales/index'
import { Route as MapIndexImport } from './routes/map/index'
import { Route as HomeIndexImport } from './routes/home/index'
import { Route as EventsIndexImport } from './routes/events/index'
import { Route as DinnerIndexImport } from './routes/dinner/index'
import { Route as CalendarIndexImport } from './routes/calendar/index'
Expand Down Expand Up @@ -44,6 +45,11 @@ const MapIndexRoute = MapIndexImport.update({
getParentRoute: () => rootRoute,
} as any)

const HomeIndexRoute = HomeIndexImport.update({
path: '/home/',
getParentRoute: () => rootRoute,
} as any)

const EventsIndexRoute = EventsIndexImport.update({
path: '/events/',
getParentRoute: () => rootRoute,
Expand Down Expand Up @@ -134,6 +140,13 @@ declare module '@tanstack/react-router' {
preLoaderRoute: typeof EventsIndexImport
parentRoute: typeof rootRoute
}
'/home/': {
id: '/home/'
path: '/home'
fullPath: '/home'
preLoaderRoute: typeof HomeIndexImport
parentRoute: typeof rootRoute
}
'/map/': {
id: '/map/'
path: '/map'
Expand Down Expand Up @@ -162,6 +175,7 @@ export interface FileRoutesByFullPath {
'/calendar': typeof CalendarIndexRoute
'/dinner': typeof DinnerIndexRoute
'/events': typeof EventsIndexRoute
'/home': typeof HomeIndexRoute
'/map': typeof MapIndexRoute
'/sales': typeof SalesIndexRoute
}
Expand All @@ -175,6 +189,7 @@ export interface FileRoutesByTo {
'/calendar': typeof CalendarIndexRoute
'/dinner': typeof DinnerIndexRoute
'/events': typeof EventsIndexRoute
'/home': typeof HomeIndexRoute
'/map': typeof MapIndexRoute
'/sales': typeof SalesIndexRoute
}
Expand All @@ -189,6 +204,7 @@ export interface FileRoutesById {
'/calendar/': typeof CalendarIndexRoute
'/dinner/': typeof DinnerIndexRoute
'/events/': typeof EventsIndexRoute
'/home/': typeof HomeIndexRoute
'/map/': typeof MapIndexRoute
'/sales/': typeof SalesIndexRoute
}
Expand All @@ -204,6 +220,7 @@ export interface FileRouteTypes {
| '/calendar'
| '/dinner'
| '/events'
| '/home'
| '/map'
| '/sales'
fileRoutesByTo: FileRoutesByTo
Expand All @@ -216,6 +233,7 @@ export interface FileRouteTypes {
| '/calendar'
| '/dinner'
| '/events'
| '/home'
| '/map'
| '/sales'
id:
Expand All @@ -228,6 +246,7 @@ export interface FileRouteTypes {
| '/calendar/'
| '/dinner/'
| '/events/'
| '/home/'
| '/map/'
| '/sales/'
fileRoutesById: FileRoutesById
Expand All @@ -242,6 +261,7 @@ export interface RootRouteChildren {
CalendarIndexRoute: typeof CalendarIndexRoute
DinnerIndexRoute: typeof DinnerIndexRoute
EventsIndexRoute: typeof EventsIndexRoute
HomeIndexRoute: typeof HomeIndexRoute
MapIndexRoute: typeof MapIndexRoute
SalesIndexRoute: typeof SalesIndexRoute
}
Expand All @@ -255,6 +275,7 @@ const rootRouteChildren: RootRouteChildren = {
CalendarIndexRoute: CalendarIndexRoute,
DinnerIndexRoute: DinnerIndexRoute,
EventsIndexRoute: EventsIndexRoute,
HomeIndexRoute: HomeIndexRoute,
MapIndexRoute: MapIndexRoute,
SalesIndexRoute: SalesIndexRoute,
}
Expand All @@ -279,6 +300,7 @@ export const routeTree = rootRoute
"/calendar/",
"/dinner/",
"/events/",
"/home/",
"/map/",
"/sales/"
]
Expand Down Expand Up @@ -307,6 +329,9 @@ export const routeTree = rootRoute
"/events/": {
"filePath": "events/index.tsx"
},
"/home/": {
"filePath": "home/index.tsx"
},
"/map/": {
"filePath": "map/index.tsx"
},
Expand Down
71 changes: 71 additions & 0 deletions src/routes/home/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { createFileRoute } from '@tanstack/react-router'
// import { VStack } from '../../components'

// const homeItems = [
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// { title: "test", description: "Hello world." },
// ]

// User.fetchSingle().then(user => {
// console.log(user)
// })

// const supabaseService = new SupabaseService()
// SupabaseService.fetchSingle(User, "c7e80cb0-7d3d-411f-9983-5e3addf62980", { email: "ncuapp@test.com" }).then(
// response => {
// console.log(response)
// }
// )

// SupabaseService.fetchMultiple(User).then(
// response => {
// console.log(response)
// }
// )

// async function fetch() {
// const { data } = await supabase.from('members').select('*')
// console.log(data)
// }
// fetch()

export const Route = createFileRoute('/home/')({

component: () => <div>
<section className='h-[200px] mb-[40px] bg-yellow-50 relative border-b-[5px] border-gray-600'>

<div className='flex flex-col absolute left-[10px] bottom-[5px]'>
<h1 className='font-bold text-4xl text-black'>這裡最多七個字</h1>
<h2 className='font-bold text-black'>中文系 二年級</h2>
</div>

{/* <div className='
w-[125px] h-[125px]
absolute right-[15px] bottom-[-25px]
border-[4px] border-gray-600 rounded-full bg-gray-500
'></div> */}

<div className="avatar absolute right-[15px] bottom-[-25px]">
<div className="ring-gray-300 w-[125px] rounded-full ring">
<img src="https://img.daisyui.com/images/stock/photo-1534528741775-53994a69daeb.webp" />
</div>
</div>

<span className='
absolute top-[20px] right-[15px]
text-black font-bold text-xs
'>
變更個人檔案
</span>
</section>
</div >,
})
84 changes: 84 additions & 0 deletions src/services/SupabaseService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import GenericModel from "../models/GenericModel";
import { supabase } from "../utils/supabase";

// Supabase service for quickly accessing data
export default class SupabaseService {


/* Fetch a single object
*
* Parameters
* ----------
* modelClass: class
* The target model class
* targetID: str
* The target record ID
* criteria: Dictionary
* Extra search criteria, please follow the DATABASE_MAPPING of the target model
*
* Returns
* -------
* The target object (of the target model type). Null if record is not found.
*/
public static async fetchSingle<T extends GenericModel>(
modelClass: new() => T,
targetID: String,
criteria?: Partial<Record<string, any>>,
): Promise<T | null> {
Comment on lines +23 to +27
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

Fix TypeScript type declarations.

The method signature has some type-related issues:

  • Use lowercase string instead of String
  • Consider adding proper typing for criteria instead of using Record<string, any>
 public static async fetchSingle<T extends GenericModel>(
     modelClass: new() => T,
-    targetID: String,
-    criteria?: Partial<Record<string, any>>,
+    targetID: string,
+    criteria?: Partial<Record<keyof T, unknown>>,
 ): Promise<T | null> {
📝 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
public static async fetchSingle<T extends GenericModel>(
modelClass: new() => T,
targetID: String,
criteria?: Partial<Record<string, any>>,
): Promise<T | null> {
public static async fetchSingle<T extends GenericModel>(
modelClass: new() => T,
targetID: string,
criteria?: Partial<Record<keyof T, unknown>>,
): Promise<T | null> {
🧰 Tools
🪛 Biome (1.9.4)

[error] 25-25: Don't use 'String' as a type.

Use lowercase primitives for consistency.
Safe fix: Use 'string' instead

(lint/complexity/noBannedTypes)

🪛 eslint (1.23.1)

[error] 25-25: Don't use String as a type. Use string instead

(@typescript-eslint/ban-types)


[error] 26-26: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

const model = (modelClass as any)
const tableName = model.TABLE_NAME

var query = supabase.from(tableName)
.select('*')
.eq("uuid", targetID)

for (const key in criteria)
if (model.DATABASE_MAP[key])
query = query.eq(model.DATABASE_MAP[key], criteria[key])

const data = await query;
Comment on lines +31 to +39
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

Improve query construction and error handling.

Several improvements can be made to the query construction:

  • Use const instead of var
  • Add error handling for the Supabase query
  • Consider validating UUID format before query
-        var query = supabase.from(tableName)
+        const query = supabase.from(tableName)
                         .select('*')
                         .eq("uuid", targetID)
         
         for (const key in criteria)
             if (model.DATABASE_MAP[key])
                 query = query.eq(model.DATABASE_MAP[key], criteria[key])
 
-        const data = await query;
+        const { data, error } = await query;
+        if (error) {
+            throw new Error(`Failed to fetch ${tableName}: ${error.message}`);
+        }

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

🧰 Tools
🪛 eslint (1.23.1)

[error] 31-33: Unexpected var, use let or const instead.

(no-var)


if (!data.data || data.data.length == 0) {
return null
}
const record = data.data[0]

return model.parseJson(record)
}




/* Fetch an array of objects
*
* Parameters
* ----------
* modelClass: class
* The target model class
*
* Returns
* -------
* Array of objects (of the target model type).
* Returns empty array if no records can be found.
*/
public static async fetchMultiple<T extends GenericModel>(
modelClass: new() => T,
): Promise<Array<T>> {
const model = (modelClass as any)
const tableName = model.TABLE_NAME

const data = await supabase.from(tableName)
.select('*')

const records = data.data

const results: Array<T> = []

records?.forEach(record => {
const user = model.parseJson(record)
results.push(user)
})

return results
}
Comment on lines +64 to +83
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 pagination and improve error handling.

The current implementation has several potential issues:

  • Fetching all records without pagination could lead to performance issues
  • Error handling is missing
  • The variable name 'user' is misleading as it could be any model type
 public static async fetchMultiple<T extends GenericModel>(
     modelClass: new() => T,
+    limit: number = 100,
+    offset: number = 0
 ): Promise<Array<T>> {
     const model = (modelClass as any)
     const tableName = model.TABLE_NAME

-    const data = await supabase.from(tableName)
-                    .select('*')
+    const { data, error } = await supabase.from(tableName)
+                    .select('*')
+                    .range(offset, offset + limit - 1)

+    if (error) {
+        throw new Error(`Failed to fetch ${tableName}: ${error.message}`);
+    }
                     
     const records = data.data

     const results: Array<T> = []

     records?.forEach(record => {
-        const user = model.parseJson(record)
-        results.push(user)
+        const item = model.parseJson(record)
+        results.push(item)
     })

     return results
 }
📝 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
public static async fetchMultiple<T extends GenericModel>(
modelClass: new() => T,
): Promise<Array<T>> {
const model = (modelClass as any)
const tableName = model.TABLE_NAME
const data = await supabase.from(tableName)
.select('*')
const records = data.data
const results: Array<T> = []
records?.forEach(record => {
const user = model.parseJson(record)
results.push(user)
})
return results
}
public static async fetchMultiple<T extends GenericModel>(
modelClass: new() => T,
limit: number = 100,
offset: number = 0
): Promise<Array<T>> {
const model = (modelClass as any)
const tableName = model.TABLE_NAME
const { data, error } = await supabase.from(tableName)
.select('*')
.range(offset, offset + limit - 1)
if (error) {
throw new Error(`Failed to fetch ${tableName}: ${error.message}`);
}
const records = data.data
const results: Array<T> = []
records?.forEach(record => {
const item = model.parseJson(record)
results.push(item)
})
return results
}
🧰 Tools
🪛 eslint (1.23.1)

[error] 67-67: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

}
Loading