From a8ca1e29c45b4fac7506bb0f872a7c041ece64eb Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Tue, 17 Jul 2012 16:58:02 +0200 Subject: [PATCH 4/4] Bug 7688: Follow-up: Fixes and improvements - constraint between subscription and subscription_frequencies changed (added ON DELETE/ON UPDATE) - constraint between subscription and subscription_numberpatterns changed (added ON DELETE/ON UPDATE) - When deleting a frequency or a numbering pattern, ask for a confirmation if it's still used by subscriptions - updatedatabase.pl do not blindly insert numbering pattern anymore. Now it will first check if a similar pattern already exists in database. - "Numbering" reworded to "Formatting", text inputs replaced by hard-coded selects - Avoid creating a new numbering pattern with no name - On subscription-numberpatterns.pl, replace Calendar by jQueryUI datepicker --- C4/Serials/Frequency.pm | 30 ++++++++ C4/Serials/Numberpattern.pm | 28 +++++++ installer/data/mysql/kohastructure.sql | 4 +- installer/data/mysql/updatedatabase.pl | 53 +++++++++---- .../prog/en/modules/serials/subscription-add.tt | 57 +++++++++++--- .../en/modules/serials/subscription-frequencies.tt | 34 ++++++++- .../modules/serials/subscription-numberpatterns.tt | 78 +++++++++++++++----- serials/subscription-add.pl | 10 +++ serials/subscription-frequencies.pl | 18 ++++- serials/subscription-numberpatterns.pl | 24 +++++- 10 files changed, 287 insertions(+), 49 deletions(-) diff --git a/C4/Serials/Frequency.pm b/C4/Serials/Frequency.pm index 0559d03..d5c1cbd 100644 --- a/C4/Serials/Frequency.pm +++ b/C4/Serials/Frequency.pm @@ -35,6 +35,8 @@ BEGIN { &AddSubscriptionFrequency &ModSubscriptionFrequency &DelSubscriptionFrequency + + &GetSubscriptionsWithFrequency ); } @@ -244,6 +246,34 @@ sub DelSubscriptionFrequency { $sth->execute($frequencyid); } +=head3 GetSubscriptionsWithFrequency + + my @subs = GetSubscriptionsWithFrequency($frequencyid); + +Returns all subscriptions that are using a particular frequency + +=cut + +sub GetSubscriptionsWithFrequency { + my ($frequencyid) = @_; + + return unless $frequencyid; + + my $dbh = C4::Context->dbh; + my $query = qq{ + SELECT * + FROM subscription + LEFT JOIN biblio ON subscription.biblionumber = biblio.biblionumber + WHERE periodicity = ? + }; + my $sth = $dbh->prepare($query); + my @results; + if ($sth->execute($frequencyid)) { + @results = @{ $sth->fetchall_arrayref({}) }; + } + return @results; +} + 1; __END__ diff --git a/C4/Serials/Numberpattern.pm b/C4/Serials/Numberpattern.pm index 1fb5ea9..4e35803 100644 --- a/C4/Serials/Numberpattern.pm +++ b/C4/Serials/Numberpattern.pm @@ -38,6 +38,7 @@ BEGIN { &ModSubscriptionNumberpattern &DelSubscriptionNumberpattern + &GetSubscriptionsWithNumberpattern ); } @@ -253,6 +254,33 @@ sub DelSubscriptionNumberpattern { $sth->execute($numberpatternid); } +=head3 GetSubscriptionsWithNumberpattern + + my @subs = GetSubscriptionsWithNumberpattern($numberpatternid); + +Returns all subscriptions that are using a particular numbering pattern + +=cut + +sub GetSubscriptionsWithNumberpattern { + my ($numberpatternid) = @_; + + return unless $numberpatternid; + + my $dbh = C4::Context->dbh; + my $query = qq{ + SELECT * + FROM subscription + LEFT JOIN biblio ON subscription.biblionumber = biblio.biblionumber + WHERE numberpattern = ? + }; + my $sth = $dbh->prepare($query); + my @results; + if ($sth->execute($numberpatternid)) { + @results = @{ $sth->fetchall_arrayref({}) }; + } + return @results; +} 1; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 1105143..ee0d662 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1927,8 +1927,8 @@ CREATE TABLE `subscription` ( `graceperiod` int(11) NOT NULL default '0', `enddate` date default NULL, PRIMARY KEY (`subscriptionid`), - CONSTRAINT subscription_ibfk_1 FOREIGN KEY (periodicity) REFERENCES subscription_frequencies (id), - CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) + CONSTRAINT subscription_ibfk_1 FOREIGN KEY (periodicity) REFERENCES subscription_frequencies (id) ON DELETE SET NULL ON UPDATE CASCADE, + CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) ON DELETE SET NULL ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -- diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 40c8a43..5deee00 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -5677,6 +5677,15 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) |); + my $check_numberpatterns_sth = $dbh->prepare(qq| + SELECT * FROM subscription_numberpatterns + WHERE add1 = ? AND add2 = ? AND add3 = ? + AND every1 = ? AND every2 = ? AND every3 = ? + AND whenmorethan1 = ? AND whenmorethan2 = ? AND whenmorethan3 = ? + AND setto1 = ? AND setto2 = ? AND setto3 = ? + AND numberingmethod = ? + LIMIT 1 + |); my $update_subscription_sth = $dbh->prepare(qq| UPDATE subscription SET numberpattern = ?, @@ -5686,20 +5695,36 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { my $i = 1; while(my $sub = $sth->fetchrow_hashref) { - # Create a new numbering pattern for each subscription - my $ok = $insert_numberpatterns_sth->execute( - "Backup pattern $i", 4+$i, "Automatically created pattern by updatedatabase", $sub->{numberingmethod}, - "X", $sub->{add1}, $sub->{every1}, $sub->{whenmorethan1}, $sub->{setto1}, undef, - "Y", $sub->{add2}, $sub->{every2}, $sub->{whenmorethan2}, $sub->{setto2}, undef, - "Z", $sub->{add3}, $sub->{every3}, $sub->{whenmorethan3}, $sub->{setto3}, undef + $check_numberpatterns_sth->execute( + $sub->{add1}, $sub->{add2}, $sub->{add3}, + $sub->{every1}, $sub->{every2}, $sub->{every3}, + $sub->{whenmorethan1}, $sub->{whenmorethan2}, $sub->{whenmorethan3}, + $sub->{setto1}, $sub->{setto2}, $sub->{setto3}, + $sub->{numberingmethod} ); - if($ok) { - my $id = $dbh->last_insert_id(undef, undef, 'subscription_numberpatterns', undef); - # Link to subscription_numberpatterns and subscription_frequencies - $update_subscription_sth->execute($id, - $frequencies_mapping->{$sub->{periodicity}}, $sub->{subscriptionid}); + my $p = $check_numberpatterns_sth->fetchrow_hashref; + if (defined $p) { + # Pattern already exists, link to it + $update_subscription_sth->execute($p->{id}, + $frequencies_mapping->{$sub->{periodicity}}, + $sub->{subscriptionid}); + } else { + # Create a new numbering pattern for this subscription + my $ok = $insert_numberpatterns_sth->execute( + "Backup pattern $i", 4+$i, "Automatically created pattern by updatedatabase", $sub->{numberingmethod}, + "X", $sub->{add1}, $sub->{every1}, $sub->{whenmorethan1}, $sub->{setto1}, undef, + "Y", $sub->{add2}, $sub->{every2}, $sub->{whenmorethan2}, $sub->{setto2}, undef, + "Z", $sub->{add3}, $sub->{every3}, $sub->{whenmorethan3}, $sub->{setto3}, undef + ); + if($ok) { + my $id = $dbh->last_insert_id(undef, undef, 'subscription_numberpatterns', undef); + # Link to subscription_numberpatterns and subscription_frequencies + $update_subscription_sth->execute($id, + $frequencies_mapping->{$sub->{periodicity}}, + $sub->{subscriptionid}); + } + $i++; } - $i++; } # Remove now useless columns @@ -5724,8 +5749,8 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { ADD COLUMN countissuesperunit INTEGER NOT NULL DEFAULT 1 AFTER periodicity, ADD COLUMN skip_serialseq BOOLEAN NOT NULL DEFAULT 0 AFTER irregularity, ADD COLUMN locale VARCHAR(80) DEFAULT NULL AFTER numberpattern, - ADD CONSTRAINT subscription_ibfk_1 FOREIGN KEY (periodicity) REFERENCES subscription_frequencies (id), - ADD CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) + ADD CONSTRAINT subscription_ibfk_1 FOREIGN KEY (periodicity) REFERENCES subscription_frequencies (id) ON DELETE SET NULL ON UPDATE CASCADE, + ADD CONSTRAINT subscription_ibfk_2 FOREIGN KEY (numberpattern) REFERENCES subscription_numberpatterns (id) ON DELETE SET NULL ON UPDATE CASCADE |); print "Upgrade to $DBversion done (Add subscription_frequencies and subscription_numberpatterns tables)\n"; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt index dfb0bd0..bfebddd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-add.tt @@ -212,6 +212,9 @@ function modifyAdvancedPattern() { $("#advancedpredictionpatternt input").each(function() { $(this).attr("readonly", false); }); + $("#advancedpredictionpatternt select").each(function() { + $(this).attr("disabled", false); + }); $("#restoreadvancedpatternbutton").show(); $("#saveadvancedpatternbutton").show(); @@ -234,6 +237,11 @@ function restoreAdvancedPattern() { $(this).val(unescape(globalnumpatterndata[id])); } }); + $("#advancedpredictionpatternt select").each(function() { + $(this).attr("disabled", true); + var id = $(this).attr('id'); + $(this).val(unescape(globalnumpatterndata[id])); + }); $("#restoreadvancedpatternbutton").hide(); $("#saveadvancedpatternbutton").hide(); @@ -310,6 +318,11 @@ function testPredictionPattern() { } function saveAdvancedPattern() { + if ($("#patternname").val().length == 0) { + alert(_("Please enter a name for this pattern")); + return false; + } + // Check if patternname already exists, and modify pattern // instead of creating it if so var found = 0; @@ -321,7 +334,10 @@ function saveAdvancedPattern() { }); var cnfrm = 1; if(found){ - cnfrm = confirm(_("This pattern already exists. Do you want to modify it")); + var msg = _("This pattern name already exists. Do you want to modify it?") + + "\n" + _("Warning: it will modify the pattern for all subscriptions") + + _("that are using it."); + cnfrm = confirm(msg); } if(cnfrm) { @@ -343,11 +359,15 @@ function saveAdvancedPattern() { "/cgi-bin/koha/serials/create-numberpattern.pl", ajaxData, function(data){ - if(found == 0){ - $("#numberpattern").append(""); + if (data.numberpatternid) { + if(found == 0){ + $("#numberpattern").append(""); + } + $("#numberpattern").val(data.numberpatternid); + numberpatternload(); + } else { + alert(_("Something went wrong. Unable to create a new numbering pattern.")); } - $("#numberpattern").val(data.numberpatternid); - numberpatternload(); } ); } @@ -660,7 +680,7 @@ $(document).ready(function() {
  • - + + If empty, system locale is used
  • @@ -771,10 +798,18 @@ $(document).ready(function() { - Numbering - - - + [% BLOCK numbering_select %] + + [% END %] + Formatting + [% PROCESS numbering_select name="numbering1" %] + [% PROCESS numbering_select name="numbering2" %] + [% PROCESS numbering_select name="numbering3" %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-frequencies.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-frequencies.tt index d390042..8f8a4ab 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-frequencies.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-frequencies.tt @@ -44,6 +44,10 @@ function check_form() { return false; } +function show_blocking_subs() { + $("#blocking_subs").show(); +} + $(document).ready(function() { $("#issuesperunit").change(function() { var value = $(this).val(); @@ -140,10 +144,36 @@ $(document).ready(function() { [% ELSE %] +

    Frequencies

    + [% IF still_used %] +
    +

    + This frequency is still used by [% subscriptions.size %] + subscription(s). Do you still want to delete it? +

    +

    Show subscriptions

    + + +
    + + + + +
    +
    + +
    +
    + [% END %] New frenquency [% IF (frequencies_loop.size) %] -

    Frenquencies

    @@ -165,7 +195,7 @@ $(document).ready(function() { [% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-numberpatterns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-numberpatterns.tt index 72a3885..4ab79f4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-numberpatterns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-numberpatterns.tt @@ -53,6 +53,10 @@ }); } [% END %] + +function show_blocking_subs() { + $("#blocking_subs").show(); +} //]]> @@ -158,10 +162,30 @@ - - - - + [% BLOCK numbering_select %] + + [% END %] + + + +
    [% frequency.displayorder %] Modify | - Delete + Delete
    NumberingFromatting[% PROCESS numbering_select name="numbering1" value=numbering1 %][% PROCESS numbering_select name="numbering2" value=numbering2 %][% PROCESS numbering_select name="numbering3" value=numbering3 %]
    @@ -188,18 +212,7 @@
  • - - Show Calendar - +
  • @@ -212,7 +225,12 @@
  • - + If empty, system locale is used
  • @@ -249,6 +267,32 @@ [% ELSE %]

    Number patterns

    + [% IF still_used %] +
    +

    + This pattern is still used by [% subscriptions.size %] + subscription(s). Do you still want to delete it? +

    +

    Show subscriptions

    + + +
    + + + + +
    +
    + +
    +
    + [% END %] New numbering pattern [% IF (numberpatterns_loop.size) %] diff --git a/serials/subscription-add.pl b/serials/subscription-add.pl index cddd485..42a9091 100755 --- a/serials/subscription-add.pl +++ b/serials/subscription-add.pl @@ -210,6 +210,16 @@ if ($op eq 'addsubscription') { } $template->param(numberpatterns => \@numberpatternloop); + # Get installed locales + # FIXME this will not work with all environments. + # If call to locale fails, @locales will be an empty array, which is fine. + my @locales = map { + chomp; + # we don't want POSIX and C locales + /^C|^POSIX$/ ? () : $_ + } `locale -a`; + $template->param(locales => \@locales); + output_html_with_http_headers $query, $cookie, $template->output; } diff --git a/serials/subscription-frequencies.pl b/serials/subscription-frequencies.pl index 1997674..53de1c7 100755 --- a/serials/subscription-frequencies.pl +++ b/serials/subscription-frequencies.pl @@ -92,7 +92,23 @@ if($op && ($op eq 'savenew' || $op eq 'savemod')) { } elsif($op && $op eq 'del') { my $frequencyid = $input->param('frequencyid'); - DelSubscriptionFrequency($frequencyid); + if ($frequencyid) { + my $confirm = $input->param('confirm'); + if ($confirm) { + DelSubscriptionFrequency($frequencyid); + } else { + my @subs = GetSubscriptionsWithFrequency($frequencyid); + if (@subs) { + $template->param( + frequencyid => $frequencyid, + still_used => 1, + subscriptions => \@subs + ); + } else { + DelSubscriptionFrequency($frequencyid); + } + } + } } diff --git a/serials/subscription-numberpatterns.pl b/serials/subscription-numberpatterns.pl index e7f6480..2647672 100755 --- a/serials/subscription-numberpatterns.pl +++ b/serials/subscription-numberpatterns.pl @@ -104,10 +104,16 @@ if($op && ($op eq 'new' || $op eq 'modify')) { my @frequencies = GetSubscriptionFrequencies(); my @subtypes; push @subtypes, { value => $_ } for (qw/ issues weeks months /); + my @locales = map { + chomp; + /^C|^POSIX$/ ? () : $_ + } `locale -a`; + $template->param( $op => 1, frequencies_loop => \@frequencies, subtypes_loop => \@subtypes, + locales => \@locales, DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(), ); output_html_with_http_headers $input, $cookie, $template->output; @@ -116,8 +122,22 @@ if($op && ($op eq 'new' || $op eq 'modify')) { if($op && $op eq 'del') { my $id = $input->param('id'); - if(defined $id) { - DelSubscriptionNumberpattern($id); + if ($id) { + my $confirm = $input->param('confirm'); + if ($confirm) { + DelSubscriptionNumberpattern($id); + } else { + my @subs = GetSubscriptionsWithNumberpattern($id); + if (@subs) { + $template->param( + id => $id, + still_used => 1, + subscriptions => \@subs + ); + } else { + DelSubscriptionNumberpattern($id); + } + } } } -- 1.7.10.4