Skip to content

[18.0][IMP] partner_second_lastname: add name order 'Lastname, Firstname SecondLastname'#2309

Open
BhaveshHeliconia wants to merge 1 commit into
OCA:18.0from
HeliconiaIO:18.0-imp-partner_second_lastname
Open

[18.0][IMP] partner_second_lastname: add name order 'Lastname, Firstname SecondLastname'#2309
BhaveshHeliconia wants to merge 1 commit into
OCA:18.0from
HeliconiaIO:18.0-imp-partner_second_lastname

Conversation

@BhaveshHeliconia

Copy link
Copy Markdown
Contributor

This PR adds a new name formatting option: Lastname, Firstname SecondLastname (key: last_first_comma2).

Improvements:

  • Added last_first_comma2 to res.config.settings.
  • Implemented computed name logic and inverse name logic in res.partner for the new format.
  • Fixed a potential IndexError in _get_computed_name when lastname is missing.
  • Updated documentation in CONFIGURE.md.
  • Added comprehensive tests in test_config.py, test_multiple_names.py, and test_name.py.

return [(k, new_labels[k]) if k in new_labels else (k, v) for k, v in options]
result = [(k, new_labels[k]) if k in new_labels else (k, v) for k, v in options]
# Separate format where only the first lastname is followed by a comma
result.append(("last_first_comma2", "Lastname, Firstname SecondLastname"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why didn't you add it directly to the new_labels dict?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@luisDIXMIT Here, if we add it to 'new_labels' dict, then it won't get added in 'result' as we're looping on 'options' and 'options' don't have 'last_first_comma2', so we've added it separately afterwards.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, thanks for explaining me!

@luisDIXMIT luisDIXMIT left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested on runboat and LGTM!

@legalsylvain legalsylvain left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we add new key, we should propose changes in related OCA modules. don't you think ?
At least :
./hr/hr_employee_firstname
./hr/hr_employee_second_lastname/
./partner-contact/partner_second_lastname/

Thanks !

@BhaveshHeliconia

Copy link
Copy Markdown
Contributor Author

If we add new key, we should propose changes in related OCA modules. don't you think ? At least : ./hr/hr_employee_firstname ./hr/hr_employee_second_lastname/ ./partner-contact/partner_second_lastname/

Thanks !

Yes, you're right. This should also be handled in the hr_employee_second_lastname module. I'll take a look and propose the necessary changes there as well.

@BhaveshHeliconia

Copy link
Copy Markdown
Contributor Author

@legalsylvain here is the PR for hr_employee_second_lastname: OCA/hr#1554

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.

3 participants