Skip to content

Calendar service->main#284

Open
yaelirosner wants to merge 19 commits into
mainfrom
Calendar_Service
Open

Calendar service->main#284
yaelirosner wants to merge 19 commits into
mainfrom
Calendar_Service

Conversation

@yaelirosner

@yaelirosner yaelirosner commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

update middleware and fix reviews

res.status(HttpStatusCode.OK).json({
IsSucceeded: true,
statusCode: HttpStatusCode.OK,
message: "Connection to Google Calendar was successful! You can close the window."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consult AI, I feel something is not accurate in this sentence

default:
return res.status(500).json({ message: "Internal server error." });
}
next(err);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!

}
}
export async function syncCalendar(req: Request, res: Response) {
const bedgNumber = Number(req.query.userID);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

badgeNumber (pleach ensure I'm correct)

import { Request, Response, NextFunction } from 'express';


export class AppCalendarError extends Error {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class name is a bit unclear. Please consult AI

};

export const generateGoogleAuthUrl = (state: string, employee_email: string): string => {
export const initializeAuthSession = async (employeeEmail: string, state: string): Promise<string> => {

@yehudits yehudits Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function taht insert data to database should be named accordingly.
You can rename this function maybe, but the best option (I think) is to extract just lines 16- 28 to separate function

if (isPersonal) return 'Frontal personal meeting';
return 'Frontal team meeting';
if (isOnline && isPersonal) return MeetingType.ONLINE_PERSONAL_MEETING;
if (isOnline) return MeetingType.ONLINE_TEAM_MEETING;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

return MeetingType.FRONTAL_TEAM_MEETING;
}

function mapEventToMeeting(event: any, badgeNumber: number): Meeting | null {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please divide to some functions.

There is a common practice - create a separate service and function for mapping and converting between model and db. Can we implement it here?

const lastWeek = new Date();
lastWeek.setDate(lastWeek.getDate() - 7);
const nextMonth = new Date();
nextMonth.setDate(nextMonth.getDate() + 30);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create functions for nextMonth.getDate() + 30 and for lastWeek.getDate() - 7). Their name should descibe their goal

): Promise<void> {
const decryptedToken = decryptToken(refreshToken); // פענוח הטוקן

const { data: user, error: userError } = await supabase

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions that use db should be separated from rest of the logic (maybe even another file and sometimes new layer just for the db connection

@Efrat8277 Efrat8277 requested a review from yehudits June 25, 2026 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants