-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: typebox mutations #1221
feat: typebox mutations #1221
Conversation
Reviewer's Guide by SourceryThis pull request migrates the mutation schemas from Zod to TypeBox. This change enhances type safety, improves validation, and provides better integration with the existing codebase. No diagrams generated as the changes look simple and do not need a visual representation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @roderik - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment explaining why Zod is being replaced with TypeBox.
- The
safeParse
function should be updated to handle errors in a more user-friendly way, perhaps by returning a result type instead of throwing an error.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Review instructions: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 3 issues found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
/** | ||
* Redacts sensitive fields in an object by replacing their values with asterisks | ||
*/ | ||
export function redactSensitiveFields(obj: unknown): unknown { |
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.
🚨 suggestion (security): Review the redaction utility logic.
Currently, the function redacts the 'pincode' field by replacing its value with asterisks. Consider whether additional sensitive fields should be redacted to meet security requirements.
}), | ||
valueInBaseCurrency: z.fiatCurrencyAmount(), | ||
}); | ||
export function CreateBondSchema({ |
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.
issue (complexity): Consider using a default constant schema for common cases and allowing customization only when necessary to reduce cognitive overhead and simplify usage for the majority of cases where the options don't vary.
The additional layer of parameterization does increase cognitive overhead when these options rarely vary. Consider maintaining a default constant schema and only allowing customization when needed. For example, you could define a default schema and then write a simple wrapper for the rare cases that require customization:
// Define default values
const DEFAULTS = {
maxCap: 1000000,
minCap: 1,
maxFaceValue: 1000000,
minFaceValue: 1,
decimals: 2,
};
// Default constant schema
export const DefaultCreateBondSchema = t.Object({
assetName: t.String({
description: "The name of the bond",
minLength: 1,
}),
// … other fields remain unchanged
cap: t.Amount(DEFAULTS.maxCap, DEFAULTS.minCap, DEFAULTS.decimals, {
description: "Maximum issuance amount",
errorMessage: "Must be at least 1",
}),
faceValue: t.Amount(
DEFAULTS.maxFaceValue,
DEFAULTS.minFaceValue,
DEFAULTS.decimals,
{
description: "Face value of the bond",
errorMessage: "Must be at least 1",
}
),
// … remaining fields
});
// Use a simple function for customization when needed
export function CreateBondSchema(options: Partial<typeof DEFAULTS> = {}) {
const { maxCap, minCap, maxFaceValue, minFaceValue, decimals } = {
...DEFAULTS,
...options,
};
return t.Object({
assetName: t.String({
description: "The name of the bond",
minLength: 1,
}),
// … other fields remain as in DefaultCreateBondSchema
cap: t.Amount(maxCap, minCap, decimals, {
description: "Maximum issuance amount",
errorMessage: "Must be at least 1",
}),
faceValue: t.Amount(maxFaceValue, minFaceValue, decimals, {
description: "Face value of the bond",
errorMessage: "Must be at least 1",
}),
// … remaining fields
});
}
This way, most of your code can directly use the simpler DefaultCreateBondSchema
while the function remains available for exceptional cases.
userAddress: z.address(), | ||
assettype: z.assetType(), | ||
}); | ||
export function DisallowUserSchema() { |
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.
issue (complexity): Consider memoizing the schema to avoid recreating it every time the function is called, which reduces unnecessary indirection and improves performance without changing the API.
If you don’t need runtime customization, you can simplify by memoizing the schema rather than wrapping the definition in a function that creates a new schema every time. For example, you could define the schema once and then export a getter that returns the memoized instance:
const _disallowUserSchema = t.Object(
{
address: t.EthereumAddress({
description: "The contract address",
}),
pincode: t.Pincode({
description: "The pincode for signing the transaction",
}),
userAddress: t.EthereumAddress({
description: "The address of the user to disallow",
}),
assettype: t.AssetType({
description: "The type of asset",
}),
},
{
description: "Schema for validating disallow user mutation inputs",
}
);
export function DisallowUserSchema() {
return _disallowUserSchema;
}
export type DisallowUserInput = StaticDecode<ReturnType<typeof DisallowUserSchema>>;
This approach reduces the extra indirection by ensuring the schema is created only once while preserving the existing API.
pincode: z.pincode(), | ||
assettype: z.assetType(), | ||
}); | ||
export function WithdrawSchema({ |
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.
issue (complexity): Consider exposing a constant/default schema for common configurations and a parameterized function for specific cases to simplify code usage and reduce perceived complexity
You could simplify code usage by exposing a constant/default schema and, only when necessary, the parameterized function. For example, if most cases use a common configuration, create a default export that wraps your dynamic function:
const DEFAULT_MAX = 1000;
const DEFAULT_MIN = 1;
const DEFAULT_DECIMALS = 18;
export const withdrawSchemaDefault = WithdrawSchema({
maxAmount: DEFAULT_MAX,
minAmount: DEFAULT_MIN,
decimals: DEFAULT_DECIMALS,
});
export type WithdrawInput = StaticDecode<ReturnType<typeof WithdrawSchema>>;
Then use withdrawSchemaDefault
in most places and call WithdrawSchema
with custom values only when really needed. This makes usage clearer and reduces perceived complexity.
decimals?: number; | ||
} = {}) { | ||
const maxAmount = balance ? Number(balance) : undefined; | ||
const min = minAmount ? minAmount : decimals ? 10 ** -decimals : 1; |
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.
suggestion (code-quality): Avoid unneeded ternary statements (simplify-ternary
)
const min = minAmount ? minAmount : decimals ? 10 ** -decimals : 1; | |
const min = minAmount || (decimals ? 10 ** -decimals : 1); |
Explanation
It is possible to simplify certain ternary statements into either use of an||
or !
.This makes the code easier to read, since there is no conditional logic.
options?: SchemaOptions | ||
) => | ||
t.Number({ | ||
minimum: min, | ||
minimum: min ? min : decimals ? 10 ** -decimals : 0, |
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.
suggestion (code-quality): Avoid unneeded ternary statements (simplify-ternary
)
minimum: min ? min : decimals ? 10 ** -decimals : 0, | |
minimum: min || (decimals ? 10 ** -decimals : 0), |
Explanation
It is possible to simplify certain ternary statements into either use of an||
or !
.This makes the code easier to read, since there is no conditional logic.
commit 8064a65 Author: janb87 <[email protected]> Date: Mon Mar 24 12:07:17 2025 +0100 improved schema commit 60b6155 Author: janb87 <[email protected]> Date: Mon Mar 24 11:44:44 2025 +0100 fix type commit 066b19f Merge: 43ffd40 2913e09 Author: Jan Bevers <[email protected]> Date: Mon Mar 24 11:39:57 2025 +0100 Merge branch 'main' into jan/fix-schema-errors commit 43ffd40 Author: janb87 <[email protected]> Date: Mon Mar 24 11:39:30 2025 +0100 fix commit 9eff6eb Author: janb87 <[email protected]> Date: Mon Mar 24 11:30:24 2025 +0100 fix: asset creation and activity commit 2913e09 Author: Roderik van der Veer <[email protected]> Date: Mon Mar 24 11:06:07 2025 +0100 feat: typebox mutations (#1221) Migrate Zod validation schemas to TypeBox for improved type safety and schema definition across multiple mutations, including bond creation, fund creation, contact addition, tokenized deposit creation, asset transfer, equity creation, yield schedule setting, cryptocurrency creation, stablecoin creation, asset withdrawal, collateral updating, asset burning, asset minting, bond topping up, asset freezing, and general asset transferring. This change enhances the reliability and maintainability of the application's data validation processes. Enhancements: - Replace Zod schemas with TypeBox schemas for various mutations to leverage its static type checking and schema definition capabilities. - Update the safe-action client to use typeboxAdapter for validation. - Improve error logging for TypeBox validation errors by including input data and error details. - Add descriptions to TypeBox schemas for better documentation and clarity. - Implement amount validation using TypeBox's Amount function, providing more control over minimum and maximum values. - Add RoleMap type to validate user roles selection Tests: - Ensure all mutations with schema changes continue to function correctly and validate data as expected. commit f29892e Author: Jan Bevers <[email protected]> Date: Mon Mar 24 10:51:21 2025 +0100 fix: user onboarding + user details (#1223) Fixes user onboarding and user details functionality by adjusting data types, modifying user detail retrieval, and updating authentication configurations. Bug Fixes: - Fixes an issue where event counts were incorrectly typed as BigInt in the AssetActivitySchema, changing them to Number. - Fixes an issue where user data and account data were not being merged correctly in the user list query. - Makes the currency field optional during user authentication.
commit 810f6c1 Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:22:05 2025 +0100 fix user currency commit 15b1096 Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:21:39 2025 +0100 cleanup commit a4bb1c0 Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:18:22 2025 +0100 fix test commit adc106e Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:17:38 2025 +0100 resolve merge conflicts commit 9809eff Merge: c52dbd4 b2d9026 Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:17:28 2025 +0100 Merge branch 'main' into fix/validation-erros * main: fix: asset creation and activity (#1224) feat: typebox mutations (#1221) fix: user onboarding + user details (#1223) commit c52dbd4 Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:16:52 2025 +0100 Squashed commit of the following: commit 8064a65 Author: janb87 <[email protected]> Date: Mon Mar 24 12:07:17 2025 +0100 improved schema commit 60b6155 Author: janb87 <[email protected]> Date: Mon Mar 24 11:44:44 2025 +0100 fix type commit 066b19f Merge: 43ffd40 2913e09 Author: Jan Bevers <[email protected]> Date: Mon Mar 24 11:39:57 2025 +0100 Merge branch 'main' into jan/fix-schema-errors commit 43ffd40 Author: janb87 <[email protected]> Date: Mon Mar 24 11:39:30 2025 +0100 fix commit 9eff6eb Author: janb87 <[email protected]> Date: Mon Mar 24 11:30:24 2025 +0100 fix: asset creation and activity commit 2913e09 Author: Roderik van der Veer <[email protected]> Date: Mon Mar 24 11:06:07 2025 +0100 feat: typebox mutations (#1221) Migrate Zod validation schemas to TypeBox for improved type safety and schema definition across multiple mutations, including bond creation, fund creation, contact addition, tokenized deposit creation, asset transfer, equity creation, yield schedule setting, cryptocurrency creation, stablecoin creation, asset withdrawal, collateral updating, asset burning, asset minting, bond topping up, asset freezing, and general asset transferring. This change enhances the reliability and maintainability of the application's data validation processes. Enhancements: - Replace Zod schemas with TypeBox schemas for various mutations to leverage its static type checking and schema definition capabilities. - Update the safe-action client to use typeboxAdapter for validation. - Improve error logging for TypeBox validation errors by including input data and error details. - Add descriptions to TypeBox schemas for better documentation and clarity. - Implement amount validation using TypeBox's Amount function, providing more control over minimum and maximum values. - Add RoleMap type to validate user roles selection Tests: - Ensure all mutations with schema changes continue to function correctly and validate data as expected. commit f29892e Author: Jan Bevers <[email protected]> Date: Mon Mar 24 10:51:21 2025 +0100 fix: user onboarding + user details (#1223) Fixes user onboarding and user details functionality by adjusting data types, modifying user detail retrieval, and updating authentication configurations. Bug Fixes: - Fixes an issue where event counts were incorrectly typed as BigInt in the AssetActivitySchema, changing them to Number. - Fixes an issue where user data and account data were not being merged correctly in the user list query. - Makes the currency field optional during user authentication. commit b2d9026 Author: Jan Bevers <[email protected]> Date: Mon Mar 24 12:15:59 2025 +0100 fix: asset creation and activity (#1224) Improve type safety by updating schemas to handle null values and use safe parsing for transaction details and list existence checks. commit 291b21c Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 12:05:53 2025 +0100 unit test wait for single transaction commit 2913e09 Author: Roderik van der Veer <[email protected]> Date: Mon Mar 24 11:06:07 2025 +0100 feat: typebox mutations (#1221) Migrate Zod validation schemas to TypeBox for improved type safety and schema definition across multiple mutations, including bond creation, fund creation, contact addition, tokenized deposit creation, asset transfer, equity creation, yield schedule setting, cryptocurrency creation, stablecoin creation, asset withdrawal, collateral updating, asset burning, asset minting, bond topping up, asset freezing, and general asset transferring. This change enhances the reliability and maintainability of the application's data validation processes. Enhancements: - Replace Zod schemas with TypeBox schemas for various mutations to leverage its static type checking and schema definition capabilities. - Update the safe-action client to use typeboxAdapter for validation. - Improve error logging for TypeBox validation errors by including input data and error details. - Add descriptions to TypeBox schemas for better documentation and clarity. - Implement amount validation using TypeBox's Amount function, providing more control over minimum and maximum values. - Add RoleMap type to validate user roles selection Tests: - Ensure all mutations with schema changes continue to function correctly and validate data as expected. commit f29892e Author: Jan Bevers <[email protected]> Date: Mon Mar 24 10:51:21 2025 +0100 fix: user onboarding + user details (#1223) Fixes user onboarding and user details functionality by adjusting data types, modifying user detail retrieval, and updating authentication configurations. Bug Fixes: - Fixes an issue where event counts were incorrectly typed as BigInt in the AssetActivitySchema, changing them to Number. - Fixes an issue where user data and account data were not being merged correctly in the user list query. - Makes the currency field optional during user authentication. commit 6e8ce1f Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 10:48:16 2025 +0100 fix assets stats schema commit 892f4b7 Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 10:45:24 2025 +0100 cleanup console log commit 791903f Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 10:44:42 2025 +0100 fix asset activity fragment commit 70a7dbe Author: Snigdha Singh <[email protected]> Date: Mon Mar 24 10:04:12 2025 +0100 fix fund predict address
Summary by Sourcery
Migrate Zod validation schemas to TypeBox for improved type safety and schema definition across multiple mutations, including bond creation, fund creation, contact addition, tokenized deposit creation, asset transfer, equity creation, yield schedule setting, cryptocurrency creation, stablecoin creation, asset withdrawal, collateral updating, asset burning, asset minting, bond topping up, asset freezing, and general asset transferring. This change enhances the reliability and maintainability of the application's data validation processes.
Enhancements:
Tests: