Partner lastname 2 #153

yajo · 2015-08-11T09:05:43Z

Aiming for OCA/l10n-spain#174, this is an alternative for #141.

It adds a second lastname to contacts.

yajo · 2015-08-12T09:16:26Z

After last commits, I think it's production-ready.

This is the 2nd try of #105, FTR.

rafaelbn · 2015-08-12T13:43:31Z

@yajo @antespi @eLBati please review.

We have two PR for two modules with different name and same functionality.

#105
#141

@yajo I think we must change name. Better partner_second_lastname, as is how it is known for this field internationally.

After @antespi and @eLBati review we will close PR 141 and go ahead with this one and the new name.

I would not want to add more modules to this project until we discover the problem of issue #154. It's a general issue, no body can deleted res.partner.

yajo · 2015-08-13T09:00:10Z

We have two PR for two modules with different name and same functionality.

I know, but #105 was closed, and #141 depends on #140 (and both lack ✅).

I think we must change name. Better partner_second_lastname

No problem.

I would not want to add more modules to this project until we discover the problem of issue #154. It's a general issue, no body can deleted res.partner.

Let's keep that discussion in #154's thread. BTW you did not answer there to last comments.

eLBati · 2015-08-13T10:30:54Z

partner_second_lastname/README.rst

+This module was written to extend the functionality of ``partner_firstname`` to
+support having a second lastname for contact partners.
+
+In some countries, it"s important to have a second last name for contacts.


it"s -> it's

rafaelbn · 2015-08-14T18:34:44Z

@yajo as you see in #154 even though you get a ✅ Bugs exists.

Please add test for:

deleting res.partner
Automatic merge res.partner (this is also deduplicate contacts function)

partner_firstname and partner_second_lastname addons are critical, this affect to the full functionality of Odoo. It could be more easy if core assume that anyone in the worldwide has name and family name at least!

Thanks

antespi · 2015-08-19T09:37:18Z

