I think uid 1 should not be affected by the agreement page. Not really a problem ticking on a box . Rather the fact that I find it scary that the admin is forced to be locked outside the admin area until he accepts the agreement(only drush can save the day). What if the module conflicts with another by a weird twist of fate!! Not really a major issue, but still makes sense :)

Comments

pagaille’s picture

pagaille commented

I'm not sure about excluding the admin user automatically, as some sites require that all users accept an agreement (even the admin). But maybe an option to exclude the admin user would do the trick?

charos’s picture

charos commented

I guess it depends on the need of other users. My only concern was the case of locking admin out of the system in case something goes wrong. Maybe exclude the redirection to agreement in /admin/* ? Merely a suggestion!

pagaille’s picture

pagaille commented

In the latest version you can add admin/* to the list of paths to exclude. Or even just admin/build/modules/list.

itserich’s picture

itserich commented

I have this set to user role: authenticated and frequency: on every log in.

I also use multiple screens.

As admin, User 1, it requires me to click I Agree each time I switch screens.

Setting page visibility to - Show on every page except the listed pages: admin/* - does not solve the problem.

Wondering if it is my configuration or a glitch?

pagaille’s picture

pagaille commented

For now, admin users have to accept the agreement, just like all other users. Adding admin/* to the list of pages that are excepted just prevents any pages matching the admin/* path from triggering the agreement. If the admin user (or any other user) goes to any page aside from something matching the admin/* path, they will have to accept the agreement before they can proceed. The admin/* tip was just to help avoid an endless loop where that could potentially occur should the Agreement module be buggy... this way, the admin user could get to the Agreement settings page and disable it without having to accept the agreement first.

So there's nothing wrong in your config options - it's just confusing that admin user <> admin/* path. Hope this helps....

itserich’s picture

itserich commented

Thanks. Not sure I understand, but overall the module works great.

yuriy.babenko’s picture

yuriy.babenko commented
Status: Active » Postponed

I agree with pagaille - we shouldn't automatically assume that the admin user should be excluded, and the admin/* wildcard should be flexible enough for most needs. Postponing this for now, to be reviewed at a later date.

yuriy.babenko’s picture

yuriy.babenko commented

Just noticed that I previously added an exception (in the code) to automatically exclude uid=1 from the agreement. I no longer agree with that approach, so I'm removing the exclusion from the master branch. This note is more for maintainers than anyone else... :)

pagaille’s picture

pagaille commented
Status: Postponed » Closed (fixed)

Looks like the admin user is indeed excluded in the 6.x-2.0-rc2 release, because of the new "skip agreement" permission (which the admin user will always have). Will keep in 6.x-2.0.