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

User Cannot Edit Display Name #17606

Closed
turtlepod opened this Issue Nov 7, 2017 · 13 comments

Comments

Projects
None yet
6 participants
@turtlepod
Member

turtlepod commented Nov 7, 2017

Prerequisites

  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate
  • The issue still exists against the latest master branch of WooCommerce on Github (this is not the same version as on WordPress.org!)
  • I have attempted to find the simplest possible steps to reproduce the issue
  • I have included a failing test as a pull request (Optional)

Steps to reproduce the issue

  1. In My Account Page the welcome message are

"Hello {name} (not {name}? Log out)"

The {name} is using display name.

  1. But when they edit their profile via "Account Details" (endpoint), the options are only "first name" and "last name"

So, they will never able to change their display name.

Possible solution: also update display name, maybe using "{first name} {last name}" format on account details save action.

Use case: miss type my name as "Favid" when I register, and try to update my name via account details buy the display name in dashboard message remain the same.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Nov 7, 2017

Member

How about just not using display name on account pages?

Member

mikejolley commented Nov 7, 2017

How about just not using display name on account pages?

@claudiosanches

This comment has been minimized.

Show comment
Hide comment
@claudiosanches

claudiosanches Nov 7, 2017

Member

Possible solution: also update display name, maybe using "{first name} {last name}" format on account details save action.

This is not an option, since we had trouble before: #15020

But since display name can be used in comments and other public locations, probably easier including a new option.

Member

claudiosanches commented Nov 7, 2017

Possible solution: also update display name, maybe using "{first name} {last name}" format on account details save action.

This is not an option, since we had trouble before: #15020

But since display name can be used in comments and other public locations, probably easier including a new option.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Nov 7, 2017

Member

@claudiosanches I mean on private account pages use actual name. Display name is for public places.

Member

mikejolley commented Nov 7, 2017

@claudiosanches I mean on private account pages use actual name. Display name is for public places.

@claudiosanches

This comment has been minimized.

Show comment
Hide comment
@claudiosanches

claudiosanches Nov 7, 2017

Member

@mikejolley still this not covers if your name is wrong, since we don't allow to edit the display name once that is not an email.

Not to mention the behavior of saving a display name is not the same in all places:

wp_update_user( apply_filters( 'woocommerce_update_customer_args', array(
'ID' => $customer->get_id(),
'role' => $customer->get_role(),
'display_name' => $customer->get_first_name() . ' ' . $customer->get_last_name(),
), $customer ) );

// If the display name is an email, update to the user's full name.
if ( is_email( $customer->get_display_name() ) ) {
$customer->set_display_name( $data['billing_first_name'] . ' ' . $data['billing_last_name'] );
}

Both uses first and last names, but on My Account it's different:

// Prevent emails being displayed, or leave alone.
$user->display_name = is_email( $current_user->display_name ) ? $user->first_name : $current_user->display_name;

And another problem is that in none of that places are considered formatting for some languages/countries, like we do in:

/**
* Get a formatted billing full name.
*
* @return string
*/
public function get_formatted_billing_full_name() {
/* translators: 1: first name 2: last name */
return sprintf( _x( '%1$s %2$s', 'full name', 'woocommerce' ), $this->get_billing_first_name(), $this->get_billing_last_name() );
}

Member

claudiosanches commented Nov 7, 2017

@mikejolley still this not covers if your name is wrong, since we don't allow to edit the display name once that is not an email.

Not to mention the behavior of saving a display name is not the same in all places:

wp_update_user( apply_filters( 'woocommerce_update_customer_args', array(
'ID' => $customer->get_id(),
'role' => $customer->get_role(),
'display_name' => $customer->get_first_name() . ' ' . $customer->get_last_name(),
), $customer ) );

// If the display name is an email, update to the user's full name.
if ( is_email( $customer->get_display_name() ) ) {
$customer->set_display_name( $data['billing_first_name'] . ' ' . $data['billing_last_name'] );
}

Both uses first and last names, but on My Account it's different:

// Prevent emails being displayed, or leave alone.
$user->display_name = is_email( $current_user->display_name ) ? $user->first_name : $current_user->display_name;

And another problem is that in none of that places are considered formatting for some languages/countries, like we do in:

