From 3339de75ab090f526667cc622c0902dadfff0671 Mon Sep 17 00:00:00 2001 From: "Brian \"Moses\" Hall" Date: Tue, 5 Nov 2024 14:00:25 -0500 Subject: [PATCH] #182 Remove support for institution name in menu items (#185) * Tidy `sub MenuItems` * Remove __INST__ support and add basic tests * Add database seeds necessary for testing --- cgi/CRMS.pm | 38 +++++++-------- docker/db/sql/006_crms_institutions.sql | 63 +++++++++++++++++++++++++ docker/db/sql/007_crms_users.sql | 7 +++ docker/db/sql/008_crms_menus.sql | 11 +++++ docker/db/sql/009_crms_menuitems.sql | 43 +++++++++++++++++ t/CRMS.t | 16 +++++++ 6 files changed, 157 insertions(+), 21 deletions(-) create mode 100644 docker/db/sql/006_crms_institutions.sql create mode 100644 docker/db/sql/007_crms_users.sql create mode 100644 docker/db/sql/008_crms_menus.sql create mode 100644 docker/db/sql/009_crms_menuitems.sql diff --git a/cgi/CRMS.pm b/cgi/CRMS.pm index c029e2ad..91d30a7f 100755 --- a/cgi/CRMS.pm +++ b/cgi/CRMS.pm @@ -7165,38 +7165,34 @@ sub Menus } # Returns aref of arefs to name, url, target, and rel -sub MenuItems -{ +# Call with crms.menus.id value or special "docs" keyword. +# TODO: turning menu items into URLs is done in three different template files +# (nav.tt, home.tt, top.tt partial) and is hard to read. This routine or a utility +# should return a complete URL. +sub MenuItems { my $self = shift; my $menu = shift; my $user = shift || $self->get('user'); - $menu = $self->SimpleSqlGet('SELECT id FROM menus WHERE docs=1 LIMIT 1') if $menu eq 'docs'; - my $q = $self->GetUserQualifications($user); - my ($inst, $iname); + my $menu_items = []; + if ($menu eq 'docs') { + $menu = $self->SimpleSqlGet('SELECT id FROM menus WHERE docs=1 LIMIT 1'); + } + my $qualifications = $self->GetUserQualifications($user); my $sql = 'SELECT name,href,restricted,target,page FROM menuitems WHERE menu=? ORDER BY n ASC'; my $ref = $self->SelectAll($sql, $menu); - my @all = (); - foreach my $row (@{$ref}) - { - my $r = $row->[2]; - my $page = $row->[4]; - if ($self->DoQualificationsAndRestrictionsOverlap($q, $r) || - $self->DoesUserHavePageAccess($user, $page)) - { - $inst = $self->GetUserProperty($user, 'institution') unless defined $inst; - $iname = $self->GetInstitutionName($inst, 1) unless defined $iname; - my $name = $row->[0]; - $name =~ s/__INST__/$iname/; + foreach my $row (@$ref) { + my ($name, $href, $restricted, $target, $page) = @$row; + if ($self->DoQualificationsAndRestrictionsOverlap($qualifications, $restricted) || + $self->DoesUserHavePageAccess($user, $page)) { my $rel = ''; - if ($row->[3] && $row->[3] eq '_blank' && $row->[1] =~ m/^http/i) - { + if ($target && $target eq '_blank' && $href =~ m/^http/i) { $rel = 'rel="noopener"'; } - push @all, [$name, $self->MenuPath($row->[1]), $row->[3], $rel]; + push @$menu_items, [$name, $self->MenuPath($href), $target, $rel]; } } - return \@all; + return $menu_items; } sub GetUserQualifications diff --git a/docker/db/sql/006_crms_institutions.sql b/docker/db/sql/006_crms_institutions.sql new file mode 100644 index 00000000..069fbe9c --- /dev/null +++ b/docker/db/sql/006_crms_institutions.sql @@ -0,0 +1,63 @@ +use crms; + +LOCK TABLES `institutions` WRITE; +/*!40000 ALTER TABLE `institutions` DISABLE KEYS */; +INSERT INTO `institutions` VALUES + (1,'University of Michigan','Michigan','umich.edu'), + (3,'Indiana University','Indiana','iu.edu'), + (5,'University of Wisconsin-Madison','Wisconsin','wisc.edu'), + (7,'University of Minnesota','Minnesota','umn.edu'), + (9,'Columbia University','Columbia','columbia.edu'), + (11,'Princeton University','Princeton','princeton.edu'), + (13,'Northwestern University','Northwestern','northwestern.edu'), + (15,'Stanford University','Stanford','stanford.edu'), + (17,'McGill University','McGill','mcgill.ca'), + (19,'Penn State University','Penn State','psu.edu'), + (21,'Johns Hopkins University','JHU','jhu.edu'), + (23,'Cornell University','Cornell','cornell.edu'), + (25,'University of North Carolina at Chapel Hill','UNC Chapel Hill','unc.edu'), + (27,'University of Arizona','Arizona','arizona.edu'), + (29,'Amherst College','Amherst','amherst.edu'), + (31,'Ohio State University','Ohio State','osu.edu'), + (33,'Boston College','Boston','bc.edu'), + (35,'DePaul University','DePaul','depaul.edu'), + (37,'University of Chicago','Chicago','uchicago.edu'), + (39,'Michigan State University','Michigan State','msu.edu'), + (41,'University of Massachusetts Amherst','UMass Amherst','umass.edu'), + (43,'University of Illinois','UIUC','illinois.edu'), + (45,'New York University','New York','nyu.edu'), + (47,'Lafayette College','Lafayette','lafayette.edu'), + (49,'University of Pittsburgh','Pitt','pitt.edu'), + (51,'University of Florida','Florida','ufl.edu'), + (53,'University of Mississippi','Mississippi','olemiss.edu'), + (55,'Texas A&M University','Texas A&M','tamu.edu'), + (57,'UCLA','UCLA','ucla.edu'), + (59,'University of Pennsylvania','Pennsylvania','upenn.edu'), + (61,'University of Tennessee Knoxville','UT Knoxville','utk.edu'), + (63,'Wake Forest University','Wake Forest','wfu.edu'), + (65,'University of Virginia','Virginia','virginia.edu'), + (67,'Georgia State University','GSU','gsu.edu'), + (69,'University of California Santa Cruz','UCSC','ucsc.edu'), + (71,'Washington State University','Washington State','wsu.edu'), + (73,'Temple University','Temple','temple.edu'), + (74,'Texas State University','Texas State','txstate.edu'), + (76,'University of Houston','Houston','uh.edu'), + (78,'University of California San Diego','UCSD','ucsd.edu'), + (79,'Duke University','Duke','duke.edu'), + (81,'California Digital Library','CDL','ucop.edu'), + (83,'University of Notre Dame','Notre Dame','nd.edu'), + (85,'UCSF','UCSF','ucsf.edu'), + (87,'Baylor University','Baylor','baylor.edu'), + (89,'UC Irvine','UC Irvine','uci.edu'), + (91,'Dartmouth College','Dartmouth','dartmouth.edu'), + (93,'University of Maryland','Maryland','umd.edu'), + (94,'University of Alberta','Alberta','ualberta.ca'), + (96,'McMaster University','McMaster','mcmaster.ca'), + (98,'University of Kentucky','Kentucky','uky.edu'), + (100,'University of British Columbia','UBC','ubc.ca'), + (102,'University of Texas at San Antonio','UTSA','utsa.edu'), + (103,'Queen\'s University','Queen\'s','queensu.ca'), + (104,'University of Oklahoma','Oklahoma','ou.edu'), + (106,'West Virginia University','W Virginia','wvu.edu'); +/*!40000 ALTER TABLE `institutions` ENABLE KEYS */; +UNLOCK TABLES; diff --git a/docker/db/sql/007_crms_users.sql b/docker/db/sql/007_crms_users.sql new file mode 100644 index 00000000..e6a64e98 --- /dev/null +++ b/docker/db/sql/007_crms_users.sql @@ -0,0 +1,7 @@ +use crms; + +LOCK TABLES `users` WRITE; +/*!40000 ALTER TABLE `users` DISABLE KEYS */; + INSERT INTO `users` VALUES ('autocrms','autocrms','CRMS Default Admin',1,1,1,1,NULL,'',1,NULL,NULL); +/*!40000 ALTER TABLE `users` ENABLE KEYS */; +UNLOCK TABLES; diff --git a/docker/db/sql/008_crms_menus.sql b/docker/db/sql/008_crms_menus.sql new file mode 100644 index 00000000..02971a30 --- /dev/null +++ b/docker/db/sql/008_crms_menus.sql @@ -0,0 +1,11 @@ +USE crms; + +LOCK TABLES `menus` WRITE; +/*!40000 ALTER TABLE `menus` DISABLE KEYS */; +INSERT INTO `menus` VALUES (0,'Review','minor',NULL,0,NULL), + (1,'Search/Browse','major',NULL,1,NULL), + (2,'Documentation','total',NULL,2,1), + (3,'Stats/Reports','orange',NULL,3,NULL), + (4,'Administrative','red',NULL,4,NULL); +/*!40000 ALTER TABLE `menus` ENABLE KEYS */; +UNLOCK TABLES; diff --git a/docker/db/sql/009_crms_menuitems.sql b/docker/db/sql/009_crms_menuitems.sql new file mode 100644 index 00000000..f2c628c4 --- /dev/null +++ b/docker/db/sql/009_crms_menuitems.sql @@ -0,0 +1,43 @@ +USE crms; + +LOCK TABLES `menuitems` WRITE; +/*!40000 ALTER TABLE `menuitems` DISABLE KEYS */; +INSERT INTO `menuitems` VALUES + (0,'Review','crms?p=review','review','r',NULL,0), + (0,'Provisional Matches','crms?p=provisionals','undReviews','e',NULL,1), + (0,'Conflicts','crms?p=conflicts','expert','e',NULL,2), + (0,'My Unprocessed Reviews','crms?p=editReviews','editReviews','r',NULL,3), + (0,'My Held Reviews','crms?p=holds','holds','r',NULL,4), + (0,'Automatic Rights Inheritance','crms?p=inherit;auto=1','inherit','ea',NULL,5), + (0,'Rights Inheritance Pending Ppproval','crms?p=inherit;auto=0','inherit','ea',NULL,6), + (1,'Historical Reviews','crms?p=adminHistoricalReviews','adminHistoricalReviews',NULL,NULL,0), + (1,'Active Reviews','crms?p=adminReviews','adminReviews','eax',NULL,1), + (1,'All Held Reviews','crms?p=adminHolds','adminHolds','ea',NULL,2), + (1,'Queue','/crms/queue','queue','ea',NULL,3), + (1,'Final Determinations','crms?p=exportData','exportData','ea',NULL,4), + (1,'Candidates','crms?p=candidates','candidates','ea',NULL,5), + (1,'Filtered Volumes','crms?p=und','und','ea',NULL,6), + (2,'CRMS Documentation','https://www.hathitrust.org/CRMSdocumentation',NULL,NULL,'_blank',1), + (2,'Review Search Help','/crms/web/pdf/ReviewSearchHelp.pdf',NULL,NULL,'_blank',7), + (2,'Review Search Terms','/crms/web/pdf/ReviewSearchTerms.pdf',NULL,NULL,'_blank',8), + (2,'User Levels/Privileges','/crms/web/pdf/UserLevelsPrivileges.pdf',NULL,'a','_blank',12), + (2,'System-Generated Reviews','/crms/web/pdf/SystemGeneratedReviews.pdf',NULL,'a','_blank',14), + (2,'CRMS Status Codes','/crms/web/pdf/CRMSStatusCodes.pdf',NULL,'a','_blank',15), + (3,'My Review Stats','crms?p=userRate','userRate','r',NULL,1), + (3,'All Review Stats','crms?p=adminUserRate','adminUserRate','ea',NULL,2), + (3,'System Summary','crms?p=queueStatus','queueStatus','ea',NULL,8), + (3,'Export Stats','crms?p=exportStats','exportStats','ea',NULL,9), + (4,'User Accounts','/crms/users','adminUser','eai',NULL,1), + (4,'Institutions','crms?p=institutions','institutions','a',NULL,2), + (4,'Query Rights Database','crms?p=rights','rights',NULL,NULL,3), + (4,'Query Bibliographic Rights','crms?p=bib_rights','rights',NULL,NULL,3), + (4,'Track Volumes','crms?p=track','track',NULL,NULL,4), + (4,'All Locked Volumes','crms?p=adminQueue','adminQueue','ea',NULL,5), + (4,'Add to Queue','/crms/queue/new','queueAdd','ea',NULL,6), + (4,'Set System Status','crms?p=systemStatus','systemStatus','ea',NULL,7), + (4,'System Administration','crms?p=debug','debug','s',NULL,8), + (4,'Projects','crms?p=projects','projects','a',NULL,9), + (4,'Keio Data','crms?p=keio','keio','a',NULL,10), + (4,'Licensing','crms?p=licensing','licensing','a',NULL,11); +/*!40000 ALTER TABLE `menuitems` ENABLE KEYS */; +UNLOCK TABLES; diff --git a/t/CRMS.t b/t/CRMS.t index f35f2b07..2f86f969 100755 --- a/t/CRMS.t +++ b/t/CRMS.t @@ -95,6 +95,22 @@ subtest '#LinkToJira' => sub { 'DEV-000'); }; +subtest '#MenuItems' => sub { + subtest 'with menu id' => sub { + my $items = $crms->MenuItems(1, 'autocrms'); + isa_ok($items, 'ARRAY', 'returns arrayref'); + ok(scalar @$items > 0, 'returns at least one item'); + is(scalar @{$items->[0]}, 4, 'items are 4-element arrayrefs'); + }; + + subtest 'with "docs" special keyword' => sub { + my $items = $crms->MenuItems('docs', 'autocrms'); + isa_ok($items, 'ARRAY', 'returns arrayref'); + ok(scalar @$items > 0, 'returns at least one item'); + is(scalar @{$items->[0]}, 4, 'items are 4-element arrayrefs'); + }; +}; + subtest '#GetProjectsRef' => sub { isa_ok($crms->GetProjectsRef, 'ARRAY'); };