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

Update MoneyBeam account, add HolderName #6968

Merged

Conversation

ghost
Copy link

@ghost ghost commented Dec 16, 2023

Same procedure as #6965.

  • Add HolderName, mandatory for new accounts.
  • Older, existing MoneyBeam accounts do not have HolderName, and retain their age status.
  • If buying from an old account that does not have HolderName, Trader Chat can be used.

@pazza83


moneybeam-1


moneybeam-3


moneybeam-2

Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

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

Thank you for opening this PR @yonson2023!

Here are the needed changes to get your PR merged:

  1. The start payment screen shows an empty "Account owner full name" text field for old accounts during a trade. This could confuse users and increase the number of support requests. It'd be better to hide this field for old accounts (without a holder name).
    start_payment_hide_account_owner_field_if_empty

  2. The "Account owner full name" text field is empty for old accounts on the "Manage accounts" screen (Accounts -> Manage Accounts -> Select an old MoneyBeam account). It's better to hide the text field for empty holder names (old accounts).
    dont_show_holder_name_test_field_if_old_account

  3. The text fields on the "Start payment using ..." screen are below each other. This is inconsistent to our other account types. There should be two text fields next to each other (see SEPA example).

MoneyBeam:

moneybeam_new_screen_fields_below_each_other

SEPA:

sepa_text_fields_next_each_other

@ghost ghost closed this Jan 9, 2024
@ghost ghost force-pushed the money_beam_add_holder_name branch from 9b507b5 to 3f8fedd Compare January 9, 2024 23:45
@ghost ghost reopened this Jan 9, 2024
@ghost
Copy link
Author

ghost commented Jan 10, 2024

text fields on the "Start payment using ..." screen are below each other. This is inconsistent to our other account types. There should be two text fields next to each other (see SEPA example).

Fixed.


Regarding cases when an old account supplies no Account Holder Name. I think it is important to show that the field was not supplied (is empty) rather than hiding it from view. It prompts the user to either (a) update/recreate his own account with the necessary info, or (b) contact his trading peer via trader chat to request the info for successful payment verification (which also lets the trading peer know they need to update/recreate their account information).

@pazza83
Copy link

pazza83 commented Jan 10, 2024

If the account has not provided a name because their account was created prior to name being required please can we show "Ask trader to provide account name if needed"

This could shown in the account summary, payment receipt, payment confirmation, and support ticket screens etc.

@alvasw
Copy link
Contributor

alvasw commented Jan 10, 2024

Regarding cases when an old account supplies no Account Holder Name. I think it is important to show that the field was not supplied (is empty) rather than hiding it from view. It prompts the user to either (a) update/recreate his own account with the necessary info, or (b) contact his trading peer via trader chat to request the info for successful payment verification (which also lets the trading peer know they need to update/recreate their account information).

What do you think about writing into text field that the user can ask the other party his holder name? Additionally, we can add text on the trader's side with the old MoneyBeam account that he has to tell the other party his holder name when asked.

If the account has not provided a name because their account was created prior to name being required please can we show "Ask trader to provide account name if needed"

This could shown in the account summary, payment receipt, payment confirmation, and support ticket screens etc.

I looked into the account aging/signing code. We could migrate all "MoneyBeam" accounts to "MoneyBeam accounts with holder name" while retaining the old account age and adding the holder name to the salt. Afterward, we can show a popup before starting/taking a trade to users with old accounts to enter their holder name. But it's too much for this PR.

@ghost
Copy link
Author

ghost commented Jan 12, 2024

Done.

moneybeam1

@ghost ghost force-pushed the money_beam_add_holder_name branch from e41d74f to 9e5fa8f Compare January 12, 2024 21:41
Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

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

ACK

Copy link
Contributor

@alejandrogarcia83 alejandrogarcia83 left a comment

Choose a reason for hiding this comment

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

utACK

@alejandrogarcia83 alejandrogarcia83 merged commit ccd4b2c into bisq-network:master Jan 14, 2024
3 checks passed
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.

None yet

3 participants