Add qfStrategy to qfRounds #1903
Conversation
WalkthroughThis pull request introduces a new migration and updates to the QF (Quadratic Funding) round entity to support a new strategy classification. A new enum Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
migration/1736719823637-MarkRoundsStrategy.ts (1)
14-22: Ensure safe rollback by dropping dependencies firstThe down migration attempts to drop the column before dropping the enum type. While this works, it's better to explicitly handle dependencies in reverse order.
Apply this diff to improve the rollback sequence:
public async down(queryRunner: QueryRunner): Promise<void> { await queryRunner.query(` - ALTER TABLE "qf_round" - DROP COLUMN "qfStrategy"; - `); - await queryRunner.query(` - DROP TYPE "qf_strategy_enum"; + DROP TYPE IF EXISTS "qf_strategy_enum" CASCADE; `); }src/entities/qfRound.ts (2)
23-26: Standardize enum value casingThe enum values use inconsistent casing: 'Cocm' vs 'regular'. Consider using UPPER_SNAKE_CASE for enum values as it's a common convention.
Apply this diff to standardize the casing:
export enum QfStrategyEnum { - Cocm = 'cocm', - Regular = 'regular', + COCM = 'COCM', + REGULAR = 'REGULAR', }
28-30: Add documentation for the enum valuesThe enum values lack documentation explaining their purpose and when to use each strategy.
Add JSDoc comments to document the enum:
+/** + * Defines the strategy for Quadratic Funding rounds + * @enum {string} + */ export enum QfStrategyEnum { + /** COCM (Cost of Capital Method) strategy for specialized funding calculations */ COCM = 'COCM', + /** Regular strategy for standard quadratic funding calculations */ REGULAR = 'REGULAR', }src/server/adminJs/tabs/qfRoundTab.ts (1)
314-322: Enhance qfStrategy field configurationThe current configuration lacks validation and could benefit from a custom component for better user experience.
Consider enhancing the field configuration:
qfStrategy: { + type: 'string', isVisible: { filter: false, list: false, show: true, new: true, edit: true, }, + custom: { + required: true, + }, + components: { + edit: adminJs.bundle('./components/QFStrategySelect'), + show: adminJs.bundle('./components/QFStrategyDisplay'), + }, },Would you like me to help create these custom components for a better user interface?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
migration/1736719823637-MarkRoundsStrategy.ts(1 hunks)src/entities/qfRound.ts(3 hunks)src/server/adminJs/tabs/qfRoundTab.ts(1 hunks)
🔇 Additional comments (1)
src/entities/qfRound.ts (1)
108-115: Consider making qfStrategy required for productionThe field is marked as nullable which might lead to inconsistent states. Consider making it required with a default value.
Let's verify if there are any existing QF rounds that would be affected:
If there are no existing rounds or if you can handle migration, consider this change:
@Field(_type => QfStrategyEnum, { nullable: true }) @Column({ type: 'enum', enum: QfStrategyEnum, - default: QfStrategyEnum.Regular, - nullable: true, + default: QfStrategyEnum.REGULAR, + nullable: false, }) - qfStrategy?: QfStrategyEnum; + qfStrategy: QfStrategyEnum;
| await queryRunner.query(` | ||
| ALTER TABLE "qf_round" | ||
| ADD COLUMN "qfStrategy" "qf_strategy_enum" DEFAULT 'regular'; | ||
| `); |
There was a problem hiding this comment.
Fix the migration sequence: Create enum type before using it
The migration attempts to add a column of type qf_strategy_enum before creating the enum type itself. This will fail as the type doesn't exist yet.
Apply this diff to fix the sequence:
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`
- ALTER TABLE "qf_round"
- ADD COLUMN "qfStrategy" "qf_strategy_enum" DEFAULT 'regular';
- `);
- await queryRunner.query(`
CREATE TYPE "qf_strategy_enum" AS ENUM ('cocm', 'regular');
+ `);
+ await queryRunner.query(`
+ ALTER TABLE "qf_round"
+ ADD COLUMN "qfStrategy" "qf_strategy_enum" DEFAULT 'regular';
`);
}📝 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.
| await queryRunner.query(` | |
| ALTER TABLE "qf_round" | |
| ADD COLUMN "qfStrategy" "qf_strategy_enum" DEFAULT 'regular'; | |
| `); | |
| await queryRunner.query(` | |
| CREATE TYPE "qf_strategy_enum" AS ENUM ('cocm', 'regular'); | |
| `); | |
| await queryRunner.query(` | |
| ALTER TABLE "qf_round" | |
| ADD COLUMN "qfStrategy" "qf_strategy_enum" DEFAULT 'regular'; | |
| `); |
CarlosQ96
force-pushed
the
hotfix_add_qf_strategy
branch
from
020e20b to
469c830
Compare
* throw error on un-vouching the givbackseligible projects * update message * optimize the approveMultipleProjects * make projects verified if they become givbacksEligible * prevent approve or reject draft projects * fix records * fix conditions * use redirectUrl * fix tests * add unverifyProjectsTestCases * add test:projectVerificationTab * temporary comment * fix typo * send email when project verification status changed * Feat/Generating public user data * added tests for querying user basic data * add includeUnlisted to FilterProjectQueryInputParams * return proper projects * add recipient address to streams when nonexistent (#1890) * started endaoment update feature * Superfluid Base Support (#1893) * finish project endpoint for superfluid to read * add filters for network and tokens for recurringdonations * fix verification logic and emails for base network streams * Update src/resolvers/recurringDonationResolver.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * comment network test and add cbBTC to seeds --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * remove project validation from anchor contract * Add networkId logic to superfluid subgraphs (#1896) * add networkId logic to superfluid subgraphs * remove networkId from api call to superfluid * fix eslint * fix linkedin scope * fix user info link to user info * cron job for sitemap generating * adding additional projects to Endaoment list * started cronjob * finished cron job * Feature cluster matching (#1862) * add cluster matching entity * add cluster matching adapters * finish cocm adapter * improve error handling for cluster matching * comment broken contract tests by missing eth_getCode Method * add feedback to handle qf cases * add cluster matching job to bootstrap file * fix coderabbit feedback PR * termine worker if an exception is raised * fix updateUser condition to handle email undefined case * fixed one variable; added cronjob env suggested by Carlos * removed redundant code * check config value * fix calling env variable * fix/removing endaomentId from update * add qfStrategy to qfRounds (#1903) * update bootstrap.js adding check endaoment * adding sitemap cronjob to bootstrap * additional logger data * fine tuninnig log * improve logger * fixing endaoment id * Set default zero for power balance snapshot on no return from balance aggregator (#1732) Ref #1655 * Fix/Sitemap env variables * fix missing prefix for url * fix matching cap calculation * fix data insertion for cluster matching * add user passport score null case to clustermatching queries * fix error handling in cocm adapter * add cluster matching sync timestamp and logs (#1913) * add cluster matching sync timestamp and logs * add nullability to clustermathicng syncAT * fix db call in worker for cluster matching * add uniquness constraint to estimatedclustedMatching * handle undefined case for instant power boosting services * better error handling in worker job * fixing prettier problem * Disable cluster matching --------- Co-authored-by: Cherik <[email protected]> Co-authored-by: kkatusic <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Mitch <[email protected]> Co-authored-by: Lovel George <[email protected]> Co-authored-by: Amin Latifi <[email protected]>
For determining what to show in the UI
Summary by CodeRabbit
Release Notes
qfStrategyto the rounds table.