From bf0594c1e8c52bb18affb02a1dfd69c344aa61e2 Mon Sep 17 00:00:00 2001 From: IRHM Date: Sat, 18 Jul 2026 10:34:56 +0100 Subject: [PATCH] Fix `Include Previously Watched` regression and update profile stats to not use the setting Index also added to Fixes https://github.com/sbondCo/Watcharr/issues/1027 --- CHANGELOG.md | 9 +++ server/database/entity/activity.go | 2 +- .../auth/authmiddleware/auth_middleware.go | 4 + server/feature/profile/profile.go | 80 +++++++------------ server/feature/user/user.go | 2 +- server/feature/watched/watched.go | 21 ++++- server/feature/watched/watched_refine.go | 37 ++++++++- server/watcharr.go | 7 +- src/routes/(app)/profile/+page.svelte | 4 +- 9 files changed, 102 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f3a818b..d6ee0eee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,15 @@ These changes are awaiting release: +## Changed + +- Profile: Stats no longer care about the `Include Previously Watched` setting. All previously watched items will be counted in the stats now. +- Activity: Added `index` to `WatchedID` column to speed up queries. + +## Fixed + +- `Include Previously Watched` regression (fixes https://github.com/sbondCo/Watcharr/issues/1027). + # [4.0.1] - 2026-07-16T10:12:00Z ## Changed diff --git a/server/database/entity/activity.go b/server/database/entity/activity.go index 33f76c64..8f52333e 100644 --- a/server/database/entity/activity.go +++ b/server/database/entity/activity.go @@ -46,7 +46,7 @@ type Activity struct { // secured (users can only view their own activities). UserID uint `json:"-" gorm:"not null"` // ID of watched list item this activity is linked to. - WatchedID uint `json:"watchedId" gorm:"not null"` + WatchedID uint `json:"watchedId" gorm:"not null;index"` // Type of activity. Type ActivityType `json:"type" gorm:"not null"` // Holds custom data (ex, if rating changed, this can diff --git a/server/feature/auth/authmiddleware/auth_middleware.go b/server/feature/auth/authmiddleware/auth_middleware.go index 88427e6f..fdcd7ae6 100644 --- a/server/feature/auth/authmiddleware/auth_middleware.go +++ b/server/feature/auth/authmiddleware/auth_middleware.go @@ -14,6 +14,10 @@ import ( // Auth middleware // If db is passed, extra user info from the database will be fetched. +// +// **NOTE:** Instead of providing the `db` parameter, it is probably better to +// fetch what you need in the handler directly! We might follow that pattern +// from now on and potentially remove `db` from this func in the future. func AuthRequired(db *gorm.DB, cfg *config.ServerConfig) gin.HandlerFunc { return func(c *gin.Context) { slog.Debug("AuthRequired middleware hit") diff --git a/server/feature/profile/profile.go b/server/feature/profile/profile.go index f826bc0d..0ba76c82 100644 --- a/server/feature/profile/profile.go +++ b/server/feature/profile/profile.go @@ -1,7 +1,6 @@ package profile import ( - "encoding/json" "errors" "log/slog" "time" @@ -28,63 +27,38 @@ func NewService(db *gorm.DB) *Service { } } -// Check if content has been previsouly watched by looking for related activity. +// Checks if item has been previously watched by scanning for any activity +// that counts as a play. func (s *Service) hasBeenPreviouslyWatched(a *[]entity.Activity) bool { - wp := false - var relatedActivity []entity.Activity for _, v := range *a { - if v.Type == entity.ADDED_WATCHED || - v.Type == entity.IMPORTED_ADDED_WATCHED || - v.Type == entity.IMPORTED_WATCHED || - v.Type == entity.STATUS_CHANGED { - relatedActivity = append(relatedActivity, v) + if v.CountAsPlay { + return true } } - if len(relatedActivity) <= 0 { - return false - } - for _, ra := range relatedActivity { - if ra.Type == entity.IMPORTED_ADDED_WATCHED { - wp = true - break - } else if ra.Type == entity.ADDED_WATCHED || ra.Type == entity.IMPORTED_WATCHED { - if ra.Data == "" { - continue - } - var v map[string]any - err := json.Unmarshal([]byte(ra.Data), &v) - if err != nil { - slog.Error("Checking ADDED_WATCHED or IMPORTED_WATCHED.. failed to parse json data", "error", err) - continue - } - if status, ok := v["status"]; ok { - if status == "FINISHED" { - wp = true - break - } - } - } else if ra.Type == entity.STATUS_CHANGED { - if ra.Data == "FINISHED" { - wp = true - break - } - } - } - return wp + return false } // Gets any data required for profile page func (s *Service) getProfile(userId uint) (Profile, error) { + // Get user. user := new(entity.User) res := s.db.Model(&entity.User{}).Where("id = ?", userId).Take(&user) if res.Error != nil { - slog.Error("Failed to get profile:", "error", res.Error.Error()) + slog.Error("Failed to get profile:", + "error", res.Error) return Profile{}, errors.New("failed to get profile") } + + // Process stats. watched := new([]entity.Watched) - res = s.db.Model(&entity.Watched{}).Preload("Content").Preload("Activity").Where("user_id = ?", userId).Find(&watched) + res = s.db.Model(&entity.Watched{}). + Preload("Content"). + Preload("Activity"). + Where("user_id = ?", userId). + Find(&watched) if res.Error != nil { - slog.Error("Profile: Failed to get watched for processing:", "error", res.Error.Error()) + slog.Error("Profile: Failed to get watched for processing:", + "error", res.Error) return Profile{}, errors.New("failed to get watched for processing") } var ( @@ -95,11 +69,12 @@ func (s *Service) getProfile(userId uint) (Profile, error) { ) for _, w := range *watched { isFinished := false - if w.Status == entity.FINISHED { - isFinished = true - } else if *user.IncludePreviouslyWatched && s.hasBeenPreviouslyWatched(&w.Activity) { - // If status is not finished and user has IncludePreviouslyWatched enabled, - // then we can also check if content hasBeenPreviouslyWatched. + // Note: Deliberately always checking `hasBeenPreviouslyWatched` for any + // items without status set to FINISHED without checking users + // `IncludePreviouslyWatched` setting, because that setting is useful + // for filters, BUT not for these stats. I think it is always expected + // that all previously watched stuff is included in finished stats. + if w.Status == entity.FINISHED || s.hasBeenPreviouslyWatched(&w.Activity) { isFinished = true } if isFinished { @@ -107,7 +82,8 @@ func (s *Service) getProfile(userId uint) (Profile, error) { continue } c := *w.Content - if c.Type == entity.SHOW { + switch c.Type { + case entity.SHOW: showsWatched++ // This aint a science, just a very inaccurate guesstimate. if c.NumberOfEpisodes != 0 { @@ -116,9 +92,11 @@ func (s *Service) getProfile(userId uint) (Profile, error) { showRuntime = c.Runtime } showsWatchedRuntime += showRuntime * c.NumberOfEpisodes - slog.Debug("calcualted", "show", c.Title, "runti", showRuntime*c.NumberOfEpisodes) + slog.Debug("profile stat calculated", + "show", c.Title, + "runti", showRuntime*c.NumberOfEpisodes) } - } else if c.Type == entity.MOVIE { + case entity.MOVIE: moviesWatched++ moviesWatchedRuntime += c.Runtime } diff --git a/server/feature/user/user.go b/server/feature/user/user.go index 4b6da1ed..6416efab 100644 --- a/server/feature/user/user.go +++ b/server/feature/user/user.go @@ -65,7 +65,7 @@ func (s *Service) UserUpdate(userId uint, ur entity.UserSettings) (entity.UserSe } func (s *Service) UserGetSettings(userId uint) (entity.UserSettings, error) { - slog.Debug("user update request running", "user_id", userId) + slog.Debug("UserGetSettings: Request running.", "user_id", userId) user := new(entity.User) res := s.db.Where("id = ?", userId).Take(&user) if res.Error != nil { diff --git a/server/feature/watched/watched.go b/server/feature/watched/watched.go index 5c8bd1a4..82baa4d7 100644 --- a/server/feature/watched/watched.go +++ b/server/feature/watched/watched.go @@ -22,11 +22,16 @@ type GameProvider interface { GetOrCache(igdbID int) (entity.Game, error) } +type UserProvider interface { + UserGetSettings(userId uint) (entity.UserSettings, error) +} + type Service struct { db *gorm.DB cp ContentProvider gameProvider GameProvider activityProvider domain.ActivityAddProvider + userProvider UserProvider } func NewService( @@ -34,12 +39,14 @@ func NewService( cp ContentProvider, gameProvider GameProvider, activityProvider domain.ActivityAddProvider, + userProvider UserProvider, ) *Service { return &Service{ db, cp, gameProvider, activityProvider, + userProvider, } } @@ -74,8 +81,16 @@ func (s *Service) GetWatchedPage( "user_id", userId, "pagination_params", pp, "wr", wr) - watched := new([]entity.Watched) + pRes := &util.PaginationResponse[entity.Watched, util.None]{} + + // Get user settings. + userSettings, err := s.userProvider.UserGetSettings(userId) + if err != nil { + return *pRes, errors.New("failed to get user settings") + } + + watched := new([]entity.Watched) res := s.db. Model(&entity.Watched{}). Where(&entity.Watched{UserID: userId}) @@ -101,7 +116,7 @@ func (s *Service) GetWatchedPage( Preload("WatchedSeasons"). Preload("WatchedEpisodes"). // Apply filters first. - Scopes(watchedRefineFilter(wr)). + Scopes(watchedRefineFilter(wr, &userSettings)). // Then count results (after filter); Count(&pRes.TotalResults). // Now calculate pagination properties with a TotalResults @@ -164,7 +179,7 @@ func (s *Service) getPublicWatched( Preload("WatchedSeasons"). Preload("WatchedEpisodes"). // Apply filters first. - Scopes(watchedRefineFilter(wr)). + Scopes(watchedRefineFilter(wr, nil)). // Then count results (after filter); Count(&pRes.TotalResults). // Now calculate pagination properties with a TotalResults diff --git a/server/feature/watched/watched_refine.go b/server/feature/watched/watched_refine.go index 20d16904..22970e11 100644 --- a/server/feature/watched/watched_refine.go +++ b/server/feature/watched/watched_refine.go @@ -38,15 +38,41 @@ func refineFilterType(db *gorm.DB, ft []util.SupportedMedia) { } // Applies 'Status' filter. -func refineFilterStatus(db *gorm.DB, f []entity.WatchedStatus) { +func refineFilterStatus( + db *gorm.DB, + f []entity.WatchedStatus, + userSettings *entity.UserSettings, +) { if len(f) <= 0 { return } + // Process the input data. + fIncludesFinished := false for i := range f { // Ensure string **case** is valid WatchedStatus by converting to uppercase. f[i] = entity.WatchedStatus(strings.ToUpper(string(f[i]))) + if f[i] == entity.FINISHED { + fIncludesFinished = true + slog.Debug("refineFilterStatus: f includes FINISHED") + } + } + // Apply the query. + if fIncludesFinished && + userSettings != nil && util.Deref(userSettings.IncludePreviouslyWatched, false) { + slog.Debug("refineFilterStatus: Performing query that includes previously watched.") + db. + // The WHERE here is wrapped in parenthesis so that the `OR` doesn't + // intefere with the main query confusing its AND/ORs. + Where(`(watcheds.status IN ? OR EXISTS ( + SELECT 1 + FROM activities + WHERE activities.watched_id = watcheds.id + AND activities.count_as_play = 1 + ))`, f) + } else { + slog.Debug("refineFilterStatus: Performing standard query.") + db.Where(`watcheds.status IN ?`, f) } - db.Where("watcheds.status IN ?", f) } // Applies sorts to list. @@ -125,11 +151,14 @@ func refineSortPinned(db *gorm.DB) { // list data. // gorm scope for applying filters to watched -func watchedRefineFilter(wr domain.WatchedGetPageRequest) func(db *gorm.DB) *gorm.DB { +func watchedRefineFilter( + wr domain.WatchedGetPageRequest, + userSettings *entity.UserSettings, +) func(db *gorm.DB) *gorm.DB { return func(db *gorm.DB) *gorm.DB { // Apply filters refineFilterType(db, wr.FilterType) - refineFilterStatus(db, wr.FilterStatus) + refineFilterStatus(db, wr.FilterStatus, userSettings) return db } } diff --git a/server/watcharr.go b/server/watcharr.go index ee69b8a7..dc9754b5 100644 --- a/server/watcharr.go +++ b/server/watcharr.go @@ -206,7 +206,12 @@ func main() { userService := user.NewService(db) userManageService := user.NewManageService(db) gameService := game.NewService(db, &br.Cfg.TWITCH, activityService) - watchedService := watched.NewService(db, contentService, gameService, activityService) + watchedService := watched.NewService( + db, + contentService, + gameService, + activityService, + userService) watchedSeasonService := season.NewService(db, activityService) watchedEpisodeService := episode.NewService( db, diff --git a/src/routes/(app)/profile/+page.svelte b/src/routes/(app)/profile/+page.svelte index 6541eed0..cb2b9b7a 100644 --- a/src/routes/(app)/profile/+page.svelte +++ b/src/routes/(app)/profile/+page.svelte @@ -297,7 +297,7 @@ { includePreviouslyWatchedDisabled = false; - // Get profile stats again - getProfilePromise = getProfile(); }); }} />