From fe2b2a52ecfcb6b91493b89bcabe8397510756e2 Mon Sep 17 00:00:00 2001 From: eric-kitagawa Date: Fri, 17 Apr 2026 05:25:57 -0400 Subject: [PATCH] fix: cursor pagination on room number + room id tiebreaks --- backend/internal/handler/rooms.go | 37 +++++++++++++++++++------- backend/internal/handler/rooms_test.go | 33 ++++++++++++----------- backend/internal/repository/rooms.go | 30 ++++++++++++--------- 3 files changed, 63 insertions(+), 37 deletions(-) diff --git a/backend/internal/handler/rooms.go b/backend/internal/handler/rooms.go index cd697a21c..de84f1d78 100644 --- a/backend/internal/handler/rooms.go +++ b/backend/internal/handler/rooms.go @@ -3,7 +3,9 @@ package handler import ( "context" "errors" + "fmt" "strconv" + "strings" "github.com/generate/selfserve/internal/errs" "github.com/generate/selfserve/internal/httpx" @@ -13,7 +15,7 @@ import ( ) type RoomsRepository interface { - FindRoomsWithOptionalGuestBookingsByFloor(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) + FindRoomsWithOptionalGuestBookingsByFloor(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) FindAllFloors(ctx context.Context, hotelID string) ([]int, error) FindRoomByID(ctx context.Context, hotelID string, id string) (*models.RoomWithOptionalGuestBooking, error) } @@ -26,6 +28,24 @@ func NewRoomsHandler(repo RoomsRepository) *RoomsHandler { return &RoomsHandler{repo: repo} } +func parseFilterRoomsCursor(cursor string) (roomNumber int, roomID string, err error) { + if cursor == "" { + return 0, "", nil + } + + parts := strings.Split(cursor, ":") + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return 0, "", errs.BadRequest("invalid cursor") + } + + roomNumber, err = strconv.Atoi(parts[0]) + if err != nil { + return 0, "", errs.BadRequest("invalid cursor") + } + + return roomNumber, parts[1], nil +} + // FilterRooms godoc // @Summary List rooms with filters // @Description Retrieves rooms with optional floor filters and cursor pagination, including any active guest bookings @@ -50,21 +70,18 @@ func (h *RoomsHandler) FilterRooms(c *fiber.Ctx) error { return err } - cursorRoomNumber := 0 - if body.Cursor != "" { - cursorRoomNumber, err = strconv.Atoi(body.Cursor) - if err != nil { - return errs.BadRequest("invalid cursor") - } + cursorRoomNumber, cursorRoomID, err := parseFilterRoomsCursor(body.Cursor) + if err != nil { + return err } - rooms, err := h.repo.FindRoomsWithOptionalGuestBookingsByFloor(c.Context(), &body, hotelID, cursorRoomNumber) + rooms, err := h.repo.FindRoomsWithOptionalGuestBookingsByFloor(c.Context(), &body, hotelID, cursorRoomNumber, cursorRoomID) if err != nil { return errs.InternalServerError() } page := utils.BuildCursorPage(rooms, body.Limit, func(r *models.RoomWithOptionalGuestBooking) string { - return strconv.Itoa(r.RoomNumber) + return fmt.Sprintf("%d:%s", r.RoomNumber, r.ID) }) return c.JSON(page) @@ -126,4 +143,4 @@ func (h *RoomsHandler) GetFloors(c *fiber.Ctx) error { } return c.JSON(floors) -} +} \ No newline at end of file diff --git a/backend/internal/handler/rooms_test.go b/backend/internal/handler/rooms_test.go index 981073b2a..abd354d76 100644 --- a/backend/internal/handler/rooms_test.go +++ b/backend/internal/handler/rooms_test.go @@ -16,13 +16,13 @@ import ( ) type mockRoomsRepository struct { - findRoomsFunc func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) + findRoomsFunc func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) findFloorsFunc func(ctx context.Context, hotelID string) ([]int, error) findRoomByIDFunc func(ctx context.Context, hotelID string, id string) (*models.RoomWithOptionalGuestBooking, error) } -func (m *mockRoomsRepository) FindRoomsWithOptionalGuestBookingsByFloor(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { - return m.findRoomsFunc(ctx, filter, hotelID, cursorRoomNumber) +func (m *mockRoomsRepository) FindRoomsWithOptionalGuestBookingsByFloor(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { + return m.findRoomsFunc(ctx, filter, hotelID, cursorRoomNumber, cursorRoomID) } func (m *mockRoomsRepository) FindAllFloors(ctx context.Context, hotelID string) ([]int, error) { @@ -44,7 +44,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { t.Parallel() mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return []*models.RoomWithOptionalGuestBooking{ { Room: models.Room{ @@ -87,7 +87,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { doe := "Doe" pic := "https://example.com/jane.jpg" mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return []*models.RoomWithOptionalGuestBooking{ { Room: models.Room{RoomNumber: 202, Floor: 2, SuiteType: "deluxe", RoomStatus: "occupied"}, @@ -130,7 +130,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { t.Parallel() mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return []*models.RoomWithOptionalGuestBooking{}, nil }, } @@ -163,7 +163,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { } mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return rooms, nil }, } @@ -190,12 +190,14 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { var capturedFilter *models.FilterRoomsRequest var capturedHotelID string - var capturedCursor int + var capturedCursorNumber int + var capturedCursor string mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { capturedFilter = filter capturedHotelID = hotelID - capturedCursor = cursorRoomNumber + capturedCursorNumber = cursorRoomNumber + capturedCursor = cursorRoomID return []*models.RoomWithOptionalGuestBooking{}, nil }, } @@ -204,7 +206,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { h := NewRoomsHandler(mock) app.Post("/rooms", h.FilterRooms) - req := httptest.NewRequest("POST", "/rooms", strings.NewReader(`{"cursor":"200","limit":10}`)) + req := httptest.NewRequest("POST", "/rooms", strings.NewReader(`{"cursor":"200:530e8400-e458-41d4-a716-446655440222","limit":10}`)) req.Header.Set("Content-Type", "application/json") req.Header.Set(hotelIDHeader, testHotelID) resp, err := app.Test(req) @@ -214,14 +216,15 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { require.NotNil(t, capturedFilter) assert.Equal(t, 10, capturedFilter.Limit) assert.Equal(t, testHotelID, capturedHotelID) - assert.Equal(t, 200, capturedCursor) + assert.Equal(t, 200, capturedCursorNumber) + assert.Equal(t, "530e8400-e458-41d4-a716-446655440222", capturedCursor) }) t.Run("returns 400 when hotel_id header is missing", func(t *testing.T) { t.Parallel() mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return nil, nil }, } @@ -244,7 +247,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { t.Parallel() mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return nil, errors.New("db error") }, } @@ -266,7 +269,7 @@ func TestRoomsHandler_FilterRooms(t *testing.T) { t.Parallel() mock := &mockRoomsRepository{ - findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { + findRoomsFunc: func(ctx context.Context, filter *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { return nil, nil }, } diff --git a/backend/internal/repository/rooms.go b/backend/internal/repository/rooms.go index c278c21e8..c02077c2b 100644 --- a/backend/internal/repository/rooms.go +++ b/backend/internal/repository/rooms.go @@ -20,23 +20,31 @@ func NewRoomsRepository(pool *pgxpool.Pool) *RoomsRepository { return &RoomsRepository{db: pool} } -func (r *RoomsRepository) FindRoomsWithOptionalGuestBookingsByFloor(ctx context.Context, filters *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int) ([]*models.RoomWithOptionalGuestBooking, error) { +func (r *RoomsRepository) FindRoomsWithOptionalGuestBookingsByFloor(ctx context.Context, filters *models.FilterRoomsRequest, hotelID string, cursorRoomNumber int, cursorRoomID string) ([]*models.RoomWithOptionalGuestBooking, error) { limit := utils.ResolveLimit(filters.Limit) + var cursorNumber any + var cursorID any + if cursorRoomID == "" { + cursorNumber = nil + cursorID = nil + } else { + cursorNumber = cursorRoomNumber + cursorID = cursorRoomID + } // Paginate before joining with guests rows, err := r.db.Query(ctx, ` WITH paginated_rooms AS ( SELECT id, room_number, floor, suite_type, room_status, is_accessible FROM rooms - WHERE hotel_id = $4 + WHERE hotel_id = $5 AND ($1::int[] IS NULL OR floor = ANY($1)) - AND room_number > $2 - ORDER BY room_number ASC - LIMIT $3 + AND ($2::int IS NULL OR (room_number, id) > ($2::int, $3::uuid)) + ORDER BY room_number ASC, id ASC + LIMIT $4 ) SELECT pr.id, pr.room_number, pr.floor, pr.suite_type, pr.room_status, pr.is_accessible, - CASE WHEN COUNT(guest_bookings.id) > 0 THEN 'active' ELSE 'inactive' END AS booking_status, json_agg( json_build_object( 'id', guests.id, @@ -48,11 +56,11 @@ func (r *RoomsRepository) FindRoomsWithOptionalGuestBookingsByFloor(ctx context. FROM paginated_rooms pr LEFT JOIN guest_bookings ON pr.id = guest_bookings.room_id AND guest_bookings.status = 'active' - AND guest_bookings.hotel_id = $4 + AND guest_bookings.hotel_id = $5 LEFT JOIN guests ON guests.id = guest_bookings.guest_id GROUP BY pr.id, pr.room_number, pr.floor, pr.suite_type, pr.room_status, pr.is_accessible - ORDER BY pr.room_number ASC`, - filters.Floors, cursorRoomNumber, limit+1, hotelID) + ORDER BY pr.room_number ASC, pr.id ASC`, + filters.Floors, cursorNumber, cursorID, limit+1, hotelID) if err != nil { return nil, err @@ -65,7 +73,6 @@ func (r *RoomsRepository) FindRoomsWithOptionalGuestBookingsByFloor(ctx context. var guestsJSON json.RawMessage err := rows.Scan( &rb.ID, &rb.RoomNumber, &rb.Floor, &rb.SuiteType, &rb.RoomStatus, &rb.IsAccessible, - &rb.BookingStatus, &guestsJSON, ) if err != nil { @@ -112,7 +119,6 @@ func (r *RoomsRepository) FindRoomByID(ctx context.Context, hotelID string, id s row := r.db.QueryRow(ctx, ` SELECT r.id, r.room_number, r.floor, r.suite_type, r.room_status, r.is_accessible, - CASE WHEN COUNT(gb.id) > 0 THEN 'active' ELSE 'inactive' END AS booking_status, json_agg( json_build_object( 'id', g.id, @@ -132,7 +138,7 @@ func (r *RoomsRepository) FindRoomByID(ctx context.Context, hotelID string, id s var rb models.RoomWithOptionalGuestBooking var guestsJSON json.RawMessage - err := row.Scan(&rb.ID, &rb.RoomNumber, &rb.Floor, &rb.SuiteType, &rb.RoomStatus, &rb.IsAccessible, &rb.BookingStatus, &guestsJSON) + err := row.Scan(&rb.ID, &rb.RoomNumber, &rb.Floor, &rb.SuiteType, &rb.RoomStatus, &rb.IsAccessible, &guestsJSON) if err != nil { if errors.Is(err, pgx.ErrNoRows) { return nil, errs.ErrNotFoundInDB