From 26885cdbdb8e9e83292e4ce0ecc096d93e92956b Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 6 Sep 2024 14:20:21 +0100 Subject: [PATCH] Bug 10190: Modal form improvements We now fetch all rules on modal open and the effective rule on context selection. This allows us to compare the specific values of the rule we're adding/editing to the fallback values present in the all rules set. We use the fallback rule values as placeholders in the input boxes to highlight to the end user what value will be used should they choose to wipe the input. We also allow wiping inputs and pass 'null' to the API appropriately. Finally, we set a minimum value for delay when adding a new rule to highlight to the user that the new trigger will be added after existing triggers. --- .../CirculationTriggersFormAdd.vue | 362 +++++++++++------- .../CirculationTriggers/TriggersTable.vue | 65 +++- 2 files changed, 283 insertions(+), 144 deletions(-) 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 45849640e9c..c0dbce2d3cf 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 @@ -12,13 +12,15 @@

{{ $__("Trigger context") }}

-

{{ $__("Existing rules") }}

-

{{ $__("Notice") }} {{ " " + newTriggerNumber - 1 }}

+

{{ $__("Existing rules") }}

+

{{ $__("Notice") }} {{ " " + newTriggerNumber - 1 }}

@@ -30,7 +32,7 @@ > -
  • +
  • - - + +
    + + {{ $__("Yes") }} + + + {{ $__("No") }} + + + {{ $__("Fallback to default") }} + + ({{ fallbackRule.restricts === 1 ? $__("Yes") : $__("No") }}) + +
  • @@ -209,23 +233,23 @@ export default { categories: null, itemTypes: null, circRules: [], - circRuleTrigger: { + newRule: { item_type_id: "*", library_id: "*", patron_category_id: "*", delay: null, notice: null, mtt: null, - restrict: "0", + restrict: null, }, - effectiveRule: { + fallbackRule: { item_type_id: "*", library_id: "*", patron_category_id: "*", delay: null, notice: null, mtt: null, - restrict: "0", + restrict: null, }, newTriggerNumber: 1, mtts: [ @@ -239,68 +263,53 @@ export default { fine: null, chargeperiod: null, lengthunit: null, + numberOfTriggers: null, }, editMode: false, ruleBeingEdited: null, + triggerBeingEdited: null, } }, beforeRouteEnter(to, from, next) { next(vm => { vm.getLibraries().then(() => vm.getCategories().then(() => - vm.getItemTypes().then(() => { - const { query } = to - vm.checkForExistingRules(query).then( - () => (vm.initialized = true) - ) - }) + vm.getItemTypes().then(() => + vm.getCircRules().then(() => { + const { query } = to + vm.checkForExistingRules(query).then( + () => (vm.initialized = true) + ) + }) + ) ) ) }) }, + computed: { + minDelay() { + const lastRule = this.circRules[this.newTriggerNumber - 2]; + return lastRule ? parseInt(lastRule[`overdue_${this.newTriggerNumber - 1}_delay`]) + 1 : 0; + } + }, methods: { async addCircRule(e) { e.preventDefault() const context = { - library_id: this.circRuleTrigger.library_id || "*", - item_type_id: this.circRuleTrigger.item_type_id || "*", + library_id: this.newRule.library_id || "*", + item_type_id: this.newRule.item_type_id || "*", patron_category_id: - this.circRuleTrigger.patron_category_id || "*", + this.newRule.patron_category_id || "*", } - const delay = - this.circRuleTrigger.delay === this.effectiveRule.delay - ? null - : this.circRuleTrigger.delay - const notice = - this.circRuleTrigger.notice === this.effectiveRule.notice - ? null - : this.circRuleTrigger.notice - const restrict = - this.circRuleTrigger.restrict === this.effectiveRule.restrict - ? null - : this.circRuleTrigger.restrict - const mtt = - this.circRuleTrigger.mtt.join(",") === - this.effectiveRule.mtt.join(",") - ? null - : this.circRuleTrigger.mtt.join(",") - const circRule = { context, } - circRule[`overdue_${this.newTriggerNumber}_delay`] = delay - circRule[`overdue_${this.newTriggerNumber}_notice`] = notice - circRule[`overdue_${this.newTriggerNumber}_restrict`] = restrict - circRule[`overdue_${this.newTriggerNumber}_mtt`] = mtt - - if (this.editMode) { - Object.keys(circRule).forEach(key => { - if (key === "context") return - if (!circRule[key]) delete circRule[key] - }) - } + circRule[`overdue_${this.newTriggerNumber}_delay`] = this.newRule.delay + circRule[`overdue_${this.newTriggerNumber}_notice`] = this.newRule.notice + circRule[`overdue_${this.newTriggerNumber}_restrict`] = this.newRule.restrict + circRule[`overdue_${this.newTriggerNumber}_mtt`] = this.newRule.mtt && this.newRule.mtt.length ? this.newRule.mtt.join(",") : null const client = APIClient.circRule await client.circRules.update(circRule).then( @@ -354,6 +363,16 @@ export default { error => {} ) }, + async getCircRules() { + const client = APIClient.circRule + await client.circRules.getAll({}, { effective: false }).then( + rules => { + const { rulesPerTrigger } = this.splitCircRulesByTriggerNumber(rules) + this.circRules = rulesPerTrigger.length ? rulesPerTrigger : rules + }, + error => {} + ) + }, async handleContextChange() { await this.checkForExistingRules() }, @@ -362,95 +381,158 @@ export default { const editMode = routeParams && routeParams.triggerNumber if (editMode) { this.editMode = editMode - this.ruleBeingEdited = routeParams.triggerNumber + this.triggerBeingEdited = routeParams.triggerNumber } const library_id = routeParams && routeParams.library_id ? routeParams.library_id - : this.circRuleTrigger.library_id || "*" + : this.newRule.library_id || "*" const item_type_id = routeParams && routeParams.item_type_id ? routeParams.item_type_id - : this.circRuleTrigger.item_type_id || "*" + : this.newRule.item_type_id || "*" const patron_category_id = routeParams && routeParams.patron_category_id ? routeParams.patron_category_id - : this.circRuleTrigger.patron_category_id || "*" + : this.newRule.patron_category_id || "*" const params = { library_id, item_type_id, patron_category_id, } + // Fetch effective ruleset for context const client = APIClient.circRule await client.circRules.getAll({}, params).then( rules => { - if (rules.length === 0) { - this.newTriggerNumber = 1 - } else { - const { rulesPerTrigger } = - this.splitCircRulesByTriggerNumber(rules) - this.circRules = rulesPerTrigger - this.ruleInfo = { - issuelength: rules[0].issuelength, - decreaseloanholds: rules[0].decreaseloanholds, - fine: rules[0].fine, - chargeperiod: rules[0].chargeperiod, - lengthunit: rules[0].lengthunit, - } + this.ruleBeingEdited = rules[0] + this.ruleBeingEdited.context = params + const regex = /overdue_(\d+)_delay/g + const numberOfTriggers = Object.keys(rules[0]).filter( + key => regex.test(key) && rules[0][key] !== null + ).length + const splitRules = this.filterCircRulesByContext(this.ruleBeingEdited) + this.newTriggerNumber = editMode + ? routeParams.triggerNumber + : numberOfTriggers + 1 + this.assignTriggerValues( + splitRules, + this.newTriggerNumber, + params + ) + + this.ruleInfo = { + issuelength: rules[0].issuelength, + decreaseloanholds: rules[0].decreaseloanholds, + fine: rules[0].fine, + chargeperiod: rules[0].chargeperiod, + lengthunit: rules[0].lengthunit, + numberOfTriggers: numberOfTriggers, } }, - error => {} + error => { } ) + }, + filterCircRulesByContext(effectiveRule) { + const context = effectiveRule.context; + + // Filter rules that match the context + let contextRules = this.circRules.filter(rule => { + return Object.keys(context).every(key => { + return context[key] === rule.context[key]; + }); + }); + + // Calculate the number of 'overdue_X_' triggers in the effectiveRule + const regex = /overdue_(\d+)_delay/g; + const numberOfTriggers = Object.keys(effectiveRule).filter( + key => regex.test(key) && effectiveRule[key] !== null + ).length; + + // Ensure there is one contextRule per 'X' from 1 to numberOfTriggers + for (let i = 1; i <= numberOfTriggers; i++) { + // Check if there's already a rule for overdue_X_ in contextRules + const matchingRule = contextRules.find(rule => rule[`overdue_${i}_delay`] !== undefined); + + if (!matchingRule) { + // Create a new rule with the same context and null overdue_X_* keys + const placeholderRule = { + context: { ...context }, // Clone the context + [`overdue_${i}_delay`]: null, + [`overdue_${i}_notice`]: null, + [`overdue_${i}_mtt`]: null, + [`overdue_${i}_restrict`]: null + }; + + // Add the new rule to contextRules + contextRules.push(placeholderRule); + } + } - params.effective = false - await client.circRules.getAll({}, params).then( - rules => { - if (rules.length === 0) { - this.newTriggerNumber = 1 + // Sort contextRules by the 'X' value in 'overdue_X_delay' + contextRules.sort((a, b) => { + const getX = rule => { + const match = Object.keys(rule).find(key => regex.test(key)); + return match ? parseInt(match.match(/\d+/)[0], 10) : 0; + }; + + return getX(a) - getX(b); + }); - this.assignTriggerValues( - "circRuleTrigger", - this.circRules, - this.newTriggerNumber, - params - ) - this.assignTriggerValues( - "effectiveRule", - this.circRules, - this.newTriggerNumber, - params - ) - } else { - const regex = /overdue_(\d+)_delay/g - const numberOfTriggers = Object.keys(rules[0]).filter( - key => regex.test(key) && rules[0][key] !== null - ).length - this.newTriggerNumber = editMode - ? routeParams.triggerNumber - : numberOfTriggers + 1 - const { rulesPerTrigger } = - this.splitCircRulesByTriggerNumber(rules) - const triggerToDisplay = editMode - ? routeParams.triggerNumber - : numberOfTriggers - this.assignTriggerValues( - "circRuleTrigger", - rulesPerTrigger, - triggerToDisplay - ) - this.assignTriggerValues( - "effectiveRule", - rulesPerTrigger, - triggerToDisplay - ) - } - }, - error => {} - ) + return contextRules; + }, + findFallbackRule(currentContext, key) { + + // Filter rules to only those with non-null values for the specified key and not the current context + const relevantRules = this.circRules.filter( + rule => { + return Object.keys(currentContext).some(key => currentContext[key] !== rule.context[key]) + && rule[key] !== null + && rule[key] !== undefined; + } + ); + + // Function to calculate specificity score + const getSpecificityScore = ruleContext => { + let score = 0 + if ( + ruleContext.library_id !== "*" && + ruleContext.library_id === currentContext.library_id + ) + score += 4 + if ( + ruleContext.patron_category_id !== "*" && + ruleContext.patron_category_id === + currentContext.patron_category_id + ) + score += 2 + if ( + ruleContext.item_type_id !== "*" && + ruleContext.item_type_id === currentContext.item_type_id + ) + score += 1 + return score + } + + // Sort the rules based on specificity score, descending + const sortedRules = relevantRules.sort((a, b) => { + return ( + getSpecificityScore(b.context) - + getSpecificityScore(a.context) + ) + }) + + // If no rule found, return null + if (sortedRules.length === 0) { + return null + } + + // Get the value from the most specific rule + const bestRule = sortedRules[0] + return bestRule[key] }, - assignTriggerValues(prop, rules, triggerNumber, context = null) { - this[prop] = { + assignTriggerValues(rules, triggerNumber, context = null) { + this.newRule = { item_type_id: context ? context.item_type_id : rules[triggerNumber - 1].context.item_type_id || "*", @@ -461,21 +543,27 @@ export default { ? context.patron_category_id : rules[triggerNumber - 1].context.patron_category_id || "*", - delay: rules[triggerNumber - 1][ + delay: rules[triggerNumber - 1] ? rules[triggerNumber - 1][ `overdue_${triggerNumber}_delay` - ], - notice: rules[triggerNumber - 1][ + ] : null, + notice: rules[triggerNumber - 1] ? rules[triggerNumber - 1][ `overdue_${triggerNumber}_notice` - ], - mtt: rules[triggerNumber - 1][`overdue_${triggerNumber}_mtt`] + ] : null, + mtt: rules[triggerNumber - 1] ? rules[triggerNumber - 1][`overdue_${triggerNumber}_mtt`] ? rules[triggerNumber - 1][ `overdue_${triggerNumber}_mtt` ].split(",") - : [], - restrict: + : [] : null, + restrict: rules[triggerNumber - 1] ? rules[triggerNumber - 1][ `overdue_${triggerNumber}_restrict` - ], + ] : null, + } + this.fallbackRule = { + delay: this.findFallbackRule(context, `overdue_${triggerNumber}_delay`), + notice: this.findFallbackRule(context, `overdue_${triggerNumber}_notice`), + mtt: this.findFallbackRule(context, `overdue_${triggerNumber}_mtt`), + restrict: this.findFallbackRule(context, `overdue_${triggerNumber}_restrict`) } }, }, diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/TriggersTable.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/TriggersTable.vue index a1ba0e476dd..590fe682763 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/TriggersTable.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/Admin/CirculationTriggers/TriggersTable.vue @@ -36,13 +36,13 @@ @@ -100,10 +100,11 @@ }" > {{ + findEffectiveRule(rule, `overdue_${modal ? i + 1 : triggerNumber}_notice`).value !== '' ? handleTransport( findEffectiveRule(rule, `overdue_${modal ? i + 1 : triggerNumber}_mtt`).value, "email" - ) + ) : '' }} @@ -116,10 +117,11 @@ }" > {{ + findEffectiveRule(rule, `overdue_${modal ? i + 1 : triggerNumber}_notice`).value !== '' ? handleTransport( findEffectiveRule(rule, `overdue_${modal ? i + 1 : triggerNumber}_mtt`).value, "print" - ) + ) : '' }} @@ -132,10 +134,11 @@ }" > {{ + findEffectiveRule(rule, `overdue_${modal ? i + 1 : triggerNumber}_notice`).value !== '' ? handleTransport( findEffectiveRule(rule, `overdue_${modal ? i + 1 : triggerNumber}_mtt`).value, "sms" - ) + ) : '' }} @@ -185,6 +188,7 @@ export default { "triggerNumber", "modal", "ruleBeingEdited", + "triggerBeingEdited", "categories", "itemTypes", "letters", @@ -204,8 +208,55 @@ export default { handleRestrictions(value) { return value === "1" ? this.$__("Yes") : this.$__("No") }, + filterCircRulesByContext(effectiveRule) { + const context = effectiveRule.context; + + // Filter rules that match the context + let contextRules = this.circRules.filter(rule => { + return Object.keys(context).every(key => { + return context[key] === rule.context[key]; + }); + }); + + // Calculate the number of 'overdue_X_' triggers in the effectiveRule + const regex = /overdue_(\d+)_delay/g; + const numberOfTriggers = Object.keys(effectiveRule).filter( + key => regex.test(key) && effectiveRule[key] !== null + ).length; + + // Ensure there is one contextRule per 'X' from 1 to numberOfTriggers + for (let i = 1; i <= numberOfTriggers; i++) { + // Check if there's already a rule for overdue_X_ in contextRules + const matchingRule = contextRules.find(rule => rule[`overdue_${i}_delay`] !== undefined); + + if (!matchingRule) { + // Create a new rule with the same context and null overdue_X_* keys + const newRule = { + context: { ...context }, // Clone the context + [`overdue_${i}_delay`]: null, + [`overdue_${i}_notice`]: null, + [`overdue_${i}_mtt`]: null, + [`overdue_${i}_restrict`]: null + }; + + // Add the new rule to contextRules + contextRules.push(newRule); + } + } + + // Sort contextRules by the 'X' value in 'overdue_X_delay' + contextRules.sort((a, b) => { + const getX = rule => { + const match = Object.keys(rule).find(key => regex.test(key)); + return match ? parseInt(match.match(/\d+/)[0], 10) : 0; + }; + + return getX(a) - getX(b); + }); + + return contextRules; + }, filterCircRulesByTabNumber(number) { - if (this.modal) return this.circRules return this.circRules.filter( rule => rule.triggerNumber === number && @@ -226,7 +277,7 @@ export default { if (ruleSet[key] === null) { // Filter rules to only those with non-null values for the specified key const relevantRules = this.circRules.filter( - rule => rule[key] !== null + rule => rule[key] !== null && rule[key] !== undefined ) // Function to calculate specificity score -- 2.46.0