JASPER-788: Signing and Digital Workflow: Update Order payload to reflect updates to the payload structure from CSO (PART 2)#1030
Conversation
| } | ||
|
|
||
| [Fact] | ||
| public async Task SubmitOrder_ReturnsFailure_WhenCsoSubmitFails() |
There was a problem hiding this comment.
The changes to this test seem to change what is being tested here:
- original test, mimic an error response from CSO and how it is handled.
- new test, return a success from CSO but an error mapping the response.
Recommend at least updating the name of this test, and adding a test to check that CSO returning an error is handled.
| return null; | ||
| } | ||
|
|
||
| var userGuid = _httpContextAccessor.HttpContext?.User?.ProvjudUserGuid(); |
There was a problem hiding this comment.
just want to make sure this isn't a functional regression.
The old logic would default to the logged in user's guid and fall back to the order's guid, which would allow for some delegation or impersonation.
The new logic always uses the order's judge identifier. That means that if I review an order as an admin impersonating a user (ie. with the judge selector), that order will look like it was reviewed by the order's user and not the impersonating admin.
| actionDto.OrderTerms ??= Array.Empty<OrderTerm>(); | ||
| var actionDto = Mapper.Map<JudicialAction>(orderDto); | ||
| actionDto.OrderTerms ??= []; | ||
| actionDto.ReviewedBy = new Reviewer |
There was a problem hiding this comment.
do we really want to use getValueOrDefault in these cases?
I think a default will send an identifier of 0 to CSO.
We may be better off throwing these failures as errors to fail fast.
| && o.OrderRequest.Referral.SentToPartId.Equals(dto.Referral.SentToPartId) | ||
| && o.OrderRequest.Referral.ReferredDocumentId == dto.Referral.ReferredDocumentId); | ||
| && o.OrderRequest.Referral.ReferredDocumentId == dto.Referral.ReferredDocumentId | ||
| && (o.Status == OrderStatus.Pending || o.Status == OrderStatus.AwaitingDocumentation)); |
There was a problem hiding this comment.
interesting, so this allows an order to be completed, and then re-submitted and completed again?
I didn't think that was supported by CSO... Just curious, have you tested this path?
| /// <exception cref="ApiException">A server side error occurred.</exception> | ||
| public virtual async System.Threading.Tasks.Task<FileResponse> GetJudicialDocumentAsync(System.Guid transaction_Id, double? agencyId, double documentId, DocumentApplicationName documentApplicationName, string applicationCode, string documentFlatten, System.Threading.CancellationToken cancellationToken) | ||
| { | ||
| if (documentId == null) |
There was a problem hiding this comment.
this is a bit weird for generated code - as double is a non-nullable value type. Perhaps something wrong with the client generation settings?
| config.NewConfig<OrderDto, JudicialAction>() | ||
| .Map(dest => dest.SignatureApplied, src => src.Signed) | ||
| .Map(dest => dest.Comment, src => src.Comments) | ||
| .Map(dest => dest.Document, src => FromBase64OrNull( |
There was a problem hiding this comment.
Similar question here, does it make sense to map to null? I feel like that will result in potential loss of document content - which is probably an error and not something we can ignore and map to null. Especially if we aren't logging that the base64 conversion failed.
ronaldo-macapobre
force-pushed
the
feature/JASPER-788-part-2
branch
from
dbcf809 to
cc75c71
Compare
ronaldo-macapobre
requested a deployment
to
lz-test
GitHub Actions
Waiting
ronaldo-macapobre
requested a deployment
to
lz-test
GitHub Actions
Waiting
There was a problem hiding this comment.
Majority of the change is due to file scope namespace update.
ronaldo-macapobre
requested a deployment
to
lz-prod
GitHub Actions
Waiting
ronaldo-macapobre
requested a deployment
to
lz-prod
GitHub Actions
Waiting
ronaldo-macapobre
force-pushed
the
feature/JASPER-788-part-2
branch
from
1356656 to
9adc831
Compare
ronaldo-macapobre
requested a deployment
to
lz-test
GitHub Actions
Waiting
ronaldo-macapobre
requested a deployment
to
lz-prod
GitHub Actions
Waiting
ronaldo-macapobre
requested a deployment
to
lz-prod
GitHub Actions
Waiting
| @@ -0,0 +1,20 @@ | |||
| { | |||
There was a problem hiding this comment.
can you provide some context as to why this was included in this PR?
ronaldo-macapobre
force-pushed
the
feature/JASPER-788-part-2
branch
from
245fc7f to
2c55020
Compare
ronaldo-macapobre
had a problem deploying
to
lz-dev
GitHub Actions
Failure
- Implement OrderDocumentStrategy to handle Order related files. - Update OrderService to populate info that used to be available from previous schema - Import Judicial client from CSO team that will replace the mock code.
- Assign raj.PersonId to order.JudgeId in OrderReminderJob
- Convert DocumentData/SupportingDocumentData to Byte[] when mapping OrderDto to JudicialAction - Only allow updating of Orders for Pending and AwaitingDocumentation status
- Replace PasswordTokenRequest to ClientCredentialsToken
- Replace PasswordTokenRequest to ClientCredentialsToken
- Ensure guid is uppercase for both provjud and idir
- Update RejectedDate, SignedDate and ActionDate as DateTime types. RejectedDate, SignedDate is changed to nullable. - Fix UserSeeder to handle JudgeId
- Update mapping for base64 to throw exception if value is null
ronaldo-macapobre
force-pushed
the
feature/JASPER-788-part-2
branch
from
2c55020 to
eef5d95
Compare
|
2 participants
Pull Request for JIRA Ticket: JASPER-788
Issue ticket number and link
https://jira.justice.gov.bc.ca/browse/JASPER-788
Description
.vscode/launch.jsonfor enable debugging .NET code easier.❗ The only missing piece is where we will be sourcing the
PaasSeqNoas we pulling this value from the request payload which seems incorrect.Type of change
How Has This Been Tested?
Checklist: