Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #778 (closed defect: fixed)

Opened 13 years ago

Last modified 12 years ago

[PATCH] Patch to make emailAddress and displayName optional in default soprovider

Reported by: anonymous Owned by: anonymous
Priority: normal Milestone: 0.9a5
Component: Identity Version: 0.9a4
Severity: normal Keywords:
Cc: steve@…

Description

This patch makes the default soprovider more suitable for a wider range of uses by making the email address and display name optional. It does remove the alternateId status of emailAddress.

Tim Lesher has rightly expressed some concern. (  http://tinyurl.com/rfwqu )

My thinking is that:

  1. TG 1.0 will be used in internet apps, intranet apps, and apps in small mom and pop shops for which the term "intranet" would be overkill... as would be requiring an email address and (to a lesser extent) display name. The current tg_user enforces a decidedly "Internet App" flavor.
  1. As Jorge Godoy pointed out on the Turbogears list, it's easier to add constraints than to remove them.
  1. The TG 1.0 API will be fixed in stone and we will be living with it for a while.
  1. The fields are useful to a large enough percentage of people that it makes sense to keep them for (optional) use than to remove them entirely.

Attachments

emailaddress_displayname_optional.patch Download (529 bytes) - added by steve@… 13 years ago.
Patch to make email adress and display name optional in default soprovider
ident_email_dispname_optional.patch Download (751 bytes) - added by steve@… 13 years ago.
Patch to make email adress and display name optional in default soprovider

Change History

Changed 13 years ago by steve@…

Patch to make email adress and display name optional in default soprovider

Changed 13 years ago by steve@…

Patch to make email adress and display name optional in default soprovider

comment:1 Changed 13 years ago by steve@…

The first patch is against 0.9a4. (Oops, selected the wrong file. Sorry.)

The second ( ident_email_dispname_optional.patch ) is against the trunk.

comment:2 Changed 13 years ago by godoy

Hi Steve! Thanks for the patches.

Just for you to know, the next version from what is in the trunk will be 0.9a5 and the next version after 0.9a4 will also be 0.9a5, so only the "second" patch would be needed right now. But it is nice to have the first one for people that need this right now and doesn't follow the trunk.

I'm OK with applying those to the trunk but we should also mention this change in the docs.

After this gets applied -- if it does get applied --, would you mind adding a note at http://trac.turbogears.org/turbogears/wiki/IdentityManagement that since revision (yet to be defined) these fields are optional and the by_user_name method can't be used anymore? You might even add a link to this bug by writing #778 (just like you're seeing here, Trac will make the hyperlink automagically ;-)).

Thanks for your help.

comment:3 Changed 13 years ago by jorge.vargas

I don't think this is a good idea, we will start patching it over to fit this type of app or that type of app.

As you mention TG is being use in diferent types of apps therefore should have diferent default identities which just confuses people out.

So If there is too much troubles with the default provider I believe the best thing to do is have NO default, and make everyone have to define a class with a specific name (or some configurable value) that will be the default provider.

Then by documentation suggest diferent ways of going at it. Right now it is weird to have your model.py with all your classes except the one defining your model and and the extend part is not good since you have to go to more then one table to retrieve data.

comment:4 Changed 13 years ago by anonymous

Well, I really can't disagree with the idea of having tg_user defined in model.py, with username and password fields (and no others) being a requirement of the model. That would be heaven.

Unfortunately, I'm not up to coding that and the fact is that we currently do have a default model with requirements that get in the way.

It is much easier for the programmer to simply extend tg_user to add fields or constraints than it is to get around constraints that don't make sense for the particular app.

Including some commonly used fields seems like a good idea to me. But *requiring* them is where they start to get in the way. Particularly when they are required to be unique.

comment:5 Changed 13 years ago by jvanasco@…

I added an alternate approach to this over here: http://trac.turbogears.org/turbogears/ticket/780

The alternate version uses configuration options in Identity to decide if user_name or user_email is the primary key and/or required.

If this patch is used instead, it would be nice to see user_name bumped up to 255 chars (now its 16), as that would let people use user_name to store email addresses ( as some apps use email as an id, not a user designed name )

comment:6 Changed 13 years ago by godoy

  • Cc steve@… added

Steve, could you provide us with a patch for saprovider.py as well? Thanks!

comment:7 Changed 13 years ago by godoy

(Just to make it clear: just against the trunk, OK? There's no need to patch against 0.9a4)

comment:8 Changed 13 years ago by godoy

jvanasco: ammendments to both steve's patches are welcome.

Even though this is a bit weird and I wouldn't call it good design I can see some usefulness in duplicating what you "had" with email_address (by_email_address) at the user_id (using it with an email address to log in the application).

comment:9 Changed 13 years ago by kevin

  • Status changed from new to closed
  • Resolution set to fixed

The model classes have moved to model.py in quickstart projects, so anyone who wants can just remove or modify the extra fields.

Note: See TracTickets for help on using tickets.