-
Notifications
You must be signed in to change notification settings - Fork 0
[APB-11045][APB-APB-11015] Success and FailedNonFixable emails #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 43 commits
7c02966
4477295
232a634
1c4e007
d9d3647
f35b4f4
853c591
c016314
86b63ef
d9be6f4
c5f5830
d169df3
fe20e1a
0a0b6f6
6c1d1a3
7dd4bc5
6059914
e646367
d4099ae
16b4cf0
b05ea41
08ad1f1
05e90e0
04d21ea
5f2e1de
33b9ad9
92720e9
cb1cf32
9de4d92
cb938ac
a0eb410
ff2a7e7
246c12b
c3f7ba1
c9b5789
aa08cd3
45cad5a
dd34816
357f824
8023e25
c28f356
aa8850b
63e98e9
6cd0888
f4839cb
d2dcab6
cb08a60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| /* | ||
| * Copyright 2026 HM Revenue & Customs | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package uk.gov.hmrc.agentregistrationrisking.connectors | ||
|
|
||
| import play.api.http.Status.ACCEPTED | ||
| import uk.gov.hmrc.agentregistration.shared.util.Errors | ||
| import uk.gov.hmrc.agentregistrationrisking.config.AppConfig | ||
| import uk.gov.hmrc.agentregistrationrisking.model.EmailInformation | ||
| import uk.gov.hmrc.agentregistrationrisking.util.FutureUtil.andLogOnFailure | ||
| import uk.gov.hmrc.http.client.HttpClientV2 | ||
|
|
||
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
| import scala.concurrent.ExecutionContext | ||
|
|
||
| @Singleton | ||
| class EmailConnector @Inject() ( | ||
| appConfig: AppConfig, | ||
| httpClient: HttpClientV2 | ||
| )(using ExecutionContext) | ||
| extends Connector: | ||
|
|
||
| def sendEmail(emailInformation: EmailInformation)(using RequestHeader): Future[Unit] = | ||
| val url: URL = url"$baseUrl/hmrc/email" | ||
| httpClient | ||
| .post(url) | ||
| .withBody(Json.toJson(emailInformation)) | ||
| .execute[HttpResponse] | ||
| .map: response => | ||
| response.status match | ||
| case ACCEPTED => () | ||
| case status => | ||
| Errors.throwUpstreamErrorResponse( | ||
| httpMethod = "POST", | ||
| url = url, | ||
| status = status, | ||
| response = response, | ||
| info = s"Failed to send email for template ${emailInformation.templateId}" | ||
| ) | ||
| .andLogOnFailure(s"Failed to send email for template ${emailInformation.templateId}") | ||
|
|
||
| private val baseUrl: String = appConfig.emailBaseUrl |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* | ||
| * Copyright 2026 HM Revenue & Customs | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package uk.gov.hmrc.agentregistrationrisking.model | ||
|
|
||
| import play.api.libs.json.Json | ||
| import play.api.libs.json.OFormat | ||
|
|
||
| final case class EmailInformation( | ||
| to: Seq[String], | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was wondering if it might be worth using a dedicated type for the email address, such as EmailAddress, instead of a plain String. It already seems to handle the necessary JSON serialization, and it could make the code more type-safe overall.
|
||
| templateId: String, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we consider using a dedicated type for this as well? Since we only have a small set of email template values, it might make sense to model them as an enum. That way, we could reduce the risk of passing incompatible or unsupported values. |
||
| parameters: Map[String, String], | ||
| force: Boolean = false, | ||
| eventUrl: Option[String] = None, | ||
| onSendUrl: Option[String] = None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was wondering if those fields are still needed, as they don’t seem to be used anywhere. Maybe it would make sense to remove them to avoid maintaining code that isn’t actually in use? |
||
| ) | ||
|
|
||
| object EmailInformation: | ||
| given OFormat[EmailInformation] = Json.format[EmailInformation] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,23 @@ extends Repo[ApplicationReference, ApplicationForRisking]( | |
| ) | ||
| ).toFuture() | ||
|
|
||
| // def findReadyForSubscription(): Future[Seq[ApplicationForRisking]] = collection | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was wondering what the purpose of this commented-out code is. |
||
| // .find( | ||
| // Filters.and( | ||
| // Filters.size("failures", 0), | ||
| // Filters.eq("isSubscribed", false) | ||
| // ) | ||
| // ).toFuture() | ||
|
|
||
| def findApplicationsPendingAction(): Future[Seq[ApplicationForRisking]] = collection | ||
| .find( | ||
| Filters.and( | ||
| Filters.exists(FieldNames.entityRiskingResult), | ||
| Filters.eq(FieldNames.isSubscribed, false), | ||
| Filters.eq("isEmailSent", false) | ||
| ) | ||
| ).toFuture() | ||
|
|
||
| // def findReadyForSubscription(): Future[Seq[ApplicationForRisking]] = collection | ||
| // .find( | ||
| // Filters.and( | ||
|
|
@@ -95,6 +112,22 @@ extends Repo[ApplicationReference, ApplicationForRisking]( | |
| ) | ||
| ).toFuture() | ||
| } | ||
| def findSubscribedReadyForSuccessEmail(): Future[Seq[ApplicationForRisking]] = collection | ||
| .find( | ||
| Filters.and( | ||
| Filters.eq("isSubscribed", true), | ||
| Filters.eq("isEmailSent", false) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we consider using a |
||
| ) | ||
| ).toFuture() | ||
|
|
||
| def updateEmailSent(applicationReference: ApplicationReference): Future[UpdateResult] = collection | ||
| .updateOne( | ||
| Filters.eq(FieldNames.applicationReference, applicationReference.value), | ||
| Updates.combine( | ||
| Updates.set("isEmailSent", true), | ||
| Updates.set(FieldNames.lastUpdatedAt, Instant.now(clock).toString) | ||
| ) | ||
| ).toFuture() | ||
|
|
||
| // when named ApplicationForRiskingRepo, Scala 3 compiler complains | ||
| // about cyclic reference error during compilation ... | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| /* | ||
| * Copyright 2026 HM Revenue & Customs | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package uk.gov.hmrc.agentregistrationrisking.services | ||
|
|
||
| import uk.gov.hmrc.agentregistration.shared.risking.RiskingOutcome | ||
| import uk.gov.hmrc.agentregistration.shared.util.SafeEquals.=== | ||
| import uk.gov.hmrc.agentregistrationrisking.model.* | ||
| import uk.gov.hmrc.agentregistrationrisking.repository.ApplicationForRiskingRepo | ||
| import uk.gov.hmrc.agentregistrationrisking.repository.IndividualForRiskingRepo | ||
| import uk.gov.hmrc.agentregistrationrisking.util.RequestAwareLogging | ||
| import uk.gov.hmrc.agentregistrationrisking.services.RiskingOutcomeHelper._ | ||
|
|
||
| import java.time.Clock | ||
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
| import scala.concurrent.ExecutionContext | ||
| import scala.concurrent.Future | ||
|
|
||
| @Singleton | ||
| class ApplicationStatusService @Inject() ( | ||
| applicationForRiskingRepo: ApplicationForRiskingRepo, | ||
| individualForRiskingRepo: IndividualForRiskingRepo | ||
| )(using | ||
| ExecutionContext, | ||
| Clock | ||
| ) | ||
| extends RequestAwareLogging: | ||
|
|
||
| def findApprovedReadyToSubscribe(): Future[Seq[ApplicationForRisking]] = getApplicationsPendingActionWithIndividuals | ||
| .map(filterApprovedApplicationsWithIndividuals) | ||
| .map(_.map(_.application)) | ||
|
|
||
| def findNonFixableReadyForFailureEmail(): Future[Seq[ApplicationWithIndividuals]] = getApplicationsPendingActionWithIndividuals | ||
| .map(filterNonFixableApplicationsWithIndividuals) | ||
|
|
||
| private def getApplicationsPendingActionWithIndividuals: Future[Seq[ApplicationWithIndividuals]] = | ||
| for | ||
| applications <- applicationForRiskingRepo.findApplicationsPendingAction() | ||
| individuals <- individualForRiskingRepo.findByApplicationReferences(applications.map(_.applicationReference)) | ||
| yield ApplicationWithIndividuals | ||
| .merge(applications, individuals) | ||
| .filter(_.individuals.forall(_.individualRiskingResult.isDefined)) | ||
|
|
||
| private def filterApprovedApplicationsWithIndividuals(applicationsWithIndividuals: Seq[ApplicationWithIndividuals]): Seq[ApplicationWithIndividuals] = | ||
| applicationsWithIndividuals.filter: appWithIndividuals => | ||
| RiskingOutcomeHelper | ||
| .computeRiskingOutcome(appWithIndividuals) | ||
| .exists(_ === RiskingOutcome.Approved) | ||
|
|
||
| private def filterNonFixableApplicationsWithIndividuals(applicationsWithIndividuals: Seq[ApplicationWithIndividuals]): Seq[ApplicationWithIndividuals] = | ||
| applicationsWithIndividuals | ||
| .filter: appWithIndividuals => | ||
| RiskingOutcomeHelper | ||
| .computeRiskingOutcome(appWithIndividuals) | ||
| .exists(_ === RiskingOutcome.FailedNonFixable) | ||
| .map: appWithIndividuals => | ||
| val nonFixableIndividuals = appWithIndividuals.individuals.filter: individual => | ||
| individual.individualRiskingResult.exists(_.failures.outcome === RiskingOutcome.FailedNonFixable) | ||
| ApplicationWithIndividuals(appWithIndividuals.application, nonFixableIndividuals) | ||
|
|
||
| private def filterFixableApplicationsWithIndividuals(applicationsWithIndividuals: Seq[ApplicationWithIndividuals]): Seq[ApplicationWithIndividuals] = | ||
| applicationsWithIndividuals | ||
| .filter: appWithIndividuals => | ||
| RiskingOutcomeHelper | ||
| .computeRiskingOutcome(appWithIndividuals) | ||
| .exists(_ === RiskingOutcome.FailedFixable) | ||
| .map: appWithIndividuals => | ||
| val fixableIndividuals = appWithIndividuals.individuals.filter: individual => | ||
| individual.individualRiskingResult.exists(_.failures.outcome === RiskingOutcome.FailedFixable) | ||
| ApplicationWithIndividuals(appWithIndividuals.application, fixableIndividuals) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we consider renaming EmailInformation to SendEmailRequest to better reflect that it represents the actual HTTP request used when calling the email microservice? It seems we already follow this naming convention in other connectors, so it might be good to stay consistent with that.