-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Restore firstname lastname on address #6159
base: main
Are you sure you want to change the base?
Restore firstname lastname on address #6159
Conversation
- Unignored `firstname` and `lastname` field in the address model. - Updated validations, attributes, and views to handle 'firstname' and 'lastname' separately. - Modified tests, factories, and locales to align with the new structure. - Introduced a 'set_full_name' method callback to concatenate 'firstname' and 'lastname' for backward compatibility.
Update the address handling logic in the APIs and tests to for 'firstname' and 'lastname' fields, instead of using a single 'name' field.
- Update the backend logic to separate 'name', 'firstname' and 'lastname' for better data handling. - Adjusted user and address forms, search functionality, and related views to use the new fields.
Update the address handling logic in the admin components and tests for 'firstname' and 'lastname' fields, instead of using a single 'name' field.
I'd like this to be optional. Either dependent on the store, or as a global configuration - something like store.separate_first_and_last_name or Spree::Config.separate_first_and_last_name The way it's currently implemented, the store I work with that works perfectly fine with a single name field will have their Furthermore, I think The configuration can also be used to display / hide the |
Yeah, I'm with @mamhoff. There ware services out there that handle name as a unified field, which is preferable due to... reality. That said, a huge number of services that expect separate fields, and other reasons why separate fields are preferable for many orgs. Option to choose needs to remain, even if it's kind of annoying from a maintenance perspective. The alternative creates too much work for existing stores working with unified names. |
@jarednorman @mamhoff Right now if not used first name and last name have no impact on name. This is a very aggressive stance, I'd love to call it a strongly opinionated default incentevize migrate back to these fields also because the pain of splitting those fields is much bigger than the pain of joining them. If that approach has no chance for majority we can put it behind a configuration flag, I wouldn't necessarily motivate people to fragment the DB structure that way but I can understand your point. @tvdeyen @jarednorman @kennyadsl |
I am imagining a `firstname` attribute and use the current `name` attribute as either the full name or last name.
The first-name should not be mandatory by default and it should be probably hidden in forms.
That way we do not have three hard to align fields and reflect what already is the case in ie. Germany where most address forms only have the (last) name attribute mandatory.
All of this should be a - by default disabled - feature toggle (ie. `enable_address_firstname`). Maybe we even have a second toggle for showing / hiding the first-name field and validation separately.
|
@jarednorman @kennyadsl is that something you can get on board with? |
If we have both On a side note, it's the same option that Shopify offers: I'm ok with this path, as soon as the solution is backward compatible. |
Ok, so here's probably the easiest way to do it then:
Is that ok for everybody? |
This is true for the starter front end maybe, but not for admin forms. I suggest just displaying/hiding the field on the preference then. If we keep the field and only require one of them, we will end with inconsistent data. Examples
*) required vs.
*) required and
*) required
I would prefer to use Oh, and maybe we should call the field "given_name"? 🤔 |
@tvdeyen I don't want to argue about naming schemes, buuuuuuuuuuuttttt:
Microsoft: first_name
I know you would :) but can we try to find something for newbies that is friendly. |
Summary
Description:
This PR introduces updates to the address model, admin components, backend, API, and associated logic to handle firstname and lastname as separate fields, replacing the previous name field providing backwards compatibility to existing implementations of name. The updates aim to improve data handling and increase clarity when working with user and address-related data.
For the purpose of not breaking name and provide a smooth transistion if first and last name are used name is compiled with the concatenated value allowing the fields to coexist.
Changes:
Admin: Updated admin components to use firstname and lastname fields separately in place of the name field.
Backend: Modified backend logic to separate name into firstname and lastname, adjusting user and address forms, search functionality, and related views.
API: Updated the API and its associated tests to manage firstname and lastname fields separately, replacing the name field.
Core: Unignored the firstname and lastname fields in the address model. Introduced a set_full_name method for backward compatibility, concatenating firstname and lastname. Updated tests, factories, and locales to align with the new model structure.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: