Conversation
PS-529 - for TG make sure to return only payments, without not rewards
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| validateIsoDate(startDate, "start-date"); | ||
| validateIsoDate(endDate, "end-date"); | ||
| if (startDate >= endDate) { |
There was a problem hiding this comment.
[❗❗ correctness]
The comparison startDate >= endDate should be done using new Date(startDate) and new Date(endDate) to ensure proper date comparison, as string comparison may not work as expected for dates.
| console.log( | ||
| `[member-tax-aggregated-export] Exporting data for ${startDate} to ${endDate}`, | ||
| ); | ||
| const result = await pool.query(AGGREGATED_QUERY, [startDate, endDate]); |
There was a problem hiding this comment.
[security]
Consider using parameterized queries with named parameters or a library that supports query building to prevent SQL injection, even though the current usage with date parameters is low risk.
| ON UPPER(home_code."countryCode") = UPPER(mem."homeCountryCode") | ||
| LEFT JOIN lookups."Country" AS home_id | ||
| ON UPPER(home_id.id) = UPPER(mem."homeCountryCode") | ||
| WHERE mem.email IS NULL |
There was a problem hiding this comment.
[❗❗ correctness]
The condition mem.email IS NULL OR mem.email NOT ILIKE '%@wipro%' may lead to unexpected results if mem.email is NULL. Consider using COALESCE(mem.email, '') NOT ILIKE '%@wipro%' to ensure consistent filtering.
|
|
||
| validateIsoDate(startDate, "start-date"); | ||
| validateIsoDate(endDate, "end-date"); | ||
| if (startDate >= endDate) { |
There was a problem hiding this comment.
[❗❗ correctness]
The check if (startDate >= endDate) uses string comparison for dates, which may lead to incorrect results if the dates are not in the expected format. Consider parsing the dates into Date objects before comparison.
| options.output ?? `member-tax-${startDate}-to-${endDate}.csv`; | ||
| const outputPath = path.resolve(process.cwd(), outputFile); | ||
|
|
||
| const mainPool = new Pool({ connectionString: mainDatabaseUrl }); |
There was a problem hiding this comment.
[maintainability]
Consider using a connection pool configuration that includes error handling and retry logic to improve resilience against transient database connection issues.
| writeCsv(outputPath, mergedRows); | ||
| console.log(`[member-tax-export] Wrote CSV: ${outputPath}`); | ||
| } finally { | ||
| await Promise.allSettled([mainPool.end(), oldPaymentsPool.end()]); |
There was a problem hiding this comment.
[💡 maintainability]
Using Promise.allSettled ensures that all pool connections are closed, but it may mask errors in closing connections. Consider logging the results of Promise.allSettled to capture any issues during pool closure.
3 participants
https://topcoder.atlassian.net/browse/PS-529?atlOrigin=eyJpIjoiNGNlOTNjZDk4OTYyNGFkN2I3MTlmMWVhMDIzMTcyZmYiLCJwIjoiamlyYS1zbGFjay1pbnQifQ