resolve issue #81 (#83)

* fix issue #81

* changelog

CHANGELOG.md

@@ -5,6 +5,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Calendar Versioning](https://calver.org/) (`YYYY.MM.MICRO-TAG`).
 
+## [v2024.11.4-beta] - 2024-11-19
+
+### Fixed
+
+- Optimised UX when switching between list and map view ([#79](https://github.com/felix-schott/jamsessions/pull/79))
+
+### Added
+
+- Added functionality for optional rating to be submitted and stored alongside a comment (PR [#83](https://github.com/felix-schott/jamsessions/pull/83))
+
+### Changed
+
+- Changed content of InfoPopup, added link to github repo (PR [#80](https://github.com/felix-schott/jamsessions/pull/80))
+- Changed order in which venues are displayed in `AddSessionPopup` to alphabetical (PR [#82](https://github.com/felix-schott/jamsessions/pull/82))
+
 ## [v2024.11.3-beta] - 2024-11-15
 
 ### Fixed

backend/Makefile

@@ -15,7 +15,7 @@ internal/db/models.go: internal/db/schema.sql internal/db/query.sql internal/db/
 
 test-ci: internal/db/models.go bin/dbcli # run in ci pipeline
 	@TEST_DB_PORT=$(TEST_DB_PORT) docker compose -f test.docker-compose.yml up test_db -d
-	@sleep 7 # wait until the database has started up
+	@sleep 8 # wait until the database has started up
 	@if go clean -testcache && DB_URL=$(DB_URL) PATH=${PATH}:${CURDIR}/bin go test ./internal/db ./internal/geocoding ./cmd/server ./cmd/dbcli; then \
 		TEST_DB_PORT=$(TEST_DB_PORT) docker compose -f test.docker-compose.yml down test_db; \
 		docker system prune -af &> /dev/null; \
@@ -43,7 +43,7 @@ test-server:
 
 test-cli: bin/dbcli
 	TEST_DB_PORT=$(TEST_DB_PORT) docker compose -f test.docker-compose.yml up test_db -d
-	sleep 5 # wait until the database has started up
+	sleep 7 # wait until the database has started up
 	cd cmd/dbcli && PATH=${PATH}:${CURDIR}/bin DB_URL=$(DB_URL) go test || true
 	TEST_DB_PORT=$(TEST_DB_PORT) docker compose -f test.docker-compose.yml down test_db
 	docker system prune -af &> /dev/null

backend/cmd/dbcli/cli_test.go

@@ -67,6 +67,19 @@ func TestCli(t *testing.T) {
 		t.FailNow()
 	}
 
+	testSessionId2, err := queries.InsertJamSession(ctx, dbutils.InsertJamSessionParams{
+		SessionName:     "TEST_SESSION_123456",
+		Venue:           testVenueId,
+		Description:     "...",
+		StartTimeUtc:    pgtype.Timestamptz{Time: time.Date(2024, 5, 6, 3, 6, 5, 4, time.UTC), Valid: true},
+		DurationMinutes: 30,
+		Interval:        "Weekly",
+	})
+	if err != nil {
+		t.Errorf("the following error occurred when trying to insert a session record: %v", err)
+		t.FailNow()
+	}
+
 	t.Run("UpdateVenueBackline", func(t *testing.T) {
 		// temporary directory for testing
 		migrationsDirectory := t.TempDir()
@@ -120,7 +133,7 @@ func TestCli(t *testing.T) {
 		}
 		testSessionProps := types.SessionProperties{
 			SessionName:     ptr("Foo's Session"),
-			Venue:           ptr(int32(999999)), // replace this after serialisation
+			Venue:           ptr(int32(-999999)), // replace this after serialisation
 			Description:     ptr("Bla bla"),
 			StartTimeUtc:    ptr(time.Date(2024, 5, 7, 1, 1, 1, 1, time.UTC)),
 			DurationMinutes: ptr(int16(30)),
@@ -131,7 +144,7 @@ func TestCli(t *testing.T) {
 			t.Errorf("could not marshal to json: %v", err)
 			t.FailNow()
 		}
-		testSessionJson = []byte(strings.Replace(string(testSessionJson), "999999", "$new_id", -1)) // set venue to bash variable that will be evaluated  when the script runs
+		testSessionJson = []byte(strings.Replace(string(testSessionJson), "-999999", "$new_id", -1)) // set venue to bash variable that will be evaluated  when the script runs
 		if _, err := migrationutils.WriteMigration(fmt.Sprintf(`new_id=$(dbcli insert venue "%s");`+"\n"+`dbcli insert session "%s";`, testVenueJson, testSessionJson), "test_insert_session", migrationsDirectory); err != nil {
 			t.Errorf("could not write migration: %v", err)
 		}
@@ -223,4 +236,132 @@ func TestCli(t *testing.T) {
 			t.Errorf("name of inserted comment author (%v) doesn't match 'test author'", rec[0].Author)
 		}
 	})
+
+	t.Run("InsertRating", func(t *testing.T) {
+		// temporary directory for testing
+		migrationsDirectory := t.TempDir()
+		migrationsArchive := filepath.Join(migrationsDirectory, "/archive")
+
+		testRatingJson, err := json.Marshal(dbutils.InsertSessionRatingParams{
+			Session: testSessionId,
+			Rating:  ptr(int16(3)),
+		})
+		if err != nil {
+			t.Error("Couldn't marshal json", err)
+			t.FailNow()
+		}
+
+		if fp, err := migrationutils.WriteMigration(fmt.Sprintf(`dbcli insert rating "%s";`, testRatingJson), "test_insert_rating", migrationsDirectory); err != nil {
+			t.Errorf("could not write to file %v: %v", fp, err)
+		}
+
+		// run migrations
+		var stderr bytes.Buffer
+		var stdout bytes.Buffer
+		cmd := exec.Command("bash", migrationsScript, "-y")
+		cmd.Env = os.Environ()
+		cmd.Env = append(cmd.Env, "MIGRATIONS_DIRECTORY="+migrationsDirectory)
+		cmd.Env = append(cmd.Env, "MIGRATIONS_ARCHIVE="+migrationsArchive)
+		cmd.Stderr = &stderr
+		cmd.Stdout = &stdout
+
+		if err := cmd.Run(); err != nil {
+			t.Errorf("an error occured when running migrations: %v: %v", err, stderr.String())
+		}
+
+		// check db
+		rec, err := queries.GetSessionById(ctx, testSessionId)
+		if err != nil {
+			t.Error("error when retrieving inserted session record:", err)
+		}
+		if rec.Rating != 3 {
+			t.Errorf("rating in DB (%v) doesn't match 3", rec.Rating)
+		}
+	})
+
+	t.Run("InsertCommentAndRating", func(t *testing.T) {
+		// temporary directory for testing
+		migrationsDirectory := t.TempDir()
+		migrationsArchive := filepath.Join(migrationsDirectory, "/archive")
+
+		commentPayload := dbutils.InsertSessionCommentParams{
+			Session: int32(testSessionId2),
+			Content: "hey",
+		}
+		commentJson, err := json.Marshal(commentPayload)
+		if err != nil {
+			t.Error(err)
+			t.FailNow()
+		}
+
+		ratingPayload := dbutils.InsertSessionRatingParams{
+			Session: int32(testSessionId2),
+			Rating:  ptr(int16(2)),
+			Comment: ptr(int32(-999999)),
+		}
+		ratingJson, err := json.Marshal(ratingPayload)
+		if err != nil {
+			t.Error(err)
+			t.FailNow()
+		}
+		ratingJson = []byte(strings.Replace(string(ratingJson), "-999999", "$new_comment", -1))
+
+		script := fmt.Sprintf(`new_comment=$(dbcli insert comment "%s");`+"\n"+`dbcli insert rating "%s";`, commentJson, ratingJson)
+		fmt.Println(script)
+		if fp, err := migrationutils.WriteMigration(script, "test_insert_comment", migrationsDirectory); err != nil {
+			t.Errorf("could not write to file %v: %v", fp, err)
+		}
+
+		// run migrations
+		var stderr bytes.Buffer
+		var stdout bytes.Buffer
+		cmd := exec.Command("bash", migrationsScript, "-y")
+		cmd.Env = os.Environ()
+		cmd.Env = append(cmd.Env, "MIGRATIONS_DIRECTORY="+migrationsDirectory)
+		cmd.Env = append(cmd.Env, "MIGRATIONS_ARCHIVE="+migrationsArchive)
+		cmd.Stderr = &stderr
+		cmd.Stdout = &stdout
+
+		if err := cmd.Run(); err != nil {
+			t.Errorf("an error occured when running migrations: %v: %v", err, stderr.String())
+		}
+
+		// check db
+		ratingRecs, err := queries.GetRatingsBySessionId(ctx, testSessionId2)
+		if err != nil {
+			t.Error("error when retrieving inserted rating records:", err)
+		}
+		if len(ratingRecs) != 1 {
+			t.Error("epected exactly 1 rating record, got", len(ratingRecs))
+			t.FailNow()
+		}
+		if *ratingRecs[0].Rating != 2 {
+			t.Error("expected rating to be 2, got", *ratingRecs[0].Rating)
+		}
+
+		commentRecs, err := queries.GetCommentsBySessionId(ctx, testSessionId2)
+		if err != nil {
+			t.Error("error when retrieving inserted comment records:", err)
+		}
+		if len(commentRecs) != 1 {
+			t.Errorf("expected exactly 1 comment record to be returned for session %v, got %v", testSessionId2, len(commentRecs))
+			t.FailNow()
+		}
+		if *commentRecs[0].Rating == 0 {
+			t.Error("the rating field is null")
+		}
+		if *commentRecs[0].RatingID != ratingRecs[0].RatingID {
+			t.Errorf("field rating ID of comment (%v) doesn't correspond to the rating ID inserted in this test (%v)", *commentRecs[0].RatingID, ratingRecs[0].RatingID)
+		}
+		if commentRecs[0].Session != testSessionId2 {
+			t.Errorf("expected the session ID to be %v, instead got %v", testSessionId2, commentRecs[0].Session)
+		}
+		if commentRecs[0].Content != "hey" {
+			t.Errorf("content of inserted comment (%v) doesn't match 'hey'", commentRecs[0].Content)
+		}
+
+		if *commentRecs[0].Rating != 2 {
+			t.Errorf("rating ID %v in DB (%v) doesn't match 2", *commentRecs[0].RatingID, *commentRecs[0].Rating)
+		}
+	})
 }

backend/cmd/server/handlers.go

@@ -315,13 +315,13 @@ func PostSession(c *fuego.ContextWithBody[types.SessionPropertiesWithVenue]) (ty
 		}
 
 		// set venue to bash variable that will be evaluated as the real venue id during runtime
-		payload.Venue = ptr(int32(999999))
+		payload.Venue = ptr(int32(-999999))
 		sessionJson, err = json.Marshal(payload.SessionProperties)
 		if err != nil {
 			slog.Error("PostSession", "msg", err, "props", "session")
 			return types.SessionFeature[types.SessionProperties]{}, errors.New("an unknown error occured")
 		}
-		sessionJson = []byte(strings.Replace(string(sessionJson), "999999", "$new_id", -1))
+		sessionJson = []byte(strings.Replace(string(sessionJson), "-999999", "$new_id", -1))
 
 		cmd = fmt.Sprintf(`new_id=$(dbcli insert venue "%s");`+"\n"+`dbcli insert session "%s";`, venueJson, sessionJson)
 		title = fmt.Sprintf("insert_venue_%v_session_%v", *payload.VenueName, *payload.SessionName)
@@ -372,6 +372,7 @@ type CommentBody struct {
 	Session *int   `json:"session"`
 	Author  string `json:"author"`
 	Content string `json:"content"`
+	Rating  *int16 `json:"rating,omitempty"`
 }
 
 func PostCommentForSessionById(c *fuego.ContextWithBody[CommentBody]) (types.SessionFeature[types.SessionProperties], error) {
@@ -385,11 +386,37 @@ func PostCommentForSessionById(c *fuego.ContextWithBody[CommentBody]) (types.Ses
 		return types.SessionFeature[types.SessionProperties]{}, err
 	}
 	payload.Session = ptr(id)
-	j, err := json.Marshal(payload)
-	if err != nil {
-		return types.SessionFeature[types.SessionProperties]{}, err
+
+	var cmd string
+	if payload.Rating != nil {
+
+		ratingPayload := dbutils.InsertSessionRatingParams{
+			Session: int32(id),
+			Rating:  payload.Rating,
+			Comment: ptr(int32(-999999)),
+		}
+		ratingJson, err := json.Marshal(ratingPayload)
+		if err != nil {
+			return types.SessionFeature[types.SessionProperties]{}, err
+		}
+		ratingJson = []byte(strings.Replace(string(ratingJson), "-999999", "$new_comment", -1))
+
+		payload.Rating = nil
+		commentJson, err := json.Marshal(payload)
+		if err != nil {
+			return types.SessionFeature[types.SessionProperties]{}, err
+		}
+
+		cmd = fmt.Sprintf(`new_comment=$(dbcli insert comment "%s");`+"\n"+`dbcli insert rating "%s";`, commentJson, ratingJson)
+		slog.Info("PostCommentForSessionById", "mode", "commentAndRating", "cmd", cmd)
+	} else {
+		commentJson, err := json.Marshal(payload)
+		if err != nil {
+			return types.SessionFeature[types.SessionProperties]{}, err
+		}
+		cmd = fmt.Sprintf(`dbcli insert comment "%s";`, commentJson)
+		slog.Info("PostCommentForSessionById", "mode", "commentOnly", "cmd", cmd)
 	}
-	cmd := fmt.Sprintf(`dbcli insert comment "%s"`, j)
 	if _, err := migrationutils.WriteMigration(cmd, fmt.Sprintf("insert_comment_session_%v", id), migrationsDirectory); err != nil {
 		slog.Error("PostCommentForSessionById", "id", id, "msg", err)
 		return types.SessionFeature[types.SessionProperties]{}, errors.New("an unknown error occurred")

backend/cmd/server/handlers_test.go

@@ -392,7 +392,50 @@ func TestHandlers(t *testing.T) {
 			t.Errorf("error reading file: %v", err)
 		}
 
-		matched, err := regexp.Match(fmt.Sprintf(`.*.*dbcli insert comment "{\\"session\\":%v,\\"author\\":\\"\\",\\"content\\":\\"%v\\"}"`, testSession1Id, testComment), f)
+		matched, err := regexp.Match(fmt.Sprintf(`.*dbcli insert comment "{\\"session\\":%v,\\"author\\":\\"\\",\\"content\\":\\"%v\\"}"`, testSession1Id, testComment), f)
+		if err != nil {
+			t.Errorf("error when trying match with regex: %v", err)
+		}
+
+		if !matched {
+			t.Errorf("expected the regex to match. instead got file contents: %s", f)
+		}
+		// see internal/db/cli for cli tests
+	})
+
+	t.Run("PostCommentWithRating", func(t *testing.T) {
+
+		// temp directory for migrations
+		migrationsDirectory = t.TempDir()
+
+		testComment := "Test comment number 123!"
+
+		handler := fuego.HTTPHandler(s, PostCommentForSessionById)
+		req := httptest.NewRequest(http.MethodPost, fmt.Sprintf("/jamsessions/%v/comments", testSession2Id), strings.NewReader(fmt.Sprintf(`{"content": "%v", "rating": 4}`, testComment)))
+		req.SetPathValue("id", fmt.Sprint(testSession2Id))
+		w := httptest.NewRecorder()
+		handler(w, req)
+		res := w.Result()
+		if res.StatusCode != 201 {
+			t.Errorf("expected status code 201, got %v", res.StatusCode)
+		}
+
+		dir, err := os.ReadDir(migrationsDirectory)
+		if err != nil {
+			t.Errorf("couldn't read directory contents: %v", err)
+			t.FailNow()
+		}
+		if len(dir) != 1 {
+			t.Errorf("expected exactly 1 file in the directory, got %v", len(dir))
+			t.FailNow()
+		}
+
+		f, err := os.ReadFile(filepath.Join(migrationsDirectory, dir[0].Name()))
+		if err != nil {
+			t.Errorf("error reading file: %v", err)
+		}
+
+		matched, err := regexp.Match(fmt.Sprintf(`.*dbcli insert comment "{\\"session\\":%v,\\"author\\":\\"\\",\\"content\\":\\"%v\\"}".*\n.*dbcli insert rating "{\\"session\\":%v,\\"rating\\":4,\\"comment\\":\$new_comment}";`, testSession2Id, testComment, testSession2Id), f)
 		if err != nil {
 			t.Errorf("error when trying match with regex: %v", err)
 		}

backend/internal/db/db_test.go

@@ -237,6 +237,26 @@ func TestInsertAndRetrieveComment(t *testing.T) {
 	}
 }
 
+func TestGetRatingsBySessionId(t *testing.T) {
+	result, err := queries.GetRatingsBySessionId(ctx, fixtureSessionId)
+	if err != nil {
+		t.Errorf("could not retrieve ratings by session ids: %v", err)
+		t.FailNow()
+	}
+	if len(result) != 2 {
+		t.Error("expected two ratings to be returned, instead got", len(result))
+		t.FailNow()
+	}
+
+	if *result[0].Rating != 5 {
+		t.Error("expected the first rating to be 5, instead got", *result[0].Rating)
+	}
+
+	if *result[1].Rating != 1 {
+		t.Error("expected the second rating to be 1, instead got", *result[1].Rating)
+	}
+}
+
 func TestGetSessionIdsByDate(t *testing.T) {
 	result, err := queries.GetSessionIdsByDate(ctx, pgtype.Date{Time: time.Date(2024, 11, 19, 0, 0, 0, 0, time.UTC), Valid: true})
 	if err != nil {

backend/internal/db/query.sql

@@ -20,10 +20,14 @@ SELECT * FROM london_jam_sessions.venues
 WHERE venue_name = $1;
 
 -- name: GetCommentsBySessionId :many
-SELECT c.*, r.rating FROM london_jam_sessions.comments c
-LEFT OUTER JOIN london_jam_sessions.ratings r ON c.comment_id = r.rating_id
+SELECT c.*, r.rating, r.rating_id FROM london_jam_sessions.comments c
+LEFT OUTER JOIN london_jam_sessions.ratings r ON c.comment_id = r.comment
 WHERE c.session = $1;
 
+-- name: GetRatingsBySessionId :many
+SELECT * FROM london_jam_sessions.ratings
+WHERE session = $1;
+
 -- name: GetAllSessions :many
 SELECT s.*, l.*, coalesce(round(avg(rating), 2), 0)::real AS rating FROM london_jam_sessions.jamsessions s
 JOIN london_jam_sessions.venues l ON s.venue = l.venue_id