Skip to content

Commit

Permalink
Merge pull request #213 from guardian/improve-backfill
Browse files Browse the repository at this point in the history
Improve zuora/salesforce identity id backfill
  • Loading branch information
QuarpT committed Nov 15, 2018
2 parents 574a2c4 + 7a69aae commit f8cd1c8
Show file tree
Hide file tree
Showing 11 changed files with 261 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,15 @@ import play.api.libs.json.{JsValue, Json, Reads}

object GetByEmail {

sealed trait IdentityAccount
case class IdentityAccountWithValidatedEmail(identityId: IdentityId) extends IdentityAccount
case object IdentityAccountWithUnvalidatedEmail extends IdentityAccount
case class IdentityAccount(identityId: IdentityId, isUserEmailValidated: Boolean)

object RawWireModel {

case class StatusFields(userEmailValidated: Boolean)
implicit val statusFieldsReads: Reads[StatusFields] = Json.reads[StatusFields]

case class User(id: String, statusFields: StatusFields)
case class User(id: String, statusFields: Option[StatusFields])

implicit val userReads: Reads[User] = Json.reads[User]

case class UserResponse(status: String, user: User)
Expand All @@ -40,10 +39,17 @@ object GetByEmail {
}

def wireToDomainModel(userResponse: UserResponse): ClientFailableOp[IdentityAccount] = {

for {
user <- userFromResponse(userResponse)
identityId = if (user.statusFields.userEmailValidated) IdentityAccountWithValidatedEmail(IdentityId(user.id)) else IdentityAccountWithUnvalidatedEmail
} yield identityId
} yield {
val isUserEmailValidated: Boolean = userResponse
.user
.statusFields
.exists(_.userEmailValidated)

IdentityAccount(IdentityId(user.id), isUserEmailValidated)
}

}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package com.gu.identity

import com.gu.identity.GetByIdentityId.RawWireModel.{User, UserResponse}
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
import com.gu.util.resthttp.HttpOp.HttpOpWrapper
import com.gu.util.resthttp.RestRequestMaker
import com.gu.util.resthttp.RestRequestMaker.{GetRequest, RelativePath}
import com.gu.util.resthttp.Types.{ClientFailableOp, ClientSuccess, GenericError}
import play.api.libs.json.{JsValue, Json, Reads}

object GetByIdentityId {

case class IdentityUser(id: IdentityId, hasPassword: Boolean)

object RawWireModel {

case class User(id: String, hasPassword: Boolean)
implicit val userReads: Reads[User] = Json.reads[User]

case class UserResponse(status: String, user: User)
implicit val userResponseReads: Reads[UserResponse] = Json.reads[UserResponse]

}

val jsToWireModel: JsValue => ClientFailableOp[UserResponse] = RestRequestMaker.toResult[UserResponse]

def userFromResponse(userResponse: UserResponse): ClientFailableOp[User] =
userResponse match {
case UserResponse("ok", user) => ClientSuccess(user)
case error => GenericError(s"not an OK response from api: $error")
}

def wireToDomainModel(userResponse: UserResponse): ClientFailableOp[IdentityUser] = {
for {
user <- userFromResponse(userResponse)
} yield IdentityUser(IdentityId(user.id), user.hasPassword)
}

val wrapper: HttpOpWrapper[IdentityId, GetRequest, JsValue, IdentityUser] =
HttpOpWrapper[IdentityId, GetRequest, JsValue, IdentityUser](
fromNewParam = id => GetRequest(RelativePath(s"/user/${id.value}")),
toNewResponse = jsToWireModel.andThen(_.flatMap(wireToDomainModel))
)

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package com.gu.identityBackfill

import com.gu.identity.GetByEmail.IdentityAccount
import com.gu.identity.{GetByEmail, GetByIdentityId}
import com.gu.identity.GetByIdentityId.IdentityUser
import com.gu.identityBackfill.Types.EmailAddress
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
import com.gu.util.apigateway.ApiGatewayResponse
import com.gu.util.reader.Types.ApiGatewayOp
import com.gu.util.reader.Types.ApiGatewayOp.{ContinueProcessing, ReturnWithResponse}
import com.gu.util.resthttp.Types.{ClientFailableOp, ClientFailure, ClientSuccess, NotFound}

object FindExistingIdentityId {

def apply(
getByEmail: EmailAddress => ClientFailableOp[GetByEmail.IdentityAccount],
getByIdentityId: IdentityId => ClientFailableOp[GetByIdentityId.IdentityUser]
)(emailAddress: EmailAddress): ApiGatewayOp[Option[IdentityId]] = {

def continueIfNoPassword(identityId: IdentityId) = {
getByIdentityId(identityId) match {
case ClientSuccess(IdentityUser(_, false)) => ContinueProcessing(Some(identityId))
case _ => ReturnWithResponse(ApiGatewayResponse.notFound(s"identity email not validated but password is set $identityId"))
}
}

val result = getByEmail(emailAddress) match {
case ClientSuccess(IdentityAccount(identityId, true)) => ContinueProcessing(Some(identityId))
case ClientSuccess(IdentityAccount(identityId, false)) => continueIfNoPassword(identityId)
case NotFound(_) => ContinueProcessing(None)
case other: ClientFailure => ReturnWithResponse(ApiGatewayResponse.internalServerError(other.toString))
}

result.withLogging("FindExistingIdentityId")
}

}
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package com.gu.identityBackfill

import java.io.{InputStream, OutputStream}

import com.amazonaws.services.lambda.runtime.Context
import com.gu.effects.{GetFromS3, RawEffects}
import com.gu.identity.{CreateGuestAccount, GetByEmail, IdentityClient, IdentityConfig}
import com.gu.identity._
import com.gu.identityBackfill.IdentityBackfillSteps.DomainRequest
import com.gu.identityBackfill.TypeConvert._
import com.gu.identityBackfill.Types.EmailAddress
Expand All @@ -21,7 +20,6 @@ import com.gu.util.apigateway.ResponseModels.ApiResponse
import com.gu.util.apigateway.{ApiGatewayHandler, ApiGatewayRequest, ApiGatewayResponse, ResponseModels}
import com.gu.util.config.LoadConfigModule.StringFromS3
import com.gu.util.config.{LoadConfigModule, Stage}
import com.gu.util.reader.Types.ApiGatewayOp.{ContinueProcessing, ReturnWithResponse}
import com.gu.util.reader.Types._
import com.gu.util.resthttp.JsonHttp.StringHttpRequest
import com.gu.util.resthttp.RestRequestMaker.{GetRequest, PatchRequest}
Expand Down Expand Up @@ -58,6 +56,9 @@ object Handler {
val identityClient = IdentityClient(response, identityConfig)
val createGuestAccount = identityClient.wrapWith(JsonHttp.post).wrapWith(CreateGuestAccount.wrapper)
val getByEmail = identityClient.wrapWith(JsonHttp.getWithParams).wrapWith(GetByEmail.wrapper)
val getById = identityClient.wrapWith(JsonHttp.get).wrapWith(GetByIdentityId.wrapper)
val findExistingIdentityId = FindExistingIdentityId(getByEmail.runRequest, getById.runRequest) _

val countZuoraAccounts: IdentityId => ClientFailableOp[Int] = CountZuoraAccountsForIdentityId(zuoraQuerier)

lazy val sfAuth: LazyClientFailableOp[HttpOp[StringHttpRequest, RestRequestMaker.BodyAsString]] = SalesforceClient(response, sfConfig)
Expand All @@ -67,7 +68,7 @@ object Handler {
Operation(
steps = WireRequestToDomainObject(IdentityBackfillSteps(
PreReqCheck(
getByEmail.runRequest,
findExistingIdentityId,
GetZuoraAccountsForEmail(zuoraQuerier) _ andThen PreReqCheck.getSingleZuoraAccountForEmail,
countZuoraAccounts andThen PreReqCheck.noZuoraAccountsForIdentityId,
GetZuoraSubTypeForAccount(zuoraQuerier) _ andThen PreReqCheck.acceptableReaderType,
Expand Down Expand Up @@ -139,15 +140,12 @@ object Healthcheck {
sfAuth: LazyClientFailableOp[Any]
): ApiResponse =
(for {
maybeIdentityId <- getByEmail.runRequest(EmailAddress("john.duffell@guardian.co.uk"))
.toApiGatewayOp("problem with email").withLogging("healthcheck getByEmail")
identityId <- maybeIdentityId match {
case GetByEmail.IdentityAccountWithValidatedEmail(identityId) => ContinueProcessing(identityId)
case other =>
logger.error(s"failed healthcheck with $other")
ReturnWithResponse(ApiGatewayResponse.internalServerError("test identity id was not present"))
}
_ <- countZuoraAccountsForIdentityId(identityId).toApiGatewayOp("get zuora accounts for identity id")
identityAccount <- getByEmail
.runRequest(EmailAddress("john.duffell@guardian.co.uk"))
.toApiGatewayOp("problem with email")
.withLogging("healthcheck getByEmail")

_ <- countZuoraAccountsForIdentityId(identityAccount.identityId).toApiGatewayOp("get zuora accounts for identity id")
_ <- sfAuth.value.toApiGatewayOp("Failed to authenticate with Salesforce")
} yield ApiGatewayResponse.successfulExecution).apiResponse

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.gu.identityBackfill

import com.gu.identity.GetByEmail
import com.gu.identity.GetByEmail.IdentityAccountWithUnvalidatedEmail
import com.gu.identityBackfill.TypeConvert._
import com.gu.identityBackfill.Types._
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
Expand All @@ -12,26 +10,21 @@ import com.gu.util.apigateway.ApiGatewayResponse
import com.gu.util.reader.Types.ApiGatewayOp._
import com.gu.util.reader.Types._
import com.gu.util.resthttp.LazyClientFailableOp
import com.gu.util.resthttp.Types.{ClientFailableOp, ClientFailure, ClientSuccess, NotFound}
import com.gu.util.resthttp.Types.ClientFailableOp

object PreReqCheck {

case class PreReqResult(zuoraAccountId: Types.AccountId, sFContactId: SFContactId, existingIdentityId: Option[IdentityId])

def apply(
getByEmail: EmailAddress => ClientFailableOp[GetByEmail.IdentityAccount],
findExistingIdentityId: EmailAddress => ApiGatewayOp[Option[IdentityId]],
getSingleZuoraAccountForEmail: EmailAddress => ApiGatewayOp[ZuoraAccountIdentitySFContact],
noZuoraAccountsForIdentityId: IdentityId => ApiGatewayOp[Unit],
zuoraSubType: AccountId => ApiGatewayOp[Unit],
syncableSFToIdentity: SFContactId => LazyClientFailableOp[ApiGatewayOp[Unit]]
)(emailAddress: EmailAddress): ApiGatewayOp[PreReqResult] = {
for {
maybeExistingIdentityId <- (getByEmail(emailAddress) match {
case ClientSuccess(GetByEmail.IdentityAccountWithValidatedEmail(identityId)) => ContinueProcessing(Some(identityId))
case NotFound(_) => ContinueProcessing(None)
case ClientSuccess(IdentityAccountWithUnvalidatedEmail) => ReturnWithResponse(ApiGatewayResponse.notFound("identity email not validated"))
case other: ClientFailure => ReturnWithResponse(ApiGatewayResponse.internalServerError(other.toString))
}).withLogging("GetByEmail")
maybeExistingIdentityId <- findExistingIdentityId(emailAddress)
zuoraAccountForEmail <- getSingleZuoraAccountForEmail(emailAddress)
_ <- maybeExistingIdentityId.map(noZuoraAccountsForIdentityId).getOrElse(ContinueProcessing(()))
_ <- syncableSFToIdentity(zuoraAccountForEmail.sfContactId).value.toApiGatewayOp("load SF contact").flatMap(identity)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.gu.identity

import com.gu.effects.{GetFromS3, RawEffects}
import com.gu.identity.GetByEmail.IdentityAccountWithValidatedEmail
import com.gu.identity.GetByEmail.IdentityAccount
import com.gu.identityBackfill.Types.EmailAddress
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
import com.gu.test.EffectsTest
Expand All @@ -23,7 +23,7 @@ class GetByEmailEffectsTest extends FlatSpec with Matchers {
getByEmail = identityClient.wrapWith(JsonHttp.getWithParams).wrapWith(GetByEmail.wrapper)
identityId <- getByEmail.runRequest(EmailAddress("john.duffell@guardian.co.uk")).toDisjunction
} yield identityId
actual should be(\/-(IdentityAccountWithValidatedEmail(IdentityId("21814163"))))
actual should be(\/-(IdentityAccount(IdentityId("21814163"), isUserEmailValidated = true)))

}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package com.gu.identity

import com.gu.identity.GetByEmail.{IdentityAccountWithUnvalidatedEmail, IdentityAccountWithValidatedEmail}
import com.gu.identity.GetByEmail.IdentityAccount
import com.gu.identity.GetByEmailTest.{NotValidatedTestData, TestData}
import com.gu.identityBackfill.Types.EmailAddress
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
Expand All @@ -20,15 +20,19 @@ class GetByEmailTest extends FlatSpec with Matchers {
it should "get successful ok" in {
val actual = GetByEmail.wrapper.toNewResponse(Json.parse(TestData.dummyIdentityResponse))

actual should be(ClientSuccess(IdentityAccountWithValidatedEmail(IdentityId("1234"))))
actual should be(ClientSuccess(IdentityAccount(IdentityId("1234"), isUserEmailValidated = true)))
}

it should "get not validated with an error" in {
val actual = GetByEmail.wrapper.toNewResponse(Json.parse(NotValidatedTestData.dummyIdentityResponse))

actual should be(ClientSuccess(IdentityAccountWithUnvalidatedEmail))
actual should be(ClientSuccess(IdentityAccount(IdentityId("1234"), isUserEmailValidated = false)))
}

it should "get not validated if the response has no statusFields" in {
val actual = GetByEmail.wrapper.toNewResponse(Json.parse(NotValidatedTestData.identityResponseWithoutStatus))
actual should be(ClientSuccess(IdentityAccount(IdentityId("1234"), isUserEmailValidated = false)))
}
}

object GetByEmailTest {
Expand Down Expand Up @@ -87,6 +91,16 @@ object GetByEmailTest {
|}
""".stripMargin

val identityResponseWithoutStatus: String =
"""
|{
| "status": "ok",
| "user": {
| "id": "1234"
| }
|}
""".stripMargin

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.gu.identity

import com.gu.effects.{GetFromS3, RawEffects}
import com.gu.identity.GetByIdentityId.IdentityUser
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
import com.gu.test.EffectsTest
import com.gu.util.config.{LoadConfigModule, Stage}
import com.gu.util.resthttp.JsonHttp
import org.scalatest.{FlatSpec, Matchers}
import scalaz.\/-

// run this manually
class GetByIdentityIdEffectsTest extends FlatSpec with Matchers {

it should "successfull run the health check using the local code against real backend" taggedAs EffectsTest in {

val actual = for {
identityConfig <- LoadConfigModule(Stage("DEV"), GetFromS3.fetchString)[IdentityConfig]

response = RawEffects.response
identityClient = IdentityClient(response, identityConfig)
getByIdentityId = identityClient.wrapWith(JsonHttp.get).wrapWith(GetByIdentityId.wrapper)
identityId <- getByIdentityId.runRequest(IdentityId("21814163")).toDisjunction
} yield identityId
actual should be(\/-(IdentityUser(IdentityId("21814163"), hasPassword = true)))

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.gu.identity

import com.gu.identity.GetByIdentityId.IdentityUser
import com.gu.identityBackfill.salesforce.UpdateSalesforceIdentityId.IdentityId
import com.gu.util.resthttp.RestRequestMaker.{GetRequest, RelativePath}
import com.gu.util.resthttp.Types.ClientSuccess
import org.scalatest.{FlatSpec, Matchers}
import play.api.libs.json.Json

class GetByIdentityIdTest extends FlatSpec with Matchers {
it should "formulate a request" in {
GetByIdentityId.wrapper.fromNewParam(IdentityId("1234")) should be(GetRequest(RelativePath("/user/1234")))
}

it should "extract user with password" in {
val identityResponse: String =
"""
|{
| "status": "ok",
| "user": {
| "id": "1234",
| "hasPassword": true
| }
|}
""".stripMargin
val actual = GetByIdentityId.wrapper.toNewResponse(Json.parse(identityResponse))
actual should be(ClientSuccess(IdentityUser(IdentityId("1234"), hasPassword = true)))
}

it should "extract user without password" in {
val identityResponse: String =
"""
|{
| "status": "ok",
| "user": {
| "id": "1234",
| "hasPassword": false
| }
|}
""".stripMargin
val actual = GetByIdentityId.wrapper.toNewResponse(Json.parse(identityResponse))
actual should be(ClientSuccess(IdentityUser(IdentityId("1234"), hasPassword = false)))
}
}
Loading

0 comments on commit f8cd1c8

Please sign in to comment.