Generic data model and Supabase data service #125
Conversation
A generic data template model has been implemented. This model can be extended to other classes, such as User. It includes the TABLE_NAME and DATABASE_MAPPING properties for a more efficient database access. The SupabaseService has been implemented to support general data fetching. It currently supports the fetching of a single object and an array of objects. There is no need to implement the data fetching logic repeatedly for each model. - Added SupabaseService - Added GenericModel - Added User model - Added data fetching tests during Homepage initialization
- Removed VStack imports and unused template components
WalkthroughThe changes introduce a new abstract class Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SupabaseService
participant Database
User->>SupabaseService: fetchSingle(User, targetID)
SupabaseService->>Database: Query for targetID
Database-->>SupabaseService: Return record
SupabaseService-->>User: Return User instance
sequenceDiagram
participant User
participant SupabaseService
participant Database
User->>SupabaseService: fetchMultiple(User)
SupabaseService->>Database: Query for all records
Database-->>SupabaseService: Return records
SupabaseService-->>User: Return array of User instances
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (4)
src/models/GenericModel.ts (2)
1-2: LGTM! Consider documenting the contract.The abstract class declaration is appropriate for defining a generic data model contract. Consider adding JSDoc comments to document:
- The purpose of this base class
- The contract that implementing classes must fulfill
- Example usage
+/** + * Abstract base class for database models. + * Provides a contract for database mapping and JSON parsing. + * + * @example + * class User extends GenericModel { + * static TABLE_NAME = 'users'; + * static DATABASE_MAP = { id: 'id', name: 'full_name' }; + * + * static parseJson(json: Record<string, unknown>): User { + * // Implementation + * } + * } + */ export default abstract class GenericModel {
1-13: Consider additional data layer capabilities.As this class serves as the foundation for your data layer, consider adding:
- Validation methods to ensure data integrity
- Type-safe transformation utilities for handling database types
- Error handling patterns for database operations
- Methods for handling relationships between models
Example additions:
abstract class GenericModel { // ... existing code ... abstract validate(): ValidationResult; protected static transformDatabaseValue<T>( value: unknown, field: keyof DatabaseMap ): T; protected static handleDatabaseError( error: unknown, operation: DatabaseOperation ): void; }Would you like me to provide a detailed implementation of these suggestions?
🧰 Tools
🪛 Biome (1.9.4)
[error] 2-13: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 11-11: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[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
Objectas a type. TheObjecttype actually means "any non-nullish value", so it is marginally better thanunknown.
- If you want a type meaning "any object", you probably want
objectinstead.- If you want a type meaning "any value", you probably want
unknowninstead.- If you really want a type meaning "any non-nullish value", you probably want
NonNullable<unknown>instead.(@typescript-eslint/ban-types)
[error] 9-9: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/services/SupabaseService.ts (2)
1-5: Consider refactoring to standalone functions instead of static methods.The class currently only contains static methods. Consider refactoring these into standalone functions for better maintainability and to align with functional programming principles.
-export default class SupabaseService { - public static async fetchSingle... - public static async fetchMultiple... -} +export async function fetchSingle... +export async function fetchMultiple...
1-84: Consider adding comprehensive error handling and logging.The service would benefit from:
- A custom error class for Supabase-related errors
- Logging of database operations
- Retry mechanism for transient failures
Would you like me to provide an implementation for these improvements?
🧰 Tools
🪛 Biome (1.9.4)
[error] 5-84: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[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
Stringas a type. Use string instead(@typescript-eslint/ban-types)
[error] 26-26: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[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)
[error] 67-67: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/models/GenericModel.ts(1 hunks)src/models/User.ts(1 hunks)src/routeTree.gen.ts(13 hunks)src/routes/home/index.tsx(1 hunks)src/services/SupabaseService.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/home/index.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/models/GenericModel.ts
[error] 2-13: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[error] 11-11: This code is unreachable
... because this statement will throw an exception beforehand
(lint/correctness/noUnreachable)
[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)
src/models/User.ts
[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)
[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)
[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)
src/services/SupabaseService.ts
[error] 5-84: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
[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)
src/models/GenericModel.ts
[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
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead. - If you really want a type meaning "any non-nullish value", you probably want
NonNullable<unknown>instead.
(@typescript-eslint/ban-types)
[error] 9-9: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
src/models/User.ts
[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
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead. - If you really want a type meaning "any non-nullish value", you probably want
NonNullable<unknown>instead.
(@typescript-eslint/ban-types)
[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)
[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)
src/services/SupabaseService.ts
[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)
[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)
[error] 67-67: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🔇 Additional comments (3)
src/models/User.ts (1)
1-3: Verify GenericModel abstract class implementation
The User class extends GenericModel, but we need to ensure all abstract members are properly implemented.
✅ Verification successful
Implementation of abstract members is correct
The User class correctly implements all required abstract members from GenericModel:
- Static
TABLE_NAMEis defined as "members" - Static
DATABASE_MAPis implemented with the required field mappings - Static
parseJsonmethod is overridden with proper implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for GenericModel implementation to verify abstract members
ast-grep --pattern 'abstract class GenericModel {
$$$
}'
Length of output: 741
Script:
#!/bin/bash
# Search for User class implementation to verify abstract members
ast-grep --pattern 'class User extends GenericModel {
$$$
}'
Length of output: 1890
src/services/SupabaseService.ts (1)
1-2: Verify GenericModel integration.
Let's ensure the service properly integrates with all required GenericModel features.
✅ Verification successful
Integration with GenericModel is properly implemented
The SupabaseService correctly integrates with GenericModel by:
- Using proper type constraints (
<T extends GenericModel>) - Utilizing all required GenericModel features:
TABLE_NAMEfor table identificationDATABASE_MAPfor field mappingparseJsonfor object instantiation
The service implementation provides a type-safe and flexible way to perform database operations with any model that extends GenericModel.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for GenericModel implementations to verify compatibility
ast-grep --pattern 'class $_ extends GenericModel {
static TABLE_NAME = $_;
static DATABASE_MAP = $_;
}'
Length of output: 110
Script:
#!/bin/bash
# Let's first check the GenericModel implementation
cat src/models/GenericModel.ts
# Then search for any class extending GenericModel
ast-grep --pattern 'class $_ extends GenericModel'
# Also check how SupabaseService uses GenericModel
cat src/services/SupabaseService.ts
Length of output: 2676
src/routeTree.gen.ts (1)
Line range hint 1-344: LGTM! Note: This is an auto-generated file.
The changes consistently integrate the new /home/ route across all necessary interfaces and objects. The route configuration follows the established patterns used by other routes in the application.
Let's verify the route configuration consistency:
✅ Verification successful
Route configuration is properly integrated and source files exist
The verification confirms:
- Both route files exist:
src/routes/home/index.tsxandsrc/routes/home/infoCard.tsx - The HomeIndex route is consistently defined across all required sections:
- Import declaration
- Route configuration with correct path
/home/ - Type definitions in interfaces
- Route tree configuration
- The route follows the same pattern as other routes (e.g., MapIndex, SalesIndex)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the home route is consistently defined and the source file exists
# Check if the home route source file exists
if ! [ -f "src/routes/home/index.tsx" ]; then
echo "Error: Home route source file not found"
exit 1
fi
# Verify route definition consistency
rg -U --multiline '/home/' src/routeTree.gen.ts | grep -E '(path:|filePath:)'
Length of output: 189
Script:
#!/bin/bash
# Let's verify the complete route configuration for home routes
# Check both route files
echo "Checking home route files:"
ls -l src/routes/home/index.tsx src/routes/home/infoCard.tsx 2>/dev/null
echo -e "\nChecking all home route references:"
rg -A 2 "HomeIndex" src/routeTree.gen.ts
echo -e "\nVerifying route manifest entries:"
rg -A 2 '"/home' src/routeTree.gen.ts | grep -E '(filePath:|path:)'
Length of output: 1487
| // Abstract property for table name | ||
| static TABLE_NAME: string; | ||
|
|
||
| static DATABASE_MAP: Object; |
There was a problem hiding this comment.
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.
| // 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
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead. - 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 | ||
| } |
There was a problem hiding this comment.
Improve type safety and implementation of parseJson.
Several issues need to be addressed:
- Using
anytype defeats TypeScript's type safety - The return statement after throw is unreachable
- 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.
| // 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)
| 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 | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and error handling in parseJson
- The method uses
anytype which reduces type safety - Using
thisin static context could be confusing - 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.
| 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)
| 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 = "" |
There was a problem hiding this comment.
🛠️ 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.
| 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 TABLE_NAME = "members" | ||
| static DATABASE_MAP: Object = { | ||
| id: "uuid", | ||
| username: "username", | ||
| email: "fk_email", | ||
| identity: "fk_identity", | ||
| avatar: "avatar", | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and complete database mappings
- Replace the generic
Objecttype with a proper type definition for better type safety. - 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
objectinstead. - If you want a type meaning "any value", you probably want
unknowninstead. - If you really want a type meaning "any non-nullish value", you probably want
NonNullable<unknown>instead.
(@typescript-eslint/ban-types)
| public static async fetchSingle<T extends GenericModel>( | ||
| modelClass: new() => T, | ||
| targetID: String, | ||
| criteria?: Partial<Record<string, any>>, | ||
| ): Promise<T | null> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix TypeScript type declarations.
The method signature has some type-related issues:
- Use lowercase
stringinstead ofString - 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.
| 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)
| 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 | ||
| } |
There was a problem hiding this comment.
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.
| 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)
| 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; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve query construction and error handling.
Several improvements can be made to the query construction:
- Use
constinstead ofvar - 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)
通用資料模型 & 資料庫連接服務
透過建立通用模型，供各功能自行延伸，並利用 SupabaseService，使 CRUD 可集中處理。目前引入了 fetchSingle() 與 fetchMultiple() 功能，服務會同時自動轉換資料庫欄位，並以給定的 type 輸出結果。
A generic data template model has been implemented. This model can be extended to other classes, such as User. It includes the TABLE_NAME and DATABASE_MAPPING properties for a more efficient database access.
The SupabaseService has been implemented to support general data fetching. It currently supports the fetching of a single object and an array of objects. There is no need to implement the data fetching logic repeatedly for each model.
Note: 首頁 GUI 可暫時忽略
Summary by CodeRabbit
Release Notes
New Features
Usermodel to manage user data with properties likeusername,email, andbio.Bug Fixes
Documentation