Resolving more TODOs

controllers/v1/clapper/signup.go

@@ -40,7 +40,6 @@ func (r *Repos) NewSignup(c echo.Context) error {
 	}
 
 	// Check event exists
-	// TODO we might want to move this inside the service
 	e, err := r.event.Get(c.Request().Context(), eventID)
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {

controllers/v1/creator/video.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/labstack/echo/v4"
+	"gopkg.in/guregu/null.v4"
 
 	"github.com/ystv/web-api/services/creator/types/video"
 	"github.com/ystv/web-api/utils"
@@ -102,9 +103,15 @@ func (r *Repos) UpdateVideoMeta(c echo.Context) error {
 		return echo.NewHTTPError(http.StatusBadRequest, err)
 	}
 
-	v.UpdatedByID = &t.UserID
+	parsed, err := strconv.ParseInt(strconv.Itoa(t.UserID), 10, 64)
+	if err != nil {
+		err = fmt.Errorf("failed to parse user id: %w", err)
+		return echo.NewHTTPError(http.StatusBadRequest, err)
+	}
+
+	v.UpdatedByID = null.IntFrom(parsed)
 	currentDateTime := time.Now()
-	v.UpdatedAt = &currentDateTime
+	v.UpdatedAt = null.TimeFrom(currentDateTime)
 
 	err = r.video.UpdateMeta(c.Request().Context(), v)
 	if err != nil {

services/clapper/signup/signup.go

@@ -35,8 +35,6 @@ func (m *Store) New(ctx context.Context, eventID int, s clapper.NewSignup) (int,
 			return fmt.Errorf("failed to insert new signup sheet: %w", err)
 		}
 		// Check if positions have been added
-		// TODO I'm not too sure on using the signup struct for this,
-		// maybe another input variable instead?
 		if len(s.Crew) == 0 {
 			return nil
 		}

services/creator/types/video/video.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 	"strings"
 	"time"
+
+	"gopkg.in/guregu/null.v4"
 )
 
 type (
@@ -38,16 +40,16 @@ type (
 		Tags          Tag    `db:"tags" json:"tags"`
 		Status        string `db:"status" json:"status"`
 		Preset        `json:"preset"`
-		BroadcastDate time.Time  `db:"broadcast_date" json:"broadcastDate"`
-		CreatedAt     time.Time  `db:"created_at" json:"createdAt"`
-		CreatedByID   int        `db:"created_by_id" json:"createdByID"`
-		CreatedByNick string     `db:"created_by_nick" json:"createdByNick"`
-		UpdatedAt     *time.Time `db:"updated_at" json:"updatedAt,omitempty"`
-		UpdatedByID   *int       `db:"updated_by_nick" json:"updatedByID,omitempty"`
-		UpdatedByNick *string    `db:"updated_by_nick" json:"updatedByNick,omitempty"`
-		DeletedAt     *time.Time `db:"deleted_at" json:"deletedAt,omitempty"`
-		DeletedByID   *int       `db:"deleted_by_id" json:"deleteByID,omitempty"`
-		DeletedByNick *string    `db:"deleted_by_nick" json:"deleteByNick,omitempty"`
+		BroadcastDate time.Time   `db:"broadcast_date" json:"broadcastDate"`
+		CreatedAt     time.Time   `db:"created_at" json:"createdAt"`
+		CreatedByID   int         `db:"created_by_id" json:"createdByID"`
+		CreatedByNick string      `db:"created_by_nick" json:"createdByNick"`
+		UpdatedAt     null.Time   `db:"updated_at" json:"updatedAt,omitempty"`
+		UpdatedByID   null.Int    `db:"updated_by_nick" json:"updatedByID,omitempty"`
+		UpdatedByNick null.String `db:"updated_by_nick" json:"updatedByNick,omitempty"`
+		DeletedAt     null.Time   `db:"deleted_at" json:"deletedAt,omitempty"`
+		DeletedByID   null.Int    `db:"deleted_by_id" json:"deleteByID,omitempty"`
+		DeletedByNick null.String `db:"deleted_by_nick" json:"deleteByNick,omitempty"`
 	}
 	// MetaCal represents simple metadata for a calendar
 	MetaCal struct {
@@ -63,8 +65,8 @@ type (
 	}
 	// Preset represents the name and ID of a preset
 	Preset struct {
-		PresetID   *int    `db:"preset_id" json:"presetID"`
-		PresetName *string `db:"preset_name" json:"name"`
+		PresetID   null.Int    `db:"preset_id" json:"presetID"`
+		PresetName null.String `db:"preset_name" json:"name"`
 	}
 	// New is the basic information to create a video
 	New struct {

services/creator/video/get.go

@@ -86,10 +86,9 @@ func (s *Store) ListByCalendarMonth(ctx context.Context, year, month int) ([]vid
 func (s *Store) OfSeries(ctx context.Context, seriesID int) ([]video.Meta, error) {
 	var v []video.Meta
 
-	// TODO Update this select to fill all fields
 	err := s.db.SelectContext(ctx, &v,
-		`SELECT video_id, series_id, name video_name, url,
-		duration, views, tags, status, broadcast_date, created_at
+		`SELECT video_id, series_id, name AS video_name, url, description, thumbnail, duration, views, tags,
+       		status, preset_id, broadcast_date, created_at, created_by, updated_at, updated_by, deleted_at, deleted_by
 		FROM video.items
 		WHERE series_id = $1;`, seriesID)
 	if err != nil {

services/creator/video/new.go

@@ -80,7 +80,6 @@ func (s *Store) NewItem(ctx context.Context, v video.New) (int, error) {
 	})
 	if err != nil {
 		// Since we've wrapped in transaction the DB is safe, will just need to make sure s3 is back to original state
-		// TODO: Do we want to care about this outcome?
 		_, err = s.cdn.DeleteObject(ctx, &s3.DeleteObjectInput{
 			Bucket: aws.String(s.conf.ServeBucket),
 			Key:    aws.String(s.conf.IngestBucket + "/" + v.FileID[:32]),

services/creator/video/update.go

@@ -61,7 +61,7 @@ func (s *Store) UpdateMeta(ctx context.Context, m video.Meta) error {
 		return fmt.Errorf("failed to update videoItem in db: %w", err)
 	}
 
-	if m.Preset.PresetID != nil && m.Preset.PresetID != videoItem.Preset.PresetID {
+	if m.Preset.PresetID.Valid && m.Preset.PresetID != videoItem.Preset.PresetID {
 		// preset change, need to schedule new videoItem files
 		err = s.enc.RefreshVideo(ctx, m.ID)
 		if err != nil {

services/people/people.go

@@ -2,10 +2,10 @@ package people
 
 import (
 	"context"
-	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/jmoiron/sqlx"
+	"gopkg.in/guregu/null.v4"
 )
 
 type (
@@ -39,8 +39,6 @@ type (
 	}
 )
 
-// TODO Sort out pointers. They are currently here so when the json is being marshalled it will "omitempty"
-
 type (
 	// User represents a user object to be used when not all data is required
 	User struct {
@@ -57,14 +55,14 @@ type (
 	// UserFull represents a user and all columns
 	UserFull struct {
 		User
-		LastLogin *time.Time `db:"last_login" json:"lastLogin,omitempty"`
-		CreatedAt *time.Time `db:"created_at" json:"createdAt,omitempty"`
-		CreatedBy int        `db:"created_by" json:"createdBy,omitempty"`
-		UpdatedAt *time.Time `db:"updated_at" json:"updatedAt,omitempty"`
-		UpdatedBy *int       `db:"updated_by" json:"updatedBy,omitempty"`
-		DeletedAt *time.Time `db:"deleted_at" json:"deletedAt,omitempty"`
-		DeletedBy *int       `db:"deleted_by" json:"deletedBy,omitempty"`
-		Roles     []Role     `json:"roles,omitempty"`
+		LastLogin null.Time `db:"last_login" json:"lastLogin,omitempty"`
+		CreatedAt null.Time `db:"created_at" json:"createdAt,omitempty"`
+		CreatedBy int       `db:"created_by" json:"createdBy,omitempty"`
+		UpdatedAt null.Time `db:"updated_at" json:"updatedAt,omitempty"`
+		UpdatedBy null.Int  `db:"updated_by" json:"updatedBy,omitempty"`
+		DeletedAt null.Time `db:"deleted_at" json:"deletedAt,omitempty"`
+		DeletedBy null.Int  `db:"deleted_by" json:"deletedBy,omitempty"`
+		Roles     []Role    `json:"roles,omitempty"`
 	}
 	// Role represents a "group" of permissions where multiple users
 	// can have this role, and they will inherit these permissions.

services/people/permission.go

@@ -2,7 +2,12 @@ package people
 
 import (
 	"context"
+	//nolint:gosec
+	"crypto/md5"
+	"encoding/hex"
 	"fmt"
+	"log"
+	"strings"
 )
 
 var _ PermissionRepo = &Store{}
@@ -36,9 +41,17 @@ func (s *Store) ListPermissionMembersByID(ctx context.Context, permissionID int)
 	}
 
 	for _, user := range u {
-		if user.Avatar != "" {
-			// TODO: sort this out
-			user.Avatar = "https://ystv.co.uk/static/images/members/thumb/" + user.Avatar
+		switch avatar := user.Avatar; {
+		case user.UseGravatar:
+			//nolint:gosec
+			hash := md5.Sum([]byte(strings.ToLower(strings.TrimSpace(user.Email))))
+			user.Avatar = "https://www.gravatar.com/avatar/" + hex.EncodeToString(hash[:])
+		case avatar == "", strings.Contains(avatar, s.cdnEndpoint):
+		case strings.Contains(avatar, fmt.Sprintf("%d.", user.UserID)):
+			user.Avatar = "https://ystv.co.uk/static/images/members/thumb/" + avatar
+		default:
+			log.Printf("unknown avatar, user id: %d, length: %d, db string: %s, continuing", user.UserID, len(user.Avatar), user.Avatar)
+			user.Avatar = ""
 		}
 	}
 

services/public/breadcrumb.go

@@ -64,9 +64,8 @@ func (s *Store) GetVideoBreadcrumb(ctx context.Context, videoID int) ([]Breadcru
 func (s *Store) GetSeriesBreadcrumb(ctx context.Context, seriesID int) ([]Breadcrumb, error) {
 	var b []Breadcrumb
 
-	// TODO Need a bool to indicate if series is in URL
 	err := s.db.SelectContext(ctx, &b,
-		`SELECT parent.series_id as id, parent.url as url, COALESCE(parent.name, parent.url) as name
+		`SELECT parent.series_id as id, parent.url as url, COALESCE(parent.name, parent.url) as name, parent.in_url as use
 		FROM
 			video.series node,
 			video.series parent

services/public/series.go

@@ -29,7 +29,7 @@ var _ SeriesRepo = &Store{}
 
 // GetSeries provides the immediate children of children and videos
 func (s *Store) GetSeries(ctx context.Context, seriesID int) (Series, error) {
-	series, err := s.GetSeriesMeta(ctx, seriesID)
+	series, err := s.GetSeriesFullMeta(ctx, seriesID)
 	if err != nil {
 		return series, fmt.Errorf("failed to get series meta: %w", err)
 	}
@@ -48,9 +48,8 @@ func (s *Store) GetSeries(ctx context.Context, seriesID int) (Series, error) {
 }
 
 // GetSeriesMeta provides basic information for only the selected series
-// TODO probably want to swap this to return SeriesMeta instead
-func (s *Store) GetSeriesMeta(ctx context.Context, seriesID int) (Series, error) {
-	var series Series
+func (s *Store) GetSeriesMeta(ctx context.Context, seriesID int) (SeriesMeta, error) {
+	var series SeriesMeta
 	//nolint:musttag
 	err := s.db.GetContext(ctx, &series,
 		`SELECT series_id, url, name, description, thumbnail
@@ -61,6 +60,31 @@ func (s *Store) GetSeriesMeta(ctx context.Context, seriesID int) (Series, error)
 	return series, err
 }
 
+// GetSeriesFullMeta provides basic information for only the selected series
+func (s *Store) GetSeriesFullMeta(ctx context.Context, seriesID int) (Series, error) {
+	var series Series
+	//nolint:musttag
+	err := s.db.GetContext(ctx, &series,
+		`SELECT series_id, url, name, description, thumbnail
+		FROM video.series
+		WHERE series_id = $1
+		AND status = 'public';`, seriesID)
+	if err != nil {
+		return Series{}, fmt.Errorf("failed to get series: %w", err)
+	}
+
+	err = s.db.GetContext(ctx, &series.ChildVideos,
+		`SELECT video_id, series_id, name, url, description, thumbnail, broadcast_date, views, duration
+		FROM video.items
+		WHERE series_id = $1
+		AND status = 'public';`, seriesID)
+	if err != nil {
+		return Series{}, fmt.Errorf("failed to get child videos: %w", err)
+	}
+
+	return series, nil
+}
+
 // GetSeriesImmediateChildrenSeries returns series directly below the chosen series
 func (s *Store) GetSeriesImmediateChildrenSeries(ctx context.Context, seriesID int) ([]SeriesMeta, error) {
 	var seriesMeta []SeriesMeta