Skip to content
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

Send payment update message after creating case from exception record #673

Conversation

rkondratowicz
Copy link
Contributor

data.put("scanOCRData", TestCaseBuilder.ocrDataEntry("key", "value"));
data.put(ExceptionRecordFields.CONTAINS_PAYMENTS, YesNoFieldValues.NO); // no payments!
data.put(ExceptionRecordFields.ENVELOPE_ID, envelopeId);
data.put(ExceptionRecordFields.PO_BOX_JURISDICTION,jurisdiction);
Copy link
Contributor

Choose a reason for hiding this comment

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

building case data can be extracted and used in 2 tests

Copy link
Contributor

Choose a reason for hiding this comment

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

This is bound to happen some time for sure. It's all over the place. I've started in separate branch but never had a chance to progress further as more changes coming in


boolean containsPayments =
Objects.equals(
exceptionRecord.getData().get(ExceptionRecordFields.CONTAINS_PAYMENTS).toString(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we extract exceptionRecord.getData() into a variable ? It’s repeated thrice

private final CoreCaseDataApi ccdApi;

public CreateCaseCallbackService(
CreateCaseValidator validator,
ServiceConfigProvider serviceConfigProvider,
TransformationClient transformationClient,
AuthTokenGenerator s2sTokenGenerator,
PaymentsPublisher paymentsPublisher,
Copy link
Contributor

Choose a reason for hiding this comment

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

If to just reduce line change count then no point - payment happened after ccd api call + extra addition to argument list. But I'm just being picky here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't realise we were sorting class dependencies by usage order ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

we're not. just thought i'm gonna be picky and guess the reason

.map(exceptionRecord -> createNewCase(
exceptionRecord,
serviceConfigItem,
request.isIgnoreWarnings(),
idamToken,
userId
userId,
exceptionRecordData
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes no point in having exception record created prior. Can be reviewed later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sadly exceptionRecord is the request for transformation endpoint, not entire exception record we get from CCD

data.put("scanOCRData", TestCaseBuilder.ocrDataEntry("key", "value"));
data.put(ExceptionRecordFields.CONTAINS_PAYMENTS, YesNoFieldValues.NO); // no payments!
data.put(ExceptionRecordFields.ENVELOPE_ID, envelopeId);
data.put(ExceptionRecordFields.PO_BOX_JURISDICTION,jurisdiction);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is bound to happen some time for sure. It's all over the place. I've started in separate branch but never had a chance to progress further as more changes coming in

jurisdiction
)
);
paymentsPublisher.send(
Copy link
Contributor

Choose a reason for hiding this comment

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

it did raise me an eyebrow before 🙃

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview October 18, 2019 13:47 Abandoned
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview October 18, 2019 14:16 Abandoned
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview October 18, 2019 14:24 Abandoned
@rkondratowicz
Copy link
Contributor Author

closing and opening a new one - helm issues.

@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview October 18, 2019 15:42 Abandoned
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview October 18, 2019 15:46 Abandoned
@jenkins-reform-hmcts2 jenkins-reform-hmcts2 requested a deployment to preview October 18, 2019 16:36 Abandoned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants