[Bug 24606] New: Allow storing item values as a template for creating new items
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Bug ID: 24606 Summary: Allow storing item values as a template for creating new items Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Cataloging Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: m.de.rooy@rijksmuseum.nl This is a feature developed by Kyle...elsewhere... Would need to be reworked to fit into modern Koha https://github.com/liblime/LibLime-Koha/commit/ac11f3d86772891e605b58ab7b5d9... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 AspenCat Team <aspencatteam@clicweb.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aspencatteam@clicweb.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Jessie Zairo <jzairo@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jzairo@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141613 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141613&action=edit Bug 24606: Update database, add new schema file -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141614 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141614&action=edit Bug 24606: Add Koha Object(s) with unit tests -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141615&action=edit Bug 24606: Update existing schema files [DO NOT PUSH] -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141616 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141616&action=edit Bug 24606: Implement item templates This patch set implements item editor templates for community Koha. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) prove t/db_dependent/Koha/Item/Template* 5) As a non superlibrarian, enter the item editor 6) Set some item fields, save as a new template using the buttom and form below the editor. 7) Test loading a template without remembering for the session 8) Test loading a template while remembering for the session 9) Test deleting a template 10) Test updating a template 11) Create one or more shared templates 12) Log in as another non superlibrarian without the new permission manage_item_editor_templates, verify you cannot edit/delete templates shared to you 13) Enable the new permission manage_item_editor_templates, verify you can now edit and delete templates shared to you -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141617&action=edit Bug 24606: Add item template editor permission -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |david@davidnind.com --- Comment #6 from David Nind <david@davidnind.com> --- Patch no longer applies: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 24606: Update database, add new schema file Applying: Bug 24606: Add Koha Object(s) with unit tests Applying: Bug 24606: Update existing schema files [DO NOT PUSH] Using index info to reconstruct a base tree... M Koha/Schema/Result/Borrower.pm Falling back to patching base and 3-way merge... Auto-merging Koha/Schema/Result/Borrower.pm CONFLICT (content): Merge conflict in Koha/Schema/Result/Borrower.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 24606: Update existing schema files [DO NOT PUSH] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141613|0 |1 is obsolete| | Attachment #141614|0 |1 is obsolete| | Attachment #141615|0 |1 is obsolete| | Attachment #141616|0 |1 is obsolete| | Attachment #141617|0 |1 is obsolete| | --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141717 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141717&action=edit Bug 24606: Update database, add new schema file -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141718 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141718&action=edit Bug 24606: Add Koha Object(s) with unit tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141719 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141719&action=edit Bug 24606: Implement item templates This patch set implements item editor templates for community Koha. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) prove t/db_dependent/Koha/Item/Template* 5) As a non superlibrarian, enter the item editor 6) Set some item fields, save as a new template using the buttom and form below the editor. 7) Test loading a template without remembering for the session 8) Test loading a template while remembering for the session 9) Test deleting a template 10) Test updating a template 11) Create one or more shared templates 12) Log in as another non superlibrarian without the new permission manage_item_editor_templates, verify you cannot edit/delete templates shared to you 13) Enable the new permission manage_item_editor_templates, verify you can now edit and delete templates shared to you -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141720 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141720&action=edit Bug 24606: Add item template editor permission -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141721 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141721&action=edit Bug 24606: Update existing schema files [DO NOT PUSH] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #12 from David Nind <david@davidnind.com> --- The only thing I noticed is around editing and updating an already shared template. If you are not the creator, and you can have manage_item_editor_templates permissions, if you don't select 'Share template' then you make it unshared and can no longer see it. Maybe if it is a shared template, the default when updating the template is for the share template option to be selected by default. To replicate: 1. Have two librarian users, one with manage_item_editor_templates permissions (henry) the other without (casey). 2. Create and share a template as henry. 3. As casey, apply the shared template and make some changes, for example the shelving location 4. Casey can only save as a new template, can't update the existing one - as expected. 5. Change permissions for casey, so they have manage_item_editor_templates. 6. Repeat step 3 7. Select save as template and then select the template from under update shared template. 5. Don't tick the share template option. 6. Now you can't see or choose that template - it is only visible to the original creator (henry). 7. Log in as henry - you can see the template, as creator. Everything else seems to work as per the test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- Good catch! I'll get that fixed asap! Kyle (In reply to David Nind from comment #12)
The only thing I noticed is around editing and updating an already shared template.
If you are not the creator, and you can have manage_item_editor_templates permissions, if you don't select 'Share template' then you make it unshared and can no longer see it.
Maybe if it is a shared template, the default when updating the template is for the share template option to be selected by default.
To replicate: 1. Have two librarian users, one with manage_item_editor_templates permissions (henry) the other without (casey). 2. Create and share a template as henry. 3. As casey, apply the shared template and make some changes, for example the shelving location 4. Casey can only save as a new template, can't update the existing one - as expected. 5. Change permissions for casey, so they have manage_item_editor_templates. 6. Repeat step 3 7. Select save as template and then select the template from under update shared template. 5. Don't tick the share template option. 6. Now you can't see or choose that template - it is only visible to the original creator (henry). 7. Log in as henry - you can see the template, as creator.
Everything else seems to work as per the test plan.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 141972 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141972&action=edit Bug 24606: (QA follow-up) Set existing sharing setting when updating existing template -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141717|0 |1 is obsolete| | --- Comment #15 from David Nind <david@davidnind.com> --- Created attachment 142003 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142003&action=edit Bug 24606: Update database, add new schema file Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141718|0 |1 is obsolete| | --- Comment #16 from David Nind <david@davidnind.com> --- Created attachment 142004 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142004&action=edit Bug 24606: Add Koha Object(s) with unit tests Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141719|0 |1 is obsolete| | --- Comment #17 from David Nind <david@davidnind.com> --- Created attachment 142005 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142005&action=edit Bug 24606: Implement item templates This patch set implements item editor templates for community Koha. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) prove t/db_dependent/Koha/Item/Template* 5) As a non superlibrarian, enter the item editor 6) Set some item fields, save as a new template using the buttom and form below the editor. 7) Test loading a template without remembering for the session 8) Test loading a template while remembering for the session 9) Test deleting a template 10) Test updating a template 11) Create one or more shared templates 12) Log in as another non superlibrarian without the new permission manage_item_editor_templates, verify you cannot edit/delete templates shared to you 13) Enable the new permission manage_item_editor_templates, verify you can now edit and delete templates shared to you Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141720|0 |1 is obsolete| | --- Comment #18 from David Nind <david@davidnind.com> --- Created attachment 142006 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142006&action=edit Bug 24606: Add item template editor permission Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141721|0 |1 is obsolete| | --- Comment #19 from David Nind <david@davidnind.com> --- Created attachment 142007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142007&action=edit Bug 24606: Update existing schema files [DO NOT PUSH] Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141972|0 |1 is obsolete| | --- Comment #20 from David Nind <david@davidnind.com> --- Created attachment 142008 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142008&action=edit Bug 24606: (QA follow-up) Set existing sharing setting when updating existing template Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 1) QA test tools: OK Koha/Item/Template.pm FAIL Koha/Item/Templates.pm FAIL pod *** WARNING: =head3 without preceding higher level in file Koha/Item/Templates.pm *** WARNING: =head3 without preceding higher level in file Koha/Item/Templates.pm *** WARNING: =head3 without preceding higher level in file Koha/Item/Templates.pm OK Koha/Schema/Result/Borrower.pm FAIL Koha/Schema/Result/ItemEditorTemplate.pm FAIL pod coverage POD is missing for koha_object_class POD is missing for koha_objects_class OK cataloguing/additem.pl OK installer/data/mysql/atomicupdate/bug_24606.pl FAIL installer/data/mysql/kohastructure.sql FAIL tinyint_has_boolean_flag The new column item_editor_templates.is_shared is using TINYINT(1) as type but is not defined as boolean in the schema file (Koha/Schema/Result/ItemEditorTemplate.pm), see the SQL12 coding guideline OK koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc OK koha-tmpl/intranet-tmpl/prog/en/includes/str/cataloging_additem.inc FAIL koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt FAIL filters missing_filter at line 275 ( <option data-editor="1" data-shared="[% t.is_shared %]" value="[% t.id | html %]">[% t.name | html %][% IF t.is_shared %] (shared)[% END %]</option>) 2) Why the sub in cataloguing/additem.pl instead of a module? +sub get_item_from_template 3) New cookie alert! ItemEditorSessionTemplateId If this gets pushed, please don't forget updating https://wiki.koha-community.org/wiki/Use_of_Cookies. 4) Translatability You used the old style here, which is no longer necessary: + alert(MSG_TEMPLATE_NAME_REQUIRED); You can just put the message into the .js using the double-underscore: __("") Not a blocker for me, as it looks like the other messages on this template haven't been moved yet, but not opposed to a fix either :) 5) The GUI doesn't work well with the new staff interface design. The elements don't align nicely (especialy the save template one is jumping up and down a lot with the elements) and we are missing padding. I believe we should not push without having this sorted or on the way to be sorted. - We want to advertise and document this feature and then it should look nice! 5) When using "Add multiple items" all other buttons disappear, but the "Save as template" remains. Should this also be hidden? 6) 'Delete template' is greyed out, when no template is selected, but 'Apply template' is always active. Should we make 'Apply template' also depend on having a template selected first? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #22 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 142833 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142833&action=edit Bug 24606: (QA follow-up) Fix QA script issues -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #23 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 142834 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142834&action=edit Bug 24606: (QA follow-up) Update js message translation style -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #24 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 142835 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142835&action=edit Bug 24606: (QA follow-up) Fix up interface -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #25 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 142836 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142836&action=edit Bug 24606: (QA follow-up) Hide button when using multi-add -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #26 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 142837 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142837&action=edit Bug 24606: (QA follow-up) Disable 'Apply template' button if no template is selected -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #27 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #21)
1) QA test tools:
Fixed with followup!
2) Why the sub in cataloguing/additem.pl instead of a module?
To remain consistent with how this script was developed and operates. It could use an overhaul, but that's outside the scope of this bug.
3) New cookie alert!
Done!
4) Translatability
Fixed with followup!
5) The GUI doesn't work well with the new staff interface design. The elements don't align nicely (especialy the save template one is jumping up and down a lot with the elements) and we are missing padding. I believe we should not push without having this sorted or on the way to be sorted. - We want to advertise and document this feature and then it should look nice!
Fixed with followup!
5) When using "Add multiple items" all other buttons disappear, but the "Save as template" remains. Should this also be hidden?
Seems reasonable to me. Fixed with a followup!
6) 'Delete template' is greyed out, when no template is selected, but 'Apply template' is always active. Should we make 'Apply template' also depend on having a template selected first?
Also fixed with a followup! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |katrin.fischer@bsz-bw.de |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #28 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Back here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Medium patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142003|0 |1 is obsolete| | --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142877 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142877&action=edit Bug 24606: Update database, add new schema file Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142004|0 |1 is obsolete| | --- Comment #30 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142878 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142878&action=edit Bug 24606: Add Koha Object(s) with unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142005|0 |1 is obsolete| | --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142879&action=edit Bug 24606: Implement item templates This patch set implements item editor templates for community Koha. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) prove t/db_dependent/Koha/Item/Template* 5) As a non superlibrarian, enter the item editor 6) Set some item fields, save as a new template using the buttom and form below the editor. 7) Test loading a template without remembering for the session 8) Test loading a template while remembering for the session 9) Test deleting a template 10) Test updating a template 11) Create one or more shared templates 12) Log in as another non superlibrarian without the new permission manage_item_editor_templates, verify you cannot edit/delete templates shared to you 13) Enable the new permission manage_item_editor_templates, verify you can now edit and delete templates shared to you Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142006|0 |1 is obsolete| | --- Comment #32 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142880 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142880&action=edit Bug 24606: Add item template editor permission Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142007|0 |1 is obsolete| | --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142881 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142881&action=edit Bug 24606: Update existing schema files [DO NOT PUSH] Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142008|0 |1 is obsolete| | --- Comment #34 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142882 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142882&action=edit Bug 24606: (QA follow-up) Set existing sharing setting when updating existing template Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142833|0 |1 is obsolete| | --- Comment #35 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142883&action=edit Bug 24606: (QA follow-up) Fix QA script issues Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142834|0 |1 is obsolete| | --- Comment #36 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142884&action=edit Bug 24606: (QA follow-up) Update js message translation style Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142835|0 |1 is obsolete| | --- Comment #37 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142885 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142885&action=edit Bug 24606: (QA follow-up) Fix up interface Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142836|0 |1 is obsolete| | --- Comment #38 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142886 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142886&action=edit Bug 24606: (QA follow-up) Hide button when using multi-add Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142837|0 |1 is obsolete| | --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142887 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142887&action=edit Bug 24606: (QA follow-up) Disable 'Apply template' button if no template is selected Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #40 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 142888 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142888&action=edit Bug 24606: (QA follow-up) Remove string variable This string is no longer needed as the __() is used in the .js file directly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #41 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I think this a nice addition and have passed QA on this. BUT: The GUI still needs a little bit more work, so this can shine on release: https://snipboard.io/IaM4VW.jpg I noticed that it reloads the page every time you apply or clean a template, it might be a nice improvement to rely on an API call here in the future. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #42 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143020 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143020&action=edit Bug 24606: (QA follow-up) Fix up save form a bit -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #142877|0 |1 is obsolete| | Attachment #142878|0 |1 is obsolete| | Attachment #142879|0 |1 is obsolete| | Attachment #142880|0 |1 is obsolete| | Attachment #142881|0 |1 is obsolete| | Attachment #142882|0 |1 is obsolete| | Attachment #142883|0 |1 is obsolete| | Attachment #142884|0 |1 is obsolete| | Attachment #142885|0 |1 is obsolete| | Attachment #142886|0 |1 is obsolete| | Attachment #142887|0 |1 is obsolete| | Attachment #142888|0 |1 is obsolete| | Attachment #143020|0 |1 is obsolete| | --- Comment #43 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143569&action=edit Bug 24606: Update database, add new schema file Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #44 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143570 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143570&action=edit Bug 24606: Add Koha Object(s) with unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #45 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143571 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143571&action=edit Bug 24606: Implement item templates This patch set implements item editor templates for community Koha. Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) prove t/db_dependent/Koha/Item/Template* 5) As a non superlibrarian, enter the item editor 6) Set some item fields, save as a new template using the buttom and form below the editor. 7) Test loading a template without remembering for the session 8) Test loading a template while remembering for the session 9) Test deleting a template 10) Test updating a template 11) Create one or more shared templates 12) Log in as another non superlibrarian without the new permission manage_item_editor_templates, verify you cannot edit/delete templates shared to you 13) Enable the new permission manage_item_editor_templates, verify you can now edit and delete templates shared to you Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #46 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143572 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143572&action=edit Bug 24606: Add item template editor permission Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #47 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143573 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143573&action=edit Bug 24606: (QA follow-up) Set existing sharing setting when updating existing template Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #48 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143574 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143574&action=edit Bug 24606: (QA follow-up) Fix QA script issues Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #49 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143575 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143575&action=edit Bug 24606: (QA follow-up) Update js message translation style Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #50 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143576 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143576&action=edit Bug 24606: (QA follow-up) Fix up interface Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #51 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143577&action=edit Bug 24606: (QA follow-up) Hide button when using multi-add Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #52 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143578 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143578&action=edit Bug 24606: (QA follow-up) Disable 'Apply template' button if no template is selected Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #53 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143579&action=edit Bug 24606: (QA follow-up) Remove string variable This string is no longer needed as the __() is used in the .js file directly. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #54 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143580 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143580&action=edit Bug 24606: (QA follow-up) Fix up save form a bit -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #55 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 143581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143581&action=edit Bug 24606: Update existing schema files [DO NOT PUSH] -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #56 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #57 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- https://snipboard.io/0uEloj.jpg - shouldn't we hide if there is no template defined? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #58 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Use statements appear twice +use URI::Escape qw( uri_escape_utf8 ); +use URI::Escape qw( uri_escape_utf8 ); -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #59 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- There is an encoding issue when you are applying a template that has ♥ in one of its subfield Wide character in subroutine entry at /kohadevbox/koha/Koha/Item/Template.pm line 58 in Koha::Item::Template::decoded_contents at /kohadevbox/koha/Koha/Item/Template.pm line 58 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #60 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143794 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143794&action=edit Bug 24606: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #61 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143795&action=edit Bug 24606: Fix encoding issues on decoded content This patch makes the `decoded_contents` method UTF-8 encode the data before calling the relevant decode_json, as expected by the JSON library [1]. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Item/Template.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! [1] `Any decoding or encoding (e.g. to UTF-8 or UTF-16) needs to be done yourself, e.g. using the Encode module.` https://metacpan.org/pod/JSON Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #62 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 143796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143796&action=edit Bug 24606: (QA follow-up) Remove duplicate include -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #63 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #58)
Use statements appear twice +use URI::Escape qw( uri_escape_utf8 ); +use URI::Escape qw( uri_escape_utf8 );
Done. (In reply to Jonathan Druart from comment #59)
There is an encoding issue when you are applying a template that has ♥ in one of its subfield
Wide character in subroutine entry at /kohadevbox/koha/Koha/Item/Template.pm line 58
Done. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #64 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #57)
https://snipboard.io/0uEloj.jpg - shouldn't we hide if there is no template defined?
Kyle, this is not strictly a blocker, but we really need you to fix it for the release. Separate bug please. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32188 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32188 [Bug 32188] Only show template controls above item form if templates have been defined -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #65 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Tomás Cohen Arazi from comment #64)
(In reply to Jonathan Druart from comment #57)
https://snipboard.io/0uEloj.jpg - shouldn't we hide if there is no template defined?
Kyle, this is not strictly a blocker, but we really need you to fix it for the release. Separate bug please.
Filed it as: Bug 32188 - Only show template controls above item form if templates have been defined -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #66 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- The following follow-up looks highly arguable to me: Bug 24606: Fix encoding issues on decoded content - return decode_json( $self->contents ) if $self->contents; + return decode_json( encode_utf8($self->contents) ) if $self->contents; Note a few lines above (in ->store): if ( ref( $self->contents ) eq 'HASH' ) { $self->contents( encode_json( $self->contents ) ); } The reference to "Do it yourself" in the commit message is not very specific and out of its context. Since we are storing strings to the database here, I think we should refer the utf8 encoding to that layer. And we should encode/decode in the middle of the process. So I would quote another part of the JSON docs: utf8 flag disabled When utf8 is disabled (the default), then encode/decode generate and expect Unicode strings, that is, characters with high ordinal Unicode values (> 255) will be encoded as such characters, and likewise such characters are decoded as-is, no changes to them will be done, except "(re-)interpreting" them as Unicode codepoints or Unicode characters, respectively (to Perl, these are the same thing in strings unless you do funny/weird/dumb stuff). This is useful when you want to do the encoding yourself (e.g. when you want to have UTF-16 encoded JSON texts) or when some other layer does the encoding for you (for example, when printing to a terminal using a filehandle that transparently encodes to UTF-8 you certainly do NOT want to UTF-8 encode your data first and have Perl encode it another time). What does practically mean? You should switch from using decode_json and encode_json TO from_json and to_json. Leave the strings in Perl internal format during the process. encode_json Converts the given Perl data structure to a UTF-8 encoded, binary string. to_json Converts the given Perl data structure to a Unicode string by default. This needs further attention imo. (But cant fail it anymore :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #67 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- And we should encode/decode in the middle of the process. And we should NOT -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #68 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #66)
What does practically mean? You should switch from using decode_json and encode_json TO from_json and to_json. Leave the strings in Perl internal format during the process.
The docs and you say the same: decode_json expects to be passed a string with Perl's internal format representing UTF-8 characters. And that's what encode_utf8 does, and why the patch introduces it at that exact place.
This needs further attention imo. (But cant fail it anymore :)
You can't fail what is correct :-P The cool thing is that you can propose some other solution. But I'd vote against something that adds more steps to the caller. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #69 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #66)
encode_json Converts the given Perl data structure to a UTF-8 encoded, binary string. to_json Converts the given Perl data structure to a Unicode string by default.
" This module also exports to_json and from_json for backward compatibility. These are slower " -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #70 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- We are using a cookie when localStorage should be used, see bug 12410. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #71 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Tomás Cohen Arazi from comment #69)
(In reply to Marcel de Rooy from comment #66)
encode_json Converts the given Perl data structure to a UTF-8 encoded, binary string. to_json Converts the given Perl data structure to a Unicode string by default.
" This module also exports to_json and from_json for backward compatibility. These are slower "
It's not working, so it's wrong. [2022/11/17 06:44:05] [WARN] The job has not been sent to the message broker: (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Handle.pm line 200. To recreate, see bug 31351 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #72 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #68)
The docs and you say the same: decode_json expects to be passed a string with Perl's internal format representing UTF-8 characters. And that's what encode_utf8 does, and why the patch introduces it at that exact place.
Perl internal format is certainly not UTF-8. Perhaps you are saying that here, but you could well mean something else too.. Not sure what you mean. But it is clear to me that we should not use decode_json here.
You can't fail what is correct :-P That is true. But this is absolutely not correct ;)
The cool thing is that you can propose some other solution. But I'd vote against something that adds more steps to the caller. If the right thing would take one more step, you should just take it.
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #73 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- The correct pattern is in BackgroundJob: sub decoded_data { my ($self) = @_; return $self->data ? $self->json->decode( $self->data ) : undef; } sub set_encoded_data { my ( $self, $data ) = @_; return $self->data( $data ? $self->json->encode($data) : undef ); } -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #74 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Tomás Cohen Arazi from comment #69)
This module also exports to_json and from_json for backward compatibility. These are slower
Thats true. We should switch to OO interface. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #75 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> ---
From Backgrount too
sub json { my ( $self ) = @_; $self->{_json} //= JSON->new->utf8(0); # TODO Should we allow_nonref ? return $self->{_json}; } I remember that you were objecting against it too. But it is the right approach. Do not set utf8, but keep everything in Perl internal format in the middle of the process.. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #76 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #71)
[2022/11/17 06:44:05] [WARN] The job has not been sent to the message broker: (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Handle.pm line 200.
To recreate, see bug 31351
Bug 31351 has been pushed. How do you recreate the problem? And explain the connection to this report please. This is different code? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #77 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #76)
(In reply to Jonathan Druart from comment #71)
[2022/11/17 06:44:05] [WARN] The job has not been sent to the message broker: (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Handle.pm line 200.
To recreate, see bug 31351
Bug 31351 has been pushed. How do you recreate the problem? And explain the connection to this report please. This is different code?
Different code? The first patch is replacing encode_json calls with encode, and remove the utf8 encoding :) To recreate, batch mod items, add ❤ in the note, submit the job, go to the job detail => it fails go to the koha logs => see the "Wide char" error -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #78 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #77)
(In reply to Marcel de Rooy from comment #76)
(In reply to Jonathan Druart from comment #71)
[2022/11/17 06:44:05] [WARN] The job has not been sent to the message broker: (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Handle.pm line 200.
To recreate, see bug 31351
Bug 31351 has been pushed. How do you recreate the problem? And explain the connection to this report please. This is different code?
Different code? The first patch is replacing encode_json calls with encode, and remove the utf8 encoding :)
To recreate, batch mod items, add ❤ in the note, submit the job, go to the job detail => it fails go to the koha logs => see the "Wide char" error
Okay. You actually mean that Tomas is right. I am on it today. Please wait a bit. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #79 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- For the record, my comments didn't make sense. I hadn't noticed that Tomas was not modifying Koha::BackgroundJob. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #80 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #78)
(In reply to Jonathan Druart from comment #77)
(In reply to Marcel de Rooy from comment #76)
(In reply to Jonathan Druart from comment #71)
[2022/11/17 06:44:05] [WARN] The job has not been sent to the message broker: (Wide character in syswrite at /usr/lib/x86_64-linux-gnu/perl-base/IO/Handle.pm line 200.
To recreate, see bug 31351
Bug 31351 has been pushed. How do you recreate the problem? And explain the connection to this report please. This is different code?
Different code? The first patch is replacing encode_json calls with encode, and remove the utf8 encoding :)
To recreate, batch mod items, add ❤ in the note, submit the job, go to the job detail => it fails go to the koha logs => see the "Wide char" error
Okay. You actually mean that Tomas is right. I am on it today. Please wait a bit.
We complicated this discussion a bit by mixing two separate issues. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #81 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Opened bug 32242 for the Background issue now. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #82 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Tomás: I will add a patch here for your consideration now. (By way of exception :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #83 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 143989 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143989&action=edit Bug 24606: (QA follow-up) Fix encoding/decoding We should use OO interface and leave encoding to database layer. Test plan: Add a new item template containing real Unicode chars. Save. Apply on new item. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #84 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 143990 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=143990&action=edit Bug 24606: (QA follow-up) Add test for encoding/decoding NOTE: I have the impression that this unit test is quite rudimentary. I do not see the apply function tested? Test plan: Run t/db_dependent/Koha/Item/Templates.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #85 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Changed status to get attention from RM for last two follow-ups. Thx. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Keywords|additional_work_needed | --- Comment #86 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #85)
Changed status to get attention from RM for last two follow-ups. Thx.
Thanks for the follow-ups, Marcel. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #87 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Template.t fails now. But the code is wrong in the test. I'll have a look. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #88 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 144161 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144161&action=edit Bug 24606: (QA follow-up) Fix Template.t The test cases here still assumed UTF-8 enocding in the middle of the process. Can be much simpler. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_22_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 --- Comment #89 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Marcel de Rooy from comment #88)
Created attachment 144161 [details] [review] Bug 24606: (QA follow-up) Fix Template.t
The test cases here still assumed UTF-8 enocding in the middle of the process. Can be much simpler.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Follow-up pushed, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_22_11_candidate | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This new feature allows release notes| |librarians to create and | |share "item templates" | |where one or more item | |field values can be set for | |a template. Templates can | |be applied on a one by one | |basis, or set for the | |remainder of the logged in | |session. Each template may | |be optionally shared ( read | |only ) to other catalogers. | |Librarians with the | |manage_item_editor_template | |s permission may edit any | |template regardless of who | |created it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |new feature -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Documentation| |https://gitlab.com/koha-com submission| |munity/koha-manual/-/merge_ | |requests/603 Documentation| |Caroline Cyr La Rose contact| | CC| |caroline.cyr-la-rose@inlibr | |o.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32959 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32959 [Bug 32959] Item Templates will apply the same barcode each time template is applied if autobarcode is enabled -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34844 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34844 [Bug 34844] manage_item_editor_templates is missing from userpermissions.sql -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24606 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Needs documenting Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org