Skip to content

Conversation

@hussam789
Copy link

@hussam789 hussam789 commented Oct 30, 2025

User description

PR #1


PR Type

Enhancement


Description

  • Convert appStore package imports to async dynamic imports

  • Update getCalendar function to async with Promise return type

  • Update all callers of getCalendar to await the async function

  • Refactor getVideoAdapters to async with proper Promise handling

  • Update payment app access patterns to use async imports


Diagram Walkthrough

flowchart LR
  A["appStore index.ts"] -->|"dynamic imports"| B["Async module loading"]
  B -->|"awaited by"| C["getCalendar async"]
  C -->|"awaited by"| D["CalendarManager"]
  C -->|"awaited by"| E["EventManager"]
  C -->|"awaited by"| F["Booking handlers"]
  B -->|"awaited by"| G["getVideoAdapters async"]
  G -->|"awaited by"| H["videoClient"]
  B -->|"awaited by"| I["Payment functions"]
Loading

File Walkthrough

Relevant files
Enhancement
12 files
index.ts
Convert static imports to dynamic async imports                   
+29/-59 
getCalendar.ts
Make getCalendar function async with Promise                         
+2/-2     
CalendarManager.ts
Await async getCalendar calls throughout                                 
+8/-7     
videoClient.ts
Refactor getVideoAdapters to async function                           
+20/-16 
EventManager.ts
Await async getCalendar in event deletion                               
+1/-2     
reschedule.ts
Await async getCalendar in reschedule logic                           
+2/-2     
reschedule.ts
Await async getCalendar in reschedule logic                           
+2/-2     
handleCancelBooking.ts
Await async getCalendar and appStore access                           
+12/-11 
handleNewBooking.ts
Await async getCalendar in booking deletion                           
+1/-1     
deletePayment.ts
Await async appStore payment app access                                   
+1/-1     
handlePayment.ts
Await async appStore payment app access                                   
+1/-1     
bookings.tsx
Await async getCalendar and appStore access                           
+3/-3     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unawaited async loop

Description: Using forEach with an async callback causes the awaited operations (calendar deletion or
video deletion) to run untracked and may not complete or handle errors before the parent
flow proceeds.
reschedule.ts [125-132]

Referred Code
bookingRefsFiltered.forEach(async (bookingRef) => {
  if (bookingRef.uid) {
    if (bookingRef.type.endsWith("_calendar")) {
      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
      return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
    } else if (bookingRef.type.endsWith("_video")) {
      return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
    }
Unawaited async loop

Description: Using forEach with an async callback for integration deletions launches unawaited
promises, risking incomplete deletions and swallowed errors.
reschedule.ts [125-132]

Referred Code
bookingRefsFiltered.forEach(async (bookingRef) => {
  if (bookingRef.uid) {
    if (bookingRef.type.endsWith("_calendar")) {
      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
      return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
    } else if (bookingRef.type.endsWith("_video")) {
      return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
    }
Unawaited async loop

Description: Asynchronous forEach over booking references is not awaited, which can lead to race
conditions and inconsistent state when deleting calendar events.
bookings.tsx [553-560]

Referred Code
bookingRefsFiltered.forEach(async (bookingRef) => {
  if (bookingRef.uid) {
    if (bookingRef.type.endsWith("_calendar")) {
      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));

      return calendar?.deleteEvent(
        bookingRef.uid,
        builder.calendarEvent,
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unhandled promises: Using forEach with async callbacks for deletions creates unawaited promises and potential
silent failures without error handling.

Referred Code
bookingRefsFiltered.forEach(async (bookingRef) => {
  if (bookingRef.uid) {
    if (bookingRef.type.endsWith("_calendar")) {
      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
      return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
    } else if (bookingRef.type.endsWith("_video")) {
      return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
    }
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing logging: New async flows perform critical calendar/video/payment actions without adding audit logs
for action, actor, and outcome, which may hinder event reconstruction.

Referred Code
export const createEvent = async (
  credential: CredentialWithAppName,
  calEvent: CalendarEvent
): Promise<EventResult<NewCalendarEventType>> => {
  const uid: string = getUid(calEvent);
  const calendar = await getCalendar(credential);
  let success = true;
  let calError: string | undefined = undefined;

  // Check if the disabledNotes flag is set to true
  if (calEvent.hideCalendarNotes) {
    calEvent.additionalNotes = "Notes have been hidden by the organiser"; // TODO: i18n this string?
  }

  // TODO: Surface success/error messages coming from apps to improve end user visibility
  const creationResult = calendar
    ? await calendar.createEvent(calEvent).catch(async (error) => {
        success = false;
        /**
         * There is a time when selectedCalendar externalId doesn't match witch certain credential
         * so google returns 404.


 ... (clipped 88 lines)
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Log content risk: Warning logs interpolate calendarType, and without context controls it is unclear if
sensitive identifiers could be exposed in logs.

Referred Code
if (!(calendarApp && "lib" in calendarApp && "CalendarService" in calendarApp.lib)) {
  log.warn(`calendar of type ${calendarType} is not implemented`);
  return null;
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Dynamic import risk: Dynamic module access via appStore[key] is expanded to async imports without visible
validation of keys, requiring verification that keys are strictly controlled and not
user-influenced.

Referred Code
const appStore = {
  // example: import("./example"),
  applecalendar: import("./applecalendar"),
  caldavcalendar: import("./caldavcalendar"),
  closecom: import("./closecom"),
  dailyvideo: import("./dailyvideo"),
  googlecalendar: import("./googlecalendar"),
  googlevideo: import("./googlevideo"),
  hubspot: import("./hubspot"),
  huddle01video: import("./huddle01video"),
  jitsivideo: import("./jitsivideo"),
  larkcalendar: import("./larkcalendar"),
  office365calendar: import("./office365calendar"),
  office365video: import("./office365video"),
  plausible: import("./plausible"),
  salesforce: import("./salesforce"),
  zohocrm: import("./zohocrm"),
  sendgrid: import("./sendgrid"),
  stripepayment: import("./stripepayment"),
  tandemvideo: import("./tandemvideo"),
  vital: import("./vital"),


 ... (clipped 10 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Await promises within a loop correctly

Replace the forEach loop with an async callback with
Promise.all(bookingRefsFiltered.map(async ...)) to ensure all asynchronous
operations within the loop are awaited.

packages/app-store/vital/lib/reschedule.ts [125-133]

-bookingRefsFiltered.forEach(async (bookingRef) => {
-  if (bookingRef.uid) {
-    if (bookingRef.type.endsWith("_calendar")) {
-      const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
-      return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
-    } else if (bookingRef.type.endsWith("_video")) {
-      return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
+await Promise.all(
+  bookingRefsFiltered.map(async (bookingRef) => {
+    if (bookingRef.uid) {
+      if (bookingRef.type.endsWith("_calendar")) {
+        const calendar = await getCalendar(credentialsMap.get(bookingRef.type));
+        return calendar?.deleteEvent(bookingRef.uid, builder.calendarEvent);
+      } else if (bookingRef.type.endsWith("_video")) {
+        return deleteMeeting(credentialsMap.get(bookingRef.type), bookingRef.uid);
+      }
     }
-  }
-});
+  })
+);

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a fire-and-forget issue where async functions inside a forEach are not awaited, which could lead to unhandled promise rejections and race conditions. Using Promise.all with map is the correct way to handle this.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants