Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions cron.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import UtsavDb from './models/utsav_db.model.js';
import { sendCancellationEmail, sendOpenBookingEmail } from './helpers/mailer.helper.js';
import {
getBooking,
getBookings,
getBookingType,
getBookingTypeFromBooking
} from './helpers/booking.helper.js';
Expand Down Expand Up @@ -109,19 +110,26 @@ async function getUnpaidOnlineBookingsAndTransactions(bookings, transactions) {
.subtract(MAX_APP_PAYMENT_DURATION, 'minutes');
const pendingTransactions = await getPendingTransactions(cancelTimeFilter);

const bookingsByType = {};

for (const transaction of pendingTransactions) {
const bookingType = getBookingType(transaction);
// TODO: optimize, get all bookings at once

// Food bookings are handled in a special way
if (bookingType != TYPE_FOOD) {
const booking = await getBooking(bookingType, transaction.bookingid);
if (booking) {
bookings.push(booking);
if (!bookingsByType[bookingType]) {
bookingsByType[bookingType] = new Set();
}
bookingsByType[bookingType].add(transaction.bookingid);
}
transactions.push(transaction);
}

for (const [bookingType, bookingIdsSet] of Object.entries(bookingsByType)) {
const bookingIds = Array.from(bookingIdsSet);
const fetchedBookings = await getBookings(bookingType, bookingIds);
bookings.push(...fetchedBookings);
}
}

async function cancelBookings(systemUser, bookings, userBookingIds, openBookings, t) {
Expand Down
51 changes: 51 additions & 0 deletions helpers/booking.helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,57 @@ export async function getBooking(bookingType, bookingid) {
return booking;
}

export async function getBookings(bookingType, bookingids) {
if (!bookingids || bookingids.length === 0) {
return [];
}

let bookings = [];

switch (bookingType) {
case TYPE_ROOM:
bookings = await RoomBooking.findAll({
where: { bookingid: bookingids }
});
break;

case TYPE_FLAT:
bookings = await FlatBooking.findAll({
where: { bookingid: bookingids }
});
break;

case TYPE_ADHYAYAN:
bookings = await ShibirBookingDb.findAll({
where: { bookingid: bookingids }
});
break;

case TYPE_FOOD:
bookings = await FoodDb.findAll({
where: { id: bookingids }
});
break;

case TYPE_TRAVEL:
bookings = await TravelDb.findAll({
where: { bookingid: bookingids }
});
break;

case TYPE_UTSAV:
bookings = await UtsavBooking.findAll({
where: { bookingid: bookingids }
});
break;

default:
throw new ApiError(400, `${ERR_INVALID_BOOKING_TYPE}: ${bookingType}`);
}

return bookings;
}
Comment on lines +87 to +136
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The getBookings function can be made more concise and easier to maintain by using a map to associate booking types with their corresponding models and primary key fields. This avoids the repetitive switch statement, reduces code duplication, and makes it simpler to add or modify booking types in the future.

export async function getBookings(bookingType, bookingids) {
  const modelMapping = {
    [TYPE_ROOM]: { model: RoomBooking, key: 'bookingid' },
    [TYPE_FLAT]: { model: FlatBooking, key: 'bookingid' },
    [TYPE_ADHYAYAN]: { model: ShibirBookingDb, key: 'bookingid' },
    [TYPE_FOOD]: { model: FoodDb, key: 'id' },
    [TYPE_TRAVEL]: { model: TravelDb, key: 'bookingid' },
    [TYPE_UTSAV]: { model: UtsavBooking, key: 'bookingid' }
  };

  const mapping = modelMapping[bookingType];
  if (!mapping) {
    throw new ApiError(400, `${ERR_INVALID_BOOKING_TYPE}: ${bookingType}`);
  }

  const { model, key } = mapping;
  return model.findAll({
    where: { [key]: bookingids }
  });
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jules please take a look

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've updated the getBookings function to use let/const for better adherence to modern JavaScript standards. I also added an early return for empty bookingids to make the function more robust.


export function ifMigrated(transaction) {
let bookingid = transaction.bookingid;
let description = transaction.description;
Expand Down
80 changes: 80 additions & 0 deletions tests/optimize_booking_retrieval.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { expect, it, describe, mock } from "bun:test";
import {
TYPE_ROOM,
TYPE_FLAT,
TYPE_ADHYAYAN,
TYPE_FOOD,
TYPE_TRAVEL,
TYPE_UTSAV
} from '../config/constants.js';

mock.module("../models/associations.js", () => ({
RoomBooking: { findAll: async (q) => q.where.bookingid.map(id => ({ bookingid: id })) },
FlatBooking: { findAll: async (q) => q.where.bookingid.map(id => ({ bookingid: id })) },
ShibirBookingDb: { findAll: async (q) => q.where.bookingid.map(id => ({ bookingid: id })) },
FoodDb: { findAll: async (q) => q.where.id.map(id => ({ id })) },
TravelDb: { findAll: async (q) => q.where.bookingid.map(id => ({ bookingid: id })) },
UtsavBooking: { findAll: async (q) => q.where.bookingid.map(id => ({ bookingid: id })) },
CardDb: {},
Transactions: {},
ShibirDb: {},
Departments: {},
MaintenanceDb: {},
GateRecord: {},
CentreDb: {},
BulkFoodBooking: {},
FoodPhysicalPlate: {},
RoomDb: {},
FlatDb: {},
UtsavDb: {},
UtsavPackagesDb: {},
AdminRoles: {},
AdminUsers: {},
Roles: {},
Menu: {},
Cities: {},
States: {},
Countries: {},
GuestDb: {},
GuestRelationship: {},
RazorpayWebhook: {},
RazorpaySettlement: {},
SupportTickets: {},
BlockDates: {},
Updates: {},
AdhyayanFeedback: {},
RazorpaySettlementRecon: {},
ShibirAttendanceDb: {}
}));

const { getBookings } = await import('../helpers/booking.helper.js');

describe('getBookings', () => {
it('should return empty array for empty bookingids', async () => {
const result = await getBookings(TYPE_ROOM, []);
expect(result).toEqual([]);
});

it('should call RoomBooking.findAll for TYPE_ROOM', async () => {
const ids = ['room1'];
const result = await getBookings(TYPE_ROOM, ids);
expect(result).toHaveLength(1);
expect(result[0].bookingid).toBe('room1');
});

it('should call FoodDb.findAll for TYPE_FOOD', async () => {
const ids = [1];
const result = await getBookings(TYPE_FOOD, ids);
expect(result).toHaveLength(1);
expect(result[0].id).toBe(1);
});

it('should throw error for invalid booking type', async () => {
try {
await getBookings('INVALID', ['id1']);
expect(true).toBe(false); // Should not reach here
} catch (e) {
expect(e.message).toContain('Invalid booking type');
}
});
Comment on lines +72 to +79
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This test for checking exceptions can be simplified by using the expect().rejects.toThrow() assertion provided by the bun:test framework. This makes the test more readable and idiomatic, avoiding the manual try/catch block.

  it('should throw error for invalid booking type', async () => {
    await expect(getBookings('INVALID', ['id1'])).rejects.toThrow(
      'Invalid booking type'
    );
  });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jules please take a look

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've added these tests using Bun's test runner to verify the batch fetching logic and error handling for invalid booking types. The tests confirm that getBookings correctly maps booking types to their respective models and uses the appropriate field for IDs (e.g., id for food bookings and bookingid for others).

});