Bug 10190: (QA follow-up): UI/UX bug fixes - phase two

Fix: handle exiting modal
 - prevent data reload when the user exits the modal
 - as preparation for the above: refactor to use one handler only
Fix: new trigger saves with delay defined
 - Identified an old refactor omission - we were still passing 'i' despite the from now relying on this.triggerNumber
Fix: return to correct trigger tab after save or reset
Fix: restore delete button and bold italic info
 - restore display, fixes a regression introduced by the previous UI/UX patch set
Fix: order dropdown options by description or name (not code)
Fix: circ rule resets
 - restore reset funtionality
  (mtt was still occasionally passed as a string, and would then get compared to an array. Ensure this is not the case.)
 - do not trigger the data refresh if the current ruleset did not get deleted
 - make sure triggerNumber is defined when navigation is caused by reset
 - add the notice code in brackets to letter names for display (eg. Overdue notice (ODUE))

Assisted-by: Claude Opus 4.6 (Anthropic)

AI used to help trace back potential bug sources and draft fixes. Changes were reviewed, tweaked, and commited manually and gradually (with a pre-submission commit squash)

Sponsored-by: Glasgow Colleges Library Group <https://library.cityofglasgowcollege.ac.uk>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Chloe Zermatten 2026-04-16 09:32:38 +00:00 committed by Pedro Amorim
parent 0fa640b387
commit 37a9684669
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
6 changed files with 98 additions and 41 deletions

View File

