Forum OpenACS Improvement Proposals (TIPs): TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user".

Collapse
1: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user".
Posted by Don Baccus on 05/30/09 02:10 AM
There's no reason to use generic storage (i.e. the value placed in acs_attribute_values rather than the object itself) for this field. You can only have one bio per person (generic storage is mostly useful for many-to-one relationships, even then I'd suggest it's mostly a bad idea).

Why change this? Generic storage, with the potential of many values for an attribute for each object, is a bit difficult to handle when auto-generating forms and the like, such as is done with the dynamic types package.

xotcl-core, for instance, specifically excludes bio and bio_mime_type from attributes it will deal with.

I'm proposing moving it from "person" to "user" because the code only supports bios for actual users, and it makes sense to me that it should be this way.

This also simplifies the code dealing with bios a bit, though in order to ease upgrade problems I'm not changing the structure of the Tcl API (you have to be able to log in to upgrade...).

I've already implemented this for PG complete with an upgrade script which copies over the existing biographies, deletes the old copy, drops the attribute, etc. I'll do it for Oracle after approval (most of the queries aren't RDBMS-specific anyway).

Also I propose dropping the "bio_mime_type" column as it is unused.

Collapse
5: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 1)
Posted by Emmanuelle Raffenne on 05/30/09 12:47 PM
I approve with reserve about dropping the bio_mime_type column. It is actually used (not in core but in .LRN). Any text datatype can be potentially HTML, especially since we have HTML editors such as tinymce and xinha.

How would the mime type of "bio" be handled then?

Note: Regarding dynamic types, that reminds me association between attributes for a more complex semantic but that goes beyond what we want to get initially.

Collapse
6: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 5)
Posted by Don Baccus on 05/30/09 02:12 PM
Before suggesting dropping bio_mime_type I did the following in my complete checkout of all current openacs packages, "openacs-4-complete":

Goshawk:~ acs$ cd openacs-4-complete/packages
gGoshawk:packages acs$ grep -r -l bio_mime_type .
./acs-subsite/sql/postgresql/attribute.sql
./xotcl-core/tcl/05-db-procs.tcl
Goshawk:packages acs$

xotcl-core is Gustaf's kludge to ignore the attributes "bio" and "bio_mime_type" because his code doesn't handle attributes defined to use generic rather than type-specific storage.

acs-subsite/sql/postgresql/attribute.sql creates the bio_mime_type attribute.

Note: it isn't created for oracle so if it is really used somewhere in .LRN, it is somewhere that only supports PG, and is not in cvs.openacs.org:/cvsroot (unless it is in contrib, which I didn't check).

Are you guys using it locally?

bio should really use the rich text stuff regardless, I thought about making that change but it's more than I'm prepared to do right now.

Collapse
7: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 6)
Posted by Gustaf Neumann on 05/30/09 02:46 PM
Don, i don't understand, why you are poking on xotcl-core. xotcl-core never claimed to support the externally stored attributes, so it simply excludes it. Most likely Malte's code for xo-dyntypes extends xotcl-core to handle it, but i did not have time to look at it.

It was rather a kludge at the first place to define bio this way. Don's suggestion to move it from persons to users and to turn it into a regular attribute makes perfectly sense.

Emma, the mime-type for the bio could be stored in the bio field together with the content (as two element list) by using the rich-text widget for editing. This is also a kind of a kludge, but used all over OpenACS.

Collapse
8: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 6)
Posted by Emmanuelle Raffenne on 05/30/09 02:46 PM
No, it's not my local, it's .LRN. But you are right, it doesn't use the "bio_mime_type", it doesn't even store it... It converts it from/to HTML each time it is edited (gross!).

So, yeah, dropping bio_mime_type won't hurt.

Collapse
9: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 7)
Posted by Don Baccus on 05/30/09 02:50 PM
I'm not "poking at it".

I grep'd to find all the uses of bio_mime_type, which is the right thing to do before removing it, no?

When I saw the xotcl-core reference, well, of course I wanted to look to see what you were doing with it.

And I stated above that you're kludging around the use of it.

Since that is all xotcl-core is doing with it, it is safe to get rid of it, i.e. I won't break xotcl-core by doing so.

Surely you don't prefer that I just remove stuff without first checking to make sure I won't break stuff like xotcl-core???

Collapse
10: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 7)
Posted by Emmanuelle Raffenne on 05/30/09 02:51 PM
Gustaf,

Yes, sounds like (more than) a bit of kludge to me. Having a proper datatype created in the DB for it would be much better.

Collapse
11: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 8)
Posted by Don Baccus on 05/30/09 02:52 PM
"It converts it from/to HTML each time it is edited (gross!)"

Yes, exactly. The behavior is totally bizarre and probably confusing to users.

I think we all agree that rich text is the right way to go, but I don't have time to do it right now.

Collapse
12: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 1)
Posted by Michael Steigman on 06/01/09 04:48 AM
If the code only supports bios for actual users, perhaps it should be changed? It would seem that if there's any rationale for keeping the person abstraction around, it's the place for a bio, no? Not intentionally trying to stir things up but this will come into play when we start thinking about where we're going to attach the profile attributes. (I lean towards person on this issue and that would make NO sense if bio is part of a user type).
Collapse
13: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 12)
Posted by Don Baccus on 06/02/09 12:48 AM
the whole party/person/user abstraction, while a good one, is poorly implemented and messy.

If I leave it in person, less code needs changing, in fact the Tcl API needn't change a bit.

I'll leave it there for now and if some day people want to have a global argument about the person vs. user abstraction I'll sit and watch :)

Collapse
14: Re: TIP #133: Change "bio" from being generic storage in "person" to type-specific storage in "user". (response to 13)
Posted by Michael Steigman on 06/02/09 05:02 PM
I was kinda hoping to engender that global argument, Don :) I guess I didn't do a good enough job of stirring things up.

Don and I chatted about this offline and yes, there are multiple instances of developers adding columns in the wrong place (it appears 'users' has become the dumping ground for *anything* related to parties, even for data that relates to columns stored at other levels in the type hierarchy like, say, email). Don clarified one misconception I had about the person type per Lars' (author) intent, which was apparently for it to be a place to hold minimal amount of data on parties that are not yet full-blown users. With portrait and bio hanging off that type, it seemed to me that it was more like the place to hang non-account related attributes. I kind of liked that way of looking at it but, well, as Don pointed out, the whole situation is a little messy and has been muddied by developers over the years.

Anybody have strong (uh... or any) feelings on the matter?

Oh yeah, approved :)