Master -> develop sync #40
Conversation
[v6 PROD RELEASE] - dev -> master
PS-441 fix for phase predecessor calculations on update
Pm 2206 master
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| CREATE SCHEMA IF NOT EXISTS reviews; | ||
|
|
||
| CREATE EXTENSION IF NOT EXISTS fuzzystrmatch WITH SCHEMA skills; | ||
| CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA pg_catalog; |
There was a problem hiding this comment.
[design]
The pg_trgm extension is being created in the pg_catalog schema. Typically, extensions are created in the public schema or a specific schema relevant to the application. Ensure that this is intentional and that the application logic accounts for this schema placement.
| CREATE EXTENSION IF NOT EXISTS fuzzystrmatch WITH SCHEMA skills; | ||
| CREATE EXTENSION IF NOT EXISTS pg_trgm WITH SCHEMA pg_catalog; | ||
| CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA reviews; | ||
| CREATE EXTENSION IF NOT EXISTS "uuid-ossp" WITH SCHEMA skills; |
There was a problem hiding this comment.
[design]
The uuid-ossp extension is being created in the skills schema. This is unusual as extensions are often placed in the public schema. Verify that this schema choice is deliberate and that it aligns with the application's schema management strategy.
| JOIN pg_namespace ns ON ns.oid = idx.relnamespace | ||
| WHERE idx.relname = 'challenge_phase_challenge_open_end_idx' | ||
| AND ns.nspname = challenge_phase_schema | ||
| AND pg_get_indexdef(idx.oid) LIKE '%("challengeId", "isOpen", "scheduledEndDate", "actualEndDate", name)%' |
There was a problem hiding this comment.
[maintainability]
The pg_get_indexdef function is used with a LIKE clause to check index definitions. This approach can be fragile if the index definition changes in ways not accounted for by the LIKE pattern. Consider using a more robust method to verify index structure, if possible.
| @@ -0,0 +1,30 @@ | |||
| -- View to use in performance updates (PM-2206) | |||
| DROP VIEW IF EXISTS "challenges"."MemberChallengeAccess"; | |||
There was a problem hiding this comment.
[❗❗ correctness]
Dropping the view without checking if it exists first could lead to errors if the view is not present. Consider using DROP VIEW IF EXISTS to avoid potential errors.
| ) THEN | ||
| EXECUTE format( | ||
| 'CREATE VIEW %I.%I AS | ||
| SELECT DISTINCT r."challengeId", r."memberId" |
There was a problem hiding this comment.
[performance]
Using SELECT DISTINCT can be resource-intensive. Ensure that this is necessary for the use case, as it may impact performance.
| 'CREATE VIEW %I.%I AS | ||
| SELECT DISTINCT r."challengeId", r."memberId" | ||
| FROM resources."Resource" r | ||
| WHERE r."challengeId" IS NOT NULL |
There was a problem hiding this comment.
[performance]
The condition r."challengeId" IS NOT NULL AND r."memberId" IS NOT NULL is used to filter out null values. Ensure that these columns are indexed if they are frequently queried, to improve performance.
| terms ChallengeTerm[] | ||
| skills ChallengeSkill[] | ||
| auditLogs AuditLog[] | ||
| memberAccesses MemberChallengeAccess[] |
There was a problem hiding this comment.
[maintainability]
The addition of memberAccesses in the Challenge model introduces a new relation to MemberChallengeAccess. Ensure that the MemberChallengeAccess model is correctly populated and maintained, as any orphaned records could lead to inconsistencies. Consider adding cascading delete or update rules if appropriate.
| challenge Challenge @relation(fields: [challengeId], references: [id]) | ||
| @@id([challengeId, memberId]) |
There was a problem hiding this comment.
[❗❗ correctness]
The composite primary key @@id([challengeId, memberId]) in MemberChallengeAccess ensures uniqueness of member/challenge pairs. Verify that this constraint aligns with the intended data model and that no existing data violates this constraint.
| timelineTemplateId | ||
| ); | ||
| const { phaseDefinitionMap } = await this.getPhaseDefinitionsAndMap(); | ||
| const challengePhaseIds = new Set(_.map(challengePhases, "phaseId")); |
There was a problem hiding this comment.
[performance]
Consider using _.keyBy instead of _.map followed by new Set to directly create a set of phase IDs. This could improve performance by reducing the number of iterations over challengePhases.
| phaseId: phase.predecessor, | ||
| }); | ||
| if (_.isNil(predecessorPhase)) { | ||
| continue; |
There was a problem hiding this comment.
[correctness]
The continue statement here skips processing phases without a valid predecessor. Ensure that this behavior is intentional and that skipping these phases won't lead to incorrect scheduling or logic errors.
No description provided.