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

IBAN validation for SEPA account #6107

Merged
merged 5 commits into from Mar 18, 2022
Merged

IBAN validation for SEPA account #6107

merged 5 commits into from Mar 18, 2022

Conversation

ghost
Copy link

@ghost ghost commented Mar 16, 2022

Fixes bisq-network/growth#241
Superseds #5419

Changes:

  • when user type valid IBAN address but from country outside SEPA zone (f.e. Georgia) he see validation error: SEPA is not supported in this country
  • when user type valid SEPA zone IBAN country of bank is automatically updated
  • when user switch country of bank to value that doesn't match IBAN he see validation error: Country code invalid below IBAN field

@ghost ghost changed the title Validate IBAN to match selected country IBAN validation for SEPA account Mar 16, 2022
@alkum
Copy link
Contributor

alkum commented Mar 16, 2022

Reviewed, could confirm all changes except the last one:

when user switch country of bank to value that doesn't match IBAN he see validation error: Country code must match country of bank below IBAN field

That error is indeed shown, but ignored. A SEPA account can still be created, despite this error being shown.

(With the other error, SEPA is not supported in this country, the account cannot be created. The "Save New Account" button is disabled while the error is shown, re-enabled when error is fixed.)

Tested using sample IBANs from https://www.iban.com/structure

@ghost
Copy link
Author

ghost commented Mar 16, 2022

@alkum Thanks - please check now.

Also please wait with merge - I'll adapt Sepa Instant Form once everything will be okay on standard Sepa

@alkum
Copy link
Contributor

alkum commented Mar 16, 2022

Checked again, validation works for standard SEPA accounts.

@@ -120,14 +127,34 @@ public void addFormForAddAccount() {
sepaAccount.setCountry(country);
}

ibanInputTextField.textProperty().addListener((ov, oldValue, newValue) -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why you introduce another listener to the textProperty?

Copy link
Author

Choose a reason for hiding this comment

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

Not actually - I merged those listeners into one.

Listener is not added directly after ibanInputTextField declaration because it needs countryComboBox

@ghost ghost requested a review from ripcurlx March 17, 2022 16:21
Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK - Remaining code changes are looking fine. utACK also based on #6107 (comment)

@ripcurlx ripcurlx added this to the v1.8.5 milestone Mar 18, 2022
@ripcurlx ripcurlx merged commit 06dd94f into bisq-network:master Mar 18, 2022
@ghost ghost mentioned this pull request Mar 25, 2022
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.

Match SEPA accounts IBANs to selected participating countries
2 participants