Merge pull request #40 from brokenhandsio/fixes-after-testing

0xTim · web-flow · commit f691c43e20ed · 2020-02-17T15:22:34.000Z
Fixes after testing
diff --git a/Sources/SteamPress/Controllers/Admin/UserAdminController.swift b/Sources/SteamPress/Controllers/Admin/UserAdminController.swift
@@ -32,7 +32,7 @@ struct UserAdminController: RouteCollection {
         return try validateUserCreation(data, on: req).flatMap { createUserErrors in
             if let errors = createUserErrors {
                 let presenter = try req.make(BlogAdminPresenter.self)
-                let view = try presenter.createUserView(on: req, editing: true, errors: errors.errors, name: data.name, nameError: errors.nameError, username: data.username, usernameErorr: errors.usernameError, passwordError: errors.passwordError, confirmPasswordError: errors.confirmPasswordError, resetPasswordOnLogin: data.resetPasswordOnLogin ?? false, userID: nil, profilePicture: data.profilePicture, twitterHandle: data.twitterHandle, biography: data.biography, tagline: data.tagline, pageInformation: req.adminPageInfomation())
+                let view = try presenter.createUserView(on: req, editing: false, errors: errors.errors, name: data.name, nameError: errors.nameError, username: data.username, usernameErorr: errors.usernameError, passwordError: errors.passwordError, confirmPasswordError: errors.confirmPasswordError, resetPasswordOnLogin: data.resetPasswordOnLogin ?? false, userID: nil, profilePicture: data.profilePicture, twitterHandle: data.twitterHandle, biography: data.biography, tagline: data.tagline, pageInformation: req.adminPageInfomation())
                 return try view.encode(for: req)
             }
 
@@ -42,7 +42,11 @@ struct UserAdminController: RouteCollection {
 
             let hasher = try req.make(PasswordHasher.self)
             let hashedPassword = try hasher.hash(password)
-            let newUser = BlogUser(name: name, username: username.lowercased(), password: hashedPassword, profilePicture: data.profilePicture, twitterHandle: data.twitterHandle, biography: data.biography, tagline: data.tagline)
+            let profilePicture = data.profilePicture.isEmptyOrWhitespace() ? nil : data.profilePicture
+            let twitterHandle = data.twitterHandle.isEmptyOrWhitespace() ? nil : data.twitterHandle
+            let biography = data.biography.isEmptyOrWhitespace() ? nil : data.biography
+            let tagline = data.tagline.isEmptyOrWhitespace() ? nil : data.tagline
+            let newUser = BlogUser(name: name, username: username.lowercased(), password: hashedPassword, profilePicture: profilePicture, twitterHandle: twitterHandle, biography: biography, tagline: tagline)
             if let resetPasswordRequired = data.resetPasswordOnLogin, resetPasswordRequired {
                 newUser.resetPasswordRequired = true
             }
@@ -69,25 +73,31 @@ struct UserAdminController: RouteCollection {
                 throw Abort(.internalServerError)
             }
 
-            return try self.validateUserCreation(data, editing: true, on: req).flatMap { errors in
+            return try self.validateUserCreation(data, editing: true, existingUsername: user.username, on: req).flatMap { errors in
                 if let editUserErrors = errors {
                     let presenter = try req.make(BlogAdminPresenter.self)
-                    let view = try presenter.createUserView(on: req, editing: true, errors: editUserErrors.errors, name: data.name, nameError: errors?.nameError ?? false, username: data.username, usernameErorr: errors?.usernameError ?? false, passwordError: editUserErrors.passwordError, confirmPasswordError: editUserErrors.confirmPasswordError, resetPasswordOnLogin: data.resetPasswordOnLogin ?? false, userID: nil, profilePicture: data.profilePicture, twitterHandle: data.twitterHandle, biography: data.biography, tagline: data.tagline, pageInformation: req.adminPageInfomation())
+                    let view = try presenter.createUserView(on: req, editing: true, errors: editUserErrors.errors, name: data.name, nameError: errors?.nameError ?? false, username: data.username, usernameErorr: errors?.usernameError ?? false, passwordError: editUserErrors.passwordError, confirmPasswordError: editUserErrors.confirmPasswordError, resetPasswordOnLogin: data.resetPasswordOnLogin ?? false, userID: user.userID, profilePicture: data.profilePicture, twitterHandle: data.twitterHandle, biography: data.biography, tagline: data.tagline, pageInformation: req.adminPageInfomation())
                     return try view.encode(for: req)
                 }
 
                 user.name = name
                 user.username = username.lowercased()
-                user.profilePicture = data.profilePicture
-                user.twitterHandle = data.twitterHandle
-                user.biography = data.biography
-                user.tagline = data.tagline
+                
+                let profilePicture = data.profilePicture.isEmptyOrWhitespace() ? nil : data.profilePicture
+                let twitterHandle = data.twitterHandle.isEmptyOrWhitespace() ? nil : data.twitterHandle
+                let biography = data.biography.isEmptyOrWhitespace() ? nil : data.biography
+                let tagline = data.tagline.isEmptyOrWhitespace() ? nil : data.tagline
+                
+                user.profilePicture = profilePicture
+                user.twitterHandle = twitterHandle
+                user.biography = biography
+                user.tagline = tagline
 
                 if let resetPasswordOnLogin = data.resetPasswordOnLogin, resetPasswordOnLogin {
                     user.resetPasswordRequired = true
                 }
 
-                if let password = data.password {
+                if let password = data.password, password != "" {
                     let hasher = try req.make(PasswordHasher.self)
                     user.password = try hasher.hash(password)
                 }
@@ -127,7 +137,7 @@ struct UserAdminController: RouteCollection {
     }
 
     // MARK: - Validators
-    private func validateUserCreation(_ data: CreateUserData, editing: Bool = false, on req: Request) throws -> EventLoopFuture<CreateUserErrors?> {
+    private func validateUserCreation(_ data: CreateUserData, editing: Bool = false, existingUsername: String? = nil, on req: Request) throws -> EventLoopFuture<CreateUserErrors?> {
         var createUserErrors = [String]()
         var passwordError = false
         var confirmPasswordError = false
@@ -144,7 +154,7 @@ struct UserAdminController: RouteCollection {
             usernameError = true
         }
 
-        if !editing || data.password != nil {
+        if !editing || !data.password.isEmptyOrWhitespace() {
             if data.password.isEmptyOrWhitespace() {
                 createUserErrors.append("You must specify a password")
                 passwordError = true
@@ -156,7 +166,7 @@ struct UserAdminController: RouteCollection {
             }
         }
 
-        if let password = data.password {
+        if let password = data.password, password != "" {
             if password.count < 10 {
                 createUserErrors.append("Your password must be at least 10 characters long")
                 passwordError = true
@@ -177,14 +187,17 @@ struct UserAdminController: RouteCollection {
         }
 
         var usernameUniqueError: EventLoopFuture<String?>
-
         let usersRepository = try req.make(BlogUserRepository.self)
         if let username = data.username {
-            usernameUniqueError = usersRepository.getUser(username: username.lowercased(), on: req).map { user in
-                if user != nil {
-                    return "Sorry that username has already been taken"
-                } else {
-                    return nil
+            if editing && data.username == existingUsername {
+                usernameUniqueError = req.future(nil)
+            } else {
+                usernameUniqueError = usersRepository.getUser(username: username.lowercased(), on: req).map { user in
+                    if user != nil {
+                        return "Sorry that username has already been taken"
+                    } else {
+                        return nil
+                    }
                 }
             }
         } else {
diff --git a/Tests/SteamPressTests/AdminTests/AdminUserTests.swift b/Tests/SteamPressTests/AdminTests/AdminUserTests.swift
@@ -79,6 +79,31 @@ class AdminUserTests: XCTestCase {
         XCTAssertEqual(response.http.status, .seeOther)
         XCTAssertEqual(response.http.headers[.location].first, "/admin/")
     }
+    
+    func testUserHasNoAdditionalInfoIfEmptyStringsSent() throws {
+        struct CreateUserData: Content {
+            static let defaultContentType = MediaType.urlEncodedForm
+            let name = "Luke"
+            let username = "lukes"
+            let password = "somepassword"
+            let confirmPassword = "somepassword"
+            let profilePicture = ""
+            let tagline = ""
+            let biography = ""
+            let twitterHandle = ""
+        }
+
+        let createData = CreateUserData()
+        _ = try testWorld.getResponse(to: createUserPath, body: createData, loggedInUser: user)
+
+        // First is user created in setup, final is one just created
+        XCTAssertEqual(testWorld.context.repository.users.count, 2)
+        let user = try XCTUnwrap(testWorld.context.repository.users.last)
+        XCTAssertNil(user.profilePicture)
+        XCTAssertNil(user.tagline)
+        XCTAssertNil(user.biography)
+        XCTAssertNil(user.twitterHandle)
+    }
 
     func testUserMustResetPasswordIfSetToWhenCreatingUser() throws {
         struct CreateUserResetData: Content {
@@ -264,6 +289,8 @@ class AdminUserTests: XCTestCase {
         XCTAssertTrue(viewErrors.contains("Your password must be at least 10 characters long"))
         let passwordError = try XCTUnwrap(presenter.createUserPasswordError)
         XCTAssertTrue(passwordError)
+        let isEditing = try XCTUnwrap(presenter.createUserEditing)
+        XCTAssertFalse(isEditing)
     }
 
     func testUserCannotBeCreatedWithEmptyName() throws {
@@ -385,6 +412,25 @@ class AdminUserTests: XCTestCase {
         XCTAssertEqual(response.http.status, .seeOther)
         XCTAssertEqual(response.http.headers[.location].first, "/admin/")
     }
+    
+    func testUserCanBeUpdatedWithSameUsername() throws {
+        struct EditUserData: Content {
+            static let defaultContentType = MediaType.urlEncodedForm
+            let name = "Leia Organa"
+            let username = "leia"
+        }
+
+        let editData = EditUserData()
+        let response = try testWorld.getResponse(to: "/admin/users/\(user.userID!)/edit", body: editData, loggedInUser: user)
+
+        XCTAssertEqual(testWorld.context.repository.users.count, 1)
+        let updatedUser = try XCTUnwrap(testWorld.context.repository.users.last)
+        XCTAssertEqual(updatedUser.username, editData.username)
+        XCTAssertEqual(updatedUser.name, editData.name)
+        XCTAssertEqual(updatedUser.userID, user.userID)
+        XCTAssertEqual(response.http.status, .seeOther)
+        XCTAssertEqual(response.http.headers[.location].first, "/admin/")
+    }
 
     func testUserCanBeUpdatedWithAllInformation() throws {
         struct EditUserData: Content {
@@ -412,6 +458,65 @@ class AdminUserTests: XCTestCase {
         XCTAssertEqual(response.http.status, .seeOther)
         XCTAssertEqual(response.http.headers[.location].first, "/admin/")
     }
+    
+    func testOptionalInfoDoesntGetUpdatedWhenEditingUsernameAndSendingEmptyValuesIfSomeAlreadySet() throws {
+        struct EditUserData: Content {
+            static let defaultContentType = MediaType.urlEncodedForm
+            let name = "Darth Vader"
+            let username = "darth_vader"
+            let twitterHandle = ""
+            let profilePicture = ""
+            let tagline = ""
+            let biography = ""
+        }
+        
+        user.profilePicture = nil
+        user.twitterHandle = nil
+        user.tagline = nil
+        user.biography = nil
+
+        let editData = EditUserData()
+        _ = try testWorld.getResponse(to: "/admin/users/\(user.userID!)/edit", body: editData, loggedInUser: user)
+
+        XCTAssertEqual(testWorld.context.repository.users.count, 1)
+        let updatedUser = try XCTUnwrap(testWorld.context.repository.users.last)
+        XCTAssertEqual(updatedUser.username, editData.username)
+        XCTAssertEqual(updatedUser.name, editData.name)
+        XCTAssertNil(updatedUser.twitterHandle)
+        XCTAssertNil(updatedUser.profilePicture)
+        XCTAssertNil(updatedUser.tagline)
+        XCTAssertNil(updatedUser.biography)
+        XCTAssertEqual(updatedUser.userID, user.userID)
+    }
+    
+    func testUpdatingOptionalInfoToEmptyValuesWhenValueOriginallySetSetsItToNil() throws {
+        struct EditUserData: Content {
+            static let defaultContentType = MediaType.urlEncodedForm
+            let name = "Darth Vader"
+            let username = "darth_vader"
+            let twitterHandle = ""
+            let profilePicture = ""
+            let tagline = ""
+            let biography = ""
+        }
+
+        user.profilePicture = "https://static.brokenhands.io/picture.png"
+        user.tagline = "Tagline"
+        user.biography = "Biography"
+        user.twitterHandle = "darthVader"
+        let editData = EditUserData()
+        _ = try testWorld.getResponse(to: "/admin/users/\(user.userID!)/edit", body: editData, loggedInUser: user)
+
+        XCTAssertEqual(testWorld.context.repository.users.count, 1)
+        let updatedUser = try XCTUnwrap(testWorld.context.repository.users.last)
+        XCTAssertEqual(updatedUser.username, editData.username)
+        XCTAssertEqual(updatedUser.name, editData.name)
+        XCTAssertNil(updatedUser.twitterHandle)
+        XCTAssertNil(updatedUser.profilePicture)
+        XCTAssertNil(updatedUser.tagline)
+        XCTAssertNil(updatedUser.biography)
+        XCTAssertEqual(updatedUser.userID, user.userID)
+    }
 
     func testWhenEditingUserResetPasswordFlagSetIfRequired() throws {
         struct EditUserData: Content {
@@ -470,6 +575,27 @@ class AdminUserTests: XCTestCase {
         XCTAssertEqual(response.http.status, .seeOther)
         XCTAssertEqual(response.http.headers[.location].first, "/admin/")
     }
+    
+    func testPasswordIsNotUpdatedWhenEmptyPasswordProvidedWhenEditingUser() throws {
+        struct EditUserData: Content {
+            static let defaultContentType = MediaType.urlEncodedForm
+            let name = "Luke"
+            let username = "lukes"
+            let password = ""
+            let confirmPassword = ""
+        }
+
+        let oldPassword = user.password
+        let editData = EditUserData()
+        let response = try testWorld.getResponse(to: "/admin/users/\(user.userID!)/edit", body: editData, loggedInUser: user)
+
+        XCTAssertEqual(testWorld.context.repository.users.count, 1)
+        let updatedUser = try XCTUnwrap(testWorld.context.repository.users.last)
+        XCTAssertEqual(updatedUser.password, oldPassword)
+        XCTAssertEqual(updatedUser.userID, user.userID)
+        XCTAssertEqual(response.http.status, .seeOther)
+        XCTAssertEqual(response.http.headers[.location].first, "/admin/")
+    }
 
     func testErrorShownWhenUpdatingUsersPasswordWithNonMatchingPasswords() throws {
         struct EditUserData: Content {
@@ -487,6 +613,7 @@ class AdminUserTests: XCTestCase {
         XCTAssertTrue(viewErrors.contains("Your passwords must match"))
         let passwordError = try XCTUnwrap(presenter.createUserPasswordError)
         let confirmPasswordError = try XCTUnwrap(presenter.createUserConfirmPasswordError)
+        XCTAssertEqual(presenter.createUserUserID, user.userID)
         XCTAssertTrue(passwordError)
         XCTAssertTrue(confirmPasswordError)
     }
@@ -509,30 +636,6 @@ class AdminUserTests: XCTestCase {
         XCTAssertTrue(passwordError)
     }
 
-    func testErrorShownWhenTryingToChangeUsersPasswordWithEmptyString() throws {
-        struct EditUserData: Content {
-            static let defaultContentType = MediaType.urlEncodedForm
-            let name = "Luke"
-            let username = "lukes"
-            let password = ""
-            let confirmPassword = ""
-            let resetPasswordOnLogin = true
-        }
-
-        let editData = EditUserData()
-        _ = try testWorld.getResponse(to: "/admin/users/\(user.userID!)/edit", body: editData, loggedInUser: user)
-
-        let viewErrors = try XCTUnwrap(presenter.createUserErrors)
-        XCTAssertTrue(viewErrors.contains("You must confirm your password"))
-        XCTAssertTrue(viewErrors.contains("You must specify a password"))
-        let passwordError = try XCTUnwrap(presenter.createUserPasswordError)
-        let confirmPasswordError = try XCTUnwrap(presenter.createUserConfirmPasswordError)
-        XCTAssertTrue(passwordError)
-        XCTAssertTrue(confirmPasswordError)
-        let resetPasswordOnLogin = try XCTUnwrap(presenter.createUserResetPasswordRequired)
-        XCTAssertTrue(resetPasswordOnLogin)
-    }
-
     func testPasswordIsActuallyHashedWhenEditingAUser() throws {
         testWorld = try! TestWorld.create(passwordHasherToUse: .reversed)
         let usersPassword = "password"
