-
Notifications
You must be signed in to change notification settings - Fork 25
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: Generic data model and Supabase data service #124
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// GenericModel class | ||
export default abstract class GenericModel { | ||
// Abstract property for table name | ||
static TABLE_NAME: string; | ||
|
||
static DATABASE_MAP: Object; | ||
|
||
// Abstract method for parsing JSON | ||
static parseJson(json: any): GenericModel { | ||
throw new Error("parseJson method not implemented."); | ||
} | ||
Comment on lines
+8
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety and error handling of parseJson method. Several improvements can be made to enhance type safety and developer experience:
- static parseJson(json: any): GenericModel {
- throw new Error("parseJson method not implemented.");
+ abstract static parseJson(json: unknown): ThisType {
+ throw new Error(
+ `${this.name}.parseJson() must be implemented to parse database records.`
+ );
+ } Consider also adding runtime type checking: protected static validateJson(json: unknown): void {
if (!json || typeof json !== 'object') {
throw new Error(`Invalid JSON data provided to ${this.name}.parseJson()`);
}
} 🧰 Tools🪛 eslint (1.23.1)[error] 9-9: 'json' is defined but never used. (@typescript-eslint/no-unused-vars) [error] 9-9: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any) |
||
} |
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety and documentation of database mappings.
- static DATABASE_MAP: Object = {
+ static DATABASE_MAP: Record<keyof User, string> = {
id: "uuid",
username: "username",
email: "fk_email",
identity: "fk_identity",
avatar: "avatar",
}
+ /** Database field mappings
+ * @property fk_email - Foreign key reference to user's email
+ * @property fk_identity - Foreign key reference to user's identity
+ */
static DATABASE_MAP: Record<keyof User, 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
(@typescript-eslint/ban-types)
Comment on lines
+6
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Database mapping is incomplete - several properties are missing The following public properties are not mapped in DATABASE_MAP but might need persistence:
This mismatch between class properties and database mappings could lead to data not being properly persisted or retrieved from the database. 🔗 Analysis chainVerify database mapping completeness. Ensure that all User properties that need database persistence are included in the DATABASE_MAP. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for potential missing database mappings
# Get all public properties from the User class
echo "Public properties in User class:"
rg "^\s+public\s+(\w+):" src/models/User.ts
# Compare with database mappings
echo -e "\nDatabase mappings:"
rg "static\s+DATABASE_MAP.*?=.*?{.*?}" src/models/User.ts -U --multiline
Length of output: 896 Script: #!/bin/bash
# Description: Check for potential missing database mappings with fixed regex
# Get all public properties from the User class (already have this)
# Now let's get the DATABASE_MAP with a proper regex
echo "Database mappings:"
rg "static\s+DATABASE_MAP.*?\{[\s\S]+?\}" src/models/User.ts
# Alternative approach using ast-grep to find the object literal
echo -e "\nUsing ast-grep to find DATABASE_MAP:"
ast-grep --pattern 'static DATABASE_MAP: Object = { $$$ }'
Length of output: 259 Script: #!/bin/bash
# Let's try a simpler approach with basic pattern matching
echo "Database mappings (simple search):"
rg "DATABASE_MAP" -A 10 src/models/User.ts
# Also get the complete User class to see the context
echo -e "\nComplete User class definition:"
ast-grep --pattern 'class User { $$$ }'
Length of output: 802 🧰 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
(@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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type definitions and add property documentation.
- 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 = ""
+ /** Unique identifier for the user */
+ public id: string = ""
+ /** User's display name */
+ public username: string = ""
+ /** User's email address */
+ public email: string = ""
+ /** User's phone number */
+ public phone: string = ""
+ /** URL to user's avatar image */
+ public avatar: string = ""
+ /** URL to user's profile background image */
+ public profileBackground: string = ""
+ /** Timestamp when user joined */
+ public joinedAt: Date = new Date()
+ /** User's role or identity type */
+ public identity: string = ""
+ /** User's academic department */
+ public department: string = ""
+ /** User's academic grade/year */
+ public grade: string = ""
+ /** User's biography or description */
+ public bio: string = "" 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 14-14: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 15-15: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 16-16: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 17-17: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 18-18: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 19-19: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 21-21: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 22-22: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 23-23: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) [error] 24-24: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) 🪛 eslint (1.23.1)[error] 14-14: Don't use (@typescript-eslint/ban-types) [error] 15-15: Don't use (@typescript-eslint/ban-types) [error] 16-16: Don't use (@typescript-eslint/ban-types) [error] 17-17: Don't use (@typescript-eslint/ban-types) [error] 18-18: Don't use (@typescript-eslint/ban-types) [error] 19-19: Don't use (@typescript-eslint/ban-types) [error] 21-21: Don't use (@typescript-eslint/ban-types) [error] 22-22: Don't use (@typescript-eslint/ban-types) [error] 23-23: Don't use (@typescript-eslint/ban-types) [error] 24-24: Don't use (@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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety and error handling in parseJson method. The current implementation has several issues that should be addressed: - 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();
+
+ try {
+ for (const [userProperty, jsonField] of Object.entries(User.DATABASE_MAP)) {
+ if (json[jsonField] !== undefined) {
+ // Type-safe assignment with validation
+ const value = json[jsonField];
+ switch(userProperty) {
+ case 'joinedAt':
+ user[userProperty] = new Date(value as string);
+ break;
+ default:
+ if (typeof value === 'string') {
+ user[userProperty as keyof User] = value;
+ }
+ }
+ }
+ }
+ } catch (error) {
+ throw new Error(`Failed to parse user data: ${error.message}`);
+ }
+
+ return user;
+ } Changes made:
📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 30-30: Using this in a static context can be confusing. this refers to the class. (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) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,89 @@ | ||||||||||||||||||||||||||||
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." }, | ||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||
Comment on lines
+4
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove or utilize the unused The -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." },
-] 📝 Committable suggestion
Suggested change
🧰 Tools🪛 eslint (1.23.1)[error] 4-4: 'homeItems' is assigned a value but never used. (@typescript-eslint/no-unused-vars) |
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// 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() | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
Comment on lines
+18
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Clean up commented data fetching implementations. There are multiple commented sections showing different approaches to data fetching. This could lead to confusion about the intended implementation. Additionally, there are hardcoded values (UUID, email) that shouldn't be committed. Consider:
|
||||||||||||||||||||||||||||
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> | ||||||||||||||||||||||||||||
Comment on lines
+47
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Externalize text content for internationalization. The hardcoded Chinese text should be moved to a localization system to support multiple languages. - <h1 className='font-bold text-4xl text-black'>這裡最多七個字</h1>
- <h2 className='font-bold text-black'>中文系 二年級</h2>
+ <h1 className='font-bold text-4xl text-black'>{t('home.title')}</h1>
+ <h2 className='font-bold text-black'>{t('home.subtitle')}</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> | ||||||||||||||||||||||||||||
Comment on lines
+57
to
+61
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace stock photo URL with a proper avatar handling system. The current implementation uses a hardcoded stock photo URL. Consider:
<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" />
+ <img
+ src={user?.avatarUrl ?? defaultAvatarUrl}
+ alt={`${user?.name}'s avatar`}
+ onError={(e) => e.currentTarget.src = defaultAvatarUrl}
+ />
</div>
</div>
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<span className=' | ||||||||||||||||||||||||||||
absolute top-[20px] right-[15px] | ||||||||||||||||||||||||||||
text-black font-bold text-xs | ||||||||||||||||||||||||||||
'> | ||||||||||||||||||||||||||||
變更個人檔案 | ||||||||||||||||||||||||||||
</span> | ||||||||||||||||||||||||||||
</section> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<VStack className='px-[10px] gap-[20px]'> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
{/* {homeItems.map((item, index) => ( | ||||||||||||||||||||||||||||
<WelcomeCard title={item.title} description={item.description} /> | ||||||||||||||||||||||||||||
))} */} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
{/* <NcuInput /> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<NcuTextarea> | ||||||||||||||||||||||||||||
Hello | ||||||||||||||||||||||||||||
</NcuTextarea> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<NcuSwitch label="test">X</NcuSwitch> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
<NcuButton>Test</NcuButton> */} | ||||||||||||||||||||||||||||
</VStack> | ||||||||||||||||||||||||||||
</div>, | ||||||||||||||||||||||||||||
}) |
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 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider refactoring to instance-based design for better testability The current static-only class design could make unit testing and dependency injection more difficult. Consider refactoring to an instance-based design: import GenericModel from "../models/GenericModel";
import { supabase } from "../utils/supabase";
-// Supabase service for quickly accessing data
-export default class SupabaseService {
+export interface ISupabaseClient {
+ from(table: string): any;
+}
+
+export default class SupabaseService {
+ constructor(private readonly client: ISupabaseClient = supabase) {} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
/* 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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
Suggested change
🧰 Tools🪛 Biome (1.9.4)[error] 25-25: Don't use 'String' as a type. Use lowercase primitives for consistency. (lint/complexity/noBannedTypes) 🪛 eslint (1.23.1)[error] 25-25: Don't use (@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]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+28
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve type safety and query building The current implementation has several areas for improvement:
- const model = (modelClass as any)
- const tableName = model.TABLE_NAME
+ const tableName = modelClass.prototype.constructor.TABLE_NAME
+ if (!tableName) {
+ throw new Error(`Table name not defined for model ${modelClass.name}`)
+ }
- var query = supabase.from(tableName)
+ let 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])
+ if (modelClass.prototype.constructor.DATABASE_MAP?.[key]) {
+ query = query.eq(
+ modelClass.prototype.constructor.DATABASE_MAP[key],
+ criteria[key]
+ )
+ } 📝 Committable suggestion
Suggested change
🧰 Tools🪛 eslint (1.23.1)[error] 28-28: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any) [error] 31-33: Unexpected var, use let or const instead. (no-var) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const data = await query; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add pagination and filtering support to fetchMultiple The current implementation has several limitations:
public static async fetchMultiple<T extends GenericModel>(
modelClass: new() => T,
+ options?: {
+ page?: number;
+ pageSize?: number;
+ filters?: Partial<Record<keyof T, unknown>>;
+ }
): Promise<Array<T>> {
- const model = (modelClass as any)
- const tableName = model.TABLE_NAME
+ const tableName = modelClass.prototype.constructor.TABLE_NAME
+ if (!tableName) {
+ throw new Error(`Table name not defined for model ${modelClass.name}`)
+ }
- const data = await supabase.from(tableName)
- .select('*')
+ let query = supabase.from(tableName).select('*')
+
+ // Apply filters
+ if (options?.filters) {
+ for (const [key, value] of Object.entries(options.filters)) {
+ if (modelClass.prototype.constructor.DATABASE_MAP?.[key]) {
+ query = query.eq(
+ modelClass.prototype.constructor.DATABASE_MAP[key],
+ value
+ )
+ }
+ }
+ }
+
+ // Apply pagination
+ if (options?.page && options?.pageSize) {
+ const start = (options.page - 1) * options.pageSize
+ query = query.range(start, start + options.pageSize - 1)
+ }
+
+ const { data, error } = await query
+
+ if (error) {
+ throw new Error(`Failed to fetch ${modelClass.name} list: ${error.message}`)
+ }
- const records = data.data
+ if (!data || data.length === 0) {
+ return []
+ }
const results: Array<T> = []
- records?.forEach(record => {
- const user = model.parseJson(record)
- results.push(user)
+ data.forEach(record => {
+ results.push(modelClass.prototype.constructor.parseJson(record))
})
return results 📝 Committable suggestion
Suggested change
🧰 Tools🪛 eslint (1.23.1)[error] 67-67: Unexpected any. Specify a different type. (@typescript-eslint/no-explicit-any) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} |
There was a problem hiding this comment.
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 of DATABASE_MAP.
The
Object
type is too broad and provides little type safety. Consider:Also, consider making these properties abstract to enforce implementation:
🧰 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. TheObject
type actually means "any non-nullish value", so it is marginally better thanunknown
.object
instead.unknown
instead.NonNullable<unknown>
instead.(@typescript-eslint/ban-types)