On 2006-08-01 17:16, Clayton Parker wrote:
> Rob,
> 
> I have checked in the test for the member along with a few other changes 
> to remember.  There are a few questions that I have for you.
> 
> 1) In order to get the roles working i needed to implement 
> IGroupsProvider and IGroupAwareRolesProvider.  This then looked at 
> getRoles to return the proper roles for the member.  So from what i 
> gather this is working the way it should.  If you set the 'Reviewer' 
> role on the member when you do member.getRoles() you get ('Reviewer',) 
> instead of ('Reviewer','Authenticated') like in CMFMember.  Is this correct?

this should be fine; the user object ends up w/ the correct roles.  it remains
to be seen if any unacceptable side effects result from this; if they do,
we'll address it then, but until then let's leave it. 
> 2) When you edit a member and add one of the stock groups it doesn't get 
> assigned to the user.  So if you select 'Administrators' and try 
> member.getUser().getGroups() you receive an empty list.  On the other 
> hand if you add a 'SimpleGroup' from membrane and add the member to the 
> references of that group then the user receives group and the roles 
> assigned to that group.  I can't figure out where to glue the stock 
> groups and remember together.  Any ideas here?  My guess is it has 
> something to do with getGroupsForPrincipal() but that has not led me to 
> the answer.

hmm... i'd think it would be handled at the PAS layer, cascading down from one
plug-in to another depending on priority.  maybe not, though, i'll take a
look. 
> 3) The way that i set up the password reset seems a bit quirky.  If you 
> are logged in as 'bob' and you go to change your password it gets 
> changed correctly but will sometimes log you out.  You can log back in 
> ok but that is a bit inconvenient :)

credentialsChanged is supposed to prevent that.  are you still seeing this
behaviour even when calling credentialsChanged? 
> Here is another scenario that 
> doesn't quite work.  You log in as 'jane' and you have the right to edit 
> bob's password.  Bob has already logged in with his current password and 
> his session is still active.  Jane resets bob's password.  Bob can't log 
> in until he closes his browser or clears his cookies.  Once the cookies 
> are cleared then he can log back in fine.  Maybe we need to hook into 
> another method on the _setPassword() method to make this work cleaner 
> rather than the credentialsChanged() from the Membership Tool.

i'm pretty sure that this problem exists for all Plone membership
implementations; it doesn't strike me as a big problem.  administrators should
be pretty aware that changing someone's password w/o alerting them to that
fact is, in fact, locking them out of their account.  actually, as i think
about it, we WANT this behaviour... there are times when an admin will change
a user's password specifically TO lock them out of their account.  we want
this to take effect immediately, not allow the user to finish the current
session. 
> Haven't had a chance to implement what was mentioned in the other emails 
> about the tools yet.  Hopefully we can clean that stuff up soon.

removing the assumption that all member objects live in portal_memberdata is
an absolute requirement.  i consider this to be a release blocker; this is one
of the fundamental improvements of remember, i don't want to code ourselves
back into a limited situation.  i might get to fixing the existing code before
you do, but i ask you (and anyone else who's listening in) to please consider
this in future efforts.  
> Also, can you take a look at the test i set up.  Let me know if it makes 
> sense and if it's set up correctly.

i did take a look, and the setup looks pretty good.  i realize that you simply
ported some tests as is from CMFMember, which is fine.  examining the tests,
though, most of them weren't actually testing anything very useful.  many were
just setting a field value on the member object and making sure the field
returned the right value; this is really testing AT more than remember.  i
edited some of the tests to be more useful, for example by checking that the
_user_ object returns the right properties when the field value is set on the
member object. 
thanks again for your efforts,

-r