summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-05-20 07:06:04 +0300
committerPaul Buetow <paul@buetow.org>2026-05-20 07:06:04 +0300
commit190bc9ecfb2e6026d0ff831b89dcc2a39843775d (patch)
tree79254f6005be3e2d84cb8857084a74682c8d8531
parent9d41de31153286103f73f6188049413a8c0d45ed (diff)
Promote allowedSetIDs to accessHelper (da)
browseService.ListMedia and ListSets each contained the same "compute permitted set IDs for this non-admin user" logic. Promote it to an exported method (h.AllowedSetIDs) on accessHelper so the permission resolution lives in one place, then call it from both sites in browse.go. Tests still pass via the existing service suite — no new unit test was needed since accessHelper is exercised through its callers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
-rw-r--r--player-server/internal/service/access.go35
-rw-r--r--player-server/internal/service/browse.go26
2 files changed, 44 insertions, 17 deletions
diff --git a/player-server/internal/service/access.go b/player-server/internal/service/access.go
index e8546d6..dd85790 100644
--- a/player-server/internal/service/access.go
+++ b/player-server/internal/service/access.go
@@ -115,3 +115,38 @@ func (h *accessHelper) verifyRestoreAccess(ctx context.Context, mediaID, userID
func (h *accessHelper) verifySetModifyAccess(ctx context.Context, setID, userID int64) error {
return h.checkSetPermission(ctx, setID, userID, model.RoleOwner)
}
+
+// AllowedSetIDs returns the set IDs the given user can read.
+//
+// For admin users it returns (nil, true, nil) — a nil slice with the isAdmin
+// flag set, meaning "no restriction, all sets are visible". Callers should
+// translate this into a repository filter that does not constrain set IDs.
+//
+// For non-admin users it returns the (possibly empty) list of set IDs the
+// user has any permission on, with isAdmin==false. An empty non-nil slice
+// means the user has no readable sets and the caller should short-circuit
+// rather than querying the repository.
+//
+// This method exists so browse.ListMedia and progress.ListInProgress can
+// share the same "what can this user see?" logic instead of reimplementing
+// the admin/permission lookup separately.
+func (h *accessHelper) AllowedSetIDs(ctx context.Context, userID int64) (ids []int64, isAdmin bool, err error) {
+ user, err := h.store.GetUserByID(ctx, userID)
+ if err != nil {
+ return nil, false, fmt.Errorf("get user: %w", err)
+ }
+ if user != nil && user.IsAdmin {
+ return nil, true, nil
+ }
+
+ perms, err := h.store.ListPermissionsByUser(ctx, userID)
+ if err != nil {
+ return nil, false, fmt.Errorf("list permissions: %w", err)
+ }
+
+ allowed := make([]int64, 0, len(perms))
+ for _, p := range perms {
+ allowed = append(allowed, p.SetID)
+ }
+ return allowed, false, nil
+}
diff --git a/player-server/internal/service/browse.go b/player-server/internal/service/browse.go
index be0ee2c..5786017 100644
--- a/player-server/internal/service/browse.go
+++ b/player-server/internal/service/browse.go
@@ -184,11 +184,6 @@ func (s *browseService) GetMediaDetail(ctx context.Context, mediaID, userID int6
}
func (s *browseService) ListMedia(ctx context.Context, userID int64, filter MediaQueryFilter) ([]model.Media, error) {
- user, err := s.store.GetUserByID(ctx, userID)
- if err != nil {
- return nil, fmt.Errorf("get user: %w", err)
- }
-
repoFilter := repository.MediaFilter{
SetID: filter.SetID,
SetIDs: filter.SetIDs,
@@ -203,27 +198,24 @@ func (s *browseService) ListMedia(ctx context.Context, userID int64, filter Medi
Sort: filter.Sort,
Limit: filter.Limit,
Offset: filter.Offset,
+ UserID: userID,
}
- if user != nil && user.IsAdmin {
- repoFilter.UserID = userID
- return s.store.ListMedia(ctx, repoFilter)
- }
-
- perms, err := s.store.ListPermissionsByUser(ctx, userID)
+ // Delegate "what sets can this user see?" to the shared accessHelper so
+ // browse and progress agree on the admin/permission semantics. Admins
+ // get isAdmin=true and an unrestricted query; non-admins get a concrete
+ // (possibly empty) allow-list that we use to short-circuit empty cases.
+ allowed, isAdmin, err := s.helper.AllowedSetIDs(ctx, userID)
if err != nil {
- return nil, fmt.Errorf("list permissions: %w", err)
+ return nil, err
}
-
- allowed := make([]int64, 0, len(perms))
- for _, p := range perms {
- allowed = append(allowed, p.SetID)
+ if isAdmin {
+ return s.store.ListMedia(ctx, repoFilter)
}
if len(allowed) == 0 {
return []model.Media{}, nil
}
repoFilter.AllowedSetIDs = allowed
- repoFilter.UserID = userID
return s.store.ListMedia(ctx, repoFilter)
}