fix(issue-16): fix critical transaction bug and high-severity repository issues

internal/kurious/adapters/sqlite_course_repository.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"time"
 
+	cerrors "git.loyso.art/frx/kurious/internal/common/errors"
 	"git.loyso.art/frx/kurious/internal/common/nullable"
 	"git.loyso.art/frx/kurious/internal/common/xslices"
 	"git.loyso.art/frx/kurious/internal/kurious/domain"
@@ -76,6 +77,20 @@ func (r *sqliteCourseRepository) List(
 	if !params.Ascending {
 		direction = "DESC"
 	}
+
+	allowedOrderBy := map[string]bool{
+		"id":         true,
+		"name":       true,
+		"created_at": true,
+		"updated_at": true,
+		"full_price": true,
+		"discount":   true,
+		"duration":   true,
+	}
+	if !allowedOrderBy[params.OrderBy] {
+		return result, fmt.Errorf("invalid order by field: %s", params.OrderBy)
+	}
+
 	query += " ORDER BY " + params.OrderBy + " " + direction
 
 	if params.Limit > 0 {
@@ -277,6 +292,9 @@ func (r *sqliteCourseRepository) Get(
 	var courseDB sqliteCourseDB
 	err = r.db.GetContext(ctx, &courseDB, query, id)
 	if err != nil {
+		if errors.Is(err, sql.ErrNoRows) {
+			return course, cerrors.ErrNotFound
+		}
 		return course, fmt.Errorf("executing query: %w", err)
 	}
 
@@ -289,7 +307,7 @@ func (r *sqliteCourseRepository) GetByExternalID(
 	return course, errors.New("not implemented")
 }
 
-func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...domain.CreateCourseParams) error {
+func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...domain.CreateCourseParams) (err error) {
 	tx, err := r.db.BeginTxx(ctx, &sql.TxOptions{Isolation: sql.LevelDefault})
 	if err != nil {
 		return fmt.Errorf("beginning tx: %w", err)
@@ -317,9 +335,9 @@ func (r *sqliteCourseRepository) CreateBatch(ctx context.Context, params ...doma
 	}
 
 	for _, param := range params {
-		_, err := stmt.ExecContext(ctx, createCourseParamsAsValues(param)...)
+		_, execErr := stmt.ExecContext(ctx, createCourseParamsAsValues(param)...)
-		if err != nil {
+		if execErr != nil {
-			return fmt.Errorf("executing statement query: %w", err)
+			return fmt.Errorf("executing statement query: %w", execErr)
 		}
 	}
 

internal/kurious/adapters/sqlite_course_repository_test.go

@@ -1,10 +1,12 @@
 package adapters
 
 import (
+	"errors"
 	"strconv"
 	"testing"
 	"time"
 
+	cerrors "git.loyso.art/frx/kurious/internal/common/errors"
 	"git.loyso.art/frx/kurious/internal/common/nullable"
 	"git.loyso.art/frx/kurious/internal/kurious/domain"
 
@@ -105,3 +107,33 @@ func (s *sqliteCourseRepositorySuite) TestListLimitOffset() {
 	s.NoError(err)
 	s.Empty(result.Courses)
 }
+
+func (s *sqliteCourseRepositorySuite) TestCreateBatchPartialFailure() {
+	cr := s.connection.CourseRepository()
+
+	baseParams := domain.CreateCourseParams{
+		SourceType: domain.SourceTypeManual,
+	}
+
+	first := baseParams
+	first.ID = "dup-id"
+
+	duplicate := baseParams
+	duplicate.ID = "dup-id"
+
+	err := cr.CreateBatch(s.ctx, first, duplicate)
+	s.Require().Error(err)
+
+	result, listErr := cr.List(s.ctx, domain.ListCoursesParams{})
+	s.Require().NoError(listErr)
+	s.Empty(result.Courses, "partial batch should be rolled back, not committed")
+	s.Zero(result.Count, "partial batch should be rolled back, not committed")
+}
+
+func (s *sqliteCourseRepositorySuite) TestGetNotFound() {
+	cr := s.connection.CourseRepository()
+
+	_, err := cr.Get(s.ctx, "does-not-exist")
+	s.Require().Error(err)
+	s.True(errors.Is(err, cerrors.ErrNotFound), "expected ErrNotFound, got %v", err)
+}

internal/kurious/adapters/sqlite_organization_repository.go

@@ -187,7 +187,7 @@ func (r *sqliteOrganizationRepository) List(ctx context.Context, params domain.L
 	}()
 
 	organizations := make([]organizationDB, 0, 1<<8)
-	err = r.db.SelectContext(ctx, &organizations, query)
+	err = r.db.SelectContext(ctx, &organizations, query, args...)
 	if err != nil {
 		return nil, fmt.Errorf("executing query: %w", err)
 	}

internal/kurious/adapters/sqlite_organization_repository_test.go

@@ -126,3 +126,45 @@ func (s *sqliteOrganzationRepositorySuite) TestCreate() {
 	expectedOrganization.UpdatedAt = gotOrganization.UpdatedAt
 	s.Equal(expectedOrganization, gotOrganization)
 }
+
+func (s *sqliteOrganzationRepositorySuite) TestListByIDs() {
+	const itemscount = 3
+	baseOrg := domain.Organization{
+		Alias:    "test-alias",
+		Name:     "test-name",
+		Site:     "test-site",
+		LogoLink: "test-logo",
+	}
+
+	ids := make([]string, 0, itemscount)
+	for i := 0; i < itemscount; i++ {
+		iStr := strconv.Itoa(i)
+		id := "test-id-" + iStr
+		ids = append(ids, id)
+
+		_, err := s.connection.Organization().Create(s.ctx, domain.CreateOrganizationParams{
+			ID:         id,
+			ExternalID: nullable.NewValue("test-ext-id-" + iStr),
+			Alias:      baseOrg.Alias,
+			Name:       baseOrg.Name,
+			Site:       baseOrg.Site,
+			LogoLink:   baseOrg.LogoLink,
+		})
+		s.Require().NoError(err)
+	}
+
+	wantedIDs := ids[:2]
+
+	gotOrgs, err := s.connection.Organization().List(s.ctx, domain.ListOrganizationsParams{
+		IDs: wantedIDs,
+	})
+	s.Require().NoError(err)
+
+	gotIDs := make([]string, 0, len(gotOrgs))
+	for _, o := range gotOrgs {
+		gotIDs = append(gotIDs, o.ID)
+	}
+
+	s.Len(gotOrgs, len(wantedIDs))
+	s.ElementsMatch(wantedIDs, gotIDs)
+}