@yajo, you have preference because you committed your contribution (#105) before me (#141).

Merge this and then I can review my improvements and propuse in a separate PR.

Thanks for your contribution.

rafaelbn · 2015-08-19T18:13:45Z

👍

rafaelbn · 2015-08-19T21:08:20Z

@eLBati @pedrobaeza could you review this PR please?

pedrobaeza · 2015-08-20T11:13:58Z

partner_second_lastname/models.py

+
+    @api.one
+    @api.depends("firstname", "lastname", "lastname2")
+    def _compute_name(self):


This should use new hooks functions to modify the behaviour instead of overwriting the original method.

Actually I need to modify it because it depends in lastname2 now. BTW, #159 complicated more than helped. I'll update and fix it here too ASAP.

antespi · 2015-08-20T11:25:52Z

👍 code review and functional review.

Some comments:

Deleting partner issue ([8.0] partner_relations - Error when deleting a res.partner #154) is still failing, but it's not related with this addon.
When creating another position (partner_contact_in_several_companies addon) appears a form with only a name field (no firstname, lastname, lastname2 fields), when filled and save then change name introducing a coma character. This addon is independent to partner_contact_in_several_companies addon, so there's no a clear solution for this, and this issue appears also with partner_firstname addon (except coma character). What do you think?

rafaelbn · 2015-09-02T14:30:33Z

@yajo @pedrobaeza @eLBati anything yo say of

When creating another position (partner_contact_in_several_companies addon) appears a form with only a name field (no firstname, lastname, lastname2 fields), when filled and save then change name introducing a coma character. This addon is independent to partner_contact_in_several_companies addon, so there's no a clear solution for this, and this issue appears also with partner_firstname addon (except coma character). What do you think?

rafaelbn · 2015-09-03T19:30:10Z

Please, one more review here? already review by @antespi and me. Thanks

yajo · 2015-09-08T10:28:30Z

All will be easier if you merge #170 before. 😉

yajo · 2015-09-08T11:06:49Z

I'm pinging @adrienpeiffer @antespi because the last commits will break #159. I need a dict in _get_inverse_name instead of a list. It's more versatile after all.

Deleting partner issue (#154) is still failing, but it's not related with this addon.

It should be tested after merging #170 (and it would be related with this one too if merged, but I already fixed it in the last commits).

It could be more easy if core assume that anyone in the worldwide has name and family name at least!

You read my mind! 😆

And about partner_contact_in_several_companies, I had no idea. It would be nice to have opinions from somebody using that module. I'll look at it.

I'd appreciate some help about the coveralls' 🚫.

yajo · 2015-09-14T08:13:57Z

Last commit removed the workaround of #154 assuming that #171 was fixing it, but at the end it does not. However the bug is not in this package nor in partner_firstname, so I'll keep this PR without the workaround in the hope the bug gets fixed soon.

I'd still like @adrienpeiffer, @antespi or somebody else to check if something is broken related to #159 because I don't know what project relies on that PR.

rafaelbn · 2015-10-23T17:55:39Z

@antespi please review. I thinks this PR is ready to merge.

moylop260 · 2015-10-24T01:50:00Z

Are the 23 commits really necessary?
(Could execute a rebase -i --autosquash oca/8.0 and fixup many ones)

moylop260 · 2015-10-24T01:52:12Z

Please, check pylint-odoo beta messages:
https://travis-ci.org/OCA/partner-contact/jobs/84057611#L177-L186

************* Module partner_firstname.__openerp__
partner_firstname/__openerp__.py:21: [C8102(manifest-required-key), ] Missing required key "license" in manifest file
************* Module partner_firstname.exceptions
partner_firstname/exceptions.py:1: [C8201(no-utf8-coding-comment), ] No UTF-8 coding comment found: Use `# coding: utf-8` or `# -*- coding: utf-8 -*-`
************* Module partner_firstname.tests.test_empty
partner_firstname/tests/test_empty.py:1: [C8201(no-utf8-coding-comment), ] No UTF-8 coding comment found: Use `# coding: utf-8` or `# -*- coding: utf-8 -*-`
************* Module partner_second_lastname.__openerp__
partner_second_lastname/__openerp__.py:5: [C8101(manifest-required-author), ] Missing author required "Odoo Community Association (OCA)" in manifest file
partner_second_lastname/__openerp__.py:5: [C8102(manifest-required-key), ] Missing required key "license" in manifest file

yajo · 2015-10-26T08:34:46Z

Rebased, squashed, fixed.

janetrp · 2015-10-28T12:25:52Z

Tested 👍 Latin people will be very happy including their mother's lastname ;)

rafaelbn · 2015-10-28T12:34:11Z

Teste in runbot 👍

rafaelbn · 2015-10-28T12:39:03Z

I think this module is ready to be merged, any opinions/comments please? Thanks

moylop260 · 2015-10-28T14:30:05Z

partner_second_lastname/README.rst

+For further information, please visit:
+
+* https://www.odoo.com/forum/help-1
+


Add try me on runbot
https://github.com/OCA/maintainer-tools/tree/master/template/module#usage

moylop260 · 2015-10-28T14:36:00Z

Tested in runbot: https://runbot.odoo-community.org/runbot/134/153
Without beta pylint-odoo messages
Runbot green
Travis green
Coverage increase

Then 👍

A little comment: Add try-me-on-runbot button in readme, please

antoniocanovas · 2015-10-28T19:03:39Z

👍

yajo · 2015-10-29T08:32:10Z

Done.

Partner lastname 2

eLBati reviewed Aug 13, 2015
View reviewed changes

rafaelbn mentioned this pull request Aug 14, 2015

[ADD] partner_second_lastname addon #141

Closed

pedrobaeza reviewed Aug 20, 2015
View reviewed changes

rafaelbn added the needs review label Oct 23, 2015

rafaelbn added this to the 8.0 milestone Oct 23, 2015

rafaelbn added the 8.0 label Oct 23, 2015

Add module partner_lastname2.

14d23db

moylop260 reviewed Oct 28, 2015
View reviewed changes

Updated README.

5b87a26

rafaelbn added a commit that referenced this pull request Oct 29, 2015

Merge pull request #153 from grupoesoc/partner_lastname2

3124dd4

Partner lastname 2

rafaelbn merged commit 3124dd4 into OCA:8.0 Oct 29, 2015

yvaucher removed the needs review label Oct 29, 2015

yajo deleted the partner_lastname2 branch October 30, 2015 11:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partner lastname 2 #153

Partner lastname 2 #153

yajo commented Aug 11, 2015

yajo commented Aug 12, 2015

rafaelbn commented Aug 12, 2015

yajo commented Aug 13, 2015

eLBati Aug 13, 2015

rafaelbn commented Aug 14, 2015

antespi commented Aug 19, 2015

rafaelbn commented Aug 19, 2015

rafaelbn commented Aug 19, 2015

pedrobaeza Aug 20, 2015

yajo Sep 8, 2015

antespi commented Aug 20, 2015

rafaelbn commented Sep 2, 2015

rafaelbn commented Sep 3, 2015

yajo commented Sep 8, 2015

yajo commented Sep 8, 2015

yajo commented Sep 14, 2015

rafaelbn commented Oct 23, 2015

moylop260 commented Oct 24, 2015

moylop260 commented Oct 24, 2015

yajo commented Oct 26, 2015

janetrp commented Oct 28, 2015

rafaelbn commented Oct 28, 2015

rafaelbn commented Oct 28, 2015

moylop260 Oct 28, 2015

moylop260 commented Oct 28, 2015

antoniocanovas commented Oct 28, 2015

yajo commented Oct 29, 2015

		For further information, please visit:

		* https://www.odoo.com/forum/help-1

Partner lastname 2 #153

Partner lastname 2 #153

Conversation

yajo commented Aug 11, 2015

yajo commented Aug 12, 2015

rafaelbn commented Aug 12, 2015

yajo commented Aug 13, 2015

eLBati Aug 13, 2015

Choose a reason for hiding this comment

rafaelbn commented Aug 14, 2015

antespi commented Aug 19, 2015

rafaelbn commented Aug 19, 2015

rafaelbn commented Aug 19, 2015

pedrobaeza Aug 20, 2015

Choose a reason for hiding this comment

yajo Sep 8, 2015

Choose a reason for hiding this comment

antespi commented Aug 20, 2015

rafaelbn commented Sep 2, 2015

rafaelbn commented Sep 3, 2015

yajo commented Sep 8, 2015

yajo commented Sep 8, 2015

yajo commented Sep 14, 2015

rafaelbn commented Oct 23, 2015

moylop260 commented Oct 24, 2015

moylop260 commented Oct 24, 2015

yajo commented Oct 26, 2015

janetrp commented Oct 28, 2015

rafaelbn commented Oct 28, 2015

rafaelbn commented Oct 28, 2015

moylop260 Oct 28, 2015

Choose a reason for hiding this comment

moylop260 commented Oct 28, 2015

antoniocanovas commented Oct 28, 2015

yajo commented Oct 29, 2015