From 5c5f6053415f69d55644fa7dfdcacce14bfe963c Mon Sep 17 00:00:00 2001 From: hongwei1 Date: Sun, 25 Feb 2018 16:38:15 +0100 Subject: [PATCH] refactor the view methods -- step4 remove the `permittedViews` method, it separate into public, private, firehose methods, and unused method `canInitiateTransactions` --- src/main/scala/code/model/BankingData.scala | 2 +- src/main/scala/code/model/User.scala | 9 ------- .../code/remotedata/RemotedataViews.scala | 3 --- .../remotedata/RemotedataViewsActor.scala | 4 --- src/main/scala/code/views/MapperViews.scala | 27 ------------------- src/main/scala/code/views/Views.scala | 9 +++---- .../code/sandbox/SandboxDataLoadingTest.scala | 4 +-- 7 files changed, 6 insertions(+), 52 deletions(-) diff --git a/src/main/scala/code/model/BankingData.scala b/src/main/scala/code/model/BankingData.scala index ce63a9f85..d75ee19f1 100644 --- a/src/main/scala/code/model/BankingData.scala +++ b/src/main/scala/code/model/BankingData.scala @@ -369,7 +369,7 @@ trait BankAccount extends MdcLoggable { */ final def permittedViews(user: Box[User]) : List[View] = { user match { - case Full(u) =>Views.views.vend.viewsUserCanAccessForAccount(u, this) + case Full(u) =>Views.views.vend.viewsUserCanAccessForAccount(u, BankIdAccountId(this.bankId, this.accountId)) case _ =>{ //TODO, this is strange ?? do we really need show all the public views??? Views.views.vend.publicViews } diff --git a/src/main/scala/code/model/User.scala b/src/main/scala/code/model/User.scala index 565a1ac2f..d3eda983e 100644 --- a/src/main/scala/code/model/User.scala +++ b/src/main/scala/code/model/User.scala @@ -91,15 +91,6 @@ trait User extends MdcLoggable { !(ViewPrivileges.count(By(ViewPrivileges.user, this.resourceUserId.value), By(ViewPrivileges.view, viewImpl.id)) == 0) } - final def canInitiateTransactions(bankAccount: BankAccount) : Box[Unit] ={ - if(Views.views.vend.viewsUserCanAccessForAccount(this, bankAccount).exists(_.canInitiateTransaction)){ - Full() - } - else { - Failure("user doesn't have access to any view that allows initiating transactions") - } - } - /** * @return the bank accounts where the user has at least access to a Private view (is_public==false) */ diff --git a/src/main/scala/code/remotedata/RemotedataViews.scala b/src/main/scala/code/remotedata/RemotedataViews.scala index b811b8ff1..4f12ae0fa 100644 --- a/src/main/scala/code/remotedata/RemotedataViews.scala +++ b/src/main/scala/code/remotedata/RemotedataViews.scala @@ -52,9 +52,6 @@ object RemotedataViews extends ObpActorInit with Views { def viewsForAccount(bankAccountId : BankIdAccountId) : List[View] = extractFuture(actor ? cc.views(bankAccountId)) - def permittedViews(user: User, bankAccountId: BankIdAccountId): List[View] = - extractFuture(actor ? cc.permittedViews(user, bankAccountId)) - def publicViews : List[View] = extractFuture(actor ? cc.publicViews()) diff --git a/src/main/scala/code/remotedata/RemotedataViewsActor.scala b/src/main/scala/code/remotedata/RemotedataViewsActor.scala index cce6bee1c..bfd080662 100644 --- a/src/main/scala/code/remotedata/RemotedataViewsActor.scala +++ b/src/main/scala/code/remotedata/RemotedataViewsActor.scala @@ -67,10 +67,6 @@ class RemotedataViewsActor extends Actor with ObpActorHelper with MdcLoggable { logger.debug("views(" + bankAccountId +")") sender ! extractResult(mapper.viewsForAccount(bankAccountId)) - case cc.permittedViews(user: User, bankAccountId: BankIdAccountId) => - logger.debug("permittedViews(" + user +", " + bankAccountId +")") - sender ! extractResult(mapper.permittedViews(user, bankAccountId)) - case cc.publicViews() => logger.debug("publicViews()") sender ! extractResult(mapper.publicViews) diff --git a/src/main/scala/code/views/MapperViews.scala b/src/main/scala/code/views/MapperViews.scala index 515a88700..6ba291d5b 100644 --- a/src/main/scala/code/views/MapperViews.scala +++ b/src/main/scala/code/views/MapperViews.scala @@ -277,33 +277,6 @@ object MapperViews extends Views with MdcLoggable { ViewImpl.findAll(ViewImpl.accountFilter(bankAccountId.bankId, bankAccountId.accountId): _*) } - /** - * This method is belong to Views trait, check the permitted views of input account for input user. - * Select all the views by user and bankAccountUID. - * - * @param user the user need to be checked for the views - * @param bankAccountId the bankAccountUID, the account will be checked the views. - * @return if find, return the view list. or return Nil. - */ - def permittedViews(user: User, bankAccountId: BankIdAccountId): List[View] = { - //TODO: do this more efficiently? - //select all views by user. - val allUserPrivs = ViewPrivileges.findAll(By(ViewPrivileges.user, user.resourceUserId.value)) - //select the Private views by BankAccountUid - val userPrivateViewsForAccount = allUserPrivs.flatMap(p => { - p.view.obj match { - case Full(v) => if( - v.isPrivate && - v.bankId == bankAccountId.bankId&& - v.accountId == bankAccountId.accountId){ - Some(v) - } else None - case _ => None - } - }) - // merge the Private and public views - (userPrivateViewsForAccount ++ publicViews ++ getAllFirehoseViews(bankAccountId, user)).distinct - } def publicViews: List[View] = { if (APIUtil.ALLOW_PUBLIC_VIEWS) diff --git a/src/main/scala/code/views/Views.scala b/src/main/scala/code/views/Views.scala index c09c55fec..24277ead5 100644 --- a/src/main/scala/code/views/Views.scala +++ b/src/main/scala/code/views/Views.scala @@ -54,19 +54,17 @@ trait Views { */ def viewsForAccount(bankAccountId : BankIdAccountId) : List[View] - def permittedViews(user: User, bankAccountId: BankIdAccountId): List[View] - final def viewsUserCanAccess(user: User): List[View] = (privateViewsUserCanAccess(user: User) ++ publicViews).distinct final def privateViewsUserCanAccess(user: User): List[View] ={ ViewPrivileges.findAll(By(ViewPrivileges.user, user.resourceUserId.value)).map(_.view.obj.toList).flatten.filter(_.isPrivate) } - final def viewsUserCanAccessForAccount(user: User, bankAccount: BankAccount) : List[View] = + final def viewsUserCanAccessForAccount(user: User, bankAccountId : BankIdAccountId) : List[View] = Views.views.vend.viewsUserCanAccess(user).filter( view => - view.bankId == bankAccount.bankId && - view.accountId == bankAccount.accountId + view.bankId == bankAccountId.bankId && + view.accountId == bankAccountId.accountId ) def getAllPublicAccounts : List[BankIdAccountId] @@ -114,7 +112,6 @@ class RemotedataViewsCaseClasses { case class removeView(viewId: ViewId, bankAccountId: BankIdAccountId) case class updateView(bankAccountId: BankIdAccountId, viewId: ViewId, viewUpdateJson: UpdateViewJSON) case class views(bankAccountId: BankIdAccountId) - case class permittedViews(user: User, bankAccountId: BankIdAccountId) case class publicViews() case class getAllPublicAccounts() case class getPublicBankAccounts(bank: Bank) diff --git a/src/test/scala/code/sandbox/SandboxDataLoadingTest.scala b/src/test/scala/code/sandbox/SandboxDataLoadingTest.scala index 025a4ae51..cbd4f5b4e 100644 --- a/src/test/scala/code/sandbox/SandboxDataLoadingTest.scala +++ b/src/test/scala/code/sandbox/SandboxDataLoadingTest.scala @@ -301,9 +301,9 @@ class SandboxDataLoadingTest extends FlatSpec with SendServerRequests with Match val owner = Users.users.vend.getUserByProviderId(defaultProvider, foundAccount.owners.toList.head.name).openOrThrowException(attemptedToOpenAnEmptyBox) //there should be an owner view - val views = Views.views.vend.permittedViews(owner, BankIdAccountId(foundAccount.bankId, foundAccount.accountId)) + val views = Views.views.vend.viewsUserCanAccessForAccount(owner, BankIdAccountId(foundAccount.bankId, foundAccount.accountId)) val ownerView = views.find(v => v.viewId.value == "owner") - ownerView.isDefined should equal(true) + owner.hasOwnerViewAccess(BankIdAccountId(foundAccount.bankId, foundAccount.accountId)) should equal(true) //and the owners should have access to it Views.views.vend.getOwners(ownerView.get).map(_.idGivenByProvider) should equal(account.owners.toSet)