Summary: | Clicking save doesn't fill auto-populated fields in authority and biblio editor | ||
---|---|---|---|
Product: | Koha | Reporter: | David Nind <david> |
Component: | MARC Authority data support | Assignee: | Andrii Nugged <nugged> |
Status: | Pushed to oldstable --- | QA Contact: | Aleisha Amohia <aleisha> |
Severity: | major | ||
Priority: | P5 - low | CC: | aleisha, fridolin.somers, nugged, ssammons |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: |
This fixes a regression between Koha 24.11, and 25.05 and main. When adding a new authority or bibliographic record, clicking save (without filling in any fields) now restores filling in the auto-populated fields such as 000, 003, and 008.
|
|
Version(s) released in: |
25.11.00,25.05.02,24.11.08
|
Circulation function: | |
Attachments: |
Bug 40092: Fill auto-populated control fields in MARC editors
Bug 40092: Fill auto-populated control fields in MARC editors Bug 40092: Fill auto-populated control fields in MARC editors |
Description
David Nind
2025-06-08 07:47:39 UTC
See Mattermost chat discussion (8 June 2025): https://chat.koha-community.org/koha-community/pl/akiuk7i73b8qmd38haaxrq7z5o We're preparing to upgrade our production to 25.05, so we've been doing thorough beta testing with librarians with our Finnish libraries. During this testing, we discovered that the main MARC records interface is also affected by the same issue. We found why, prepared a fix, and post-checked with librarians that this bug is gone. The root cause is in the JavaScript function `AreMandatoriesNotOk`, which uses hardcoded rewriting of classes, so it removed recently announced `framework_plugin` calss: elt.setAttribute('class', 'input_marceditor noEnterSubmit ' + notFilledClass); so our solution is to expand hardcoded line with the class: elt.setAttribute('class', 'input_marceditor noEnterSubmit framework_plugin ' + notFilledClass); or, better yet, or, if that does not creates any other border cases, completely replace the hardcoded class assignment with a more flexible method: elt.classList.add(notFilledClass); Code below. Created attachment 183124 [details] [review] Bug 40092: Fill auto-populated control fields in MARC editors Problem: Clicking Save doesn't fill control fields automatically in both the authority and bibliographic MARC editors. Regression introduced when class `framework_plugin` was added: `AreMandatoriesNotOk()` rewrites the element `class` attribute with a hard‑coded string, dropping existing classes. Because the plugins that populate 000/003/005/008 rely on `framework_plugin`, the inputs are skipped and the fields remain empty. This patch stops clobbering the attribute and instead appends the visual 'notFilled' class via `elt.classList.add()`. The one-line fix is applied in both `authorities.tt` and `addbiblio.tt`. Test plan: Authority editor 1. Authorities → New authority → choose framework (e.g. Topical term). 2. Leave all fields blank, click Save. Before patch: 000, 003, 008 remain blank; required tags turn yellow. 3. Apply patch, hard‑refresh, repeat steps 1–2. After patch: control fields auto‑populate as in 24.11. Bibliographic editor 1. Cataloguing → New record (Not in Advanced MARC editor). 2. Click Save without entering data. Before patch: control fields stay blank; record cannot be saved. 3. Apply patch, hard‑refresh, repeat. After patch: record is blocked, but 000/005/008 auto‑populate. Both editors behave like 24.11; mandatory highlighting still works. one thing to decide: I put both variants in code, one commented out so we should throw it away if second more elegant (and future-for-new-classes-proof) one is better and works: // elt.setAttribute('class','input_marceditor noEnterSubmit framework_plugin ' + notFilledClass); or .classList.add(notFilledClass); I tried the proposed solution as I was also experiencing no auto-population on 000,003,005,and 008 in cataloging coming from 23 to 25.05. I just tried adding a new MARC record and just pressed save (no data entry) and now the fields 000,003,005,and 008 all populate as they did before. I have the lines commented out as recommended. Created attachment 183159 [details] [review] Bug 40092: Fill auto-populated control fields in MARC editors Problem: Clicking Save doesn't fill control fields automatically in both the authority and bibliographic MARC editors. Regression introduced when class `framework_plugin` was added: `AreMandatoriesNotOk()` rewrites the element `class` attribute with a hard‑coded string, dropping existing classes. Because the plugins that populate 000/003/005/008 rely on `framework_plugin`, the inputs are skipped and the fields remain empty. This patch stops clobbering the attribute and instead appends the visual 'notFilled' class via `elt.classList.add()`. The one-line fix is applied in both `authorities.tt` and `addbiblio.tt`. Test plan: Authority editor 1. Authorities → New authority → choose framework (e.g. Topical term). 2. Leave all fields blank, click Save. Before patch: 000, 003, 008 remain blank; required tags turn yellow. 3. Apply patch, hard‑refresh, repeat steps 1–2. After patch: control fields auto‑populate as in 24.11. Bibliographic editor 1. Cataloguing → New record (Not in Advanced MARC editor). 2. Click Save without entering data. Before patch: control fields stay blank; record cannot be saved. 3. Apply patch, hard‑refresh, repeat. After patch: record is blocked, but 000/005/008 auto‑populate. Both editors behave like 24.11; mandatory highlighting still works. Signed-off-by: David Nind <david@davidnind.com> Notes for QA phase: we need to decide if `.classList.add(notFilledClass);` is universal and do not interfere/future proof (looks like). If so, then remove another proposed but commented out solution `// elt.setAttribute...`. It would also be nice to have a test for this, if it is possible (I have no idea how to create them myself!). That would help avoid breaking the feature again in the future. I changed the status to major, ideally so that this can be fixed in the first maintenance release for 25.05. Created attachment 183914 [details] [review] Bug 40092: Fill auto-populated control fields in MARC editors Problem: Clicking Save doesn't fill control fields automatically in both the authority and bibliographic MARC editors. Regression introduced when class `framework_plugin` was added: `AreMandatoriesNotOk()` rewrites the element `class` attribute with a hard‑coded string, dropping existing classes. Because the plugins that populate 000/003/005/008 rely on `framework_plugin`, the inputs are skipped and the fields remain empty. This patch stops clobbering the attribute and instead appends the visual 'notFilled' class via `elt.classList.add()`. The one-line fix is applied in both `authorities.tt` and `addbiblio.tt`. Test plan: Authority editor 1. Authorities → New authority → choose framework (e.g. Topical term). 2. Leave all fields blank, click Save. Before patch: 000, 003, 008 remain blank; required tags turn yellow. 3. Apply patch, hard‑refresh, repeat steps 1–2. After patch: control fields auto‑populate as in 24.11. Bibliographic editor 1. Cataloguing → New record (Not in Advanced MARC editor). 2. Click Save without entering data. Before patch: control fields stay blank; record cannot be saved. 3. Apply patch, hard‑refresh, repeat. After patch: record is blocked, but 000/005/008 auto‑populate. Both editors behave like 24.11; mandatory highlighting still works. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> (In reply to Andrii Nugged from comment #7) > Notes for QA phase: we need to decide if `.classList.add(notFilledClass);` > is universal and do not interfere/future proof (looks like). If so, then > remove another proposed but commented out solution `// elt.setAttribute...`. Thank you Andrii. I decided to use the 'safer' commented out solution because then we're clear on exactly which classes should be present. This is a symptom of a bug which happens earlier in the process, where the framework_plugin class isn't passed back to the template as it should, probably in a Perl script, which we would be able to add a test for... but since this is a major bug, let's get this fix in and work out how to programmatically ensure the right classes are passed through later. Nice work everyone! Pushed to main for 25.11 Nice work everyone! Pushed to 25.05.x Pushed to 24.11.x for 24.11.08 |