-
Notifications
You must be signed in to change notification settings - Fork 18
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
Allow refresh concurrently of materialized views #1611
Conversation
WalkthroughThis update introduces several database migration scripts to create and manage materialized views for calculating and ranking power values for projects and users. Additionally, it enhances performance by adding the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 11
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- migration/1717643016553-ProjectFuturePowerView_V2.ts (1 hunks)
- migration/1717643739652-ProjectPowerView_V2.ts (1 hunks)
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts (1 hunks)
- migration/1717645768886-UserProjectPowerView_V2.ts (1 hunks)
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts (1 hunks)
- migration/1717646612482-ProjectActualMatchingView_V16.ts (1 hunks)
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts (1 hunks)
- migration/1717648653115-ProjectInstantPowerView_V2.ts (1 hunks)
- src/repositories/instantBoostingRepository.ts (2 hunks)
- src/repositories/projectPowerViewRepository.ts (3 hunks)
- src/repositories/userProjectPowerViewRepository.ts (1 hunks)
- src/services/projectViewsService.ts (2 hunks)
Additional context used
GitHub Check: test
migration/1717644442966-ProjectUserInstantPowerView_V2.ts
[failure] 32-32:
'queryRunner' is defined but never used. Allowed unused args must match /^_/umigration/1717645768886-UserProjectPowerView_V2.ts
[failure] 35-35:
'queryRunner' is defined but never used. Allowed unused args must match /^_/umigration/1717648491606-LastSnapshotProjectPowerView_V2.ts
[failure] 74-74:
'queryRunner' is defined but never used. Allowed unused args must match /^_/umigration/1717643016553-ProjectFuturePowerView_V2.ts
[failure] 62-62:
'queryRunner' is defined but never used. Allowed unused args must match /^_/umigration/1717643739652-ProjectPowerView_V2.ts
[failure] 61-61:
'queryRunner' is defined but never used. Allowed unused args must match /^_/umigration/1717646612482-ProjectActualMatchingView_V16.ts
[failure] 183-183:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
Additional comments not posted (13)
migration/1717644442966-ProjectUserInstantPowerView_V2.ts (1)
6-29
: The SQL queries for creating the materialized view and indexes are correctly structured and follow best practices for PostgreSQL.src/repositories/userProjectPowerViewRepository.ts (1)
59-59
: The update to useREFRESH MATERIALIZED VIEW CONCURRENTLY
is correctly implemented and aligns with the PR objectives to allow concurrent refreshes.migration/1717645768886-UserProjectPowerView_V2.ts (1)
5-30
: The SQL queries for creating the materialized view and indexes are correctly structured and follow best practices for PostgreSQL.migration/1717646357435-ProjectEstimatedMatchingView_V2.ts (1)
6-43
: The SQL queries for creating the materialized view and indexes are correctly structured and follow best practices for PostgreSQL.migration/1717648653115-ProjectInstantPowerView_V2.ts (2)
6-55
: The implementation of theup
method looks correct and follows best practices for managing materialized views and indexes in PostgreSQL.
57-64
: Thedown
method is correctly implemented to safely remove the materialized view and its indexes, ensuring clean rollback capabilities.migration/1717648491606-LastSnapshotProjectPowerView_V2.ts (1)
6-70
: The SQL queries in theup
method are well-structured and correctly manage the lifecycle of the materialized view and its indexes.migration/1717643016553-ProjectFuturePowerView_V2.ts (1)
6-58
: The SQL queries in theup
method are correctly structured to handle the creation of the materialized view and its indexes efficiently.migration/1717643739652-ProjectPowerView_V2.ts (1)
4-56
: The SQL queries in theup
method are well-structured and correctly manage the lifecycle of the materialized view and its indexes.src/repositories/projectPowerViewRepository.ts (1)
92-92
: LGTM! Consider adding specific error handling for the database queries within this function.Also applies to: 101-101
src/repositories/instantBoostingRepository.ts (2)
109-109
: LGTM! Ensure that the materialized viewproject_user_instant_power_view
supports concurrent refresh.
96-96
: LGTM! Ensure that the materialized viewproject_instant_power_view
supports concurrent refresh.migration/1717646612482-ProjectActualMatchingView_V16.ts (1)
183-183
: Consider adding a comment explaining why thedown
method is empty, to clarify that it's intentional to avoid data loss.Tools
GitHub Check: test
[failure] 183-183:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> {} |
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.
The down
method is empty. Consider implementing it to ensure that the migration can be safely rolled back if necessary.
Tools
GitHub Check: test
[failure] 74-74:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> {} |
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.
The down
method is empty. It's important to implement this method to allow for a clean rollback of the migration if needed.
Tools
GitHub Check: test
[failure] 62-62:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> {} |
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.
The down
method is empty. Consider implementing it to ensure that the migration can be safely rolled back if necessary.
Tools
GitHub Check: test
[failure] 61-61:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
@@ -69,7 +69,7 @@ export const refreshProjectPowerView = async (): Promise<void> => { | |||
try { | |||
return AppDataSource.getDataSource().query( | |||
` | |||
REFRESH MATERIALIZED VIEW project_power_view | |||
REFRESH MATERIALIZED VIEW CONCURRENTLY project_power_view |
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.
Tip
Codebase Verification
The migration scripts for project_power_view
do not include the WITH (fillfactor = 100)
clause, which is required for concurrent refresh. This means that the materialized view might not support concurrent refresh as currently defined.
migration/1717643739652-ProjectPowerView_V2.ts
migration/1662915983385-ProjectPowerView.ts
Please update the materialized view definition to include the necessary clause for concurrent refresh.
Analysis chain
LGTM! Ensure that the materialized view project_power_view
supports concurrent refresh.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the materialized view supports concurrent refresh.
# Test: Check the view definition for the `CONCURRENTLY` support.
psql -d your_database -c "\\d+ project_power_view"
Length of output: 123
Script:
#!/bin/bash
# Description: Search for the definition of the materialized view `project_power_view` in the codebase.
rg "CREATE MATERIALIZED VIEW project_power_view" -A 10
Length of output: 56
Script:
#!/bin/bash
# Description: Find SQL files or migration scripts and search for the definition of the materialized view `project_power_view`.
# Step 1: Find SQL files or migration scripts.
fd -e sql -e up.sql -e down.sql -e migration.sql > sql_files.txt
# Step 2: Search within those files for the definition of the materialized view.
rg "CREATE MATERIALIZED VIEW project_power_view" -A 10 -f sql_files.txt
Length of output: 124
Script:
#!/bin/bash
# Description: Search for any references to `project_power_view` in the codebase.
rg "project_power_view"
Length of output: 5500
Script:
#!/bin/bash
# Description: Display the contents of the migration scripts to verify the definition of the materialized view `project_power_view`.
cat migration/1717643739652-ProjectPowerView_V2.ts
cat migration/1662915983385-ProjectPowerView.ts
Length of output: 6582
`); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> {} |
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.
Consider implementing the down
method to provide rollback capabilities by dropping the created indexes and materialized view.
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`DROP INDEX IF EXISTS project_user_instant_power_view_project_id, PROJECT_USER_INSTANT_POWER_VIEW_TOTAL_POWER, project_user_instant_power_view_project_id;`);
await queryRunner.query(`DROP MATERIALIZED VIEW IF EXISTS PUBLIC.PROJECT_USER_INSTANT_POWER_VIEW;`);
}
Tools
GitHub Check: test
[failure] 32-32:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
Rename the unused parameter queryRunner
to _queryRunner
to resolve the static analysis issue.
- public async down(queryRunner: QueryRunner): Promise<void> {}
+ public async down(_queryRunner: QueryRunner): Promise<void> {}
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.
public async down(queryRunner: QueryRunner): Promise<void> {} | |
public async down(_queryRunner: QueryRunner): Promise<void> {} |
Tools
GitHub Check: test
[failure] 32-32:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> {} |
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.
Consider implementing the down
method to provide rollback capabilities by dropping the created indexes and materialized view.
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`DROP INDEX IF EXISTS user_project_power_view_id_idx, USER_PROJECT_POWER_VIEW_POWER_BOOSTED, USER_PROJECT_POWER_VIEW_PROJECT_ID;`);
await queryRunner.query(`DROP MATERIALIZED VIEW IF EXISTS PUBLIC.USER_PROJECT_POWER_VIEW;`);
}
Tools
GitHub Check: test
[failure] 35-35:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
Rename the unused parameter queryRunner
to _queryRunner
to resolve the static analysis issue.
- public async down(queryRunner: QueryRunner): Promise<void> {}
+ public async down(_queryRunner: QueryRunner): Promise<void> {}
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.
public async down(queryRunner: QueryRunner): Promise<void> {} | |
public async down(_queryRunner: QueryRunner): Promise<void> {} |
Tools
GitHub Check: test
[failure] 35-35:
'queryRunner' is defined but never used. Allowed unused args must match /^_/u
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> { |
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.
Consider implementing the down
method to provide rollback capabilities by dropping the created indexes and materialized view.
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`DROP INDEX IF EXISTS idx_project_estimated_matching_project_id, idx_project_estimated_matching_qf_round_id, idx_project_estimated_matching_unique;`);
await queryRunner.query(`DROP MATERIALIZED VIEW IF EXISTS project_estimated_matching_view;`);
}
@@ -9,7 +9,7 @@ export const refreshProjectEstimatedMatchingView = async (): Promise<void> => { | |||
try { | |||
return AppDataSource.getDataSource().query( | |||
` | |||
REFRESH MATERIALIZED VIEW project_estimated_matching_view | |||
REFRESH MATERIALIZED VIEW CONCURRENTLY project_estimated_matching_view |
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.
Tip
Codebase Verification
The extracted definitions of the materialized view project_estimated_matching_view
do not include the WITH NO DATA
clause, which is necessary for supporting concurrent refreshes. This means that the materialized view may not support the CONCURRENTLY
keyword during refresh operations.
To ensure the materialized view supports concurrent refresh, you should modify the view definition to include the WITH NO DATA
clause.
- File:
migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
- File:
migration/1685958638251-ProjectEstimatedMatchingView.ts
Add the WITH NO DATA
clause at the end of the CREATE MATERIALIZED VIEW
statement in both files.
Analysis chain
LGTM! Ensure that the materialized view project_estimated_matching_view
supports concurrent refresh.
The extracted lines show the creation of the materialized view project_estimated_matching_view
, but they do not indicate whether the view supports concurrent refresh. To verify this, we need to check if the view is created with the WITH NO DATA
clause, which is a prerequisite for using REFRESH MATERIALIZED VIEW CONCURRENTLY
.
Let's search for the complete definition of the materialized view to see if it includes the WITH NO DATA
clause.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the materialized view supports concurrent refresh.
# Test: Check the view definition for the `CONCURRENTLY` support.
psql -d your_database -c "\\d+ project_estimated_matching_view"
Length of output: 136
Script:
#!/bin/bash
# Description: Search for the definition of the materialized view `project_estimated_matching_view` in the codebase.
ast-grep --lang sql --pattern $'CREATE MATERIALIZED VIEW project_estimated_matching_view AS $$$'
Length of output: 204
Script:
#!/bin/bash
# Description: Search for the definition of the materialized view `project_estimated_matching_view` in the codebase.
rg 'CREATE MATERIALIZED VIEW project_estimated_matching_view'
Length of output: 312
Script:
#!/bin/bash
# Description: Extract the lines around the `CREATE MATERIALIZED VIEW` statement in the identified files.
rg -A 10 -B 2 'CREATE MATERIALIZED VIEW project_estimated_matching_view' migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
rg -A 10 -B 2 'CREATE MATERIALIZED VIEW project_estimated_matching_view' migration/1685958638251-ProjectEstimatedMatchingView.ts
Length of output: 1436
Script:
#!/bin/bash
# Description: Extract the complete definition of the materialized view `project_estimated_matching_view` to check for the `WITH NO DATA` clause.
rg -A 50 -B 2 'CREATE MATERIALIZED VIEW project_estimated_matching_view' migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
rg -A 50 -B 2 'CREATE MATERIALIZED VIEW project_estimated_matching_view' migration/1685958638251-ProjectEstimatedMatchingView.ts
Length of output: 3954
@@ -22,7 +22,7 @@ | |||
try { | |||
return AppDataSource.getDataSource().query( | |||
` | |||
REFRESH MATERIALIZED VIEW project_actual_matching_view | |||
REFRESH MATERIALIZED VIEW CONCURRENTLY project_actual_matching_view |
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.
Tip
Codebase Verification
The materialized view project_actual_matching_view
does not currently support concurrent refresh as the CONCURRENTLY
keyword is not present in its definitions.
- Files to review and modify:
migration/1717646612482-ProjectActualMatchingView_V16.ts
migration/1713700147145-project_actual_matchin_view_15.ts
migration/1713545913826-project_actual_matching_v14_.ts
migration/1712205556308-ProjectActualMatchingV12.ts
migration/1710768644383-project_actual_matching_v13_.ts
migration/1710322367912-project_actual_matching_v12_.ts
migration/1710322367912-project_actual_matching_v11_.ts
migration/1709755736838-project_actual_matching_v10.ts
migration/1709740250342-project_actual_matching_v9.ts
migration/1709625907739-project_actual_matching_v8.ts
migration/1709468854359-project_actual_matching_v6.ts
migration/1708511332373-project_actual_matching_v6.ts
migration/1708341627263-change_type_of_field_in_user_passport_score.ts
migration/1708280336872-project_actual_matching_view_v5.ts
migration/1708269524103-project_actual_matching_view_v4.ts
migration/1708252061900-create_project_actual_matching_view_v3.ts
migration/1707892354692-project_actual_matching_view_v2.ts
Please update the view definitions to support concurrent refresh.
Analysis chain
LGTM! Ensure that the materialized view project_actual_matching_view
supports concurrent refresh.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the materialized view supports concurrent refresh.
# Test: Check the view definition for the `CONCURRENTLY` support.
psql -d your_database -c "\\d+ project_actual_matching_view"
Length of output: 133
Script:
#!/bin/bash
# Description: Search for the definition of the materialized view `project_actual_matching_view` in the codebase.
# This will help us verify if the view supports concurrent refresh.
rg 'CREATE MATERIALIZED VIEW.*project_actual_matching_view' -A 10
Length of output: 19824
452c729
to
ba9bf5d
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- migration/1717643016553-ProjectFuturePowerView_V2.ts (1 hunks)
- migration/1717643739652-ProjectPowerView_V2.ts (1 hunks)
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts (1 hunks)
- migration/1717645768886-UserProjectPowerView_V2.ts (1 hunks)
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts (1 hunks)
- migration/1717646612482-ProjectActualMatchingView_V16.ts (1 hunks)
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts (1 hunks)
- migration/1717648653115-ProjectInstantPowerView_V2.ts (1 hunks)
- src/repositories/instantBoostingRepository.ts (2 hunks)
- src/repositories/projectPowerViewRepository.ts (3 hunks)
- src/repositories/userProjectPowerViewRepository.ts (1 hunks)
- src/services/projectViewsService.ts (2 hunks)
Files skipped from review as they are similar to previous changes (11)
- migration/1717643016553-ProjectFuturePowerView_V2.ts
- migration/1717643739652-ProjectPowerView_V2.ts
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts
- migration/1717645768886-UserProjectPowerView_V2.ts
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts
- migration/1717648653115-ProjectInstantPowerView_V2.ts
- src/repositories/instantBoostingRepository.ts
- src/repositories/projectPowerViewRepository.ts
- src/repositories/userProjectPowerViewRepository.ts
- src/services/projectViewsService.ts
Additional comments not posted (2)
migration/1717646612482-ProjectActualMatchingView_V16.ts (2)
183-183
: Verify the intention behind the emptydown
method.Is the empty
down
method intentional to prevent data loss, or should it include logic to revert the changes made by theup
method?
6-181
: Ensure SQL queries are optimized and follow best practices.
9802fc5
to
d535a67
Compare
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.
@CarlosQ96 I think there is a necessary step to add for this to work.
Check the comment here:
#1611 (comment)
Also some tests are still failing and saying that the materialized view cannot be refreshed concurrently
d535a67
to
43c5dbe
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- migration/1717643016553-ProjectFuturePowerView_V2.ts (1 hunks)
- migration/1717643739652-ProjectPowerView_V2.ts (1 hunks)
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts (1 hunks)
- migration/1717645768886-UserProjectPowerView_V2.ts (1 hunks)
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts (1 hunks)
- migration/1717646612482-ProjectActualMatchingView_V16.ts (1 hunks)
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts (1 hunks)
- migration/1717648653115-ProjectInstantPowerView_V2.ts (1 hunks)
- src/repositories/instantBoostingRepository.ts (2 hunks)
- src/repositories/projectPowerViewRepository.ts (3 hunks)
- src/repositories/userProjectPowerViewRepository.ts (1 hunks)
- src/services/projectViewsService.ts (2 hunks)
- src/views/projectPowerView.ts (1 hunks)
- test/pre-test-scripts.ts (4 hunks)
Files skipped from review as they are similar to previous changes (12)
- migration/1717643016553-ProjectFuturePowerView_V2.ts
- migration/1717643739652-ProjectPowerView_V2.ts
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts
- migration/1717645768886-UserProjectPowerView_V2.ts
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
- migration/1717646612482-ProjectActualMatchingView_V16.ts
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts
- migration/1717648653115-ProjectInstantPowerView_V2.ts
- src/repositories/instantBoostingRepository.ts
- src/repositories/projectPowerViewRepository.ts
- src/repositories/userProjectPowerViewRepository.ts
- src/services/projectViewsService.ts
Additional comments not posted (2)
src/views/projectPowerView.ts (1)
17-19
: Unique index addition aligns with PostgreSQL requirements for concurrent refresh.This change is crucial for enabling concurrent refreshes of the materialized view, ensuring data integrity and performance improvements.
test/pre-test-scripts.ts (1)
37-44
: Updated imports to new migration script versions.The updates are correctly reflecting the new versions of the migration scripts, ensuring that the test environment matches the latest database schema changes.
43c5dbe
to
f97865a
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (14)
- migration/1717643016553-ProjectFuturePowerView_V2.ts (1 hunks)
- migration/1717643739652-ProjectPowerView_V2.ts (1 hunks)
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts (1 hunks)
- migration/1717645768886-UserProjectPowerView_V2.ts (1 hunks)
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts (1 hunks)
- migration/1717646612482-ProjectActualMatchingView_V16.ts (1 hunks)
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts (1 hunks)
- migration/1717648653115-ProjectInstantPowerView_V2.ts (1 hunks)
- src/repositories/instantBoostingRepository.ts (2 hunks)
- src/repositories/projectPowerViewRepository.ts (1 hunks)
- src/repositories/userProjectPowerViewRepository.ts (1 hunks)
- src/services/projectViewsService.ts (2 hunks)
- src/views/projectPowerView.ts (1 hunks)
- test/pre-test-scripts.ts (4 hunks)
Files skipped from review as they are similar to previous changes (13)
- migration/1717643016553-ProjectFuturePowerView_V2.ts
- migration/1717643739652-ProjectPowerView_V2.ts
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts
- migration/1717645768886-UserProjectPowerView_V2.ts
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
- migration/1717646612482-ProjectActualMatchingView_V16.ts
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts
- migration/1717648653115-ProjectInstantPowerView_V2.ts
- src/repositories/instantBoostingRepository.ts
- src/repositories/projectPowerViewRepository.ts
- src/repositories/userProjectPowerViewRepository.ts
- src/services/projectViewsService.ts
- src/views/projectPowerView.ts
Additional comments not posted (2)
test/pre-test-scripts.ts (2)
37-44
: The updated import statements correctly reference the new migration classes.
Line range hint
489-509
: TherunMigrations
function correctly utilizes the new migration classes to perform database migrations.
f97865a
to
cd21204
Compare
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- migration/1717643016553-ProjectFuturePowerView_V2.ts (1 hunks)
- migration/1717643739652-ProjectPowerView_V2.ts (1 hunks)
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts (1 hunks)
- migration/1717645768886-UserProjectPowerView_V2.ts (1 hunks)
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts (1 hunks)
- migration/1717646612482-ProjectActualMatchingView_V16.ts (1 hunks)
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts (1 hunks)
- migration/1717648653115-ProjectInstantPowerView_V2.ts (1 hunks)
- src/repositories/instantBoostingRepository.ts (2 hunks)
- src/repositories/projectPowerViewRepository.test.ts (2 hunks)
- src/repositories/projectPowerViewRepository.ts (3 hunks)
- src/repositories/userProjectPowerViewRepository.ts (1 hunks)
- src/services/projectViewsService.ts (2 hunks)
- src/views/projectPowerView.ts (1 hunks)
- test/pre-test-scripts.ts (4 hunks)
Files skipped from review due to trivial changes (1)
- src/views/projectPowerView.ts
Files skipped from review as they are similar to previous changes (12)
- migration/1717643016553-ProjectFuturePowerView_V2.ts
- migration/1717643739652-ProjectPowerView_V2.ts
- migration/1717644442966-ProjectUserInstantPowerView_V2.ts
- migration/1717645768886-UserProjectPowerView_V2.ts
- migration/1717646357435-ProjectEstimatedMatchingView_V2.ts
- migration/1717646612482-ProjectActualMatchingView_V16.ts
- migration/1717648491606-LastSnapshotProjectPowerView_V2.ts
- migration/1717648653115-ProjectInstantPowerView_V2.ts
- src/repositories/instantBoostingRepository.ts
- src/repositories/projectPowerViewRepository.ts
- src/repositories/userProjectPowerViewRepository.ts
- src/services/projectViewsService.ts
Additional comments not posted (1)
test/pre-test-scripts.ts (1)
37-44
: Updated migration imports correctly reflect new versions.The changes to the import statements and the usage of these imports in the
runMigrations
function correctly reflect the updates to the migration files as described in the PR. This ensures that the correct versions are used during the test setup.Also applies to: 489-509
// the concurrently are affected by other tests TODO: FIX | ||
// assert.equal(projectPowers[0].powerRank, 1); | ||
// assert.equal(projectPowers[0].projectId, project2.id); | ||
// assert.equal(projectPowers[1].powerRank, 2); | ||
// assert.equal(projectPowers[1].projectId, project1.id); | ||
// assert.equal(projectPowers[2].powerRank, 3); | ||
// assert.equal(projectPowers[3].powerRank, 3); |
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.
Address test interference issues to ensure reliable outcomes.
The commented-out assertions in the test cases indicate interference from other tests. This can lead to unreliable test results. Consider improving test isolation or using a different strategy to ensure that tests do not affect each other. Once resolved, re-enable these assertions to ensure the functionality is correctly verified.
Also applies to: 383-396
Done! There is a test failing but ill go test in staging first, its a value. But i think its the test environment |
* Change for Project Ownship * i have fixed the issue for transfer project ownership in adminBro and fixed for eslint * Revert "651 run migration files as js file (#1604)" This reverts commit 5140a93. * Update staging-pipeline.yml * Update staging-pipeline.yml * Update staging-pipeline.yml * Create funding.json * Fix issue in db migration scripts (#1608) * Change commands to use typeorm cli * Select migration files based on node env * Fix eslint errors on staging branch * add cache to mainCategories query * add cache to findQfRounds query * add cache to findQfRounds query * fix MainCategory test case * removed unnecessary comments and changed email to admin id * fix: filter unique donors based on passport score and knownAsSybilAddress * set cache duration as env var * allow refresh concurrently of materialized views (#1611) * add logger to updateProjectStatistics * fix: add new field isDataAnalysisDone to QF Round * fix: refactor condition where data analysis shouldn't be edited * fix: issue while listing recurrring donations * revert concurrent flag for failed views * fix: requested change (using subquery to get unique donors count) * add getUserAnalysisScore * revert all materialized views concurrent flag * reactivate concurrent flag for all materialized views * move refresh estimated matching view last * Add some error logs and complete some error messages (#1623) * move refreshProjectEstimatedMatchingView * fix: retrieve correct usd value of donations for an archived QF round * skip if there's a wrong tx hash * skip if there's a wrong error in insertDonationsFromQfRoundHistory * Get giv price from coingecko instead of subgraph (#1621) * Get giv price from coingecko instead of subgraph related to Giveth/giveth-dapps-v2#4168 * Fix linter errors * Remove poignArt stuff * remove unused logs * remove unused logs * force findActiveQfRound to use read replica * rewrite updateProjectStatistics * move refreshProjectEstimatedMatchingView * optimize updateProjectStatistics * ProjectEstimatedMatchingView V3 * resolve ProjectEstimatedMatchingView V2 errors * update donationRepository.test.ts * update donationRepository.ts * update project.ts * update ProjectEstimatedMatchingView.ts * update projectRepository.ts * update projectService.ts with new estimated matching structure * update qfRoundHistory.ts with new estimated matching structure * update qfRoundRepository.test.ts with new estimated matching structure * update qfRoundRepository.ts with new estimated matching structure * update qfRoundResolver.ts with new estimated matching structure * update qfUtils.ts with new estimated matching structure * add verified status * add minimumPassportScore to getProjectQfRoundStats * add check for begin and end date to getProjectQfRoundStats * remove refreshProjectEstimatedMatchingView from tests * fix qfRoundRepository.test.ts tests * remove qfRound stats from projects when added or removed from QF round * update added projects only * update added projects only * optimize refreshProjectEstimatedMatchingView * set NODE_ENV for refreshProjectEstimatedMatchingView * merge getQfRoundUniqueDonors and getQfRoundTotalDonations * fix adding valueUsd to query * add userQfRoundModelScore * remove minimumPassportScore from getProjectQfRoundStats * remove user join from getProjectQfRoundStats * remove insertDonationsFromQfRoundHistory from syncDonationStatusWithBlockchainNetwork * enable replica * projectVerificationRepository.ts replace .save with update method * projectVerificationRepository.ts replace .save with update method * remove unused fields from logs * remove user from badge warning * optimize projectsWithoutUpdateAfterTimeFrame * Zkevm integration (#1635) * Integrate with zkevm chain related to #1617 #1615 #1618 #1619 #1614 #1620 * Add coingeckoId for wbtc zkevm token * Fix lint errors * Remove duplicate keys in provider.ts * Fix AAve token on zkevm chain * fix merge conflicts * add projectId to projectRepository.ts * remove unnecessary updatedAt * fix: get unique_donors based on user wallet address not added to sybil table * add cache to findActiveQfRound * Add on conflicrt do nothing for add zkevm tokens * Fix add polygon zkevm token migration * remove updateTotalDonationsOfProject from migrations * replace updateTotalDonationsOfProject with updateProjectStatistics * replace updateTotalDonationsOfProject with updateProjectStatistics * remove sumDonationValueUsd from project * do email verification on project verification form through Ortto * add solana to available networks for qfround * remove knownAsSybilAddress from user * wrong query cache in some functions * User Story - Rejected project owner reason * use networkId instead of optimism-only flag (#1653) * Use networkId instead of optimismOnly flag * Update graphql queries for tests * fatal errors (#1640) * change critical error logs to fatal * add fatal log for donation verification failure with time threshold * added on final step for verification project data * Replace ortto External Embedded Webform for Onboarding guide * fix: findArchivedQfRounds query * fix subscribeOnboarding args * change output to boolean * WIP: add API key to request & add fetch MBD score record from DB * fixed last step verification * fixed typo * WIP: add field to user that holds user MBD score for an active round * disable DB Replica on staging * fetch data uere with the score from MBD & remove non needed query * Fix some test cases about stable coin donations * fix: issue with adminJs authentication when user has hash as null * "Last Update" on project card is not correct * Fix some test cases about stable coin donations (#1665) * Fix some test cases about stable coin donations * Fix few test cases of projectResolver * Fix few test cases of projectResolver * Fix test cases of project resolver * Fix donationResolver test cases * Bump follow-redirects from 1.15.5 to 1.15.6 (#1414) Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.5 to 1.15.6. - [Release notes](https://github.com/follow-redirects/follow-redirects/releases) - [Commits](follow-redirects/follow-redirects@v1.15.5...v1.15.6) --- updated-dependencies: - dependency-name: follow-redirects dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mohammadranjbarz <[email protected]> * Bump @solana/web3.js from 1.87.6 to 1.87.7 (#1492) Bumps [@solana/web3.js](https://github.com/solana-labs/solana-web3.js) from 1.87.6 to 1.87.7. - [Release notes](https://github.com/solana-labs/solana-web3.js/releases) - [Commits](https://github.com/solana-labs/solana-web3.js/commits) --- updated-dependencies: - dependency-name: "@solana/web3.js" dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mohammadranjbarz <[email protected]> * Bump express from 4.18.2 to 4.19.2 (#1430) Bumps [express](https://github.com/expressjs/express) from 4.18.2 to 4.19.2. - [Release notes](https://github.com/expressjs/express/releases) - [Changelog](https://github.com/expressjs/express/blob/master/History.md) - [Commits](expressjs/express@4.18.2...4.19.2) --- updated-dependencies: - dependency-name: express dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mohammadranjbarz <[email protected]> * Bump braces from 3.0.2 to 3.0.3 (#1629) Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mohammadranjbarz <[email protected]> * Bump undici from 5.28.3 to 5.28.4 (#1454) Bumps [undici](https://github.com/nodejs/undici) from 5.28.3 to 5.28.4. - [Release notes](https://github.com/nodejs/undici/releases) - [Commits](nodejs/undici@v5.28.3...v5.28.4) --- updated-dependencies: - dependency-name: undici dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: mohammadranjbarz <[email protected]> * fix types * Updated project verification form resolver test fixed previos 3 test with terms and condition update and added one last step test when user complete form and clicked "FINISH" button * fix extracting last comment * get donation to giveth with donation box analytics (#1661) * Add a method for finding relevant donations to the donationRepository.ts * Add a method for calculating donations metrics to the donationService.ts * Add a gql resolver for donations metrics to the donationResolver.ts * Add unit test for gql query resolver * Fix division by zero issue * times 100 average percentage to represent percent * add useDonationBox field to donation and draftDonation * fill useDonationBox field with correct data in migrations * Change donationMetrics endpoint based on change in data schema * remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add default value for useDonationBox field Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add false removed imports * add default value for useDonationBox field Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix tests based on changes * fix tests * send useDonationBox as last arg and make it optional * add relevant donation tx hash to donation creation flow * calculate related donations based on relevant donation tx hash * add unit tests * update unit test * fix bug * fix bug in donation repository and tests are pass * Clear All donations after donation metrics test cases * Add new function for deleting project from db and use it to don't affect other test cases * comment new test cases to ensure bug is related to that or not * Clear donations after tests * Comments new test cases in donationRepository.test.ts * uncomment tests and Clean up test effects on DB * Remove project addresses too * Add a new method to testUtils.ts for removing project from db by id * change donation dates to now * Remove clear donations sections * Change update count to a bigger number to pass test --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add test for fetching and refreshing qfroundscore for model * revert changes on package,json * revert changes on package,json * fix migration error * fix migration error * fix: turn minimumValidUsdValue to nullable when querying * remove updatedAt from projects tab * Remove old donation and add new ones (#1674) * remove verified from projects tab edit menu * remove verified from projectVerificationTab edit menu * fix total donations query * fix archived_round totals * fix latestUpdate column if not exists * fix join for totals * fix total donations with subquery * fix add some error handling and remove commented interface * fix: resolve conflict * fix: resolve conflict * fix USD value not showing * fix: convert timestap retrieved from RPC provider to milisconds * fix: donation test cases (milliseconds) * fix: donation test cases (milliseconds) * Endaoment integration (#1663) * Remove giving block projects that don't have any donation related to #1599 * Add needed fields to organization entity for endaoment projects related to Giveth/Roadmap#111 * Remove giving-blocks stuff and endaoment integration thing * Add categories to endaoment projects * Fix test cases * Fix some test cases about stable coin donations * Add endaoment categories * Fix remove giving-blocks projects migration * Add default image for endaoment projects * Fix linter * Fix add useDonationBox to draft_donation migration * Fix migration of adding useDonationBox to donation * Fix eslint error * Fix disabling recurring donations for endaoment projects * Change givingBlock organization to endaoment in pre-test script * Remove test case of accepted tokens of giving blocks * Relate all base, optimism and mainnet tokens to endaoment organization related to #1626 * Fix migration query syntax * fix: donation test cases (milliseconds) * fix: donation test cases (milliseconds) * Modify ormconfig for running migrations * Run ts files for migrations * Make isRecipient of project_addresses of endaoment project true * Fix type error of one of endaoment projects * add test case for filling prices * Modify add endaoment organization migration to pass the tests * fix: convert minimumUserAnalysisScore to Float on qfRound entity * fix: revert migration changes * add log for filling prices * feat: add github on project socialmedia enum * Merge master to staging (#1696) * Release relevant donations (#1687) * get donation to giveth with donation box analytics (#1661) * Add a method for finding relevant donations to the donationRepository.ts * Add a method for calculating donations metrics to the donationService.ts * Add a gql resolver for donations metrics to the donationResolver.ts * Add unit test for gql query resolver * Fix division by zero issue * times 100 average percentage to represent percent * add useDonationBox field to donation and draftDonation * fill useDonationBox field with correct data in migrations * Change donationMetrics endpoint based on change in data schema * remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add default value for useDonationBox field Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add false removed imports * add default value for useDonationBox field Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix tests based on changes * fix tests * send useDonationBox as last arg and make it optional * add relevant donation tx hash to donation creation flow * calculate related donations based on relevant donation tx hash * add unit tests * update unit test * fix bug * fix bug in donation repository and tests are pass * Clear All donations after donation metrics test cases * Add new function for deleting project from db and use it to don't affect other test cases * comment new test cases to ensure bug is related to that or not * Clear donations after tests * Comments new test cases in donationRepository.test.ts * uncomment tests and Clean up test effects on DB * Remove project addresses too * Add a new method to testUtils.ts for removing project from db by id * change donation dates to now * Remove clear donations sections * Change update count to a bigger number to pass test --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Remove old donation and add new ones (#1674) * Remove unused variable --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * release analytics dashboard dropdown menu related chagnes (#1686) * use networkId instead of optimism-only flag (#1653) * Use networkId instead of optimismOnly flag * Update graphql queries for tests * Change network id to float in gql schema to fix unit tests --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Add organization filter to projectsTab.ts in admin panel related to #1599 (comment) * Fill description summary for endaoment projects related to #1599 (comment) * Fix eslint errors * Replace special characters of slugs of endaoment projects with - related to Giveth/giveth-dapps-v2#4280 (comment) * Fix eslint errors * return hardcoded as user MBD score * rename qfRound minimumUserAnalysisScore field * Resolve conflicts (#1703) * Release relevant donations (#1687) * get donation to giveth with donation box analytics (#1661) * Add a method for finding relevant donations to the donationRepository.ts * Add a method for calculating donations metrics to the donationService.ts * Add a gql resolver for donations metrics to the donationResolver.ts * Add unit test for gql query resolver * Fix division by zero issue * times 100 average percentage to represent percent * add useDonationBox field to donation and draftDonation * fill useDonationBox field with correct data in migrations * Change donationMetrics endpoint based on change in data schema * remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * remove unused import Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add default value for useDonationBox field Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * add false removed imports * add default value for useDonationBox field Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix tests based on changes * fix tests * send useDonationBox as last arg and make it optional * add relevant donation tx hash to donation creation flow * calculate related donations based on relevant donation tx hash * add unit tests * update unit test * fix bug * fix bug in donation repository and tests are pass * Clear All donations after donation metrics test cases * Add new function for deleting project from db and use it to don't affect other test cases * comment new test cases to ensure bug is related to that or not * Clear donations after tests * Comments new test cases in donationRepository.test.ts * uncomment tests and Clean up test effects on DB * Remove project addresses too * Add a new method to testUtils.ts for removing project from db by id * change donation dates to now * Remove clear donations sections * Change update count to a bigger number to pass test --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Remove old donation and add new ones (#1674) * Remove unused variable --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * release analytics dashboard dropdown menu related chagnes (#1686) * use networkId instead of optimism-only flag (#1653) * Use networkId instead of optimismOnly flag * Update graphql queries for tests * Change network id to float in gql schema to fix unit tests --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Modify endaoment categories and relate them to projects related to #1664 (comment) * Add canUseOnFrontend to category so can show a category but don't allow users to select them when create/update projects related to #1685 * Fix project resolver test case * fix CPU spikes * add new test case for project update * fix test case * Fix add endaoment integration * Filter isActive and canUseOnFrontend categories in projectResolver * Filter isActive and canUseOnFrontend categories in projectResolver * Return canUseOnFrontend in main category list * Comment migrations related to endaoment (#1706) * 1.24.0 * Test ci/cd * Fix build problem --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: Lovel George <[email protected]> Co-authored-by: Lovel George <[email protected]> Co-authored-by: Ramin <[email protected]> Co-authored-by: Moe Shehab <[email protected]> Co-authored-by: Mitch <[email protected]> Co-authored-by: ali ebrahimi <[email protected]> Co-authored-by: Meriem-BM <[email protected]> Co-authored-by: CarlosQ96 <[email protected]> Co-authored-by: Carlos <[email protected]> Co-authored-by: kkatusic <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Reshzera <[email protected]>
Based off docs from postgresql:
https://www.postgresqltutorial.com/postgresql-views/postgresql-materialized-views/
And https://www.postgresql.org/docs/current/sql-refreshmaterializedview.html
Related to the issue: #1607
To use concurrent flag we must have at least 1 unique index on the views.
Summary by CodeRabbit
New Features
Enhancements
CONCURRENTLY
option toREFRESH MATERIALIZED VIEW
statements for improved performance during concurrent operations.Testing
Database
projectId
andround
columns inprojectPowerView
.