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

ISREST-1006: add sepa component #165

Closed
wants to merge 5 commits into from

Conversation

skoch-intershop
Copy link
Contributor

@skoch-intershop skoch-intershop commented Mar 18, 2020

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

@dhhyi dhhyi added the feature New feature or request label Mar 18, 2020
@SGrueber SGrueber assigned skoch-intershop and SGrueber and unassigned SGrueber Mar 18, 2020
@@ -0,0 +1,20 @@
export class HostedPaymentPageParametersMapper {
static fromData(body: { name: string; value: string }[]): { name: string; value: string }[] {
Copy link
Collaborator

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) {
Copy link
Collaborator

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) {
Copy link
Collaborator

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
Copy link
Collaborator

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">
Copy link
Collaborator

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 {
Copy link
Collaborator

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 {
Copy link
Collaborator

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() {
Copy link
Collaborator

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
Copy link
Collaborator

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

@@ -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!",
Copy link
Collaborator

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

@skoch-intershop skoch-intershop force-pushed the feature/add_concardis_sepa_component branch from cdb0045 to 117d994 Compare March 26, 2020 06:59
@SGrueber SGrueber force-pushed the feature/add_concardis_sepa_component branch 2 times, most recently from 04f4efb to 1c667e1 Compare March 31, 2020 09:29
shauke pushed a commit that referenced this pull request Mar 31, 2020
- requires CONCARDIS 1.7.2 release and an ICM release 7.10.16.6+
@shauke
Copy link
Collaborator

shauke commented Apr 1, 2020

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.

shauke pushed a commit that referenced this pull request Apr 1, 2020
- requires CONCARDIS 1.7.2 release and an ICM release 7.10.16.6+
@shauke
Copy link
Collaborator

shauke commented Apr 1, 2020

Merged with 0.18.1 hotfix release changes in the develop branch (c0db224).

@shauke shauke closed this Apr 1, 2020
@shauke shauke deleted the feature/add_concardis_sepa_component branch April 14, 2020 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants