DTR-3713: CIS MRF Business Functions F18d,18f,18g#68
Open
yanshengzhangHMRC wants to merge 6 commits intomainfrom
Open
DTR-3713: CIS MRF Business Functions F18d,18f,18g#68yanshengzhangHMRC wants to merge 6 commits intomainfrom
yanshengzhangHMRC wants to merge 6 commits intomainfrom
Conversation
|
jamalosman
reviewed
Mar 10, 2026
app/uk/gov/hmrc/constructionindustryscheme/connectors/ChrisConnector.scala
Outdated
Show resolved
Hide resolved
app/uk/gov/hmrc/constructionindustryscheme/connectors/ChrisConnector.scala
Outdated
Show resolved
Hide resolved
|
jassalrichy
reviewed
Mar 10, 2026
| def createMonthlyNilReturnRequestJson(payload: BuiltSubmissionPayload): JsValue = | ||
| XmlToJsonConvertor.convertXmlToJson(payload.envelope.toString) match { | ||
| case XmlConversionResult(true, Some(json), _) => json | ||
| case XmlConversionResult(false, _, Some(error)) => Json.obj("error" -> error) |
Contributor
There was a problem hiding this comment.
unit test for case XmlConversionResult(false, _, Some(error))
Contributor
Author
There was a problem hiding this comment.
Hi @jassalrichy , I think it's already covered in unit test below, but kindly let me know if concern persists
"return error JSON when XML conversion fails"
jassalrichy
reviewed
Mar 10, 2026
| XmlToJsonConvertor.convertXmlToJson(payload.envelope.toString) match { | ||
| case XmlConversionResult(true, Some(json), _) => json | ||
| case XmlConversionResult(false, _, Some(error)) => Json.obj("error" -> error) | ||
| case _ => Json.obj("error" -> "unexpected conversion failure") |
Contributor
Author
There was a problem hiding this comment.
Hi @jassalrichy , I think it's already covered in unit test below, but kindly let me know if concern persists
"return unexpected conversion failure when neither json nor error provided"
jassalrichy
reviewed
Mar 10, 2026
| res: SubmissionResult | ||
| )(implicit hc: HeaderCarrier): Future[Result] = | ||
| validateCorrelationId(correlationId, res.meta.correlationId) match { | ||
| case Left(reason) => |
Contributor
There was a problem hiding this comment.
unit test for case Left(reason)
Contributor
Author
There was a problem hiding this comment.
added now
jassalrichy
reviewed
Mar 10, 2026
| ) | ||
| } | ||
| } | ||
| .recover { case ex => |
Contributor
Author
There was a problem hiding this comment.
added now
jassalrichy
reviewed
Mar 10, 2026
| val sent = sentRaw.trim | ||
| val ack = ackRaw.trim | ||
|
|
||
| if (sent.isEmpty || ack.isEmpty) Left("empty correlationId") |
Contributor
There was a problem hiding this comment.
unit test for if (sent.isEmpty || ack.isEmpty)
unit test for if (sent != ack)
Contributor
Author
There was a problem hiding this comment.
added now
jassalrichy
reviewed
Mar 10, 2026
Contributor
There was a problem hiding this comment.
We point to stubs by default in application.conf
These ports are overridden by using appplication.no.stubs.conf
Contributor
Author
There was a problem hiding this comment.
forget to deselect them in the last commit, updated now
jassalrichy
approved these changes
Mar 10, 2026
jamalosman
approved these changes
Mar 11, 2026
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.