diff --git a/cgi/CRMS.pm b/cgi/CRMS.pm index 33e08045..27e36e55 100755 --- a/cgi/CRMS.pm +++ b/cgi/CRMS.pm @@ -7281,7 +7281,8 @@ sub Rights my $proj = $self->SimpleSqlGet('SELECT project FROM queue WHERE id=?', $id); $proj = 1 unless defined $proj; my @all = (); - my $sql = 'SELECT r.id,CONCAT(a.name,"/",rs.name),r.description,a.name,rs.name FROM rights r'. + my $sql = 'SELECT r.id,CONCAT(a.name,"/",rs.name),r.description,a.name,rs.name,a.dscr,rs.dscr'. + ' FROM rights r'. ' INNER JOIN attributes a ON r.attr=a.id'. ' INNER JOIN reasons rs ON r.reason=rs.id'. ' INNER JOIN projectrights pr ON r.id=pr.rights'. @@ -7295,7 +7296,8 @@ sub Rights { push @all, {'id' => $row->[0], 'rights' => $row->[1], 'description' => $row->[2], 'n' => $n, - 'attr' => $row->[3], 'reason' => $row->[4]}; + 'attr' => $row->[3], 'reason' => $row->[4], + 'attr_dscr' => $row->[5], 'reason_dscr' => $row->[6]}; $n++; } return \@all if $order; @@ -8021,4 +8023,32 @@ sub Field008Formatter { CRMS::Field008Formatter->new; } +# TODO: move to a Utilities class or module. +# Right now the partials do not have access to the Utilities module directly +# so until that gets refactored keep this here so it can be used in a template. +# This is only used with output of CRMS::Rights for the rights.tt partial. +sub array_to_pairs { + my $self = shift; + my $array = shift; + + my $pairs = []; + if (!scalar @$array) { + return $pairs; + } + foreach my $element (@$array) { + # If there is nothing in the pairs list, or if the last entry contains two elements, add a new one-item arrayref. + # Otherwise add as the second half of the pair under construction. + if (scalar @$pairs == 0 || scalar @{$pairs->[-1]} == 2) { + push @$pairs, [$element]; + } else { + push @{$pairs->[-1]}, $element; + } + } + # Final non-pair in case of odd array, [x] -> [x, undef] + if (scalar @{$pairs->[-1]} == 1) { + push @{$pairs->[-1]}, undef; + } + return $pairs; +} + 1; diff --git a/cgi/Project/SBCR.pm b/cgi/Project/SBCR.pm new file mode 100644 index 00000000..06d9450c --- /dev/null +++ b/cgi/Project/SBCR.pm @@ -0,0 +1,144 @@ +package SBCR; +use parent 'Project'; + +use strict; +use warnings; + +use lib $ENV{'SDRROOT'} . '/crms/lib'; +use CRMS::Entitlements; + +sub new { + my $class = shift; + return $class->SUPER::new(@_); +} + +# ========== REVIEW ========== # +sub ValidateSubmission { + my $self = shift; + my $cgi = shift; + + my @errs; + my $params = $self->extract_parameters($cgi); + return 'You must select a rights/reason combination' unless $params->{rights}; + my $rights_data = CRMS::Entitlements->new(crms => $self->{crms})->rights_by_id($params->{rights}); + my $attr = $rights_data->{attribute_name}; + my $reason = $rights_data->{reason_name}; + my $rights = $rights_data->{name}; + # Renewal information + my $renNum = $params->{renNum}; + my $renDate = $params->{renDate}; + # ADD and pub date + my $date = $params->{date}; + my $actual = $params->{actual}; + # Note and note category + my $note = $params->{note}; + my $category = $params->{category}; + if ($date && $date !~ m/^-?\d{1,4}$/) { + push @errs, 'date must be only decimal digits'; + } + if (($reason eq 'add' || $reason eq 'exp') && !$date) { + push @errs, "*/$reason must include a numeric year"; + } + ## ic/ren requires a renewal number and date + if ($rights eq 'ic/ren') { + if (!$renNum || !$renDate) { + push @errs, 'ic/ren must include renewal id and renewal date'; + } + } + if ($actual && $actual !~ m/^\d{4}(-\d{4})?$/) { + push @errs, 'Actual Publication Date must be a date or a date range (YYYY or YYYY-YYYY)'; + } + return join ', ', @errs; +} + +# Extract Project-specific data from the CGI into a struct +# that will be encoded as JSON string in the reviewdata table. +sub ExtractReviewData { + my $self = shift; + my $cgi = shift; + + my $params = $self->extract_parameters($cgi); + my $data = {}; + $data->{'renNum'} = $params->{renNum} if $params->{renNum}; + $data->{'renDate'} = $params->{renDate} if $params->{renDate}; + $data->{'date'} = $params->{date} if $params->{date}; + $data->{'pub'} = 1 if $params->{pub}; + $data->{'crown'} = 1 if $params->{crown}; + $data->{'actual'} = $params->{actual} if $params->{actual}; + $data->{'approximate'} = 1 if $params->{approximate}; + return $data; +} + +sub FormatReviewData { + my $self = shift; + my $id = shift; + my $json = shift; + + # FIXME pretty() isn't needed here? + my $jsonxs = JSON::XS->new->utf8->canonical(1)->pretty(0); + my $data = $jsonxs->decode($json); + my @lines; + my $renewal_fmt = $self->format_renewal_data($data->{renNum}, $data->{renDate}); + if ($renewal_fmt) { + push @lines, $renewal_fmt; + } + if ($data->{date}) { + my $date_type = ($data->{pub})? 'Pub' : 'ADD'; + push @lines, "$date_type $data->{date}"; + } + if ($data->{crown}) { + push @lines, "Crown \x{1F451}"; + } + if ($data->{actual}) { + push @lines, "Actual Pub Date $data->{actual}"; + } + if ($data->{approximate}) { + push @lines, "Approximate Pub Date"; + } + return { + 'id' => $id, + 'format' => join('
', @lines), + 'format_long' => '' + }; +} + +sub ReviewPartials { + return [ + 'top', + 'bibdata_sbcr', + 'expertDetails_sbcr', + 'authorities', + 'sbcr_form' + ]; +} + +# extract CGI parameters into a hashref +# values are stripped +# Note: this might be useful to apply much earlier in the call chain, would +# decouple project modules from CGI +# Would have to think carefully about other possible side effect data transformations, +# don't know if it's appropriate to delve into the semantics of the review +# parameters too deeply. +sub extract_parameters { + my $self = shift; + my $cgi = shift; + + my $params = {}; + foreach my $name ($cgi->param) { + my $value = $cgi->param($name); + $value =~ s/\A\s+|\s+\z//ug; + $params->{$name} = $value; + } + return $params; +} + +sub format_renewal_data { + my $self = shift; + my $renNum = shift || ''; + my $renDate = shift || ''; + + return '' unless $renNum || $renDate; + return "Renewal $renNum / $renDate"; +} + +1; diff --git a/cgi/partial/bibdata_sbcr.tt b/cgi/partial/bibdata_sbcr.tt new file mode 100644 index 00000000..78f643d9 --- /dev/null +++ b/cgi/partial/bibdata_sbcr.tt @@ -0,0 +1,21 @@ +
+ + + + + + + + [% projs = crms.GetUserProjects() %] + [% IF projs.size > 1 %] + + [% END %] +
ID: [% htid %]
Pub Date: + + [% data.bibdata.display_date || '(unknown)' %] + +
Country: [% data.bibdata.country %]
Current Rights: [% crms.CurrentRightsString(htid) || 'unknown' %]
+ Project: [% data.project.name %] +
+
diff --git a/cgi/partial/copyrightForm.tt b/cgi/partial/copyrightForm.tt index 4eddcdf7..86193534 100644 --- a/cgi/partial/copyrightForm.tt +++ b/cgi/partial/copyrightForm.tt @@ -74,13 +74,7 @@ - + [% WHILE n < of %] diff --git a/cgi/partial/expertDetails_sbcr.tt b/cgi/partial/expertDetails_sbcr.tt new file mode 100644 index 00000000..af0622e5 --- /dev/null +++ b/cgi/partial/expertDetails_sbcr.tt @@ -0,0 +1,51 @@ +[% # Note: this should be merged with expertDetails.tt when we have a way to pass %] +[% # per-partial config from a project's ReviewPartials implementation %] +[% # and then it's just a matter of returning e.g. {file => 'expertDetails', config => {expert_only => 0}} %] +
+
Rights/Reason: - [% IF !ren %] - - [% END %] -
+ [% IF status == 3 %] + + [% ELSIF status == 2 %] + + [% END %] + + [% totalReviews = crms.GetReviewCount(htid, 1) %] + [% IF totalReviews > 0 %] + + + [% END %] + [% totalReviews = crms.GetReviewCount(htid) %] + [% IF totalReviews > 0 %] + + [% END %] + [% IF info.addedby %] + + [% END %] + [% status = crms.GetSystemStatus() %] + [% IF status.2 %] + + + + [% END %] + + diff --git a/cgi/partial/import_user.tt b/cgi/partial/import_user.tt new file mode 100644 index 00000000..319bbcc9 --- /dev/null +++ b/cgi/partial/import_user.tt @@ -0,0 +1,32 @@ +[% # When an expert is adjudicating a conflict, make it possible to import user-entered data %] +[% # rather than manually entering it (though there is that option if expert needs to add or modify %] + +[% IF expert && importUser %] + [% reviews = data.reviews %] + [% IF reviews.keys.size %] + + + + + [% FOREACH user IN reviews.keys.sort %] + + + + + [% END %] +
+
+ Import user review: +
+
+ [% # Note there is no JS that actually causes this image to be displayed %] + [% # Since importing user review data is just a matter of swapping in local JSON data %] + [% # It would probably be safe to remove this %] + +
+
+ [% END %] +[% END %] \ No newline at end of file diff --git a/cgi/partial/notes.tt b/cgi/partial/notes.tt new file mode 100644 index 00000000..b1e4805a --- /dev/null +++ b/cgi/partial/notes.tt @@ -0,0 +1,12 @@ +
+ + + + . +
diff --git a/cgi/partial/rights.tt b/cgi/partial/rights.tt new file mode 100644 index 00000000..1fa48a9b --- /dev/null +++ b/cgi/partial/rights.tt @@ -0,0 +1,56 @@ + +[% # Note: before trying to use this with projects other than SBCR, need to add a prediction loader in the after Rights/Reason %] +[% # and modify the JS togglePredictionLoader() routine to take an id parameter. %] +[% # SBCR has a prediction loader for the renDate so we need to trigger a loader by id instead of %] +[% # assuming there will only be one loader per operational pane, and its id will be "predictionLoader" %] +[% rights = crms.Rights(htid, 1) %] +
+
+ + Rights/Reason Help + + [% # This is an experimental version of the "rights reference" tooltip which uses %] + [% # attr/reason descriptions from ht_rights rather than our own, which are not well %] + [% # maintained and contain many blanks. %] + [% # KH says: we do not need specialized CRMS rights descriptions. %] + [% # When projects other than SBCR begin to use this, the `description` column from %] + [% # the `crms.rights` table can be removed %] + [% FOR right IN rights %] + [% right.rights %] - [% right.attr_dscr %] / [% right.reason_dscr %]
+ [% END %] +
+
+
+ [% # TODO: the rights structure is needed by every project. %] + [% # We should provide it to the templates and let them derive layouts as needed %] + [% # e.g., convert to two columns, convert to pairs, via utility routines %] + [% rights = crms.array_to_pairs(crms.Rights(htid)) %] + + + + + + [% FOREACH rights_pair IN rights %] + + [% right = rights_pair.0 %] + + + + + [% END %] +
Rights/Reason:
+ + + + [% right = rights_pair.1 %] + [% IF right %] + + + [% END %] +
+
+ diff --git a/cgi/partial/sbcr_form.tt b/cgi/partial/sbcr_form.tt new file mode 100644 index 00000000..d773a36b --- /dev/null +++ b/cgi/partial/sbcr_form.tt @@ -0,0 +1,329 @@ +[% cgi = crms.get('cgi') %] +[% importUser = cgi.param('importUser') %] + +[% # Set up values from CGI (in case of error) or existing review (editing). %] +[% u_renNum = (error)? cgi.param('renNum') : data.reviews.$user.data.renNum %] +[% u_renDate = (error)? cgi.param('renDate') : data.reviews.$user.data.renDate %] +[% u_date = (error)? cgi.param('date') : data.reviews.$user.data.date %] +[% u_pub = (error)? cgi.param('pub') : data.reviews.$user.data.pub %] +[% u_crown = (error)? cgi.param('crown') : data.reviews.$user.data.crown %] +[% u_actual = (error)? cgi.param('actual') : data.reviews.$user.data.actual %] +[% u_approximate = (error)? cgi.param('approximate') : data.reviews.$user.data.approximate %] +[% u_rights = (error)? cgi.param('rights') : data.reviews.$user.rights %] +[% u_category = (error)? cgi.param('category') : data.reviews.$user.category %] +[% u_note = (error)? cgi.param('note') : data.reviews.$user.note %] +[% u_swiss = (error)? cgi.param('swiss') : data.reviews.$user.swiss %] +[% u_hold = (error)? cgi.param('hold') : data.reviews.$user.hold %] + +[% writing_hand_tag = "writing hand" %] +[% author_death_date_text = "Author Death Date: " _ writing_hand_tag %] +[% publication_date_text = "Publication Date:" %] + +
+
+ + + + + + + [% viafwarn = crms.VIAFWarning(htid, data.record) %] + [% IF viafwarn %] + [% viafwarn = "Warning: possible foreign author: " _ viafwarn %] + + [% END %] + + + + + + + + + + + + + + + + [% label = (u_pub)? publication_date_text : author_death_date_text %] + + + + + + [% display = "table-row" %] + [% IF u_pub %] + [% display = "none" %] + [% END %] + + + + + + + + + + + + + + + + + +
$viafwarn
+ + + +
+ + + +
+ + + + + +
+ + + + + +
+ + + +
+ + +
+ + +
+ +
+ [% INCLUDE partial/rights.tt %] + [% INCLUDE partial/notes.tt %] + +
+ + + + + +
+
+ + [% IF expert %] + + [% checked = (u_swiss || ((status == 2 || status == 3) && data.project.SwissByDefault())) %] + + + + [% END %] + [% holds = crms.CountHolds() %] + [% IF !hold %] + [% hold = crms.HoldForItem(htid, user) %] + [% END %] + + + + + + + +
+ +
+ =5 && !hold)? 'disabled="disabled"':'' %] + [% (hold)? 'checked="checked"':'' %] + onclick="toggleVisibility('expiry');"/> +
+ =5)? 'style="color:red;"':'' %]> + + You currently have [% holds %] out of the maximum 5 volumes on hold. + + +
+ [% INCLUDE partial/import_user.tt %] +
+
+ + + +
diff --git a/docker/db/sql/001_crms_schema.sql b/docker/db/sql/001_crms_schema.sql index 7c6decad..5a4fdb9a 100644 --- a/docker/db/sql/001_crms_schema.sql +++ b/docker/db/sql/001_crms_schema.sql @@ -1037,7 +1037,8 @@ CREATE TABLE `rights` ( `attr` tinyint(4) NOT NULL, `reason` tinyint(4) NOT NULL, `description` text, - PRIMARY KEY (`id`) + PRIMARY KEY (`id`), + UNIQUE KEY `unique_attr_reason` (`attr`,`reason`) ) ENGINE=InnoDB AUTO_INCREMENT=26 DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; diff --git a/lib/CRMS/Entitlements.pm b/lib/CRMS/Entitlements.pm new file mode 100644 index 00000000..b308b69a --- /dev/null +++ b/lib/CRMS/Entitlements.pm @@ -0,0 +1,129 @@ +package CRMS::Entitlements; + +# NOTE THIS IS A TEMPORARY NAME +# CRMS::Rights conflicts with a method defined in the CRMS module +# Once this module is proven, it can replace some/all of the rights/attributes/reasons +# methods in CRMS.pm + +# Manages an in-memory copy of the crms.rights table +# and through it the attributes and reasons it ties together. + +# As of CRMS version 8.7.1 the crms.rights table has a UNIQUE constraint on the attr,reason +# combination. As a result, a method like `rights_by_attribute_reason` need never worry +# about handling more than one result. + +use strict; +use warnings; + +use Data::Dumper; + +# This is a singleton. Rights, attributes, and reasons are static and can be cached. +# Derivatives based on project_rights, if any, should not be cached because they can change. +my $ONE_TRUE_ENTITLEMENTS; + +sub new { + my ($class, %args) = @_; + if (!$ONE_TRUE_ENTITLEMENTS) { + my $self = bless {}, $class; + # TODO: once we have a standalone DB module this can go away. + my $crms = $args{crms}; + if (!defined $crms) { + die "CRMS::Entitlements module needs CRMS instance."; + } + $self->{crms} = $crms; + # Eager load lookup tables + $self->_load_tables; + $ONE_TRUE_ENTITLEMENTS = $self; + } + return $ONE_TRUE_ENTITLEMENTS; +} + +sub rights_by_id { + my $self = shift; + my $id = shift; + + return $self->{rights}->{$id}; +} + +sub rights_by_attribute_reason { + my $self = shift; + my $attribute = shift; + my $reason = shift; + + # Translate attribute and reason into names if numeric + if ($attribute =~ m/^\d+$/) { + $attribute = $self->attribute_by_id($attribute)->{name}; + } + if ($reason =~ m/^\d+$/) { + $reason = $self->reason_by_id($reason)->{name}; + } + return $self->{rights_by_name}->{"$attribute/$reason"}; +} + +# Returns a hashref with the fields id, type, dscr, name just as they appear in the +# `attributes` table +sub attribute_by_id { + my $self = shift; + my $id = shift; + + return $self->{attributes_by_id}->{$id}; +} + +# Returns a hashref with the fields id, type, dscr, name just as they appear in the +# `attributes` table +sub attribute_by_name { + my $self = shift; + my $name = shift; + + return $self->{attributes_by_name}->{$name}; +} + +# Returns a hashref with the fields id, dscr, name just as they appear in the +# `reasons` table +sub reason_by_id { + my $self = shift; + my $id = shift; + + return $self->{reasons_by_id}->{$id}; +} + +# Returns a hashref with the fields id, dscr, name just as they appear in the +# `reasons` table +sub reason_by_name { + my $self = shift; + my $name = shift; + + return $self->{reasons_by_name}->{$name}; +} + +# Set up slightly duplicative lookup tables for fast attribute/reason access by id or by name. +# Also set up rights lookup by id. +sub _load_tables { + my $self = shift; + + # crms.attributes + my $sql = 'SELECT * FROM attributes ORDER BY id'; + $self->{attributes_by_id} = $self->{crms}->GetDb->selectall_hashref($sql, 'id'); + $self->{attributes_by_name} = $self->{crms}->GetDb->selectall_hashref($sql, 'name'); + # crms.reasons + $sql = 'SELECT * FROM reasons ORDER BY id'; + $self->{reasons_by_id} = $self->{crms}->GetDb->selectall_hashref($sql, 'id'); + $self->{reasons_by_name} = $self->{crms}->GetDb->selectall_hashref($sql, 'name'); + # crms.rights + $self->{rights} = {}; + $self->{rights_by_name} = {}; + $sql = 'SELECT * FROM rights ORDER BY id'; + $self->{rights} = $self->{crms}->GetDb->selectall_hashref($sql, 'id'); + # Decorare each entry with attribute and reason names + foreach my $id (keys %{$self->{rights}}) { + my $rights = $self->{rights}->{$id}; + my $attr_name = $self->attribute_by_id($rights->{attr})->{name}; + my $reason_name = $self->reason_by_id($rights->{reason})->{name}; + $rights->{attribute_name} = $attr_name; + $rights->{reason_name} = $reason_name; + $rights->{name} = "$attr_name/$reason_name"; + $self->{rights_by_name}->{$rights->{name}} = $rights; + } +} + +1; diff --git a/lib/CRMS/Version.pm b/lib/CRMS/Version.pm index 865238d5..72c554b9 100644 --- a/lib/CRMS/Version.pm +++ b/lib/CRMS/Version.pm @@ -3,6 +3,6 @@ package CRMS::Version; use strict; use warnings; -our $VERSION = '8.7.2'; +our $VERSION = '8.8.0'; 1; diff --git a/t/CRMS.t b/t/CRMS.t index 4fa3f68d..9eec53ce 100755 --- a/t/CRMS.t +++ b/t/CRMS.t @@ -191,4 +191,16 @@ subtest '#Field008Formatter' => sub { isa_ok $crms->Field008Formatter, "CRMS::Field008Formatter"; }; +subtest '#Field008Formatter' => sub { + isa_ok $crms->Field008Formatter, "CRMS::Field008Formatter"; +}; + +subtest 'array_to_pairs' => sub { + is_deeply($crms->array_to_pairs([]), [], 'no elements'); + is_deeply($crms->array_to_pairs([1]), [[1, undef]], 'one element'); + is_deeply($crms->array_to_pairs([1, 2]), [[1, 2]], 'two elements'); + is_deeply($crms->array_to_pairs([1, 2, 3]), [[1, 2], [3, undef]], 'three elements'); + is_deeply($crms->array_to_pairs([1, 2, 3, 4]), [[1, 2], [3, 4]], 'four elements'); +}; + done_testing(); diff --git a/t/Project.t b/t/Project.t index 595102a6..5d0c42a6 100755 --- a/t/Project.t +++ b/t/Project.t @@ -2,11 +2,14 @@ use strict; use warnings; -BEGIN { unshift(@INC, $ENV{'SDRROOT'}. '/crms/cgi'); } use Test::More; +use lib $ENV{'SDRROOT'} . '/crms/cgi'; +use lib $ENV{'SDRROOT'} . '/crms/lib'; + use CRMS; +use CRMS::Entitlements; my $dir = $ENV{'SDRROOT'}. '/crms/cgi/Project'; opendir(DIR, $dir) or die "Can't open $dir\n"; @@ -20,6 +23,12 @@ foreach my $file (sort @files) } my $crms = CRMS->new; +my $entitlements = CRMS::Entitlements->new(crms => $crms); +# Rights ids used across subtests + +my $ic_cdpp_rights_id = $entitlements->rights_by_attribute_reason('ic', 'cdpp')->{id}; +my $und_nfi_rights_id = $entitlements->rights_by_attribute_reason('und', 'nfi')->{id}; +my $und_ren_rights_id = $entitlements->rights_by_attribute_reason('und', 'ren')->{id}; my $project = Project->new(crms => $crms); subtest '#queue_order' => sub { @@ -30,5 +39,327 @@ subtest '#PresentationOrder' => sub { is($project->PresentationOrder, undef, 'default project has no PresentationOrder'); }; +subtest 'ValidateSubmission' => sub { + subtest 'no rights selected' => sub { + my $cgi = CGI->new; + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/rights\/reason combination/, 'error displayed'); + }; + + subtest 'und/nfi must include note category and note text' => sub { + subtest 'with category and note' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_nfi_rights_id); + $cgi->param('category', 'Edition'); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include note category and note text/, 'no error'); + }; + + subtest 'without category' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_nfi_rights_id); + # Setting the category explicitly to empty string is needed to avoid + # "uninitialized value $category" warnings in Project.pm. + # These can be all removed when that is fixed with a default empty string value. + $cgi->param('category', ''); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include note category and note text/, 'error displayed'); + }; + + subtest 'with neither' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_nfi_rights_id); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include note category and note text/, 'error displayed'); + }; + }; + + subtest 'ic/ren must include renewal id and renewal date' => sub { + my $ic_ren_rights_id = $entitlements->rights_by_attribute_reason('ic', 'ren')->{id}; + subtest 'with renewal data' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_ren_rights_id); + $cgi->param('renNum', 'R123'); + $cgi->param('renDate', '4Jun23'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include renewal id and renewal date/, 'no error'); + }; + + subtest 'with just renewal id' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_ren_rights_id); + $cgi->param('renNum', 'R123'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include renewal id and renewal date/, 'error displayed'); + }; + + subtest 'without renewal data' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_ren_rights_id); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include renewal id and renewal date/, 'error displayed'); + }; + }; + + subtest 'pd/ren should not include renewal info' => sub { + my $pd_ren_rights_id = $entitlements->rights_by_attribute_reason('pd', 'ren')->{id}; + subtest 'without renewal info' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $pd_ren_rights_id); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/should not include renewal info/, 'no error'); + }; + + # FIXME: next two tests show arguably incorrect behavior, the error should be triggered if either + # renNum or renDate is present. The assumption has been that renDate will always be present + # if renNum is, and that's maybe not quite true. + subtest 'with renNum' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $pd_ren_rights_id); + $cgi->param('renNum', 'R123'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/should not include renewal info/, 'no error'); + }; + + subtest 'with renDate' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $pd_ren_rights_id); + $cgi->param('renDate', '4Jun23'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/should not include renewal info/, 'no error'); + }; + + subtest 'with both' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $pd_ren_rights_id); + $cgi->param('renDate', '4Jun23'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/should not include renewal info/, 'error displayed'); + }; + }; + + subtest 'pd*/cdpp must not include renewal data' => sub { + foreach my $attr ('pd', 'pdus') { + my $rights = $entitlements->rights_by_attribute_reason($attr, 'cdpp')->{id}; + subtest "$attr with renewal number" => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('renNum', 'R123'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must not include renewal info/, 'error displayed'); + }; + + subtest "$attr with renewal date" => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('renDate', '4Jun23'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must not include renewal info/, 'error displayed'); + }; + + subtest "$attr without renewal data" => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must not include renewal info/, 'no error'); + }; + } + }; + + subtest 'pd/cdpp must include note category and note text' => sub { + my $rights = $entitlements->rights_by_attribute_reason('pd', 'cdpp')->{id}; + subtest 'with both' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('category', 'Edition'); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include note category and note text/, 'no error'); + }; + + subtest 'with note only' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('category', ''); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include note category and note text/, 'error displayed'); + }; + + subtest 'with neither' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include note category and note text/, 'error displayed'); + }; + }; + + # NOTE: this could be merged with the pd/cdpp and pdus/cdpp logic above + subtest 'ic/cdpp must not include renewal data' => sub { + subtest 'with renewal number' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_cdpp_rights_id); + $cgi->param('renNum', 'R123'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/should not include renewal info/, 'error displayed'); + }; + + subtest 'with renewal date' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_cdpp_rights_id); + $cgi->param('renDate', '4Jun23'); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/should not include renewal info/, 'error displayed'); + }; + }; + + # NOTE: this could be merged with the pd/cdpp and pdus/cdpp logic above + subtest 'ic/cdpp must include note category and note text' => sub { + subtest 'with both' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_cdpp_rights_id); + $cgi->param('category', 'Edition'); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include note category and note text/, 'no error'); + }; + + subtest 'with note only' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_cdpp_rights_id); + $cgi->param('category', ''); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include note category and note text/, 'error displayed'); + }; + + subtest 'with neither' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_cdpp_rights_id); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include note category and note text/, 'error displayed'); + }; + }; + + subtest 'und/ren must have note category Inserts/No Renewal' => sub { + subtest 'with expected category' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_ren_rights_id); + $cgi->param('category', 'Inserts/No Renewal'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/mmust have note category Inserts\/No Renewal/, 'no error'); + }; + + subtest 'without expected category' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_ren_rights_id); + $cgi->param('category', 'Edition'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must have note category Inserts\/No Renewal/, 'no error'); + }; + + subtest 'with no category' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_ren_rights_id); + $cgi->param('category', ''); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must have note category/, 'error displayed'); + }; + }; + + subtest 'Inserts/No Renewal category is only used with und/ren' => sub { + subtest 'with expected rights' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_ren_rights_id); + $cgi->param('category', 'Inserts/No Renewal'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must have rights code/, 'no error'); + }; + + subtest 'without expected rights' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $und_nfi_rights_id); + $cgi->param('category', 'Inserts/No Renewal'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must have rights code/, 'error displayed'); + }; + }; + + subtest "note optionality" => sub { + my $note_required = $crms->SimpleSqlGet('SELECT name FROM categories WHERE need_note=1 AND interface=1 AND restricted IS NULL'); + my $note_optional = $crms->SimpleSqlGet('SELECT name FROM categories WHERE need_note=0 AND interface=1 AND restricted IS NULL'); + subtest 'category without required note' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('category', $note_required); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include a note/, 'error displayed'); + }; + + subtest 'category with required note' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('category', $note_required); + $cgi->param('note', 'This is a required note'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include a note/, 'no error'); + }; + + subtest 'category without optional note' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('category', $note_optional); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include a note/, 'no error'); + }; + + subtest 'category with required note' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('category', $note_optional); + $cgi->param('note', 'This is an optional note'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include a note/, 'no error'); + }; + }; + + subtest 'must include a category if there is a note' => sub { + subtest 'note with category' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('note', 'This is a note'); + $cgi->param('category', 'Misc'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include a category/, 'no error'); + }; + + subtest 'note without category' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('category', ''); + $cgi->param('note', 'This is a note'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include a category/, 'error displayed'); + }; + }; +}; + done_testing(); diff --git a/t/Project/CrownCopyright.t b/t/Project/CrownCopyright.t new file mode 100644 index 00000000..3e6c231b --- /dev/null +++ b/t/Project/CrownCopyright.t @@ -0,0 +1,58 @@ +#!/usr/bin/perl + +use strict; +use warnings; +use utf8; + +use CGI; +use Test::More; + +use lib $ENV{'SDRROOT'} . '/crms/cgi'; +use lib $ENV{'SDRROOT'} . '/crms/lib'; +use CRMS; +use CRMS::Entitlements; + +my $crms = CRMS->new(); +my $entitlements = CRMS::Entitlements->new(crms => $crms); + +require_ok($ENV{'SDRROOT'}. '/crms/cgi/Project/CrownCopyright.pm'); + +my $sql = 'SELECT id FROM projects WHERE name="Crown Copyright"'; +my $project_id = $crms->SimpleSqlGet($sql); +my $project = CrownCopyright->new(crms => $crms, id => $project_id); +ok(defined $project); + +subtest 'ValidateSubmission' => sub { + subtest 'no rights selected' => sub { + my $cgi = CGI->new; + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/rights\/reason combination/); + }; + + subtest 'Not Government category requires und/NFI' => sub { + subtest 'Not Government category with und/nfi' => sub { + my $rights = $entitlements->rights_by_attribute_reason('und', 'nfi')->{id}; + my $cgi = CGI->new; + $cgi->param('rights', $rights); + # Setting the date explicitly to empty string is needed to avoid + # "uninitialized value $date" warnings in CrownCopyright.pm. + # These can be all removed when that is fixed with a default empty string value. + $cgi->param('date', ''); + $cgi->param('category', 'Not Government'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/Not Government category requires/); + }; + + subtest 'Not Government category without und/nfi' => sub { + my $rights = $entitlements->rights_by_attribute_reason('ic', 'ren')->{id}; + my $cgi = CGI->new; + $cgi->param('rights', $rights); + $cgi->param('date', ''); + $cgi->param('category', 'Not Government'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/Not Government category requires/); + }; + }; +}; + +done_testing(); diff --git a/t/Project/SBCR.t b/t/Project/SBCR.t new file mode 100644 index 00000000..8134af6c --- /dev/null +++ b/t/Project/SBCR.t @@ -0,0 +1,265 @@ +#!/usr/bin/perl + +use strict; +use warnings; +use utf8; + +use CGI; +use Data::Dumper; +use JSON::XS; +use Test::More; + +use lib $ENV{'SDRROOT'} . '/crms/cgi'; +use lib $ENV{'SDRROOT'} . '/crms/lib'; +use CRMS; +use CRMS::Entitlements; + +my $jsonxs = JSON::XS->new->utf8->canonical(1)->pretty(0); +# Will be used multiple times in this test suite. +my $crms = CRMS->new(); +my $entitlements = CRMS::Entitlements->new(crms => $crms); + +require_ok($ENV{'SDRROOT'}. '/crms/cgi/Project/SBCR.pm'); + +my $sql = 'SELECT id FROM projects WHERE name="SBCR"'; +my $project_id = $crms->SimpleSqlGet($sql); +my $project = SBCR->new(crms => $crms, id => $project_id); +ok(defined $project); + +subtest 'PresentationOrder' => sub { + my $order = $project->PresentationOrder; + ok(!defined $order, 'does not define a presentation order'); +}; + +subtest 'ReviewPartials' => sub { + ok(defined $project->ReviewPartials, 'defines a UI ordering'); +}; + +subtest 'ValidateSubmission' => sub { + subtest 'no rights selected' => sub { + my $cgi = CGI->new; + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/rights\/reason combination/); + }; + + subtest 'date must be only decimal digits' => sub { + subtest 'acceptable inputs' => sub { + foreach my $date ('1234', '-1234') { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('date', $date); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/date must be only decimal digits/); + } + }; + + subtest 'unacceptable inputs' => sub { + foreach my $date ('12345', '-12345', 'c.1950') { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('date', $date); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/date must be only decimal digits/); + } + }; + + subtest 'no date submitted' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/date must be only decimal digits/); + }; + }; + + subtest '*/add and */exp must include a numeric year' => sub { + foreach my $id (keys %{$entitlements->{rights}}) { + my $rights = $entitlements->{rights}->{$id}; + if ($rights->{reason_name} eq 'add' || $rights->{reason_name} eq 'exp') { + subtest "$rights->{name} with date" => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights->{id}); + $cgi->param('date', 1957); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include a numeric year/); + }; + + subtest "$rights->{name} without date" => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights->{id}); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include a numeric year/); + }; + } else { + subtest "$rights->{name} with date" => sub { + my $cgi = CGI->new; + $cgi->param('rights', $rights->{id}); + $cgi->param('date', 1957); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include a numeric year/); + }; + } + } + }; + + subtest 'ic/ren must include renewal id and renewal date' => sub { + my $ic_ren_rights_id = $entitlements->rights_by_attribute_reason('ic', 'ren')->{id}; + subtest 'with renewal data' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_ren_rights_id); + $cgi->param('renNum', 'R123'); + $cgi->param('renDate', '4Jun23'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/must include renewal id and renewal date/); + }; + + subtest 'with just renewal id' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_ren_rights_id); + $cgi->param('renNum', 'R123'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include renewal id and renewal date/); + }; + + subtest 'without renewal data' => sub { + my $cgi = CGI->new; + $cgi->param('rights', $ic_ren_rights_id); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/must include renewal id and renewal date/); + }; + }; + + subtest 'actual publication date' => sub { + subtest 'single date' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('actual', '9999'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/YYYY or YYYY-YYYY/); + }; + + subtest 'date range' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('actual', '9990-9999'); + my $err = $project->ValidateSubmission($cgi); + ok($err !~ m/YYYY or YYYY-YYYY/); + }; + + subtest 'nonsense' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('actual', 'abcde'); + my $err = $project->ValidateSubmission($cgi); + ok($err =~ m/YYYY or YYYY-YYYY/); + }; + }; + # End of ValidateSubmission subtest +}; + +subtest 'ExtractReviewData' => sub { + subtest 'with lots of data, some of it messy' => sub { + my $cgi = CGI->new; + $cgi->param('renNum', ' R123'); + $cgi->param('renDate', ' 26Sep39'); + $cgi->param('date', '1950'); + $cgi->param('pub', 'on'); + $cgi->param('crown', 'on'); + $cgi->param('actual', '1960'); + $cgi->param('approximate', 'on'); + my $extracted = $project->ExtractReviewData($cgi); + is($extracted->{renNum}, 'R123'); + is($extracted->{renDate}, '26Sep39'); + is($extracted->{date}, '1950'); + is($extracted->{pub}, 1); + is($extracted->{crown}, 1); + is($extracted->{actual}, '1960'); + is($extracted->{approximate}, 1); + }; + + subtest 'with very little data' => sub { + my $cgi = CGI->new; + my $extracted = $project->ExtractReviewData($cgi); + is_deeply($extracted, {}); + }; +}; + +subtest 'FormatReviewData' => sub { + subtest 'with lots of data' => sub { + my $data = { + renNum => 'R123', + renDate => '26Sep39', + date => '1950', + pub => 0, + crown => 1, + actual => '1960', + approximate => 1 + }; + my $json = $jsonxs->encode($data); + my $format = $project->FormatReviewData(1, $json); + ok($format->{format} =~ /Renewal/); + ok($format->{format} =~ /ADD/); + ok($format->{format} !~ /Pub/); + ok($format->{format} =~ /Crown/); + ok($format->{format} =~ /Actual/); + is($format->{id}, 1); + }; + + subtest 'with a little data' => sub { + my $data = { + date => '1960', + pub => 1 + }; + my $json = $jsonxs->encode($data); + my $format = $project->FormatReviewData(1, $json); + ok($format->{format} !~ /Renewal/); + ok($format->{format} !~ /ADD/); + ok($format->{format} =~ /Pub/); + ok($format->{format} !~ /Crown/); + ok($format->{format} !~ /Actual/); + is($format->{id}, 1); + }; + + subtest 'with no data' => sub { + my $data = {}; + my $json = $jsonxs->encode($data); + my $format = $project->FormatReviewData(1, $json); + ok($format->{format} !~ /Renewal/); + ok($format->{format} !~ /ADD/); + ok($format->{format} !~ /Pub/); + ok($format->{format} !~ /Crown/); + ok($format->{format} !~ /Actual/); + is($format->{id}, 1); + }; +}; + +subtest 'extract_parameters' => sub { + my $cgi = CGI->new; + $cgi->param('rights', 1); + $cgi->param('renNum', " R12345\n"); + my $params = $project->extract_parameters($cgi); + is($params->{rights}, 1, 'leaves rights unchanged'); + is($params->{renNum}, 'R12345', 'strips whitespace'); +}; + +subtest 'format_renewal_data' => sub { + subtest 'with no data' => sub { + ok(length $project->format_renewal_data(undef, undef) == 0); + }; + + subtest 'with only renNum' => sub { + ok($project->format_renewal_data('R123', undef) =~ m/R123/); + }; + + subtest 'with only renDate' => sub { + ok($project->format_renewal_data(undef, '1Oct51') =~ m/1Oct51/); + }; + + subtest 'with both renNum and renDate' => sub { + ok($project->format_renewal_data('R123', '1Oct51') =~ m/R123/); + ok($project->format_renewal_data('R123', '1Oct51') =~ m/1Oct51/); + }; +}; + +done_testing(); + + diff --git a/t/lib/CRMS/Entitlements.t b/t/lib/CRMS/Entitlements.t new file mode 100644 index 00000000..4c3d0009 --- /dev/null +++ b/t/lib/CRMS/Entitlements.t @@ -0,0 +1,69 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use Test::Exception; +use Test::More; + +use lib $ENV{'SDRROOT'} . '/crms/cgi'; +use lib $ENV{'SDRROOT'} . '/crms/lib'; +use CRMS; +use CRMS::Entitlements; + +my $crms = CRMS->new; + +#$CRMS::Entitlements::ONE_TRUE_ENTITLEMENTS = undef; +subtest 'new' => sub { + subtest 'Missing CRMS' => sub { + dies_ok { CRMS::Entitlements->new; }; + }; + + subtest 'CRMS supplied' => sub { + my $rights = CRMS::Entitlements->new(crms => $crms, reinit => 1); + isa_ok($rights, 'CRMS::Entitlements'); + }; +}; + +subtest 'rights_by_id' => sub { + my $rights = CRMS::Entitlements->new(crms => $crms)->rights_by_id(5); + is($rights->{id}, 5); +}; + +subtest 'rights_by_attribute_reason' => sub { + subtest 'with ids' => sub { + my $rights = CRMS::Entitlements->new(crms => $crms)->rights_by_attribute_reason(2, 7); + is($rights->{attribute_name}, 'ic'); + is($rights->{reason_name}, 'ren'); + is($rights->{name}, 'ic/ren'); + }; + + subtest 'with names' => sub { + my $rights = CRMS::Entitlements->new(crms => $crms)->rights_by_attribute_reason('ic', 'ren'); + is($rights->{attribute_name}, 'ic'); + is($rights->{reason_name}, 'ren'); + is($rights->{name}, 'ic/ren'); + }; +}; + +subtest 'attribute_by_id' => sub { + my $attr = CRMS::Entitlements->new(crms => $crms)->attribute_by_id(1); + is($attr->{name}, 'pd', 'attribute 1 is named "pd"'); +}; + +subtest 'attribute_by_name' => sub { + my $attr = CRMS::Entitlements->new(crms => $crms)->attribute_by_name('pd'); + is($attr->{id}, 1, 'attribute "pd" is id=1'); +}; + +subtest 'reason_by_id' => sub { + my $reason = CRMS::Entitlements->new(crms => $crms)->reason_by_id(1); + is($reason->{name}, 'bib', 'reason 1 is named "bib"'); +}; + +subtest 'reason_by_name' => sub { + my $reason = CRMS::Entitlements->new(crms => $crms)->reason_by_name('bib'); + is($reason->{id}, 1, 'reason "bib" is id=1'); +}; + +done_testing(); diff --git a/web/css/review.css b/web/css/review.css index a19e5809..257410fc 100644 --- a/web/css/review.css +++ b/web/css/review.css @@ -207,6 +207,9 @@ table.ReviewError { font-size:12px; } +/* + FIXME: remove the #Notes styles when all review interfaces are using .note-container class +*/ #Notes { margin-left:10px; } @@ -215,6 +218,14 @@ table.ReviewError { margin-block-start: 5px; } +.note-container { + margin-left:10px; +} + +.note-container textarea { + margin-block-start: 5px; +} + #SubmitForm { margin-block-start: 8px; } diff --git a/web/js/review.js b/web/js/review.js index 8bf76cd6..92f89e4d 100644 --- a/web/js/review.js +++ b/web/js/review.js @@ -51,12 +51,16 @@ function popRenewalDate() { var icren = document.getElementById('ICREN'); var pdusren = document.getElementById('PDUSREN'); + // TODO: this is a really clunky way of making the mapping from rights name -> + // rights id (and ultimately the radio button with that id value) available to JavaScript. + // Ideally, review.tt should query Entitlements.pm and construct JSON with name -> id map + // as a global variable. var icrenButton, pdusrenButton; if (icren) { icrenButton = document.getElementById("r" + icren.title); } - if (icren) + if (pdusren) { pdusrenButton = document.getElementById("r" + pdusren.title); } @@ -135,7 +139,7 @@ function ajaxURL(target) { function togglePredictionLoader(display) { var img = document.getElementById("predictionLoader"); if (img) { - img.style.display = display ? "block" : "none"; + img.style.display = display ? "inline-block" : "none"; } }