person title "overkill"

[iham]

making the title field read only and use it to display a combination of last_name, first_name and position is a really elegant solution!

but ... i do get confused by the way it is implemented: https://github.com/collective/collective.roster/blob/master/src/collective/roster/person.py

i don't any bit of code there. i mean ... i do. but it doesn't make sense to me. especially setting the title twice (combine last_name and first_name and in a second step add a position AND translating those strings. (the translating-thingy puzzles me completely)

i would:

1. remove the subscribers for object create/update and the adapter PersonNameFromTitle
2. make the IPerson schema a behavior instead of being "only" a schema
3. add a factory to that behavior
4. that factory implements a title setter and stores "last_name first_name, position" (by using that method, it will also index correctly without subscribers)

any suggestions or traps to know?

[datakurre]

Separate INameFromTitle-adapter and subscriber for adding the position allows to exclude the position from the generated object id.

Translate-indirection makes the title configurable using localization strings similarly to date and time formatting strings. Back then, it felt simplier and less error-prone than allowing site admins to define formatting strings in plone registry (and that was before the famous security vulnerability in str.format was found).

I don't oppose the behavior. Originally I simply thought that names are "core" enough that they should be in the main schema and not behaviors, but I still wanted to avoid having custom persistent class.

[iham]

WOW! now i got the idea behind the i18n stuff! thats really elegant! it took me a little time, but now i am very happy with that solution :)

so getting rid of the subscribers and move them to a setter would be a way to go?

[iham]

the INameFromTitle is only used once and the result is stored in the title and catalog anyways. don't see a big advantage here.

how about using a "person display title" field with keyword widget to make certain values arrangeable?

so we could add any person-related field as a translated vocabulary to choose from and define it per roster and language.

[datakurre]

I believe, INameFromTitle adapter is still required to give nice ids to person objects.

Being able to customise display title per roster sounds good, but aren’t you now replacing with that one subscriber with a lot of more code? :)

On 12 Oct 2017, 13.41 +0300, Markus Hilbert notifications@github.com, wrote:

the INameFromTitle is only used once and the result is stored in the title and catalog anyways. don't see a big advantage here. how about using a "person display title" field with keyword widget to make certain values arrangeable? so we could add any person-related field as a translated vocabulary to choose from and define it per roster and language. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[iham]

i am not quiet sure about inamefromtitle ... the id is derived from title. the title will be modified by a setter by using the order and fields you define in the roster ... i don't see a problem in having - whatever additional - field as part of the generated id.

yes and no. i don't think its more code. its "other" code. code i can more easily extend or override. to deliver a different style how the title is presented forces me to subclass almost all code of person.py.

i would like to keep the ability to arrange the title as it is need by different languages but move it to the content. so no po-file to arrange it. just plain content.

[datakurre]

Oh, you are correct about the setter. How would you then allow “lastname-firstname” as id and “lastname firstname, position” as title?

Title configuration as content is ok for me as long as there is way to have globally configurable defaults.

On 12 Oct 2017, 14.55 +0300, Markus Hilbert notifications@github.com, wrote:

i am not quiet sure about inamefromtitle ... the id is derived from title. the title will be modified by a setter by using the order and fields you define in the roster ... i don't see a problem in having - whatever additional - field as part of the generated id. yes and no. i don't think its more code. its "other" code. code i can more easily extend or override. to deliver a different style how the title is presented forces me to subclass almost all code of person.py. i would like to keep the ability to arrange the title as it is need by different languages but move it to the content. so no po-file to arrange it. just plain content. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[iham]

+1 for the global setting

the title/id

the currently easiest way would be a textline-field title_pattern with "{last_name} {first_name} (...)" as a default to use for string formating. with the iterator from Formatter.parse(title_pattern) i get all the fields i want to read from person_obj.get(iteritem, '')

title_pattern = self.title_pattern  # thats a getter; it chooses from control panel or the roster
keys = [i[1] for i in Formatter().parse(title_pattern)]
format_kwargs = {}
for key in keys:
    format_kwargs[key] = person_obj.get(key, '')
return title_pattern.format(**format_kwargs)

same can be done with the persons id (using a field named id_pattern) to keep it separated. the id can be set via the behavior setter too.

the display title

yes therefore the adapter inamefromtitle is useful. but ... isn't that already getting the name from title or id? so it might be not needed anymore.

general question about that: why differ between title and id at all? i mean, its pretty possible, that two persons can have the same first and last name. adding the position not only to the title but also to the id, wouldn't be that wrong, would it?

i read a little code to get some knowledge: the id is set via behavioral setters using the namechooser (which uses the inamefromtitle behavior)

so basically setting the id and title using setters should work.

[iham]

another question: why is the person based on dexterity.container? does it need to be a container?

[datakurre]

We allow additional content types (like collections, files, bibliography listings) below person. Those are usually easier to implement and configure as their own content type and the display on person page using viewlet.

On 17 Oct 2017, 13.02 +0200, Markus Hilbert notifications@github.com, wrote:

another question: why is the person based on dexterity.container? does it need to be a container? — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.