Skip to content
This repository has been archived by the owner on Apr 27, 2023. It is now read-only.

Added delete a user service ,Added remove icon and refresh the user list #13

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class SQLUserDAO(val database: SQLDatabase) extends UserDAO with SQLUserSchema {
}

def findById(userId: ObjectId) = findOneWhere(_.id === userId)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this empty line

def findByEmail(email: String) = findOneWhere(_.emailLowerCase === email.toLowerCase)

def findByLowerCasedLogin(login: String) = db.withTransaction { implicit session =>
Expand Down Expand Up @@ -124,7 +124,7 @@ class SQLUserDAO(val database: SQLDatabase) extends UserDAO with SQLUserSchema {
} yield (u, a, s, l)
userQuery.firstOption.map(queryUserAliases).map(untuple)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this empty line

private def queryUserAliases(tuple: (UserTuple, SQLAuth, SQLSettings, SQLLastNotif))(implicit session: Session) = {
val userId = tuple._1._1
val aliases = userAliases.where(_.userId === userId).list()
Expand All @@ -135,4 +135,11 @@ class SQLUserDAO(val database: SQLDatabase) extends UserDAO with SQLUserSchema {
val aliases = userAliases.where(_.userId === tuple._1).list()
(tuple._1, tuple._2, tuple._3, tuple._4, UserAliases(aliases.map(_.toUserAlias)))
}
}

def delete(userId: ObjectId) {
db.withTransaction { implicit session =>
users.filter(_.id === userId).delete
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,7 @@ trait UserDAO {
def countAll(): Long

def countAllActive(): Long

def delete(userId: ObjectId)

}
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,38 @@ class SQLUserDAOSpec extends FlatSpecWithSQL with ClearSQLDataAfterTest with Bef
foundUser.get.notifications.commits.get.getMillis should equal(commitDate.getMillis)
foundUser.get.notifications.followups.get.getMillis should equal(followupDate.getMillis)
}
it should "deleta a user " taggedAs RequiresDb in {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo -> 'delete'

// given
val user = UserAssembler.randomUser.withBasicAuth("user", "pass").withAdmin(set = false).withActive(set = false).get
userDAO.add(user)
val newAuth = Authentication.basic(user.authentication.username, "newpass")

// when
val modifiedUser = user.copy(authentication = newAuth, admin = true, active = true)
userDAO.delete(modifiedUser.id)
val deletedUser = userDAO.findById(user.id)

// then
assert(deletedUser === None , "Deletion was attempted but found " + deletedUser)
}
it should "deleta a user and should not show in the list " taggedAs RequiresDb in {
// given
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo -> 'delete'

val user = UserAssembler.randomUser.withBasicAuth("user", "pass").withAdmin(set = false).withActive(set = false).get
userDAO.add(user)
val newAuth = Authentication.basic(user.authentication.username, "newpass")

// when
val tobeDeletedUser = user.copy(authentication = newAuth, admin = true, active = true)
val userCountBeforeDelete = userDAO.findAll().length
userDAO.delete(tobeDeletedUser.id)
val deletedUser = userDAO.findById(user.id)
val userCountAfterDelete = userDAO.findAll().length
// then
assert(deletedUser === None , "Deletion was attempted but found " + deletedUser)
userCountAfterDelete should be(userCountBeforeDelete -1)

}

"rememberNotifications" should "store dates properly" taggedAs RequiresDb in {
// given
val user = UserAssembler.randomUser.get
Expand Down Expand Up @@ -479,4 +510,5 @@ class SQLUserDAOSpec extends FlatSpecWithSQL with ClearSQLDataAfterTest with Bef
partial should have size (2)
partial.map(_.name).toSet should be (users.map(_.name).toSet)
}

}
2 changes: 1 addition & 1 deletion codebrag-rest/src/main/scala/ScalatraBootstrap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class ScalatraBootstrap extends LifeCycle with Logging {
RepositoryUpdateScheduler.scheduleUpdates(actorSystem, repositories, commitImportService)
context.mount(new RegistrationServlet(registerService, registerNewUserUseCase, listReposAfterRegistration, listRepoBranchesAfterRegistration, watchBranchAfterRegistration, unwatchBranchAfterRegistration), Prefix + RegistrationServlet.MappingPath)
context.mount(new SessionServlet(authenticator, loginUserUseCase, userFinder), Prefix + SessionServlet.MappingPath)
context.mount(new UsersServlet(authenticator, userFinder, modifyUserDetailsUseCase, config), Prefix + UsersServlet.MappingPath)
context.mount(new UsersServlet(authenticator, userFinder, modifyUserDetailsUseCase, deleteUserUseCase, config), Prefix + UsersServlet.MappingPath)
context.mount(new UserAliasesEndpoint(authenticator, addUserAliasUseCase, deleteUserAliasUseCase), Prefix + UserAliasesEndpoint.MappingPath)
context.mount(new UsersSettingsServlet(authenticator, userDao, changeUserSettingsUseCase), Prefix + "users/settings")
context.mount(new CommitsServlet(authenticator, toReviewCommitsFinder, allCommitsFinder, reactionFinder, addCommentUseCase,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ trait Beans extends ActorSystemSupport with CommitsModule with Daos {
lazy val generateInvitationCodeUseCase = new GenerateInvitationCodeUseCase(invitationsService, userDao)
lazy val sendInvitationEmailUseCase = new SendInvitationEmailUseCase(invitationsService, userDao)
lazy val modifyUserDetailsUseCase = new ModifyUserDetailsUseCase(userDao)
lazy val deleteUserUseCase = new DeleteUserUseCase(userDao)
lazy val updateUserBrowsingContextUseCase = new UpdateUserBrowsingContextUseCase(userRepoDetailsDao)
lazy val addUserAliasUseCase = new AddUserAliasUseCase(userAliasDao, userDao)
lazy val deleteUserAliasUseCase = new DeleteUserAliasUseCase(userAliasDao)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import org.bson.types.ObjectId
import org.scalatra
import com.softwaremill.codebrag.finders.user.UserFinder
import com.softwaremill.codebrag.finders.user.ManagedUsersListView
import com.softwaremill.codebrag.usecases.user.{RegisterNewUserUseCase, ModifyUserDetailsUseCase, ModifyUserDetailsForm}
import com.softwaremill.codebrag.usecases.user.{RegisterNewUserUseCase, ModifyUserDetailsUseCase, ModifyUserDetailsForm,DeleteUserUseCase,DeleteUserForm}

class UsersServlet(
val authenticator: Authenticator,
userFinder: UserFinder,
modifyUserUseCase: ModifyUserDetailsUseCase,
deleteUserUseCase: DeleteUserUseCase,
config: CodebragConfig) extends JsonServletWithAuthentication {

get("/") {
Expand All @@ -34,8 +35,16 @@ class UsersServlet(
case _ => scalatra.Ok()
}
}

delete("/:userId") {
haltIfNotAuthenticated()
val targetUserId = new ObjectId(params("userId"))
deleteUserUseCase.execute(user.id, DeleteUserForm(targetUserId)) match {
case Left(errors) => scalatra.BadRequest(errors)
case _ => scalatra.Ok()
}
}
}


object UsersServlet {
val MappingPath = "users"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ import com.softwaremill.codebrag.finders.user.ManagedUserView
import com.softwaremill.codebrag.finders.user.ManagedUsersListView
import com.softwaremill.codebrag.domain.builder.UserAssembler
import com.softwaremill.codebrag.domain.User
import com.softwaremill.codebrag.usecases.user.ModifyUserDetailsUseCase
import com.softwaremill.codebrag.usecases.user.{ ModifyUserDetailsUseCase, DeleteUserUseCase }

class UsersServletSpec extends AuthenticatableServletSpec {

val modifyUserUseCase = mock[ModifyUserDetailsUseCase]
val deleteUserUseCase = mock[DeleteUserUseCase]
var userFinder: UserFinder = _
var config: CodebragConfig = _

Expand Down Expand Up @@ -50,7 +51,6 @@ class UsersServletSpec extends AuthenticatableServletSpec {
}
}


def configWithDemo(mode: Boolean) = {
val p = new Properties()
p.setProperty("codebrag.demo", mode.toString)
Expand All @@ -60,7 +60,7 @@ class UsersServletSpec extends AuthenticatableServletSpec {
}

class TestableUsersServlet(fakeAuthenticator: Authenticator, fakeScentry: Scentry[User])
extends UsersServlet(fakeAuthenticator, userFinder, modifyUserUseCase, config) {
extends UsersServlet(fakeAuthenticator, userFinder, modifyUserUseCase, deleteUserUseCase, config) {
override def scentry(implicit request: javax.servlet.http.HttpServletRequest) = fakeScentry
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.softwaremill.codebrag.usecases.user

import com.softwaremill.codebrag.dao.user.UserDAO
import com.softwaremill.codebrag.usecases.assertions.UserAssertions
import org.bson.types.ObjectId
import com.softwaremill.codebrag.domain.{Authentication, User}
import com.typesafe.scalalogging.slf4j.Logging
import com.softwaremill.scalaval.Validation._

case class DeleteUserForm(userId: ObjectId) {

}

class DeleteUserUseCase(protected val userDao: UserDAO) extends Logging {

import UserAssertions._

def execute(executorId: ObjectId, form: DeleteUserForm): Either[Errors, Unit] = {
assertUserWithId(executorId, mustBeActive, mustBeAdmin)(userDao)
val targetUser = loadUser(form.userId)
validateUserDetails(executorId, targetUser, form).whenOk[Unit] {
logger.debug(s"Validation passed, attempting to delete user $targetUser")
userDao.delete(form.userId)
}
}

private def loadUser(userId: ObjectId) = userDao.findById(userId).getOrElse(throw new IllegalStateException(s"User $userId not found"))

private def validateUserDetails(executorId: ObjectId, user: User, form: DeleteUserForm) = {
val changeOwnFlagsCheck = rule("userId") {
val isDeleteFlags = user.admin || user.active
(!isDeleteFlags || (isDeleteFlags && executorId != user.id), "Cannot delete own user")
}
validate(changeOwnFlagsCheck)
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package com.softwaremill.codebrag.usecases.user

import org.scalatest.{BeforeAndAfter, FlatSpec}
import org.scalatest.matchers.ShouldMatchers
import org.scalatest.mock.MockitoSugar
import com.softwaremill.codebrag.dao.user.UserDAO
import org.mockito.Matchers._
import org.mockito.Mockito._
import com.softwaremill.codebrag.domain.builder.UserAssembler
import com.softwaremill.codebrag.domain.{Authentication, User}
import com.softwaremill.codebrag.usecases.assertions.{ActiveUserStatusRequiredException, AdminRoleRequiredException}
import org.bson.types.ObjectId

class DeleteUserUseCaseSpec extends FlatSpec with ShouldMatchers with BeforeAndAfter with MockitoSugar {

val userDao = mock[UserDAO]
val useCase = new DeleteUserUseCase(userDao)

val InactiveUser = UserAssembler.randomUser.withActive(set = false).get
val ActiveUser = UserAssembler.randomUser.withActive().get

val ValidExecutor = UserAssembler.randomUser.withAdmin().withActive().get
val NonAdminExecutor = UserAssembler.randomUser.withActive().withAdmin(set = false).get
val InactiveExecutor = UserAssembler.randomUser.withActive(set = false).withAdmin().get

after {
reset(userDao)
}

it should "not delete user when executing user is neither admin nor active" in {
// given
setupReturningUserFromDB(NonAdminExecutor, InactiveExecutor)
val form = DeleteUserForm(ActiveUser.id)

// when
intercept[AdminRoleRequiredException] {
useCase.execute(NonAdminExecutor.id, form)
}
intercept[ActiveUserStatusRequiredException] {
useCase.execute(InactiveExecutor.id, form)
}

// then
verify(userDao, never()).delete(any[ObjectId])
}

it should "not allow to delete yourself" in {
// given
setupReturningUserFromDB(ValidExecutor)

// when
val ownChangeForm = DeleteUserForm(ValidExecutor.id)
val Left(result) = useCase.execute(ValidExecutor.id, ownChangeForm)

// then
result should be(Map("userId" -> List("Cannot delete own user")))
verify(userDao, never()).delete(any[ObjectId])
}


it should "delete user when validation passes" in {
// given
stubCurrentlyActiveUsersCountTo(0)
setupReturningUserFromDB(ValidExecutor, ActiveUser)

// when
val newAuth = Authentication.basic(ActiveUser.authentication.username, "secret")
val form = new DeleteUserForm(ActiveUser.id)
val result = useCase.execute(ValidExecutor.id, form)

// then
result should be('right)
val expectedUser = form
verify(userDao).delete(expectedUser.userId)
}

private def stubCurrentlyActiveUsersCountTo(activeUsersCount: Int) {
when(userDao.countAllActive()).thenReturn(activeUsersCount)
}

private def setupReturningUserFromDB(users: User*) {
users.foreach { user =>
when(userDao.findById(user.id)).thenReturn(Some(user))
}
}

}

20 changes: 20 additions & 0 deletions codebrag-ui/app/scripts/users/manageUsersPopupCtrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,37 @@ angular.module('codebrag.userMgmt')
});
};

$scope.deleteUser = function(user) {
$scope.flash.clear();
var userData = { userId: user.userId };
userMgmtService.deleteUser(userData).then(deleteSuccess(user), deleteFailed('active', user.userId))
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please unident the whole section

$scope.askForNewPassword = function(user) {
$scope.flash.clear();
var modal = popupsService.openSetUserPasswordPopup(user);
modal.result.then(function() {
$scope.flash.add('info', 'User password changed');
});
};
function deleteSuccess(user) {
$scope.flash.add('error', 'User ' + user.email + ' is deleted');
userMgmtService.loadUsers().then(function(users) {
$scope.users = users;
});
}

function modifySuccess() {
$scope.flash.add('info', 'User details changed');
}
function deleteFailed(flag, userId) {
return function(errorsMap) {
$scope.flash.add('error', ' Could not delete user ');
flattenErrorsMap(errorsMap).forEach(function(error) {
$scope.flash.add('error', error);
});
}
}

function modifyFailed(flag, user) {
return function(errorsMap) {
Expand Down
7 changes: 6 additions & 1 deletion codebrag-ui/app/scripts/users/userMgmtService.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ angular.module('codebrag.userMgmt')
return $http.put(modifyUserUrl, userData).then(null, modifyUserFailed);
};

this.deleteUser = function(userData) {
var deleteUserUrl = [usersApiUrl, '/', userData.userId].join('');
return $http.delete(deleteUserUrl).then(null, modifyUserFailed);
};

function modifyUserFailed(response) {
return $q.reject(response.data);
}
Expand All @@ -34,4 +39,4 @@ angular.module('codebrag.userMgmt')
$modal.open(config)
}

});
});
Loading