diff --git a/model/user.go b/model/user.go index da4813278..cf83399cf 100644 --- a/model/user.go +++ b/model/user.go @@ -75,8 +75,4 @@ type UserRepository interface { // Library association methods GetUserLibraries(userID string) (Libraries, error) SetUserLibraries(userID string, libraryIDs []int) error - - // TODO Remove these methods in favor of SetUserLibraries - AddUserLibrary(userID string, libraryID int) error - RemoveUserLibrary(userID string, libraryID int) error } diff --git a/persistence/artist_repository_test.go b/persistence/artist_repository_test.go index 9d159222c..62db5f961 100644 --- a/persistence/artist_repository_test.go +++ b/persistence/artist_repository_test.go @@ -363,9 +363,9 @@ var _ = Describe("ArtistRepository", func() { // Try to put the user (will fail silently if already exists) _ = ur.Put(&testUser) - // Add library association - err = ur.AddUserLibrary(currentUser.ID, 1) - // Ignore error if user-library association already exists + // Add library association using SetUserLibraries + err = ur.SetUserLibraries(currentUser.ID, []int{1}) + // Ignore error if user already has these libraries or other conflicts if err != nil && !strings.Contains(err.Error(), "UNIQUE constraint failed") && !strings.Contains(err.Error(), "duplicate key") { Expect(err).ToNot(HaveOccurred()) } @@ -633,9 +633,9 @@ var _ = Describe("ArtistRepository", func() { err := ur.Put(&unauthorizedUser) Expect(err).ToNot(HaveOccurred()) - // Then add library access - err = ur.AddUserLibrary(unauthorizedUser.ID, 1) - Expect(err).ToNot(HaveOccurred()) + // Then add library access + err = ur.SetUserLibraries(unauthorizedUser.ID, []int{1}) + Expect(err).ToNot(HaveOccurred()) // Update the user object with the libraries to simulate middleware behavior libraries, err := ur.GetUserLibraries(unauthorizedUser.ID) @@ -648,11 +648,11 @@ var _ = Describe("ArtistRepository", func() { restrictedRepo = NewArtistRepository(ctx, GetDBXBuilder()) }) - AfterEach(func() { - // Clean up: remove the user's library access - ur := NewUserRepository(request.WithUser(log.NewContext(context.TODO()), adminUser), GetDBXBuilder()) - _ = ur.RemoveUserLibrary(unauthorizedUser.ID, 1) - }) + AfterEach(func() { + // Clean up: remove the user's library access + ur := NewUserRepository(request.WithUser(log.NewContext(context.TODO()), adminUser), GetDBXBuilder()) + _ = ur.SetUserLibraries(unauthorizedUser.ID, []int{}) + }) It("CountAll returns correct count after gaining access", func() { count, err := restrictedRepo.CountAll() diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index 9ac2a0907..cf771b4e6 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -143,7 +143,7 @@ var _ = BeforeSuite(func() { // Associate users with library 1 (default test library) for i := range testUsers { - err := ur.AddUserLibrary(testUsers[i].ID, 1) + err := ur.SetUserLibraries(testUsers[i].ID, []int{1}) if err != nil { panic(err) } diff --git a/persistence/user_repository.go b/persistence/user_repository.go index 078aef672..13d1022ff 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -458,24 +458,6 @@ func (r *userRepository) SetUserLibraries(userID string, libraryIDs []int) error return nil } -func (r *userRepository) AddUserLibrary(userID string, libraryID int) error { - insert := Insert("user_library"). - Columns("user_id", "library_id"). - Values(userID, libraryID). - Suffix("ON CONFLICT (user_id, library_id) DO NOTHING") - _, err := r.executeSQL(insert) - return err -} - -func (r *userRepository) RemoveUserLibrary(userID string, libraryID int) error { - del := Delete("user_library").Where(And{ - Eq{"user_id": userID}, - Eq{"library_id": libraryID}, - }) - _, err := r.executeSQL(del) - return err -} - var _ model.UserRepository = (*userRepository)(nil) var _ rest.Repository = (*userRepository)(nil) var _ rest.Persistable = (*userRepository)(nil) diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 3b453b8f2..712194d34 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -272,31 +272,6 @@ var _ = Describe("UserRepository", func() { _ = libRepo.(*libraryRepository).delete(squirrel.Eq{"id": []int{library1.ID, library2.ID}}) }) - Describe("AddUserLibrary", func() { - It("adds a library association for a user", func() { - err := repo.AddUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - - libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) - Expect(libraries).To(HaveLen(1)) - Expect(libraries[0].ID).To(Equal(library1.ID)) - }) - - It("does not create duplicate associations", func() { - err := repo.AddUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - - // Add the same association again - err = repo.AddUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - - libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) - Expect(libraries).To(HaveLen(1)) - }) - }) - Describe("GetUserLibraries", func() { It("returns empty list when user has no library associations", func() { libraries, err := repo.GetUserLibraries("non-existent-user") @@ -305,9 +280,7 @@ var _ = Describe("UserRepository", func() { }) It("returns user's associated libraries", func() { - err := repo.AddUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - err = repo.AddUserLibrary(userID, library2.ID) + err := repo.SetUserLibraries(userID, []int{library1.ID, library2.ID}) Expect(err).To(BeNil()) libraries, err := repo.GetUserLibraries(userID) @@ -359,32 +332,6 @@ var _ = Describe("UserRepository", func() { Expect(libraries).To(HaveLen(0)) }) }) - - Describe("RemoveUserLibrary", func() { - It("removes a library association", func() { - err := repo.AddUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - err = repo.AddUserLibrary(userID, library2.ID) - Expect(err).To(BeNil()) - - err = repo.RemoveUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - - libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) - Expect(libraries).To(HaveLen(1)) - Expect(libraries[0].ID).To(Equal(library2.ID)) - }) - - It("does nothing when association doesn't exist", func() { - err := repo.RemoveUserLibrary(userID, library1.ID) - Expect(err).To(BeNil()) - - libraries, err := repo.GetUserLibraries(userID) - Expect(err).To(BeNil()) - Expect(libraries).To(HaveLen(0)) - }) - }) }) Describe("Admin User Auto-Assignment", func() { diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index 89727c6bb..9f3dd672e 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -123,19 +123,3 @@ func (u *MockedUserRepo) SetUserLibraries(userID string, libraryIDs []int) error u.UserLibraries[userID] = libraryIDs return nil } - -func (u *MockedUserRepo) AddUserLibrary(userID string, libraryID int) error { - if u.Error != nil { - return u.Error - } - // Mock: do nothing for now - return nil -} - -func (u *MockedUserRepo) RemoveUserLibrary(userID string, libraryID int) error { - if u.Error != nil { - return u.Error - } - // Mock: do nothing for now - return nil -}