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

feat: show system reject errors for requisition #1238

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

DiverDori
Copy link
Collaborator

@DiverDori DiverDori commented Aug 9, 2022

PR Type

[ ] Bugfix
[ x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no API changes)
[ ] Build-related changes
[ ] CI-related changes
[ ] Documentation content changes
[ ] Application / infrastructure changes
[ ] Other:

What Is the Current Behavior?

Requisitions that have been system rejected do not show the reject reasons.

Issue Number: Closes #

What Is the New Behavior?

System reject reasons are now delivered via REST API and will be shown.

Does this PR Introduce a Breaking Change?

[ ] Yes
[ x] No

Other Information

AB#78718

@@ -51,6 +51,7 @@ export class RequisitionMapper {
},
},
systemRejected: data.systemRejected,
systemRejectErrors: data.systemRejectErrors?.map(message => message.message),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because the message is an optional field, does it make sense to map the code here if the message is missing?
Otherwise you could remove the code from the interface if you never use it

@@ -530,7 +530,7 @@
"approval.detailspage.approval.heading": "Approval Details",
"approval.detailspage.approval_date.label": "Date of Approval",
"approval.detailspage.approval_status.label": "Approval Status",
"approval.detailspage.approval_status.system_rejected.comment": "The order could not be placed because the requisition was invalid.",
"approval.detailspage.approval_status.system_rejected.comment": "The order could not be placed because the requisition was invalid. This is caused by the following reason(s):",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use num plural here like: {{ num, plural, =0{} =1{This is caused by the following reason:} other{This is caused by the following reasons:} }}
I hope it will work - I haven't checked it out

Pls change also the German and French translation or ask Mandy to do it.

@DiverDori DiverDori force-pushed the feature/show_systemreject_errors_for_requisitions branch from 3108403 to b302224 Compare August 10, 2022 08:28
<p>{{ 'approval.detailspage.approval_status.system_rejected.comment' | translate }}</p>
<p>
{{ 'approval.detailspage.approval_status.system_rejected.comment' | translate }}
<ng-container *ngIf="requisition.systemRejectErrors">
Copy link
Collaborator

Choose a reason for hiding this comment

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

requisition.systemRejectErrors?.length

<ng-container *ngIf="requisition.systemRejectErrors">
{{
'approval.detailspage.approval_status.system_rejected.reasons.comment'
| translate: { '0': requisition.systemRejectErrors?.length ?? 0 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

{ '0': requisition.systemRejectErrors.length }

@DiverDori DiverDori force-pushed the feature/show_systemreject_errors_for_requisitions branch from b302224 to 34773c6 Compare August 10, 2022 15:08
@shauke shauke requested a review from SGrueber August 11, 2022 09:32
@SGrueber SGrueber added the enhancement Enhancement to an existing feature label Aug 11, 2022
@SGrueber SGrueber added this to the 3.0 milestone Aug 11, 2022
@SGrueber
Copy link
Collaborator

PR will be merged as soon as the missing translations have been added.

mglatter
mglatter previously approved these changes Aug 11, 2022
Copy link
Contributor

@mglatter mglatter left a comment

Choose a reason for hiding this comment

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

Localization approved

SGrueber
SGrueber previously approved these changes Aug 11, 2022
@DiverDori DiverDori dismissed stale reviews from SGrueber and mglatter via 14d6886 August 11, 2022 13:39
@SGrueber SGrueber self-requested a review August 11, 2022 14:30
@SGrueber SGrueber merged commit ace4c77 into develop Aug 11, 2022
@SGrueber SGrueber deleted the feature/show_systemreject_errors_for_requisitions branch August 11, 2022 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants