diff --git a/persistence/user_repository.go b/persistence/user_repository.go
index b2baabe94..3a7f13377 100644
--- a/persistence/user_repository.go
+++ b/persistence/user_repository.go
@@ -134,6 +134,9 @@ func (r *userRepository) Save(entity interface{}) (string, error) {
return "", rest.ErrPermissionDenied
}
u := entity.(*model.User)
+ if err := validateUsernameUnique(r, u); err != nil {
+ return "", err
+ }
err := r.Put(u)
if err != nil {
return "", err
@@ -157,6 +160,9 @@ func (r *userRepository) Update(entity interface{}, cols ...string) error {
if err := validatePasswordChange(u, usr); err != nil {
return err
}
+ if err := validateUsernameUnique(r, u); err != nil {
+ return err
+ }
err := r.Put(u)
if err == model.ErrNotFound {
return rest.ErrNotFound
@@ -186,6 +192,20 @@ func validatePasswordChange(newUser *model.User, logged *model.User) error {
return nil
}
+func validateUsernameUnique(r model.UserRepository, u *model.User) error {
+ usr, err := r.FindByUsername(u.UserName)
+ if err == model.ErrNotFound {
+ return nil
+ }
+ if err != nil {
+ return err
+ }
+ if usr.ID != u.ID {
+ return &rest.ValidationError{Errors: map[string]string{"userName": "ra.validation.unique"}}
+ }
+ return nil
+}
+
func (r *userRepository) Delete(id string) error {
usr := loggedUser(r.ctx)
if !usr.IsAdmin {
diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go
index 3fa916f9e..ea7a37437 100644
--- a/persistence/user_repository_test.go
+++ b/persistence/user_repository_test.go
@@ -2,11 +2,13 @@ package persistence
import (
"context"
+ "errors"
"github.com/astaxie/beego/orm"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
+ "github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)
@@ -144,4 +146,32 @@ var _ = Describe("UserRepository", func() {
})
})
})
+ Describe("validateUsernameUnique", func() {
+ var repo *tests.MockedUserRepo
+ var existingUser *model.User
+ BeforeEach(func() {
+ existingUser = &model.User{ID: "1", UserName: "johndoe"}
+ repo = tests.CreateMockUserRepo()
+ err := repo.Put(existingUser)
+ Expect(err).ToNot(HaveOccurred())
+ })
+ It("allows unique usernames", func() {
+ var newUser = &model.User{ID: "2", UserName: "unique_username"}
+ err := validateUsernameUnique(repo, newUser)
+ Expect(err).ToNot(HaveOccurred())
+ })
+ It("returns ValidationError if username already exists", func() {
+ var newUser = &model.User{ID: "2", UserName: "johndoe"}
+ err := validateUsernameUnique(repo, newUser)
+ Expect(err).To(BeAssignableToTypeOf(&rest.ValidationError{}))
+ Expect(err.(*rest.ValidationError).Errors).To(HaveKeyWithValue("userName", "ra.validation.unique"))
+ })
+ It("returns generic error if repository call fails", func() {
+ repo.Err = errors.New("fake error")
+
+ var newUser = &model.User{ID: "2", UserName: "newuser"}
+ err := validateUsernameUnique(repo, newUser)
+ Expect(err).To(MatchError("fake error"))
+ })
+ })
})
diff --git a/tests/mock_persistence.go b/tests/mock_persistence.go
index b7bf67e36..d877ef4c5 100644
--- a/tests/mock_persistence.go
+++ b/tests/mock_persistence.go
@@ -66,7 +66,7 @@ func (db *MockDataStore) Property(context.Context) model.PropertyRepository {
func (db *MockDataStore) User(context.Context) model.UserRepository {
if db.MockedUser == nil {
- db.MockedUser = &mockedUserRepo{}
+ db.MockedUser = CreateMockUserRepo()
}
return db.MockedUser
}
diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go
index a50968e97..11e4d427b 100644
--- a/tests/mock_user_repo.go
+++ b/tests/mock_user_repo.go
@@ -7,35 +7,48 @@ import (
"github.com/navidrome/navidrome/model"
)
-type mockedUserRepo struct {
+func CreateMockUserRepo() *MockedUserRepo {
+ return &MockedUserRepo{
+ Data: map[string]*model.User{},
+ }
+}
+
+type MockedUserRepo struct {
model.UserRepository
- data map[string]*model.User
+ Err error
+ Data map[string]*model.User
}
-func (u *mockedUserRepo) CountAll(qo ...model.QueryOptions) (int64, error) {
- return int64(len(u.data)), nil
+func (u *MockedUserRepo) CountAll(qo ...model.QueryOptions) (int64, error) {
+ if u.Err != nil {
+ return 0, u.Err
+ }
+ return int64(len(u.Data)), nil
}
-func (u *mockedUserRepo) Put(usr *model.User) error {
- if u.data == nil {
- u.data = make(map[string]*model.User)
+func (u *MockedUserRepo) Put(usr *model.User) error {
+ if u.Err != nil {
+ return u.Err
}
if usr.ID == "" {
usr.ID = base64.StdEncoding.EncodeToString([]byte(usr.UserName))
}
usr.Password = usr.NewPassword
- u.data[strings.ToLower(usr.UserName)] = usr
+ u.Data[strings.ToLower(usr.UserName)] = usr
return nil
}
-func (u *mockedUserRepo) FindByUsername(username string) (*model.User, error) {
- usr, ok := u.data[strings.ToLower(username)]
+func (u *MockedUserRepo) FindByUsername(username string) (*model.User, error) {
+ if u.Err != nil {
+ return nil, u.Err
+ }
+ usr, ok := u.Data[strings.ToLower(username)]
if !ok {
return nil, model.ErrNotFound
}
return usr, nil
}
-func (u *mockedUserRepo) UpdateLastLoginAt(id string) error {
- return nil
+func (u *MockedUserRepo) UpdateLastLoginAt(id string) error {
+ return u.Err
}
diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json
index c6904499b..2ec45ce3e 100644
--- a/ui/src/i18n/en.json
+++ b/ui/src/i18n/en.json
@@ -94,6 +94,11 @@
},
"helperTexts": {
"name": "Changes to your name will only be reflected on next login"
+ },
+ "notifications": {
+ "created": "User created",
+ "updated": "User updated",
+ "deleted": "User deleted"
}
},
"player": {
@@ -167,7 +172,8 @@
"number": "Must be a number",
"email": "Must be a valid email",
"oneOf": "Must be one of: %{options}",
- "regex": "Must match a specific format (regexp): %{pattern}"
+ "regex": "Must match a specific format (regexp): %{pattern}",
+ "unique": "Must be unique"
},
"action": {
"add_filter": "Add filter",
diff --git a/ui/src/user/DeleteUserButton.js b/ui/src/user/DeleteUserButton.js
index 44378569d..11b8e7473 100644
--- a/ui/src/user/DeleteUserButton.js
+++ b/ui/src/user/DeleteUserButton.js
@@ -3,7 +3,13 @@ import DeleteIcon from '@material-ui/icons/Delete'
import { makeStyles } from '@material-ui/core/styles'
import { fade } from '@material-ui/core/styles/colorManipulator'
import clsx from 'clsx'
-import { useDeleteWithConfirmController, Button, Confirm } from 'react-admin'
+import {
+ useDeleteWithConfirmController,
+ Button,
+ Confirm,
+ useNotify,
+ useRedirect,
+} from 'react-admin'
const useStyles = makeStyles(
(theme) => ({
@@ -22,22 +28,23 @@ const useStyles = makeStyles(
)
const DeleteUserButton = (props) => {
- const {
- resource,
- record,
- basePath,
- redirect = 'list',
- className,
- onClick,
- ...rest
- } = props
+ const { resource, record, basePath, className, onClick, ...rest } = props
+
+ const notify = useNotify()
+ const redirect = useRedirect()
+
+ const onSuccess = () => {
+ notify('resources.user.notifications.deleted')
+ redirect('/user')
+ }
+
const { open, loading, handleDialogOpen, handleDialogClose, handleDelete } =
useDeleteWithConfirmController({
resource,
record,
- redirect,
basePath,
onClick,
+ onSuccess,
})
const classes = useStyles(props)
diff --git a/ui/src/user/UserCreate.js b/ui/src/user/UserCreate.js
index 9966930d8..8a7c32124 100644
--- a/ui/src/user/UserCreate.js
+++ b/ui/src/user/UserCreate.js
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useCallback } from 'react'
import {
BooleanInput,
Create,
@@ -8,18 +8,49 @@ import {
email,
SimpleForm,
useTranslate,
+ useMutation,
+ useNotify,
+ useRedirect,
} from 'react-admin'
import { Title } from '../common'
const UserCreate = (props) => {
const translate = useTranslate()
+ const [mutate] = useMutation()
+ const notify = useNotify()
+ const redirect = useRedirect()
const resourceName = translate('resources.user.name', { smart_count: 1 })
const title = translate('ra.page.create', {
name: `${resourceName}`,
})
+
+ const save = useCallback(
+ async (values) => {
+ try {
+ await mutate(
+ {
+ type: 'create',
+ resource: 'user',
+ payload: { data: values },
+ },
+ { returnPromise: true }
+ )
+ notify('resources.user.notifications.created', 'info', {
+ smart_count: 1,
+ })
+ redirect('/user')
+ } catch (error) {
+ if (error.body.errors) {
+ return error.body.errors
+ }
+ }
+ },
+ [mutate, notify, redirect]
+ )
+
return (
} {...props}>
-
+
diff --git a/ui/src/user/UserEdit.js b/ui/src/user/UserEdit.js
index fa19abfb3..528ed365b 100644
--- a/ui/src/user/UserEdit.js
+++ b/ui/src/user/UserEdit.js
@@ -87,7 +87,9 @@ const UserEdit = (props) => {
},
{ returnPromise: true }
)
- notify('ra.notification.updated', 'info', { smart_count: 1 })
+ notify('resources.user.notifications.updated', 'info', {
+ smart_count: 1,
+ })
permissions === 'admin' ? redirect('/user') : refresh()
} catch (error) {
if (error.body.errors) {