Re: [AppDB] Allow admins to remove their admin rights

[Alexander Nicolaysen Sørnes]

På Onsdag 04 juni 2008 , 01:24:07 skrev Chris Morgan:
> What new policy on dissidents? ;-)
>
> How does this patch enable admins to remove themselves? I'm afraid I
> can't see how this change lets an admin remove their own admin status.
>
> Chris

Yea, that particular piece of code isn't very clear on what it's doing.  I 
found out that $aClean['iUserId'] would only be set when modifying another 
user, and it previously compared it to $oUser->iUserId before allowing the 
admin removal.  Now we only check for admin rights, and the oUser->iUserId == 
aClean['iUserId'] check is performed later to see if we should redirect back 
to the user admin page.
> On Tue, Jun 3, 2008 at 6:34 AM, Alexander Nicolaysen Sørnes
>
>  wrote:
> > This is in accordance with the AppDB's new policy on dissidents. :)
> >
> > Alexander N. Sørnes

[AppDB] Allow admins to remove their admin rights

[Chris Morgan]

On Wed, Jun 4, 2008 at 4:15 AM, Alexander Nicolaysen Sørnes
 wrote:
> På Onsdag 04 juni 2008 , 01:24:07 skrev Chris Morgan:
> > What new policy on dissidents? ;-)
> >
> > How does this patch enable admins to remove themselves? I'm afraid I
> > can't see how this change lets an admin remove their own admin status.
> >
> > Chris
>
> Yea, that particular piece of code isn't very clear on what it's doing.  I
> found out that $aClean['iUserId'] would only be set when modifying another
> user, and it previously compared it to $oUser->iUserId before allowing the
> admin removal.  Now we only check for admin rights, and the oUser->iUserId ==
> aClean['iUserId'] check is performed later to see if we should redirect back
> to the user admin page.

So if the admin was removing their own admin status $aClean['iUserId']
wouldn't be set? If this is the case we should document it there
because otherwise the functionality is confusing.

How does the redirect relate to removing admin rights?

Chris

[AppDB] Allow admins to remove their admin rights

[Alexander Nicolaysen Sørnes]

På Onsdag 04 juni 2008 , 17:37:15 skrev Chris Morgan:
> On Wed, Jun 4, 2008 at 4:15 AM, Alexander Nicolaysen Sørnes
>
>  wrote:
> > På Onsdag 04 juni 2008 , 01:24:07 skrev Chris Morgan:
> > > What new policy on dissidents? ;-)
> > >
> > > How does this patch enable admins to remove themselves? I'm afraid I
> > > can't see how this change lets an admin remove their own admin status.
> > >
> > > Chris
> >
> > Yea, that particular piece of code isn't very clear on what it's doing. 
> > I found out that $aClean['iUserId'] would only be set when modifying
> > another user, and it previously compared it to $oUser->iUserId before
> > allowing the admin removal.  Now we only check for admin rights, and the
> > oUser->iUserId == aClean['iUserId'] check is performed later to see if we
> > should redirect back to the user admin page.
>
> So if the admin was removing their own admin status $aClean['iUserId']
> wouldn't be set? If this is the case we should document it there
> because otherwise the functionality is confusing.
>
> How does the redirect relate to removing admin rights?
>
> Chris

If $aClean['iUserId'] isn't set, it is assumed the user is editing his own 
preferences.

As for the admin right remoal, I probably added it in the wrong place when I 
first introduced the feature.  It hadn't occured to me that anyone would want 
to do it, I guess :)

I can send a follow-up patch to add comments or clarify the code.

Alexander

[AppDB] Allow admins to remove their admin rights

[Chris Morgan]

On Wed, Jun 4, 2008 at 11:56 AM, Alexander Nicolaysen Sørnes
 wrote:
> På Onsdag 04 juni 2008 , 17:37:15 skrev Chris Morgan:
> > On Wed, Jun 4, 2008 at 4:15 AM, Alexander Nicolaysen Sørnes
> >
> >  wrote:
> > > På Onsdag 04 juni 2008 , 01:24:07 skrev Chris Morgan:
> > > > What new policy on dissidents? ;-)
> > > >
> > > > How does this patch enable admins to remove themselves? I'm afraid I
> > > > can't see how this change lets an admin remove their own admin status.
> > > >
> > > > Chris
> > >
> > > Yea, that particular piece of code isn't very clear on what it's doing.
> > > I found out that $aClean['iUserId'] would only be set when modifying
> > > another user, and it previously compared it to $oUser->iUserId before
> > > allowing the admin removal.  Now we only check for admin rights, and the
> > > oUser->iUserId == aClean['iUserId'] check is performed later to see if we
> > > should redirect back to the user admin page.
> >
> > So if the admin was removing their own admin status $aClean['iUserId']
> > wouldn't be set? If this is the case we should document it there
> > because otherwise the functionality is confusing.
> >
> > How does the redirect relate to removing admin rights?
> >
> > Chris
>
> If $aClean['iUserId'] isn't set, it is assumed the user is editing his own
> preferences.
>
> As for the admin right remoal, I probably added it in the wrong place when I
> first introduced the feature.  It hadn't occured to me that anyone would want
> to do it, I guess :)
>
> I can send a follow-up patch to add comments or clarify the code.

A follow up patch would be useful. I don't want to leave that section
undocumented if we are making changes to it.

Chris