From 3e33481fb4dc38f4ed46c53bbd26352d1c128870 Mon Sep 17 00:00:00 2001
From: Adphi <philippe.adrien.nousse@gmail.com>
Date: Fri, 18 Jan 2019 13:42:40 +0100
Subject: [PATCH] Fix User UpdateError

---
 shares.go            | 15 +++++++++------
 types/errors.go      | 11 +++++------
 types/errors_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++
 users.go             | 12 ++++++++----
 4 files changed, 68 insertions(+), 16 deletions(-)
 create mode 100644 types/errors_test.go

diff --git a/shares.go b/shares.go
index 541fb9e..c53d889 100644
--- a/shares.go
+++ b/shares.go
@@ -95,13 +95,13 @@ func (s *Shares) Delete(shareID int) error {
 // Update update share details
 // expireDate expireDate expects a well formatted date string, e.g. ‘YYYY-MM-DD’
 func (s *Shares) Update(shareUpdate types.ShareUpdate) error {
-	errs := make(chan types.UpdateError)
+	errs := make(chan *types.UpdateError)
 	var wg sync.WaitGroup
 	wg.Add(4)
 	go func() {
 		defer wg.Done()
 		if err := s.UpdatePassword(shareUpdate.ShareID, shareUpdate.Password); err != nil {
-			errs <- types.UpdateError{
+			errs <- &types.UpdateError{
 				Field: "password",
 				Error: err,
 			}
@@ -110,7 +110,7 @@ func (s *Shares) Update(shareUpdate types.ShareUpdate) error {
 	go func() {
 		defer wg.Done()
 		if err := s.UpdateExpireDate(shareUpdate.ShareID, shareUpdate.ExpireDate); err != nil {
-			errs <- types.UpdateError{
+			errs <- &types.UpdateError{
 				Field: "expireDate",
 				Error: err,
 			}
@@ -119,7 +119,7 @@ func (s *Shares) Update(shareUpdate types.ShareUpdate) error {
 	go func() {
 		defer wg.Done()
 		if err := s.UpdatePermissions(shareUpdate.ShareID, shareUpdate.Permissions); err != nil {
-			errs <- types.UpdateError{
+			errs <- &types.UpdateError{
 				Field: "permissions",
 				Error: err,
 			}
@@ -128,7 +128,7 @@ func (s *Shares) Update(shareUpdate types.ShareUpdate) error {
 	go func() {
 		defer wg.Done()
 		if err := s.UpdatePublicUpload(shareUpdate.ShareID, shareUpdate.PublicUpload); err != nil {
-			errs <- types.UpdateError{
+			errs <- &types.UpdateError{
 				Field: "publicUpload",
 				Error: err,
 			}
@@ -138,7 +138,10 @@ func (s *Shares) Update(shareUpdate types.ShareUpdate) error {
 		wg.Wait()
 		close(errs)
 	}()
-	return types.NewUpdateError(errs)
+	if err := types.NewUpdateError(errs); err != nil {
+		return err
+	}
+	return nil
 }
 
 //UpdateExpireDate updates the share's expire date
diff --git a/types/errors.go b/types/errors.go
index 044e0f6..5f73d57 100644
--- a/types/errors.go
+++ b/types/errors.go
@@ -40,17 +40,16 @@ func (e *UserUpdateError) Error() string {
 	for k, e := range e.Errors {
 		errors = append(errors, fmt.Sprintf("%s: %v", k, e))
 	}
-	return strings.Join(errors, ",")
+	return strings.Join(errors, ", ")
 }
 
 //NewUpdateError returns an UpdateError based on an UpdateError channel
-func NewUpdateError(errors chan UpdateError) *UserUpdateError {
-	var ue UserUpdateError
+func NewUpdateError(errors chan *UpdateError) *UserUpdateError {
+	ue := UserUpdateError{map[string]error{}}
 	for e := range errors {
-		if ue.Errors == nil {
-			ue.Errors = map[string]error{e.Field: e.Error}
+		if e != nil {
+			ue.Errors[e.Field] = e.Error
 		}
-		ue.Errors[e.Field] = e.Error
 	}
 	if len(ue.Errors) > 0 {
 		return &ue
diff --git a/types/errors_test.go b/types/errors_test.go
new file mode 100644
index 0000000..08bfa0c
--- /dev/null
+++ b/types/errors_test.go
@@ -0,0 +1,46 @@
+package types
+
+import (
+	"errors"
+	"github.com/stretchr/testify/assert"
+	"strconv"
+	"sync"
+	"testing"
+)
+
+func TestUserUpdateErrors(t *testing.T) {
+	exp := map[string]error{}
+	errs := make(chan *UpdateError)
+	go func() {
+		for i := 0; i < 10; i++ {
+			f := strconv.Itoa(i)
+			e := errors.New(f)
+			err := UpdateError{
+				Field: f,
+				Error: e,
+			}
+			exp[f] = e
+			errs <- &err
+		}
+		close(errs)
+	}()
+	uerrs := NewUpdateError(errs)
+	assert.Equal(t, exp, uerrs.Errors)
+	assert.NotEmpty(t, uerrs.Error())
+}
+
+func TestUserUpdateErrorsNil(t *testing.T) {
+	var wg sync.WaitGroup
+	errs := make(chan *UpdateError)
+	wg.Add(1)
+	go func() {
+		errs <- nil
+		wg.Done()
+	}()
+	go func() {
+		wg.Wait()
+		close(errs)
+	}()
+	uerrs := NewUpdateError(errs)
+	assert.Nil(t, uerrs)
+}
diff --git a/users.go b/users.go
index 80c76a7..cef1e38 100644
--- a/users.go
+++ b/users.go
@@ -191,7 +191,7 @@ func (u *Users) SendWelcomeEmail(name string) error {
 //Update takes a *types.Users struct to update the user's information
 func (u *Users) Update(user *types.UserDetails) error {
 	m := structs.Map(user)
-	errs := make(chan types.UpdateError)
+	errs := make(chan *types.UpdateError)
 	var wg sync.WaitGroup
 	for k := range m {
 		// Filter updatable fields
@@ -216,7 +216,7 @@ func (u *Users) Update(user *types.UserDetails) error {
 			go func(key string, value string) {
 				defer wg.Done()
 				if err := u.updateAttribute(user.ID, strings.ToLower(key), value); err != nil {
-					errs <- types.UpdateError{
+					errs <- &types.UpdateError{
 						Field: key,
 						Error: err,
 					}
@@ -228,7 +228,11 @@ func (u *Users) Update(user *types.UserDetails) error {
 		wg.Wait()
 		close(errs)
 	}()
-	return types.NewUpdateError(errs)
+	// Warning : we actually need to check the *err
+	if err := types.NewUpdateError(errs); err != nil {
+		return err
+	}
+	return nil
 }
 
 //UpdateEmail update the user's email
@@ -355,7 +359,7 @@ func (u *Users) baseRequest(method string, ro *req.RequestOptions, subRoutes ...
 }
 
 func ignoredUserField(key string) bool {
-	keys := []string{"Email", "DisplayName", "Phone", "Address", "Website", "Twitter", "Quota"}
+	keys := []string{"Email", "Displayname", "Phone", "Address", "Website", "Twitter", "Quota"}
 	for _, k := range keys {
 		if key == k {
 			return false