From 1cc29534af7a84bb131b95e5ded2a00c9640dbae Mon Sep 17 00:00:00 2001 From: Chloe Zermatten Date: Fri, 7 Nov 2025 12:03:26 +0000 Subject: [PATCH] Bug 10190: (follow-up) Complete Vue.js rewrite of Circulation Triggers administration interface This patch aims to resolve the following issues as reported by comment 193: - missing atomic update for overde_X_has_rules (amended to account for comment 227) - existing delays being overwritten by the client when a rule set is displayed - inability to save circulation overdue rule sets for context for which circulations rules where already set - address bug_10190.pl db script warnings (undef values were being concatenated with strings) - add a guard clause to bug_101090.pl: check that the tables the script pulls the data from do still exist, otherwise exit. - remove duplicate Letters options in the circulation triggers add / edit form --- .../data/mysql/atomicupdate/bug_10190.pl | 90 +++++++++++++++++-- .../CirculationTriggersFormAdd.vue | 60 ++++++------- .../prog/js/vue/stores/circulation-rules.js | 5 +- 3 files changed, 114 insertions(+), 41 deletions(-) diff --git a/installer/data/mysql/atomicupdate/bug_10190.pl b/installer/data/mysql/atomicupdate/bug_10190.pl index d9494d3ec0c..285c093d092 100644 --- a/installer/data/mysql/atomicupdate/bug_10190.pl +++ b/installer/data/mysql/atomicupdate/bug_10190.pl @@ -8,6 +8,11 @@ return { my ($args) = @_; my ( $dbh, $out ) = @$args{qw(dbh out)}; + # This script first pulls data from these tables, then deletes them. -> Do not run if they do not exist. + if ( !TableExists('overduerules_transport_types') || !TableExists('overduerules') ) { + exit; + } + # Populate empty overduerules table for missing categories $dbh->do( q| @@ -150,12 +155,20 @@ return { my $branchcode_key = $branchcode // ''; foreach my $i ( 1 .. 3 ) { + my $has_rules = 0; + # Insert the delay rule for group $i, skipping if it matches the most frequent delay if ( my $delay = $rule->{"delay$i"} ) { $delay ||= ''; unless ( $delay eq $most_frequent_delay{$branchcode_key}{$i} ) { $delay ||= undef; - say $out "Inserting $branchcode:$categorycode:$itemtype overdue_$i" . '_delay: ' . $delay; + say $out "Inserting " + . ( $branchcode // '' ) . ":" + . ( $categorycode // '' ) . ':' + . ( $itemtype // '' ) + . " overdue_$i" + . '_delay: ' + . ( $delay // '' ); $insert->execute( $branchcode, $categorycode, @@ -163,13 +176,20 @@ return { "overdue_$i" . '_delay', $delay ); + $has_rules = 1; } } # Insert the notice rule for group $i, skipping if it matches the most frequent notice if ( my $notice = $rule->{"letter$i"} ) { unless ( $notice eq $most_frequent_notice{$branchcode_key}{$i} ) { - say $out "Inserting $branchcode:$categorycode:$itemtype overdue_$i" . '_notice: ' . $notice; + say $out "Inserting " + . ( $branchcode // '' ) . ":" + . ( $categorycode // '' ) . ':' + . ( $itemtype // '' ) + . " overdue_$i" + . '_notice: ' + . ( $notice // '' ); $insert->execute( $branchcode, $categorycode, @@ -177,13 +197,20 @@ return { "overdue_$i" . '_notice', $notice ); + $has_rules = 1; } } # Insert the message transport type rule for group $i, skipping if it matches the most frequent mtt if ( my $mtt = $rule->{"message_transport_type_$i"} ) { unless ( $mtt eq $most_frequent_mtt{$branchcode_key}{$i} ) { - say $out "Inserting $branchcode:$categorycode:$itemtype overdue_$i" . '_mtt: ' . $mtt; + say $out "Inserting " + . ( $branchcode // '' ) . ":" + . ( $categorycode // '' ) . ':' + . ( $itemtype // '' ) + . " overdue_$i" + . '_mtt: ' + . ( $mtt // '' ); $insert->execute( $branchcode, $categorycode, @@ -191,13 +218,20 @@ return { "overdue_$i" . '_mtt', $mtt ); + $has_rules = 1; } } # Insert the restrict rule for group $i if ( my $restrict = $rule->{"debarred$i"} ) { unless ( $restrict eq $most_frequent_restrict{$branchcode_key}{$i} ) { - say $out "Inserting $branchcode:$categorycode:$itemtype overdue_$i" . '_restrict: ' . $restrict; + say $out "Inserting " + . ( $branchcode // '' ) . ":" + . ( $categorycode // '' ) . ':' + . ( $itemtype // '' ) + . " overdue_$i" + . '_restrict: ' + . ( $restrict // '' ); $insert->execute( $branchcode, $categorycode, @@ -205,8 +239,26 @@ return { "overdue_$i" . '_restrict', $restrict ); + $has_rules = 1; } } + + # Insert the has_rules rule for group $i if and only if a rule set has been added into the db for this context. + if ( $has_rules == 1 ) { + say $out "Inserting " + . ( $branchcode // '' ) . ":" + . ( $categorycode // '' ) . ':' + . ( $itemtype // '' ) + . " overdue_$i" + . "has_rules: 1"; + $insert->execute( + $branchcode, + $categorycode, + $itemtype, + "overdue_$i" . '_has_rules', + 1 + ); + } } } @@ -215,7 +267,11 @@ return { my $branchcode_value = $branchcode || undef; foreach my $i ( 1 .. 3 ) { my $most_frequent_delay = $most_frequent_delay{$branchcode}{$i}; - say $out "Inserting $branchcode_value:undef:$itemtype default most frequent delay for overdue_$i: " + say $out "Inserting " + . ( $branchcode_value // '' ) + . ":undef:" + . ( $itemtype // '' ) + . " default most frequent delay for overdue_$i: " . $most_frequent_delay; $insert->execute( $branchcode_value, @@ -226,7 +282,9 @@ return { ); my $most_frequent_notice = $most_frequent_notice{$branchcode}{$i}; - say $out "Inserting $branchcode_value:undef:undef default most frequent notice for overdue_$i: " + say $out "Inserting " + . ( $branchcode_value // '' ) + . ":undef:undef default most frequent notice for overdue_$i: " . $most_frequent_notice; $insert->execute( $branchcode_value, @@ -237,7 +295,9 @@ return { ); my $most_frequent_mtt = $most_frequent_mtt{$branchcode}{$i}; - say $out "Inserting $branchcode_value:undef:undef default most frequent mtt for overdue_$i: " + say $out "Inserting " + . ( $branchcode_value // '' ) + . ":undef:undef default most frequent mtt for overdue_$i: " . $most_frequent_mtt; $insert->execute( $branchcode_value, @@ -248,7 +308,9 @@ return { ); my $most_frequent_restrict = $most_frequent_restrict{$branchcode}{$i}; - say $out "Inserting $branchcode_value:undef:undef default most frequent restrict for overdue_$i: " + say $out "Inserting " + . ( $branchcode_value // '' ) + . ":undef:undef default most frequent restrict for overdue_$i: " . $most_frequent_restrict; $insert->execute( $branchcode_value, @@ -258,6 +320,18 @@ return { $most_frequent_restrict ); + # Insert the has_rules rule for group $i's default context + say $out "Inserting " + . ( $branchcode_value // '' ) + . ":undef:undef default has_rules for overdue_$i: 1"; + $insert->execute( + $branchcode_value, + undef, + undef, + "overdue_${i}_has_rules", + 1 + ); + } } diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue index 136fd55a0ee..d69fd2d1d19 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/CirculationTriggersFormAdd.vue @@ -638,7 +638,7 @@ export default { if (this.ruleSetToSubmit === null) { this.ruleSetToSubmit = { context: this.context, - [`overdue_${this.triggerNumber}_delay`]: this.minDelay, + [`overdue_${this.triggerNumber}_delay`]: `${this.minDelay}`, [`overdue_${this.triggerNumber}_notice`]: null, [`overdue_${this.triggerNumber}_mtt`]: null, [`overdue_${this.triggerNumber}_restrict`]: null, @@ -742,8 +742,6 @@ export default { this.currentRuleSet[`overdue_${priorTriggerNumber}_delay`] ) + 1 : 0; - this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`] = - this.minDelay; }, setMaxDelay() { const nextTriggerNumber = parseInt(this.triggerNumber) + 1; @@ -756,27 +754,21 @@ export default { : Infinity; }, setFilteredLetters() { - let library = this.context.library_id; - const branchcodeMatches = letters.filter( - letter => letter.branchcode === library - ); - const emptyBranchcodeMatches = letters.filter( - letter => letter.branchcode === "" - ); - - const uniqueCodes = [ - ...new Set( - [...branchcodeMatches, ...emptyBranchcodeMatches].map( - letter => letter.code - ) - ), - ]; + const library = this.currentRuleSet.library_id; + const byCode = new Map(); + + for (const letter of this.letters) { + if (letter.branchcode === library) { + byCode.set(letter.code, letter); // override + } else if ( + letter.branchcode === "" && + !byCode.has(letter.code) + ) { + byCode.set(letter.code, letter); + } + } - this.filteredLetters = letters.filter( - letter => - uniqueCodes.includes(letter.code) && - (letter.branchcode === library || letter.branchcode === "") - ); + this.filteredLetters = [...byCode.values()]; }, incrementDelay() { // Check for minDelay and maxDelay @@ -791,16 +783,18 @@ export default { null ) { this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`] = - min; + `${min}`; } // Increment within the valid range else { this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`] = Math.min( - this.ruleSetToSubmit[ - `overdue_${this.triggerNumber}_delay` - ] + 1, + parseInt( + this.ruleSetToSubmit[ + `overdue_${this.triggerNumber}_delay` + ] + ) + 1, max ); } @@ -809,12 +803,14 @@ export default { decrementDelay() { // Check for minDelay const min = this.minDelay !== undefined ? this.minDelay : 1; + let delay = parseInt( + this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`] + ); // Decrement only if greater than minDelay - if ( - this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`] > - min - ) { - this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`]--; + if (delay > min) { + delay--; + this.ruleSetToSubmit[`overdue_${this.triggerNumber}_delay`] = + `${delay}`; } this.setAllowSubmission(); }, diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js b/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js index 351033c088d..cf1004f2e35 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/stores/circulation-rules.js @@ -87,7 +87,10 @@ export const useCircRulesStore = defineStore("circRules", { return value.includes(type) ? $__("Yes") : $__("No"); }, hasConflict(oldRuleSet, newRuleSet, triggerNumber) { - if (!oldRuleSet) { + if ( + !oldRuleSet || + oldRuleSet[`overdue_${triggerNumber}_has_rules`] === null + ) { return false; } return ( -- 2.39.5