[Erp5-dev] proposed patch for Person

[bartek]

Hello

I noticed an undesired behaviour - setTitle on Person object duly sets
the title, which is then not displayed in the form, because it uses
first_name and last_name. And there is an inconsistency, because
getTitel returns first_name + last_name, while setTitle sets the title.

I hit this problem when I used RelationStringField which allowed
creation - I typed the name, created new Person and it came up empty,
which is not what one would expect.

What I'd suggest is the attached patch - setTitle should be the exact
reverse of getTitle, and the rest of setters is redundant.

I didn't commit it, please comment if you think it is ok or not.

Bartek
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Person_patch
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20070222/b704372d/attachment.txt>

[Jérôme Perrin]

Post by bartek
Hello
I noticed an undesired behaviour - setTitle on Person object duly sets
the title, which is then not displayed in the form, because it uses
first_name and last_name. And there is an inconsistency, because
getTitel returns first_name + last_name, while setTitle sets the title.
I hit this problem when I used RelationStringField which allowed
creation - I typed the name, created new Person and it came up empty,
which is not what one would expect.
What I'd suggest is the attached patch - setTitle should be the exact
reverse of getTitle, and the rest of setters is redundant.
I didn't commit it, please comment if you think it is ok or not.

In r13208, I added some tests for the minimal level of first name / last
name / title interaction which I think we need here (Person_view doesn't
allow to edit title, but only last_name & first_name). Thoses tests still
passes with your patch, but I'm afraid of side effects that changing the
behaviour could cause. At least, with this patch I have seen some of my unit
tests for a project importing persons from an external source failing.

The problem is only with relation field, right ? Maybe this problem can be
fixed when the relation field creates the document. Now, it just call
newContent on module, and pass arguments[1] to newContent.
One way could be to add a "Create Document Method" on relation fields that
would be a script that guess first_name, last_name from title and creates the
document then set properties.
We have the same problem each time we want to allow creation for documents
where the property is calculated (eg. reference for bank accounts), but still
I'm not sure if we use it so much that it would explain adding yet more
complexity to relation field.

[1] Anybody knows why it have to immediate reindex object ?
(MultiRelationField.py line 412)

J?rome

[Pelletier Vincent]

Post by Jérôme Perrin
[1] Anybody knows why it have to immediate reindex object ?
(MultiRelationField.py line 412)

That code is very suspicious to me, because basically we:
-create an object and index it immediately
-get its UID
-get the object from catalog using this uid

Even if I guess there might be cases where we want an object to be indexed at
creation, I guess we _never_ want to waste so much time in
storing-then-fetching the same object in catalog.

Line 680 (and below) there is a similar case :
-get the uids of all objects in a list
-iterate in the uid list:
-fetch object from catalog

I tried a patch on this code and commited, but my patch was bad and the commit
was reverted. I haven't give it another try, though I think it deserves it.