Skip to content

fix: cursor pagination on room number + room id tiebreaks #436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
eric-kitagawa wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: cursor pagination on room number + room id tiebreaks #436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 27 additions & 10 deletions backend/internal/handler/rooms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package handler
import (
"context"
"errors"
"fmt"
"strconv"
"strings"

"github.com/generate/selfserve/internal/errs"
"github.com/generate/selfserve/internal/httpx"
Expand All @@ -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)
}
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -126,4 +143,4 @@ func (h *RoomsHandler) GetFloors(c *fiber.Ctx) error {
}

return c.JSON(floors)
}
}
33 changes: 18 additions & 15 deletions backend/internal/handler/rooms_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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{
Expand Down Expand Up @@ -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"},
Expand Down Expand Up @@ -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
},
}
Expand Down Expand Up @@ -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
},
}
Expand All @@ -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
},
}
Expand All @@ -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)
Expand All @@ -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
},
}
Expand All @@ -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")
},
}
Expand All @@ -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
},
}
Expand Down
30 changes: 18 additions & 12 deletions backend/internal/repository/rooms.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
Loading