fix bugs and add tests for PostMakeReservation handler

internal/handlers/handlers.go

@@ -115,19 +115,16 @@ func (m *Repository) MakeReservation(w http.ResponseWriter, r *http.Request) {
 func (m *Repository) PostMakeReservation(w http.ResponseWriter, r *http.Request) {
 	reservation, ok := m.App.Session.Get(r.Context(), "reservation").(models.Reservation)
 	if !ok {
-		helpers.ServerError(w, errors.New("connot get item from session"))
+		m.App.Session.Put(r.Context(), "error", "can't get reservation from session")
-		return
+		http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
-	}
-	err := r.ParseForm()
-	if err != nil {
-		helpers.ServerError(w, err)
 		return
 	}
 
-	reservation.FirstName = r.Form.Get("first_name")
+	if err := r.ParseForm(); err != nil {
-	reservation.LastName = r.Form.Get("last_name")
+		m.App.Session.Put(r.Context(), "error", "can't parse form")
-	reservation.Email = r.Form.Get("email")
+		http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
-	reservation.Phone = r.Form.Get("phone")
+		return
+	}
 
 	form := forms.New(r.PostForm)
 
@@ -135,6 +132,13 @@ func (m *Repository) PostMakeReservation(w http.ResponseWriter, r *http.Request)
 	form.MinLength("first_name", 2)
 	form.IsEmail("email")
 
+	reservation.FirstName = form.Get("first_name")
+	reservation.LastName = form.Get("last_name")
+	reservation.Email = form.Get("email")
+	reservation.Phone = form.Get("phone")
+
+	// TODO: Should I check the validity of reservation.StartDate / EndDate?
+
 	if !form.Valid() {
 		data := make(map[string]interface{})
 		data["reservation"] = reservation
@@ -148,7 +152,8 @@ func (m *Repository) PostMakeReservation(w http.ResponseWriter, r *http.Request)
 
 	newReservationID, err := m.DB.InsertReservation(reservation)
 	if err != nil {
-		helpers.ServerError(w, err)
+		m.App.Session.Put(r.Context(), "error", "can't insert reservation into database")
+		http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
 		return
 	}
 
@@ -165,7 +170,8 @@ func (m *Repository) PostMakeReservation(w http.ResponseWriter, r *http.Request)
 
 	err = m.DB.InsertRoomRestriction(restriction)
 	if err != nil {
-		helpers.ServerError(w, err)
+		m.App.Session.Put(r.Context(), "error", "can't insert room restriction into database")
+		http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
 		return
 	}
 
@@ -208,6 +214,11 @@ func (m *Repository) Availability(w http.ResponseWriter, r *http.Request) {
 
 // PostAvailability is the search for availability page handler
 func (m *Repository) PostAvailability(w http.ResponseWriter, r *http.Request) {
+	if err := r.ParseForm(); err != nil {
+		m.App.Session.Put(r.Context(), "error", "can't parse form")
+		http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
+		return
+	}
 	start := r.Form.Get("start")
 	end := r.Form.Get("end")
 
@@ -265,6 +276,11 @@ type jsonResponse struct {
 
 // AvailabilityJSON is the search for availability page handler
 func (m *Repository) AvailabilityJSON(w http.ResponseWriter, r *http.Request) {
+	if err := r.ParseForm(); err != nil {
+		m.App.Session.Put(r.Context(), "error", "can't parse form")
+		http.Redirect(w, r, "/", http.StatusTemporaryRedirect)
+		return
+	}
 	sd := r.Form.Get("start")
 	ed := r.Form.Get("end")
 

internal/handlers/handlers_test.go

@@ -2,11 +2,14 @@ package handlers
 
 import (
 	"context"
+	"fmt"
 	"go-udemy-web-1/internal/models"
 	"log"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
+	"time"
 )
 
 type postData struct {
@@ -60,60 +63,197 @@ func TestHandlers(t *testing.T) {
 	}
 }
 
-func TestRepository_Reservation(t *testing.T) {
+// {{{ Make Reservation Tests
+
+var makeReservationTests = []struct {
+	name               string
+	roomID             int
+	expectedStatusCode int
+}{
+	{"ok", 1, http.StatusOK},
+	{"no session", 0, http.StatusTemporaryRedirect},
+	{"non-existant room", 100, http.StatusTemporaryRedirect},
+}
+
+func TestRepository_MakeReservation(t *testing.T) {
+	for _, test := range makeReservationTests {
 		reservation := models.Reservation{
-		RoomID: 1,
+			RoomID: test.roomID,
 			Room: models.Room{
-			ID:       1,
+				ID:       test.roomID,
-			RoomName: "General's Quarters",
+				RoomName: "Room name",
 			},
 		}
-
 		req, _ := http.NewRequest("GET", "/make-reservation", nil)
 		ctx := getCtx(req)
-
 		req = req.WithContext(ctx)
-
 		rr := httptest.NewRecorder()
-
+		if test.roomID == 0 {
+			session.Put(ctx, "reservation", nil)
+		} else {
 			session.Put(ctx, "reservation", reservation)
-
+		}
 		handler := http.HandlerFunc(Repo.MakeReservation)
 
 		handler.ServeHTTP(rr, req)
 
-	if rr.Code != http.StatusOK {
+		if rr.Code != test.expectedStatusCode {
-		t.Errorf("Reservation handler returned response code: got %d, wanted %d",
+			t.Errorf("for %s, reservation handler returned response code: got %d, wanted %d",
-			rr.Code, http.StatusOK)
+				test.name, rr.Code, http.StatusOK)
 		}
-
-	// test case where reservation is not in session (reset everything)
-	req, _ = http.NewRequest("GET", "/make-reservation", nil)
-	ctx = getCtx(req)
-	req = req.WithContext(ctx)
-	rr = httptest.NewRecorder()
-
-	handler.ServeHTTP(rr, req)
-	if rr.Code != http.StatusTemporaryRedirect {
-		t.Errorf("Reservation handler returned response code: got %d, wanted %d",
-			rr.Code, http.StatusTemporaryRedirect)
-	}
-
-	// test with non-existant room
-	req, _ = http.NewRequest("GET", "/make-reservation", nil)
-	ctx = getCtx(req)
-	req = req.WithContext(ctx)
-	rr = httptest.NewRecorder()
-	reservation.RoomID = 100
-	session.Put(ctx, "reservation", reservation)
-
-	handler.ServeHTTP(rr, req)
-	if rr.Code != http.StatusTemporaryRedirect {
-		t.Errorf("Reservation handler returned response code: got %d, wanted %d",
-			rr.Code, http.StatusTemporaryRedirect)
 	}
 }
 
+// }}}
+// {{{ PostMakeReservation tests
+
+var postMakeReservationTests = []struct {
+	name            string
+	reservationInfo []string
+	roomID          int
+
+	expectedStatusCode int
+}{
+	{
+		"ok",
+		[]string{
+			"first_name=John",
+			"last_name=Smith",
+			"email=john@smith.com",
+			"phone=1234",
+			"room_id=1",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		1, http.StatusSeeOther,
+	},
+	{
+		"no_session",
+		[]string{
+			"first_name=John",
+			"last_name=Smith",
+			"email=john@smith.com",
+			"phone=1234",
+			"room_id=0",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		0, http.StatusTemporaryRedirect,
+	},
+	{
+		"no_post_data",
+		[]string{},
+		0, http.StatusTemporaryRedirect,
+	},
+	{
+		"missing first name",
+		[]string{
+			"last_name=Smith",
+			"email=john@smith.com",
+			"phone=1234",
+			"room_id=1",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		1, http.StatusOK,
+	},
+	{
+		"wrong first name",
+		[]string{
+			"first_name=J",
+			"last_name=Smith",
+			"email=john@smith.com",
+			"phone=1234",
+			"room_id=1",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		1, http.StatusOK,
+	},
+	{
+		"wrong email",
+		[]string{
+			"first_name=John",
+			"last_name=Smith",
+			"email=john@smith",
+			"phone=1234",
+			"room_id=1",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		1, http.StatusOK,
+	},
+	{
+		"insert reservation error",
+		[]string{
+			"first_name=John",
+			"last_name=Smith",
+			"email=john@smith.com",
+			"phone=1234",
+			"room_id=2",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		2, http.StatusTemporaryRedirect,
+	},
+	{
+		"insert room restriction error",
+		[]string{
+			"first_name=John",
+			"last_name=Smith",
+			"email=john@smith.com",
+			"phone=1234",
+			"room_id=100",
+			"start_date=2050-01-01",
+			"end_date=2050-01-02",
+		},
+		100, http.StatusTemporaryRedirect,
+	},
+}
+
+func TestRepository_PostMakeReservation(t *testing.T) {
+	for _, test := range postMakeReservationTests {
+		var reqBody string
+		if len(test.reservationInfo) > 0 {
+			reqBody = test.reservationInfo[0]
+			for _, element := range test.reservationInfo[1:] {
+				reqBody = fmt.Sprintf("%s&%s", reqBody, element)
+			}
+		}
+		layout := "2006-01-02"
+		sd, _ := time.Parse(layout, "2050-01-01")
+		ed, _ := time.Parse(layout, "2050-01-02")
+		reservation := models.Reservation{
+			RoomID:    test.roomID,
+			StartDate: sd,
+			EndDate:   ed,
+		}
+
+		req, _ := http.NewRequest("POST", "/make-reservation", strings.NewReader(reqBody))
+		ctx := getCtx(req)
+		req = req.WithContext(ctx)
+		req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+		rr := httptest.NewRecorder()
+		if test.roomID == 0 {
+			session.Put(ctx, "reservation", nil)
+		} else {
+			session.Put(ctx, "reservation", reservation)
+		}
+
+		handler := http.HandlerFunc(Repo.PostMakeReservation)
+		handler.ServeHTTP(rr, req)
+
+		if rr.Code != test.expectedStatusCode {
+			fmt.Printf("for %s, reservation handler returned response code: got %d, wanted %d",
+				test.name, rr.Code, test.expectedStatusCode)
+		}
+	}
+}
+
+// }}}
+// {{{ Test Helpers
+
 func getCtx(req *http.Request) context.Context {
 	ctx, err := session.Load(req.Context(), req.Header.Get("X-Session"))
 	if err != nil {
@@ -122,3 +262,5 @@ func getCtx(req *http.Request) context.Context {
 
 	return ctx
 }
+
+// }}}

internal/handlers/setup_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/alexedwards/scs/v2"
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
-	"github.com/justinas/nosurf"
 )
 
 var functions = template.FuncMap{}
@@ -65,7 +64,7 @@ func getRoutes() http.Handler {
 	mux := chi.NewMux()
 
 	mux.Use(middleware.Recoverer)
-	// mux.Use(NoSurf)  // No need to test
+	mux.Use(WriteToConsole)
 	mux.Use(SessionLoad)
 
 	mux.Get("/", Repo.Home)
@@ -95,20 +94,6 @@ func WriteToConsole(next http.Handler) http.Handler {
 	})
 }
 
-// NoSurf adds CSRF protection to all POST requests
-func NoSurf(next http.Handler) http.Handler {
-	csrfHandler := nosurf.New(next)
-
-	csrfHandler.SetBaseCookie(http.Cookie{
-		HttpOnly: true,
-		Path:     "/",
-		Secure:   app.InProduction,
-		SameSite: http.SameSiteLaxMode,
-	})
-
-	return csrfHandler
-}
-
 // SessionLoad loads and saves the session on every request
 func SessionLoad(next http.Handler) http.Handler {
 	return session.LoadAndSave(next)

internal/repository/dbrepo/test-repo.go

@@ -12,11 +12,18 @@ func (m *testDBRepo) AllUsers() bool {
 
 // InsertReservation inserts a reservation into the database
 func (m *testDBRepo) InsertReservation(res models.Reservation) (int, error) {
+	// if the room id is 2, then fail; otherwise, pass
+	if res.RoomID == 2 {
+		return 0, errors.New("deliberate error")
+	}
 	return 1, nil
 }
 
 // InsertRoomRestriction inserts a room restriction into the database
 func (m *testDBRepo) InsertRoomRestriction(r models.RoomRestriction) error {
+	if r.RoomID == 100 {
+		return errors.New("deliberate error")
+	}
 	return nil
 }
 
@@ -36,7 +43,7 @@ func (m *testDBRepo) GetRoomById(id int) (models.Room, error) {
 	var room models.Room
 
 	if id > 2 {
-		return room, errors.New("Deliberate error")
+		return room, errors.New("deliberate error")
 	}
 	return room, nil
 }