Task #218152 Created the cron job and function to calculate the distnace between camp and attendance location#1152
Conversation
WalkthroughThe recent updates introduce a new property, Changes
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
src/src/camp/camp.service.ts (1)
Line range hint
2578-2641: Refactor themarkCampAttendancemethod to improve readability and maintainability.The
markCampAttendancemethod is quite lengthy and performs multiple operations, which makes it hard to read and maintain. Consider breaking it down into smaller, more focused methods. For example, the SQL query construction and execution can be extracted into a separate method, and the error handling logic can be encapsulated in another method. This will make the code cleaner and easier to manage.
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- src/src/attendances/attendances.service.ts (1 hunks)
- src/src/camp/camp.service.ts (2 hunks)
- src/src/cron/cron.module.ts (2 hunks)
- src/src/cron/distanceCalculation.cron.ts (1 hunks)
Additional Context Used
Path-based Instructions (4)
src/src/cron/cron.module.ts (1)
Pattern
**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:
- The code adheres to best practices associated with nodejs.
- The code adheres to best practices associated with nestjs framework.
- The code adheres to best practices recommended for performance.
- The code adheres to similar naming conventions for controllers, models, services, methods, variables."
src/src/attendances/attendances.service.ts (1)
Pattern
**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:
- The code adheres to best practices associated with nodejs.
- The code adheres to best practices associated with nestjs framework.
- The code adheres to best practices recommended for performance.
- The code adheres to similar naming conventions for controllers, models, services, methods, variables."
src/src/cron/distanceCalculation.cron.ts (1)
Pattern
**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:
- The code adheres to best practices associated with nodejs.
- The code adheres to best practices associated with nestjs framework.
- The code adheres to best practices recommended for performance.
- The code adheres to similar naming conventions for controllers, models, services, methods, variables."
src/src/camp/camp.service.ts (1)
Pattern
**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:
- The code adheres to best practices associated with nodejs.
- The code adheres to best practices associated with nestjs framework.
- The code adheres to best practices recommended for performance.
- The code adheres to similar naming conventions for controllers, models, services, methods, variables."
Additional comments not posted (4)
src/src/cron/cron.module.ts (2)
12-12: The import ofDistanceCalculationis correctly added.
27-27: The addition ofDistanceCalculationto the providers list is correctly implemented.src/src/attendances/attendances.service.ts (1)
30-30: The addition ofcamp_to_attendance_location_distancetoreturnFieldsis correctly implemented.src/src/cron/distanceCalculation.cron.ts (1)
32-69: The implementation offetchDatausing GraphQL is appropriate and well-structured.
| private async haversineFormula( | ||
| campLat: any, | ||
| campLong: any, | ||
| attendanceLat: any, | ||
| attendanceLong: any, | ||
| ) { | ||
| const sql = `SELECT CASE WHEN COALESCE(NULLIF('${campLat}', ''), 'null') = 'null' | ||
| OR COALESCE(NULLIF('${attendanceLat}', ''), 'null') = 'null' | ||
| OR COALESCE(NULLIF('${campLong}', ''), 'null') = 'null' | ||
| OR COALESCE(NULLIF('${attendanceLong}', ''), 'null') = 'null' THEN | ||
| 0::numeric | ||
| ELSE round( cast( 6371 * 2 * asin( sqrt( power(sin(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLat}', '0')::DOUBLE PRECISION, 0)) / 2), 2) + cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * power(sin(radians(COALESCE(NULLIF('${campLong}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLong}', '0')::DOUBLE PRECISION, 0)) / 2), 2) ) ) AS numeric ), 2 ) | ||
| END`; | ||
| try { | ||
| const response = ( | ||
| await this.hasuraServiceFromService.executeRawSql(sql) | ||
| ).result; | ||
| const formattedData = | ||
| this.hasuraServiceFromService.getFormattedData(response); | ||
| return formattedData[0].round; | ||
| } catch { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Consider refactoring haversineFormula to use application logic instead of SQL for distance calculation.
- private async haversineFormula(campLat: any, campLong: any, attendanceLat: any, attendanceLong: any) {
- const sql = `SELECT CASE WHEN COALESCE(NULLIF('${campLat}', ''), 'null') = 'null'
- OR COALESCE(NULLIF('${attendanceLat}', ''), 'null') = 'null'
- OR COALESCE(NULLIF('${campLong}', ''), 'null') = 'null'
- OR COALESCE(NULLIF('${attendanceLong}', ''), 'null') = 'null' THEN
- 0::numeric
- ELSE round( cast( 6371 * 2 * asin( sqrt( power(sin(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLat}', '0')::DOUBLE PRECISION, 0)) / 2), 2) + cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * power(sin(radians(COALESCE(NULLIF('${campLong}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLong}', '0')::DOUBLE PRECISION, 0)) / 2), 2) ) ) AS numeric ), 2 )
- END`;
- try {
- const response = (await this.hasuraServiceFromService.executeRawSql(sql)).result;
- const formattedData = this.hasuraServiceFromService.getFormattedData(response);
- return formattedData[0].round;
- } catch {
- return [];
- }
- }
+ private haversineFormula(campLat: number, campLong: number, attendanceLat: number, attendanceLong: number): number {
+ const R = 6371; // Radius of the Earth in km
+ const dLat = (attendanceLat - campLat) * (Math.PI / 180);
+ const dLon = (attendanceLong - campLong) * (Math.PI / 180);
+ const a = Math.sin(dLat / 2) * Math.sin(dLat / 2) +
+ Math.cos(campLat * (Math.PI / 180)) * Math.cos(attendanceLat * (Math.PI / 180)) *
+ Math.sin(dLon / 2) * Math.sin(dLon / 2);
+ const c = 2 * Math.atan2(Math.sqrt(a), Math.sqrt(1 - a));
+ return R * c; // Distance in km
+ }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.
| private async haversineFormula( | |
| campLat: any, | |
| campLong: any, | |
| attendanceLat: any, | |
| attendanceLong: any, | |
| ) { | |
| const sql = `SELECT CASE WHEN COALESCE(NULLIF('${campLat}', ''), 'null') = 'null' | |
| OR COALESCE(NULLIF('${attendanceLat}', ''), 'null') = 'null' | |
| OR COALESCE(NULLIF('${campLong}', ''), 'null') = 'null' | |
| OR COALESCE(NULLIF('${attendanceLong}', ''), 'null') = 'null' THEN | |
| 0::numeric | |
| ELSE round( cast( 6371 * 2 * asin( sqrt( power(sin(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLat}', '0')::DOUBLE PRECISION, 0)) / 2), 2) + cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * power(sin(radians(COALESCE(NULLIF('${campLong}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLong}', '0')::DOUBLE PRECISION, 0)) / 2), 2) ) ) AS numeric ), 2 ) | |
| END`; | |
| try { | |
| const response = ( | |
| await this.hasuraServiceFromService.executeRawSql(sql) | |
| ).result; | |
| const formattedData = | |
| this.hasuraServiceFromService.getFormattedData(response); | |
| return formattedData[0].round; | |
| } catch { | |
| return []; | |
| } | |
| private haversineFormula(campLat: number, campLong: number, attendanceLat: number, attendanceLong: number): number { | |
| const R = 6371; // Radius of the Earth in km | |
| const dLat = (attendanceLat - campLat) * (Math.PI / 180); | |
| const dLon = (attendanceLong - campLong) * (Math.PI / 180); | |
| const a = Math.sin(dLat / 2) * Math.sin(dLat / 2) + | |
| Math.cos(campLat * (Math.PI / 180)) * Math.cos(attendanceLat * (Math.PI / 180)) * | |
| Math.sin(dLon / 2) * Math.sin(dLon / 2); | |
| const c = 2 * Math.atan2(Math.sqrt(a), Math.sqrt(1 - a)); | |
| return R * c; // Distance in km | |
| } |
There was a problem hiding this comment.
The processData method is well-implemented. Consider enhancing error logging for better traceability.
- console.log('Error occurred while updating ', error);
+ console.error('Error occurred while updating: ', error.message);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.
| //cron runs for each hour's 30th minute | |
| @Cron('30 * * * *') | |
| private async processData() { | |
| const user = await this.fetchData(10); | |
| try { | |
| for (const userData of user) { | |
| const campData = userData.camp?.properties; | |
| const attendanceData = userData?.attendances?.[0]; | |
| const attendanceId = attendanceData.id; | |
| console.log('ATTENDANCEDATA ', attendanceData); | |
| console.log('CAMPDATA ', campData); | |
| if (!campData || !attendanceData) { | |
| console.log('SKIP - Missing campData or attendanceData'); | |
| continue; | |
| } | |
| const campLat = parseFloat(campData.lat); | |
| const campLong = parseFloat(campData.long); | |
| const attendanceLat = parseFloat(attendanceData.lat); | |
| const attendanceLong = parseFloat(attendanceData.long); | |
| console.log( | |
| 'campLat ', | |
| campLat, | |
| 'campLong ', | |
| campLong, | |
| 'attendanceLat ', | |
| attendanceLat, | |
| 'attendanceLong ', | |
| attendanceLong, | |
| ); | |
| if ( | |
| isNaN(campLat) || | |
| isNaN(campLong) || | |
| isNaN(attendanceLat) || | |
| isNaN(attendanceLong) | |
| ) { | |
| console.log('SKIP - Invalid coordinates'); | |
| continue; | |
| } | |
| const calculatedDistance = await this.haversineFormula( | |
| campLat, | |
| campLong, | |
| attendanceLat, | |
| attendanceLong, | |
| ); | |
| console.log( | |
| 'Distance between attendance and camp location ', | |
| calculatedDistance, | |
| ); | |
| const updateQuery = { | |
| query: `mutation MyMutation { | |
| update_attendance(where: {id: {_eq: ${attendanceId}}}, _set: {camp_to_attendance_location_distance: ${calculatedDistance}}) { | |
| returning { | |
| id | |
| camp_to_attendance_location_distance | |
| } | |
| } | |
| } | |
| `, | |
| }; | |
| console.log(updateQuery.query); | |
| const updatedResult = | |
| await this.hasuraServiceFromService.getData(updateQuery); | |
| console.log(JSON.stringify(updatedResult)); | |
| } | |
| } catch (error) { | |
| console.log('Error occurred while updating ', error); | |
| return []; | |
| } | |
| //cron runs for each hour's 30th minute | |
| @Cron('30 * * * *') | |
| private async processData() { | |
| const user = await this.fetchData(10); | |
| try { | |
| for (const userData of user) { | |
| const campData = userData.camp?.properties; | |
| const attendanceData = userData?.attendances?.[0]; | |
| const attendanceId = attendanceData.id; | |
| console.log('ATTENDANCEDATA ', attendanceData); | |
| console.log('CAMPDATA ', campData); | |
| if (!campData || !attendanceData) { | |
| console.log('SKIP - Missing campData or attendanceData'); | |
| continue; | |
| } | |
| const campLat = parseFloat(campData.lat); | |
| const campLong = parseFloat(campData.long); | |
| const attendanceLat = parseFloat(attendanceData.lat); | |
| const attendanceLong = parseFloat(attendanceData.long); | |
| console.log( | |
| 'campLat ', | |
| campLat, | |
| 'campLong ', | |
| campLong, | |
| 'attendanceLat ', | |
| attendanceLat, | |
| 'attendanceLong ', | |
| attendanceLong, | |
| ); | |
| if ( | |
| isNaN(campLat) || | |
| isNaN(campLong) || | |
| isNaN(attendanceLat) || | |
| isNaN(attendanceLong) | |
| ) { | |
| console.log('SKIP - Invalid coordinates'); | |
| continue; | |
| } | |
| const calculatedDistance = await this.haversineFormula( | |
| campLat, | |
| campLong, | |
| attendanceLat, | |
| attendanceLong, | |
| ); | |
| console.log( | |
| 'Distance between attendance and camp location ', | |
| calculatedDistance, | |
| ); | |
| const updateQuery = { | |
| query: `mutation MyMutation { | |
| update_attendance(where: {id: {_eq: ${attendanceId}}}, _set: {camp_to_attendance_location_distance: ${calculatedDistance}}) { | |
| returning { | |
| id | |
| camp_to_attendance_location_distance | |
| } | |
| } | |
| } | |
| `, | |
| }; | |
| console.log(updateQuery.query); | |
| const updatedResult = | |
| await this.hasuraServiceFromService.getData(updateQuery); | |
| console.log(JSON.stringify(updatedResult)); | |
| } | |
| } catch (error) { | |
| console.error('Error occurred while updating: ', error.message); | |
| return []; | |
| } |
| const campData = userData.camp?.properties; | ||
| const attendanceData = userData?.attendances?.[0]; | ||
| const attendanceId = attendanceData.id; | ||
| console.log('ATTENDANCEDATA ', attendanceData); |
There was a problem hiding this comment.
keep console log only if neccessary otherwise remove it
| console.log('CAMPDATA ', campData); | ||
|
|
||
| if (!campData || !attendanceData) { | ||
| console.log('SKIP - Missing campData or attendanceData'); |
There was a problem hiding this comment.
Here instead of console log is return statement expected?
There was a problem hiding this comment.
we are skippin the records so we cannot return anything there
|
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
src/src/cron/distanceCalculation.cron.ts (1)
82-82: Review the use of console logs for better logging practices.The use of
console.logfor debugging is common, but in a production environment, it's better to use a more robust logging framework that can handle different log levels and outputs. Consider replacingconsole.logwith a more sophisticated logging approach.Also applies to: 86-86
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- src/src/cron/distanceCalculation.cron.ts (1 hunks)
Additional Context Used
Path-based Instructions (1)
src/src/cron/distanceCalculation.cron.ts (1)
Pattern
**/*.ts: "Review the JavaScript code for conformity with the Google JavaScript style guide, highlighting any deviations. Ensure that:
- The code adheres to best practices associated with nodejs.
- The code adheres to best practices associated with nestjs framework.
- The code adheres to best practices recommended for performance.
- The code adheres to similar naming conventions for controllers, models, services, methods, variables."
Additional comments not posted (1)
src/src/cron/distanceCalculation.cron.ts (1)
73-73: Verify the cron expression for accuracy.The cron expression
'30 * * * *'indicates that theprocessDatamethod will run at the 30th minute of every hour. Confirm that this timing aligns with the intended frequency for data processing.
| const sql = `SELECT CASE WHEN COALESCE(NULLIF('${campLat}', ''), 'null') = 'null' | ||
| OR COALESCE(NULLIF('${attendanceLat}', ''), 'null') = 'null' | ||
| OR COALESCE(NULLIF('${campLong}', ''), 'null') = 'null' | ||
| OR COALESCE(NULLIF('${attendanceLong}', ''), 'null') = 'null' THEN | ||
| 0::numeric | ||
| ELSE round( cast( 6371 * 2 * asin( sqrt( power(sin(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLat}', '0')::DOUBLE PRECISION, 0)) / 2), 2) + cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * cos(radians(COALESCE(NULLIF('${campLat}', '0')::DOUBLE PRECISION, 0))) * power(sin(radians(COALESCE(NULLIF('${campLong}', '0')::DOUBLE PRECISION, 0) - COALESCE(NULLIF('${attendanceLong}', '0')::DOUBLE PRECISION, 0)) / 2), 2) ) ) AS numeric ), 2 ) |
There was a problem hiding this comment.
Ensure SQL queries are safe from injection vulnerabilities.
The current implementation of the SQL query in haversineFormula method might be vulnerable to SQL injection if not properly sanitized. Consider using parameterized queries to enhance security.
| campLat: any, | ||
| campLong: any, | ||
| attendanceLat: any, | ||
| attendanceLong: any, |
There was a problem hiding this comment.
Consider using more specific types for latitude and longitude parameters.
Using any type for latitude and longitude parameters reduces type safety. It's better to use number type for these parameters to ensure that the values being passed are of the expected type.
- campLat: any,
- campLong: any,
- attendanceLat: any,
- attendanceLong: any,
+ campLat: number,
+ campLong: number,
+ attendanceLat: number,
+ attendanceLong: number,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.
| campLat: any, | |
| campLong: any, | |
| attendanceLat: any, | |
| attendanceLong: any, | |
| campLat: number, | |
| campLong: number, | |
| attendanceLat: number, | |
| attendanceLong: number, |
| console.log(JSON.stringify(updatedResult)); | ||
| } | ||
| } catch (error) { | ||
| console.log('Error occurred while updating ', error); |
There was a problem hiding this comment.
Enhance error handling by logging more detailed information.
When logging errors, it's beneficial to log more detailed information about the error to aid in debugging. Consider logging the error message along with the stack trace.
- console.log('Error occurred while updating ', error);
+ console.error('Error occurred while updating: ', error.message, error.stack);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.
| console.log('Error occurred while updating ', error); | |
| console.error('Error occurred while updating: ', error.message, error.stack); |
YoginiTayade
changed the title
sagarkoshti1990
changed the base branch from
release-2.7.0
to
release-2.8.0
2 participants
I have ensured that following
Pull Request Checklistis taken care of before sending this PRSummary by CodeRabbit
New Features
Improvements