/**
* Get a formatted billing full name.
*
* @return string
*/
public function get_formatted_billing_full_name() {
/* translators: 1: first name 2: last name */
return sprintf( _x( '%1$s %2$s', 'full name', 'woocommerce' ), $this->get_billing_first_name(), $this->get_billing_last_name() );
}

@widianto

This comment has been minimized.

Show comment
Hide comment
@widianto

widianto Nov 9, 2017

Contributor

Hi guys, I want to contribute to woocommerce project. This is my first opensource project ever.

So what's the possible best solution to solve this issue?

  1. Providing edit display name on my-account edit page? (You say this is allowed by Wordpress, right?
  2. Or, displaying $user->first_name on Dashboard page?

Need guiding. Thanks!

Contributor

widianto commented Nov 9, 2017

Hi guys, I want to contribute to woocommerce project. This is my first opensource project ever.

So what's the possible best solution to solve this issue?

  1. Providing edit display name on my-account edit page? (You say this is allowed by Wordpress, right?
  2. Or, displaying $user->first_name on Dashboard page?

Need guiding. Thanks!

@KoolPal

This comment has been minimized.

Show comment
Hide comment
@KoolPal

KoolPal Nov 12, 2017

Hi all,

My 2 cents.

Perhaps Woocommerce should pick a thing or two from Amazon considering their headstart in ecommerce?

Woocommerce should emulate best practices from elsewhere and add value by fixing the painpoints from other platforms. Then Woocommerce will truly shine!

In Amazon, Login & Security ( Name, email & password) section can be equated with Woocommerce Account Details page. Add display name field and make it editable in this Account Details page and indicate that this would be used for reviews, etc. Do not modify this display name on every order. This will address the concern of a B2B Store Owner who used the display name to store the Company Name. https://github.com/woocommerce/woocommerce/issues/15020#issuecomment-302226494

Show Account First Name / Last Name in Dashboard Name. This will not get modified by changes in Billing First Name / Last Name during any order checkout.

In Amazon changes to billing and shipping address DO NOT modify the customer's account details. This should be replicated in Woocommerce

For reviews, Amazon permits change in display name. This could be explored for replication in Woocommerce

Thanks.

KoolPal commented Nov 12, 2017

Hi all,

My 2 cents.

Perhaps Woocommerce should pick a thing or two from Amazon considering their headstart in ecommerce?

Woocommerce should emulate best practices from elsewhere and add value by fixing the painpoints from other platforms. Then Woocommerce will truly shine!

In Amazon, Login & Security ( Name, email & password) section can be equated with Woocommerce Account Details page. Add display name field and make it editable in this Account Details page and indicate that this would be used for reviews, etc. Do not modify this display name on every order. This will address the concern of a B2B Store Owner who used the display name to store the Company Name. https://github.com/woocommerce/woocommerce/issues/15020#issuecomment-302226494

Show Account First Name / Last Name in Dashboard Name. This will not get modified by changes in Billing First Name / Last Name during any order checkout.

In Amazon changes to billing and shipping address DO NOT modify the customer's account details. This should be replicated in Woocommerce

For reviews, Amazon permits change in display name. This could be explored for replication in Woocommerce

Thanks.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Nov 13, 2017

Member

Hi @widianto

It looks like we need to (based on #17606 (comment))

  1. Leave the is_email display name handling code in place to prevent these being leaked
  2. When setting display name, format it like we do in get_formatted_billing_full_name so translations can change how name is formatted
  3. Change the display name setting in class-wc-customer-data-store.php to avoid setting to first/last name
  4. Add display name to the customer class, data store, and as a new field on the account edit page.

Thats should handle everything.

Member

mikejolley commented Nov 13, 2017

Hi @widianto

It looks like we need to (based on #17606 (comment))

  1. Leave the is_email display name handling code in place to prevent these being leaked
  2. When setting display name, format it like we do in get_formatted_billing_full_name so translations can change how name is formatted
  3. Change the display name setting in class-wc-customer-data-store.php to avoid setting to first/last name
  4. Add display name to the customer class, data store, and as a new field on the account edit page.

Thats should handle everything.

@KoolPal

This comment has been minimized.

Show comment
Hide comment
@KoolPal

KoolPal Nov 13, 2017

@mikejolley I am curious by your obvious disregard for my suggestion. Is there some criteria by which my suggestion failed to elicit a response?

KoolPal commented Nov 13, 2017

@mikejolley I am curious by your obvious disregard for my suggestion. Is there some criteria by which my suggestion failed to elicit a response?

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Nov 13, 2017

Member

@KoolPal My suggestion is inline with your suggestion.

Member

mikejolley commented Nov 13, 2017

@KoolPal My suggestion is inline with your suggestion.

@douglsmith

This comment has been minimized.

Show comment
Hide comment
@douglsmith

douglsmith Nov 14, 2017

Contributor

Keep in mind that the display name is also used to represent the user in WordPress comments and other plugins like bbPress and BuddyPress. We need to make sure any changes to the display name are okay with those uses too.

WordPress has a dropdown menu that only allows setting the display name to the username, first name, last name, first + last, or last + first. I don't know why they have those limitations, though. It doesn't stop spoofing someone because a user could just change their first or last name as desired.

Contributor

douglsmith commented Nov 14, 2017

Keep in mind that the display name is also used to represent the user in WordPress comments and other plugins like bbPress and BuddyPress. We need to make sure any changes to the display name are okay with those uses too.

WordPress has a dropdown menu that only allows setting the display name to the username, first name, last name, first + last, or last + first. I don't know why they have those limitations, though. It doesn't stop spoofing someone because a user could just change their first or last name as desired.

@widianto

This comment has been minimized.

Show comment
Hide comment
@widianto

widianto Nov 20, 2017

Contributor

@mikejolly and all, thanks for clearing out.

Again, this is my first ever contribution to open source project. I hope I can understand how to make contribution correctly and following good practices.

First, current flow are:

  • Display name created automatically from part of user's email when they register as a new user (only providing email and password)
  • So on any edit account page, user will have display_name in place, but empty First Name, Last name

My fixes:

  1. In my assumption, I need to change the code in class-wc-form-handler.php, but I still leave the is_email as you state above.
  2. For customer class, I don’t change it, because it already have for getting and setting display_name properly.
  3. I did some changes to provide update to display_name in these files:
    - includes/class-wc-form-handler.php
    - includes/data-stores/class-wc-customer-data-store.php
    - templates/myaccount/form-edit-account.php

Here is my commit:
widianto@44425d2

It provides editing for display name and saving them.

Please let me know if you have any suggestion or if I make any mistakes in that code. I'll make new branch fix if necessary to provide correct pull request later.

Contributor

widianto commented Nov 20, 2017

@mikejolly and all, thanks for clearing out.

Again, this is my first ever contribution to open source project. I hope I can understand how to make contribution correctly and following good practices.

First, current flow are:

  • Display name created automatically from part of user's email when they register as a new user (only providing email and password)
  • So on any edit account page, user will have display_name in place, but empty First Name, Last name

My fixes:

  1. In my assumption, I need to change the code in class-wc-form-handler.php, but I still leave the is_email as you state above.
  2. For customer class, I don’t change it, because it already have for getting and setting display_name properly.
  3. I did some changes to provide update to display_name in these files:
    - includes/class-wc-form-handler.php
    - includes/data-stores/class-wc-customer-data-store.php
    - templates/myaccount/form-edit-account.php

Here is my commit:
widianto@44425d2

It provides editing for display name and saving them.

Please let me know if you have any suggestion or if I make any mistakes in that code. I'll make new branch fix if necessary to provide correct pull request later.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Nov 21, 2017

Member

@widianto Send that as a pull request and we can add feedback directly.

Member

mikejolley commented Nov 21, 2017

@widianto Send that as a pull request and we can add feedback directly.

@mikejolley

This comment has been minimized.

Show comment
Hide comment
@mikejolley

mikejolley Dec 13, 2017

Member

This has a PR (almost) ready for 3.4 next year. See #17861

Member

mikejolley commented Dec 13, 2017

This has a PR (almost) ready for 3.4 next year. See #17861

@mikejolley mikejolley closed this Dec 13, 2017

claudiosanches added a commit that referenced this issue Feb 22, 2018

Merge pull request #17861 from widianto/add/17606
Provides edit display name on my-account page (#17606)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment