-
Notifications
You must be signed in to change notification settings - Fork 0
Add new recent members report as requested by sales team. Optimise TG hourly report #42
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
Conversation
…end dates from challenge phases
Enhanced Topgear hourly report by adding registration and submission …
Refactor SQL aliases in Topgear hourly report
| @@ -0,0 +1,270 @@ | |||
| WITH params AS ( | |||
| SELECT COALESCE(NULLIF($1, '')::timestamptz, TIMESTAMPTZ '2024-01-01') AS start_date | |||
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.
[maintainability]
The use of a hardcoded date TIMESTAMPTZ '2024-01-01' as a default value for start_date might lead to unexpected results if the query is used beyond this date. Consider using a more dynamic default, such as the current date or a configurable parameter.
| JOIN finance.winnings w | ||
| ON w.winning_id = p.winnings_id | ||
| JOIN params pr | ||
| ON COALESCE(p.date_paid, p.created_at) >= pr.start_date |
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.
[❗❗ correctness]
The use of COALESCE(p.date_paid, p.created_at) to determine the date for filtering payments might lead to incorrect results if date_paid is null but created_at is not. Ensure that this logic aligns with the business requirements for determining recent payments.
| JOIN eligible_members em | ||
| ON em.member_id = s."memberId" | ||
| WHERE s."memberId" IS NOT NULL | ||
| AND (s."finalScore" > 75 OR r."finalScore" > 75) |
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.
[❗❗ correctness]
The condition (s."finalScore" > 75 OR r."finalScore" > 75) might include submissions with null finalScore values if r."finalScore" is null. Consider explicitly handling null values to avoid unintended inclusions.
| ON rc.member_id = em.member_id | ||
| LEFT JOIN submissions_over_75 so | ||
| ON so.member_id = em.member_id | ||
| WHERE COALESCE(m.email, '') NOT ILIKE '%@wipro.com%' |
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.
[correctness]
The filter COALESCE(m.email, '') NOT ILIKE '%@wipro.com%' could potentially exclude valid entries if m.email is null. Consider revisiting this condition to ensure it aligns with the intended filtering logic.
| registration_end AS ( | ||
| SELECT | ||
| cp."challengeId" AS challenge_id, | ||
| MAX(COALESCE(cp."actualEndDate", cp."scheduledEndDate")) AS registration_end_date |
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.
[correctness]
Using MAX(COALESCE(cp."actualEndDate", cp."scheduledEndDate")) assumes that actualEndDate and scheduledEndDate are comparable and that actualEndDate is always preferred over scheduledEndDate. Ensure that this logic aligns with business requirements, as it may lead to unexpected results if actualEndDate is null but scheduledEndDate is not.
| submission_end AS ( | ||
| SELECT | ||
| cp."challengeId" AS challenge_id, | ||
| MAX(COALESCE(cp."actualEndDate", cp."scheduledEndDate")) AS submission_end_date |
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.
[correctness]
The use of MAX(COALESCE(cp."actualEndDate", cp."scheduledEndDate")) here also assumes that actualEndDate should take precedence over scheduledEndDate. Verify that this logic is consistent with the intended business logic, as discrepancies could lead to incorrect reporting of submission end dates.
| return { | ||
| handle: row.handle ?? null, | ||
| email: row.email ?? null, | ||
| country: alpha3ToCountryName(row.country_code), |
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.
[correctness]
The alpha3ToCountryName function is used here to convert country_code to a country name. Ensure that this function handles all possible country_code values, including null or invalid codes, to prevent unexpected errors.
| openToWork: row.open_to_work ?? null, | ||
| workHistory, | ||
| education, | ||
| trolleyIdVerified: row.trolley_id_verified ?? false, |
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.
[correctness]
The default value for trolleyIdVerified is set to false if it's null. Verify if this default behavior aligns with the business logic, as it might lead to incorrect assumptions about the verification status.
| workHistory, | ||
| education, | ||
| trolleyIdVerified: row.trolley_id_verified ?? false, | ||
| challengeWins: Number(row.challenge_wins ?? 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.
[correctness]
The conversion of challenge_wins, task_wins, registration_count, and submissions_over_75 to numbers defaults to 0 if they are null. Ensure this defaulting to 0 is intended and does not misrepresent the data.
Add new recent members report as requested by sales team. Optimise TG hourly report