-
Notifications
You must be signed in to change notification settings - Fork 87
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
ISREST-1006: add sepa component #165
Conversation
@@ -0,0 +1,20 @@ | |||
export class HostedPaymentPageParametersMapper { | |||
static fromData(body: { name: string; value: string }[]): { name: string; value: string }[] { |
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.
if this method is only used by the payment-method.mapper it should be moved to the payment-method.mapper
test and a short documentation is missing,
it is not clear, if this is a specific method for concardis direct debit a generic functionality, why is it needed?
static fromData(body: { name: string; value: string }[]): { name: string; value: string }[] { | ||
const hostedPaymentPageParameters: { name: string; value: string }[] = new Array(); | ||
|
||
if (body !== undefined) { |
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.
if (body && body.length
const hostedPaymentPageParameters: { name: string; value: string }[] = new Array(); | ||
|
||
if (body !== undefined) { | ||
for (const entry of body) { |
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.
i think you can use map here,
hostedPaymentPageParameters = body.map ( entry => { do some mapping here);
if not let's talk, don't use push
</div></li | ||
></ng-container> | ||
</div> | ||
</li></ng-container |
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.
formatting
@@ -0,0 +1,19 @@ | |||
<formly-form [form]="parameterForm" [options]="options" [model]="model" [fields]="getFieldConfig()"> | |||
<div class="offset-md-4 col-md-8"> |
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.
<div class="form-group">
is missing - padding at the bottom
* gets a parameter value from payment method | ||
* sets the general error message (key) if the parameter is not available | ||
*/ | ||
private getParamValue(name: string, errorMessage: string): string { |
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.
move method to a parent and make it protected
/** | ||
* determine errorMessages on the basis of the error code | ||
*/ | ||
getErrorMessage(code: number, fieldType: string, defaultMessage: string): string { |
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.
move to parent component
/** | ||
* cancel new payment instrument, hides and resets the parameter form | ||
*/ | ||
cancelNewPaymentInstrument() { |
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.
move to parent component
@@ -0,0 +1,9 @@ | |||
<div *ngIf="field.fieldGroupClassName" class="{{ field.fieldGroupClassName }}"> | |||
<ish-checkbox |
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.
<div class="form-group"
is missing
src/assets/i18n/en_US.json
Outdated
@@ -2679,5 +2679,17 @@ | |||
"quote.already_in_basket.error": "The quote is already in the cart.", | |||
"quote.not_found.error": "The quote could not be found.", | |||
"basket.add_quote.error": "The quote could not be added to the cart.", | |||
"quoterequest.not_editable.error": "You have already submitted the quote request. Please <a href=\"javascript:location.reload()\">reload</a> this page to view the changes." | |||
"quoterequest.not_editable.error": "You have already submitted the quote request. Please <a href=\"javascript:location.reload()\">reload</a> this page to view the changes.", | |||
"checkout.sepa.iban.error.default": "Field is required and should not be empty!", |
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.
pls, use existing error message keys, if possible, e.g. checkout.payment.debit.transfer.account.number.missing.error
if it is not possible for some keys, ask docu team for assistence and provide also German and French translations
cdb0045
to
117d994
Compare
04f4efb
to
1c667e1
Compare
...app/pages/checkout-payment/checkout-payment/payment-concardis/payment-concardis.component.ts
Outdated
Show resolved
Hide resolved
...app/pages/checkout-payment/checkout-payment/payment-concardis/payment-concardis.component.ts
Outdated
Show resolved
Hide resolved
1c667e1
to
a9b2b37
Compare
- requires CONCARDIS 1.7.2 release and an ICM release 7.10.16.6+
Changes cherry picked to 0.18.1 hotfix release branch (https://github.com/intershop/intershop-pwa/tree/chore/release_0.18.1). Will be released and merged via the hotfix branch. |
- requires CONCARDIS 1.7.2 release and an ICM release 7.10.16.6+
Merged with 0.18.1 hotfix release changes in the develop branch (c0db224). |
PR Type
[x] Feature
requires CONCARDIS 1.7.2 release ( and an ICM release 7.10.16.6+ )
What Is the Current Behavior?
It is not possible to create a Concardis SEPA Direct Debit payment instrument on the checkout payment page.
Issue Number: ISREST-1006
What Is the New Behavior?
Creation of a Concardis SEPA Direct Debit payment instrument is possible on the checkout payment page.
Does this PR Introduce a Breaking Change?
[ ] Yes
[x] No