Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider only generating dynamic list for all documents #71

Open
ericvaandering opened this issue May 31, 2018 · 4 comments
Open

Consider only generating dynamic list for all documents #71

ericvaandering opened this issue May 31, 2018 · 4 comments

Comments

@ericvaandering
Copy link
Owner

if ($UserValidation eq "certificate" || $UserValidation eq "shibboleth" ||
    (!$Public && $Preferences{Options}{DynamicFullList}{Private}) ||
    ( $Public && $Preferences{Options}{DynamicFullList}{Public})) {
  push @OtherColumns,"<a href=\"$ListBy?alldocs=1\">All documents</a>";
} else {
  push @OtherColumns,"<a href=\"$web_root/Static/Lists/$remote_user/FullList.html\">All documents</a>";
}

is in the code now and is quite complex (and more so once SSO goes in). While this may have been the right choice 18 years ago with defined password/groups and slow servers, it's probably not anymore with better authorization mechanisms and faster servers.

One report of someone being tripped up by these settings.

@lauramengel
Copy link
Collaborator

I agree that a dynamic list is usually better than a static list.
The dynamic list does not take as long to generate anymore.
Having cronjobs to generate static lists for various logons over
~60 DocDBs is less preferred (and we mostly are not generating
these static lists for fnal DocDBs anymore).

Could you add checking for FNAL SSO UserValidation to the "if"
that is the condition for doing the dynamic lists?

if ($UserValidation eq "certificate" || $UserValidation eq "shibboleth" || ...

@ericvaandering
Copy link
Owner Author

It's there already. That was a quote from the release:

$ git grep -C 5 Dynamic
cgi/DocumentDatabase-if ($UserValidation eq "certificate" || $UserValidation eq "shibboleth" || $UserValidation eq "FNALSSO" ||
cgi/DocumentDatabase:    (!$Public && $Preferences{Options}{DynamicFullList}{Private}) ||
cgi/DocumentDatabase:    ( $Public && $Preferences{Options}{DynamicFullList}{Public})) {
cgi/DocumentDatabase-  push @OtherColumns,"<a href=\"$ListBy?alldocs=1\">All documents</a>";
cgi/DocumentDatabase-} else {
cgi/DocumentDatabase-  push @OtherColumns,"<a href=\"$web_root/Static/Lists/$remote_user/FullList.html\">All documents</a>";
cgi/DocumentDatabase-}
cgi/DocumentDatabase-

@lauramengel
Copy link
Collaborator

ok, great. (In the first posting in this issue, the FNALSSO part did not show up).

@pms967
Copy link

pms967 commented Aug 17, 2023

While this may have been the right choice 18 years ago with defined password/groups and slow servers, it's probably not anymore

indeed.

Moreover, the script which should be run from Cron to generate the lists does not work on newer systems (at least, on Ubuntu server).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants