refactor: improve booking error handling and responses
Some checks failed
CI / checks (push) Has been cancelled

Refactor booking retrieval to return errors instead of nil values,
enabling more robust error handling throughout the booking, payment,
and PDF endpoints. Add custom HTTP error page rendering for not found
and internal server errors. Update interfaces and tests to match new
method signatures. This improves user feedback and code maintainability.
This commit is contained in:
Ruidy 2025-11-17 19:26:45 +01:00
parent a0b7672e9e
commit afc61e02f1
No known key found for this signature in database
GPG key ID: 705C24D202990805
8 changed files with 94 additions and 13 deletions

View file

@ -81,10 +81,13 @@ func (ps *PgStore) CardTotal(from, to time.Time) (float64, error) {
return total, nil return total, nil
} }
func (ps *PgStore) Get(id int) *booking.Booking { func (ps *PgStore) Get(id int) (*booking.Booking, error) {
var b booking.Booking var b booking.Booking
ps.db.Preload("Items").Preload("Payments").First(&b, id) res := ps.db.Preload("Items").Preload("Payments").First(&b, id)
return &b if err := res.Error; err != nil {
return nil, err
}
return &b, nil
} }
func (ps *PgStore) Create(b *booking.Booking) error { func (ps *PgStore) Create(b *booking.Booking) error {

View file

@ -16,6 +16,7 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
u "github.com/rjNemo/underscore" u "github.com/rjNemo/underscore"
"github.com/rjNemo/rentease/assets"
"github.com/rjNemo/rentease/internal/config" "github.com/rjNemo/rentease/internal/config"
"github.com/rjNemo/rentease/internal/constant" "github.com/rjNemo/rentease/internal/constant"
"github.com/rjNemo/rentease/internal/service/booking" "github.com/rjNemo/rentease/internal/service/booking"
@ -167,7 +168,11 @@ func handleBookingPage(bs *booking.Service, hc *config.Host) http.HandlerFunc {
return return
} }
b := bs.One(id) b, err := bs.One(id)
if err != nil {
renderHTTPErrorPage(w, http.StatusNotFound)
return
}
var eid string var eid string
if b.ExternalID == nil { if b.ExternalID == nil {
@ -380,7 +385,10 @@ func handleCreateItem(bs *booking.Service, hc *config.Host) http.HandlerFunc {
return return
} }
b := bs.One(bid) b, err := bs.One(bid)
if bookingLookupFailed(w, err) {
return
}
itemName := r.FormValue("item") itemName := r.FormValue("item")
itm, ok := hc.Items[itemName] itm, ok := hc.Items[itemName]
@ -550,3 +558,32 @@ func handleBookingCancel(bs *booking.Service) http.HandlerFunc {
} }
} }
} }
func bookingLookupFailed(w http.ResponseWriter, err error) bool {
if err == nil {
return false
}
if errors.Is(err, booking.ErrBookingNotFound) {
renderHTTPErrorPage(w, http.StatusNotFound)
return true
}
renderHTTPErrorPage(w, http.StatusInternalServerError)
return true
}
func renderHTTPErrorPage(w http.ResponseWriter, status int) {
pagePath := fmt.Sprintf("assets/html/HTTP%d.html", status)
page, err := assets.Static.ReadFile(pagePath)
if err != nil {
http.Error(w, http.StatusText(status), status)
return
}
w.Header().Set("Content-Type", "text/html; charset=utf-8")
w.WriteHeader(status)
if _, err := w.Write(page); err != nil {
slog.Error("failed to write error page", slog.Any("path", pagePath), slog.Any("error", err))
}
}

View file