@ -125,6 +125,7 @@ export default {
setEffectiveTriggerFilteredRuleSet,
updateCircRuleSets,
hasConflict,
compareByProperty,
scrollToElementById,
} = circRulesStore;
const {
@ -134,6 +135,7 @@ export default {
transportTypes,
patronCategories,
triggerCounts,
lastEditedTriggerNumber,
storeInitialized,
canManageAnyLibrary,
logged_in_library_id,
@ -152,9 +154,11 @@ export default {
setEffectiveTriggerFilteredRuleSet,
updateCircRuleSets,
hasConflict,
lastEditedTriggerNumber,
storeInitialized,
canManageAnyLibrary,
logged_in_library_id,
compareByProperty,
scrollToElementById,
};
},
@ -335,6 +339,7 @@ export default {
}
await this.updateCircRuleSets(ruleSetToSubmit, this.triggerNumber);
this.lastEditedTriggerNumber = this.triggerNumber;
await this.$router.push({
name: "CirculationTriggersList",
query: { refresh: Date.now() },
@ -412,7 +417,7 @@ export default {
[`overdue_${this.triggerNumber}_delay`]: this.findEffectiveRule(
this.context,
"delay",
i
this.triggerNumber
).value,
};

View File

@ -282,12 +282,17 @@ export default {
deleteRuleSet,
handleRestrictions,
} = circRulesStore;
const { libraries, itemTypes, patronCategories } =
storeToRefs(circRulesStore);
const {
libraries,
itemTypes,
patronCategories,
lastEditedTriggerNumber,
} = storeToRefs(circRulesStore);
return {
libraries,
itemTypes,
patronCategories,
lastEditedTriggerNumber,
handleContext,
handleNotice,
handleTransport,
@ -314,11 +319,14 @@ export default {
beforeRouteEnter(to, from, next) {
next(async vm => {
vm.setContext(to.query);
vm.currentRuleSet = await vm.getSelectedRuleSet({
library_id: vm.library_id,
patron_category_id: vm.patron_category_id,
item_type_id: vm.item_type_id,
});
vm.currentRuleSet = await vm.getSelectedRuleSet(
{
library_id: vm.library_id,
patron_category_id: vm.patron_category_id,
item_type_id: vm.item_type_id,
},
false
);
vm.effectiveRuleSet = vm.formatTriggerSpecificRuleSetForDisplay(
vm.currentRuleSet.context,
vm.triggerNumber,
@ -342,10 +350,12 @@ export default {
path: "/cgi-bin/koha/admin/circulation_triggers/reset",
query: {
...this.currentRuleSet.context,
triggerNumber: triggerNumber,
triggerNumber: this.triggerNumber,
},
});
return;
}
this.lastEditedTriggerNumber = this.triggerNumber;
await this.$router.push({
name: "CirculationTriggersList",
query: { refresh: Date.now() },

View File

@ -321,7 +321,7 @@
>
{{
$__(
"Bolid italic values denote fallback values where an override has not been set for the context."
"Bold italic values denote fallback values where an override has not been set for the context."
)
}}
</div>
@ -353,7 +353,7 @@ export default {
const circRulesStore = inject("circRulesStore");
const {
updateTriggerCount,
setAllRawRuleSets,
setAllFormattedRuleSets,
setAllEffectiveRuleSets,
setAllExhaustiveEffectiveRuleSets,
isLastTrigger,
@ -371,6 +371,7 @@ export default {
allExhaustiveEffectiveRuleSets,
allEffectiveRuleSets,
librariesWithRules,
lastEditedTriggerNumber,
storeInitialized,
metaInitialized,
canManageAnyLibrary,
@ -387,13 +388,14 @@ export default {
patronCategories,
updateTriggerCount,
allExhaustiveEffectiveRuleSets,
setAllRawRuleSets,
setAllFormattedRuleSets,
setAllEffectiveRuleSets,
setAllExhaustiveEffectiveRuleSets,
allEffectiveRuleSets,
librariesWithRules,
isLastTrigger,
getLibrariesWithRules,
lastEditedTriggerNumber,
storeInitialized,
metaInitialized,
from_branch,
@ -414,7 +416,7 @@ export default {
methods: {
async loadRuleSets() {
this.ruleSetInitialized = false;
await this.setAllRawRuleSets();
await this.setAllFormattedRuleSets();
this.updateTriggerCount();
this.setAllEffectiveRuleSets();
this.setAllExhaustiveEffectiveRuleSets();
@ -500,18 +502,28 @@ export default {
watch: {
$route: {
immediate: true,
handler: function (newVal, oldVal) {
async handler(newVal, oldVal) {
const fromModal = oldVal?.meta?.showModal;
this.showModal = newVal.meta && newVal.meta.showModal;
// Handle toggling page scroll as modal accessed / exited
const overflow = this.showModal ? "hidden" : "";
const body = document.querySelector("body");
body.style.overflow = overflow;
},
},
"$route.query": {
async handler() {
document.querySelector("body").style.overflow = overflow;
// Handle user exiting the modal - do not reload data.
if (
this.$route.fullPath.includes("refresh") ||
this.$route.path.endsWith("circulation_triggers")
fromModal &&
!this.showModal &&
!newVal.fullPath.includes("refresh")
) {
this.filtersInitialized = true;
return;
}
// Handle initial page loads, and successful save / edit / reset actions - reload data
if (
newVal.fullPath.includes("refresh") ||
newVal.path.endsWith("circulation_triggers")
) {
if (!this.metaInitialized) {
await new Promise(resolve => {
@ -525,11 +537,14 @@ export default {
}
await this.$nextTick();
await this.filterRuleSetsbySearchParam();
if (this.lastEditedTriggerNumber) {
this.tabSelected = `Notice ${this.lastEditedTriggerNumber}`;
this.lastEditedTriggerNumber = null;
}
await this.scrollToElementById("circ-triggers-content");
}
this.filtersInitialized = true;
},
immediate: true,
},
},
components: { TriggersTable, Toolbar, ToolbarButton },

View File

@ -30,7 +30,15 @@ export default {
setup() {
const circRulesStore = inject("circRulesStore");
circRulesStore.init(default_view, logged_in_library_id);
letters.sort(circRulesStore.compareLetterNames);
// format letters for display as "name (notice code)" in drop downs.
letters
.sort(circRulesStore.compareByProperty("name"))
.forEach(letter => {
letter.name =
letter?.name?.concat(` (${letter.code})`) ?? letter.code;
});
letters.unshift({
name: "No letter",
code: "",

View File

@ -6,6 +6,7 @@
<legend>
{{ $__("Existing overdues triggers for this context") }}
</legend>
<slot></slot>
<table>
<thead>
<th v-if="!modal" class="trigger_context">

View File

@ -21,6 +21,7 @@ export const useCircRulesStore = defineStore("circRules", () => {
currentPatronCategoryId: null,
currentItemTypeId: null,
triggerCounts: { "*": 0 },
lastEditedTriggerNumber: null,
metaInitialized: false,
// references
itemTypes: [],
@ -75,14 +76,8 @@ export const useCircRulesStore = defineStore("circRules", () => {
await this.getConfigurationOptions();
},
// utilities
compareLetterNames(a, b) {
if (a.name < b.name) {
return -1;
}
if (a.name > b.name) {
return 1;
}
return 0;
compareByProperty(property) {
return (a, b) => a[property].localeCompare(b[property]);
},
formatTriggerSpecificRuleSetForDisplay(
context,
@ -172,10 +167,7 @@ export const useCircRulesStore = defineStore("circRules", () => {
async scrollToElementById(id) {
let count = 0;
// ensures that the relevant section is loaded before we attempt to scroll it into view
while (
!document.getElementById(id) &&
count < 8
) {
while (!document.getElementById(id) && count < 8) {
await new Promise(resolve => setTimeout(resolve, 250));
count++;
}
@ -190,6 +182,24 @@ export const useCircRulesStore = defineStore("circRules", () => {
formatMttForDisplay(rawMtt) {
return rawMtt?.split(",");
},
formatRuleSetMttFields(ruleSets) {
let triggerNumber = 1;
for (const ruleSet of ruleSets) {
while (ruleSet[`overdue_${triggerNumber}_mtt`] !== undefined) {
if (
typeof ruleSet[`overdue_${triggerNumber}_mtt`] ===
"string"
) {
ruleSet[`overdue_${triggerNumber}_mtt`] =
this.formatMttForDisplay(
ruleSet[`overdue_${triggerNumber}_mtt`]
);
}
triggerNumber++;
}
triggerNumber = 1;
}
},
findEffectiveRule(
context,
ruleSuffix,
@ -436,6 +446,13 @@ export const useCircRulesStore = defineStore("circRules", () => {
...this.allDefaultLibraryRawRuleSets,
];
},
async setAllFormattedRuleSets() {
await this.setAllRawRuleSets();
this.formatRuleSetMttFields(this.allDefaultLibraryRawRuleSets);
if (this.currentLibraryId !== "*") {
this.formatRuleSetMttFields(this.allCurrentLibraryRawRuleSets);
}
},
async getSelectedRuleSet(context, effective = true) {
const rawSelectedRuleSet = await this.getRawSelectedRuleSet(
context,
@ -464,8 +481,9 @@ export const useCircRulesStore = defineStore("circRules", () => {
return;
}
const ruleSetInDb = await this.getRawSelectedRuleSet(
ruleSet.context
const ruleSetInDb = await this.getSelectedRuleSet(
ruleSet.context,
false
);
if (this.hasConflict(ruleSet, ruleSetInDb, triggerNumber)) {
@ -521,7 +539,7 @@ export const useCircRulesStore = defineStore("circRules", () => {
async getItemTypes() {
const client = APIClient.item;
let itemTypes = await client.item_types.getAll();
itemTypes.sort(this.compareByProperty("description"));
itemTypes.unshift({
item_type_id: "*",
description: $__("Default rule for all item types"),
@ -532,7 +550,7 @@ export const useCircRulesStore = defineStore("circRules", () => {
const client = APIClient.library;
let libraries = [];
libraries = await client.libraries.getAll();
libraries.sort(this.compareByProperty("name"));
libraries.unshift({
library_id: "*",
name: $__("Default rule for all libraries"),
@ -542,7 +560,7 @@ export const useCircRulesStore = defineStore("circRules", () => {
async getPatronCategories() {
const client = APIClient.patron;
let patronCategories = await client.categories.getAll();
patronCategories.sort(this.compareByProperty("name"));
patronCategories.unshift({
patron_category_id: "*",
name: $__("Default rule for all categories"),