Ticket #4382 (closed Bug: fixed)
Opened 7 years ago
Membershiptool: bad interaction with exUserFolder
| Reported by: | ajung01 | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | Past |
| Component: | Users/Groups | Version: | |
| Severity: | Keywords: | ||
| Cc: |
Description
I've installed a user source for GRUF (which uses a MySQL DB as user source + properties source).
searchForUsers() fails when calling "u = member.getUser()". In our case 'member' is an instance of GRUFUser vs. something wrapped into the context of MemberDataTool.MemberData.
The GrufUser.getUser(name) requires a 'name' argument resulting in a wrong method call.
my workaround looks like this:
try:
u = member.getUser()
except:
u = member.getUser(member.getUserName())
I am not right sure what the ideal solution is without taking a deeper look how GRUF and everything works together at this point. At least this patch works for me and makes exUserFolder working with GRUF.
Change History
comment:2 Changed 7 years ago by pjgrizel
Not sure I get it.
- The 'searchForUsers' method doesn't exist. 'searchForMembers' does in MembershipTool. Is this the one you're talking about ?
- There's no GRUFUser.getUser() method
- BasicUserFolder.getUser() is defined as a method taking a 'name' parameter
I think it'd be better to find why searchForUser() gets a 'user' object instead of a 'member' object rather than violently patching it... Plus, the try/except can be costly in a loop.
Apparently, someone has commented this line in MembershipTool.py (l.459 ):
#user = md.wrapUser(u)
What for ?
Then, can we add a test there like "if isinstance(u, MemberData): u = md.getUser..." ? I think it would be better to solve this pb.
comment:3 Changed 7 years ago by ajung01
Unfortunately I changed the setup meanwhile (customer production server, coming deadline :-) so I can not reproduce this anymore
- The 'searchForUsers' method doesn't exist. 'searchForMembers' does in MembershipTool. Is this the one you're >talking about ?
yes, searchForMembers()
- There's no GRUFUser.getUser() method
But the GRUFUser instances was wrapped in the context of the GroupUserFolder instance which provides getUser(name).
I think it'd be better to find why searchForUser() gets a 'user' object instead of a 'member' object rather than >violently patching it... Plus, the try/except can be costly in a loop.
I know...but you know what the priorities are when deadline arise at the horizon :-)
Apparently, someone has commented this line in MembershipTool.py (l.459 ):
#user = md.wrapUser(u)
What for ?
No idea...but in the context of exUserFolder this call would not make sense since exUserFolder can provide property sources other than the portal_memberdata tool. So this call makes only sense for a nacked Plone installation.
Then, can we add a test there like "if isinstance(u, MemberData): u = md.getUser..." ? I think it would be better >to solve this pb.
I'll try to dig into the problem as as I have time...
comment:4 Changed 7 years ago by pjgrizel
CI> Unfortunately I changed the setup meanwhile (customer production CI> server, coming deadline :-) so I can not CI> reproduce this anymore
Yeah, I know what it is ;)
I think it'd be better to find why searchForUser() gets a 'user' object instead of a 'member' object rather than >violently patching it... Plus, the try/except can be costly in a loop.
CI> I know...but you know what the priorities are when deadline arise at the horizon :-)
You're right, but -1 for integrating the patch into Plone ;)
CI> No idea...but in the context of exUserFolder this call would not make sense since exUserFolder CI> can provide property sources other than the portal_memberdata CI> tool. So this call makes only sense CI> for a nacked Plone installation.
-1
exUserFolder uses a way to reteive properties... LDAPUserFolder uses another way... SimpleUserFolder uses yet another way...
I did my best to provide a smooth integration protocol into GRUF, as user properties are not part of the standard Zope API. Let's keep consistant! If we're not, we'll keep tons of UF-specific code in Plone and won't be able to switch to PAS.
Now, with GRUF+LDAP, you can have what you want with user properties (Plone asking LDAPUserFolder through GRUF when it needs to fetch a property value). So, I think it'd be better to ask exUserFolder guys to agree to use Jens' API (a simple getattr overload on the User object, AFAIR).
Let's defer this bug as it's more about a product that's not part of the "stock" plone install.
comment:5 Changed 7 years ago by alecm
It appears the the getUser() is only used to get the userid/login name. Shouldn't we be able to get this directly from the wrapped memberdata object, without having to fiddle with the user folder (i.e. using member.getUserName() directly which just delegates the the underlying user, but would work without issue on an unwrapped user)?

Hm, member.getUser() should never require an argument. If it seemingly does, then my guess is there is something wrong with the wrapping and you actually acquire getUser() from the user folder (which requires an argument).