@ -34,14 +34,20 @@ func handleCreatePayment(bs *booking.Service, hc *config.Host) http.HandlerFunc
} }
} }
b := bs.One(id) b, err := bs.One(id)
if bookingLookupFailed(w, err) {
return
}
if _, err := bs.CreatePayment(b.ID, amount, r.FormValue("paymentMethod")); err != nil { if _, err := bs.CreatePayment(b.ID, amount, r.FormValue("paymentMethod")); err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError) http.Error(w, err.Error(), http.StatusInternalServerError)
return return
} }
nb := bs.One(id) nb, err := bs.One(id)
if bookingLookupFailed(w, err) {
return
}
component := view.PaymentList( component := view.PaymentList(
u.Map(nb.Payments, func(p booking.Payment) *view.PaymentViewModel { u.Map(nb.Payments, func(p booking.Payment) *view.PaymentViewModel {

View file

@ -22,7 +22,10 @@ func handlePdfCreateInvoice(bs *booking.Service, hc *config.Host) http.HandlerFu
return return
} }
b := bs.One(id) b, err := bs.One(id)
if bookingLookupFailed(w, err) {
return
}
filePath, err := bs.BuildInvoice(b, hc) filePath, err := bs.BuildInvoice(b, hc)
if err != nil { if err != nil {

View file

@ -130,5 +130,27 @@ func NewRouter(filesystem embed.FS, debug bool, origins []string) (*chi.Mux, err
fileServer := http.StripPrefix("/static/", http.FileServer(http.FS(assetsFS))) fileServer := http.StripPrefix("/static/", http.FileServer(http.FS(assetsFS)))
r.Handle("/static/*", fileServer) r.Handle("/static/*", fileServer)
notFoundHandler, err := newHTTPErrorHandler(filesystem, http.StatusNotFound)
if err != nil {
return nil, fmt.Errorf("failed to setup not found handler: %w", err)
}
r.NotFound(notFoundHandler)
return r, nil return r, nil
} }
func newHTTPErrorHandler(filesystem embed.FS, statusCode int) (http.HandlerFunc, error) {
filePath := fmt.Sprintf("assets/html/HTTP%d.html", statusCode)
page, err := fs.ReadFile(filesystem, filePath)
if err != nil {
return nil, fmt.Errorf("failed to read error page %s: %w", filePath, err)
}
return func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "text/html; charset=utf-8")
w.WriteHeader(statusCode)
if _, err := w.Write(page); err != nil {
slog.Error("failed to write error page", slog.Any("error", err))
}
}, nil
}

View file

@ -2,9 +2,12 @@ package booking
import ( import (
"context" "context"
"errors"
"log/slog" "log/slog"
"time" "time"
"gorm.io/gorm"
"github.com/rjNemo/rentease/internal/config" "github.com/rjNemo/rentease/internal/config"
stripeclient "github.com/rjNemo/rentease/internal/driver/stripe" stripeclient "github.com/rjNemo/rentease/internal/driver/stripe"
) )
@ -14,7 +17,7 @@ type Store interface {
Search(value string) []*Line Search(value string) []*Line
List(from, to time.Time) ([]*Line, error) List(from, to time.Time) ([]*Line, error)
CardTotal(from, to time.Time) (float64, error) CardTotal(from, to time.Time) (float64, error)
Get(id int) *Booking Get(id int) (*Booking, error)
Create(b *Booking) error Create(b *Booking) error
Update(b *Booking) error Update(b *Booking) error
Cancel(id int) error Cancel(id int) error
@ -95,8 +98,15 @@ func (bs Service) Create(From time.Time, To time.Time, Name, PhoneNumber, Email,
return b return b
} }
func (bs Service) One(id int) *Booking { func (bs Service) One(id int) (*Booking, error) {
return bs.store.Get(id) b, err := bs.store.Get(id)
if err != nil {
if errors.Is(err, gorm.ErrRecordNotFound) {
return nil, ErrBookingNotFound
}
return nil, err
}
return b, nil
} }
// Update updates an existing booking with new data // Update updates an existing booking with new data

View file

@ -22,7 +22,7 @@ func (bs Service) CreateStripePaymentLink(ctx context.Context, bookingID int) (s
return "", ErrStripeClientNotConfigured return "", ErrStripeClientNotConfigured
} }
b := bs.store.Get(bookingID) b, _ := bs.store.Get(bookingID)
if b == nil || b.ID == 0 { if b == nil || b.ID == 0 {
return "", ErrBookingNotFound return "", ErrBookingNotFound
} }

View file

@ -52,7 +52,7 @@ func (m *mockStore) All() []*Line { return ni
func (m *mockStore) Search(string) []*Line { return nil } func (m *mockStore) Search(string) []*Line { return nil }
func (m *mockStore) List(time.Time, time.Time) ([]*Line, error) { return nil, nil } func (m *mockStore) List(time.Time, time.Time) ([]*Line, error) { return nil, nil }
func (m *mockStore) CardTotal(time.Time, time.Time) (float64, error) { return 0, nil } func (m *mockStore) CardTotal(time.Time, time.Time) (float64, error) { return 0, nil }
func (m *mockStore) Get(int) *Booking { return nil } func (m *mockStore) Get(int) (*Booking, error) { return nil, nil }
func (m *mockStore) Create(*Booking) error { return nil } func (m *mockStore) Create(*Booking) error { return nil }
func (m *mockStore) Update(*Booking) error { return nil } func (m *mockStore) Update(*Booking) error { return nil }
func (m *mockStore) Cancel(int) error { return nil } func (m *mockStore) Cancel(int) error { return nil }