Bug 14957 - Write protecting MARC fields based on source of import
Summary: Write protecting MARC fields based on source of import
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Assignee: David Gustafsson
QA Contact: Tomás Cohen Arazi
URL: https://gitlab.com/gnucifer/koha-manu...
Keywords:
: 11519 11780 13213 18210 22354 26372 (view as bug list)
Depends on: 24788
Blocks: 30680 30681 30682 30741 33268 35155 29589 30520 30966
  Show dependency treegraph
 
Reported: 2015-10-05 14:07 UTC by Martin Stenberg
Modified: 2023-10-25 19:30 UTC (History)
41 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement enables the use of rules for merging MARC records. For example, it can be used to prevent field data from being overwritten. It is enabled using the new system preference "MARCOverlayRules". Rules are added, edited and deleted in the staff interface from Home > Koha administration > Catalog > MARC overlay rules. NOTE: A follow-up bug is being worked upon to add compatibility with bulkmarcimport.
Version(s) released in:
21.11.00


Attachments
Bug 14957: Rule based permission for MARC fields (81.46 KB, patch)
2015-10-27 11:31 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14957: Database changes (3.56 KB, patch)
2015-10-27 11:31 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14957: Add filters to relevant scripts (5.84 KB, patch)
2015-10-27 11:31 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14957: Rule based permission for MARC fields (82.59 KB, patch)
2016-05-23 20:53 UTC, Eivin Giske Skaaren
Details | Diff | Splinter Review
Bug 14957: Correct parameters for TransformMarcToKoha (940 bytes, patch)
2016-05-23 20:59 UTC, Eivin Giske Skaaren
Details | Diff | Splinter Review
Bug 14957: Key fixes to patch (19.48 KB, patch)
2016-10-19 04:23 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14957: Key fixes to patch (19.48 KB, patch)
2016-10-19 04:28 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14957: Database changes (3.56 KB, patch)
2016-10-19 09:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Add filters to relevant scripts (6.07 KB, patch)
2016-10-19 09:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Rule based permission for MARC fields (81.96 KB, patch)
2016-10-19 09:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Correct parameters for TransformMarcToKoha (940 bytes, patch)
2016-10-19 09:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Key fixes to patch (19.48 KB, patch)
2016-10-19 09:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Followup - Pod, import_lexile.pl, template and test fixes (3.45 KB, patch)
2016-10-19 09:23 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Database changes (3.56 KB, patch)
2016-11-03 07:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Add filters to relevant scripts (6.07 KB, patch)
2016-11-03 07:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Rule based permission for MARC fields (81.60 KB, patch)
2016-11-03 07:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Correct parameters for TransformMarcToKoha (940 bytes, patch)
2016-11-03 07:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Key fixes to patch (19.48 KB, patch)
2016-11-03 07:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 14957: Followup - Pod, import_lexile.pl, template and test fixes (3.45 KB, patch)
2016-11-03 07:07 UTC, Josef Moravec
Details | Diff | Splinter Review
Rule based permission for MARC fields (87.02 KB, patch)
2017-02-02 12:44 UTC, David Gustafsson
Details | Diff | Splinter Review
Rule based permission for MARC fields (86.66 KB, patch)
2017-02-02 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (90.52 KB, patch)
2017-02-03 14:09 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (90.64 KB, patch)
2017-02-27 13:09 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (90.64 KB, patch)
2017-03-06 09:59 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (90.60 KB, patch)
2017-03-06 10:04 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (90.88 KB, patch)
2017-03-14 13:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (90.77 KB, patch)
2017-03-27 12:00 UTC, Johan Larsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (91.35 KB, patch)
2017-03-30 12:24 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (92.35 KB, patch)
2017-07-04 14:15 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (68.58 KB, patch)
2018-03-07 10:16 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (68.40 KB, patch)
2018-04-25 19:22 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (67.12 KB, patch)
2018-05-07 12:27 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (122.93 KB, patch)
2018-06-20 12:07 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.56 KB, patch)
2018-06-20 12:10 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (122.93 KB, patch)
2018-06-20 12:10 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.56 KB, patch)
2018-06-20 12:11 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Refactor and fix tests and documention (122.93 KB, patch)
2018-06-20 12:11 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.56 KB, patch)
2018-06-20 12:12 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Refactor and fix tests and documentation (122.93 KB, patch)
2018-06-20 12:12 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.56 KB, patch)
2018-06-20 12:15 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Refactor and fix tests and documentation (122.93 KB, patch)
2018-06-20 12:15 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Remove exports of removed subroutines (733 bytes, patch)
2018-06-20 12:23 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.75 KB, patch)
2018-09-14 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Refactor and fix tests and documentation (123.17 KB, patch)
2018-09-14 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Remove exports of removed subroutines (733 bytes, patch)
2018-09-14 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Remove exports of removed subroutines (733 bytes, patch)
2018-09-25 12:37 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.75 KB, patch)
2018-09-25 12:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Refactor and fix tests and documentation (123.17 KB, patch)
2018-09-25 12:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Remove exports of removed subroutines (733 bytes, patch)
2018-09-25 12:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Add missing use in Biblio.pm (594 bytes, patch)
2018-10-03 13:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (66.95 KB, patch)
2018-10-22 11:57 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Refactor and fix tests and documentation (123.19 KB, patch)
2018-10-22 11:58 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Remove exports of removed subroutines (770 bytes, patch)
2018-10-22 11:58 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Add missing use in Biblio.pm (570 bytes, patch)
2018-10-22 11:58 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix control field rules regression and add rule validation (9.10 KB, patch)
2018-10-25 15:17 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix control field rules regression and add rule validation (11.17 KB, patch)
2018-10-25 15:18 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix control field rules regression and add rule validation (11.13 KB, patch)
2018-10-25 15:20 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix control field rules regression and add rule validation (11.26 KB, patch)
2018-10-25 15:23 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (4.89 KB, patch)
2018-10-30 13:50 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957 - Rule based permission for MARC fields (80.65 KB, patch)
2018-10-31 10:59 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (4.89 KB, patch)
2018-10-31 11:03 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (80.70 KB, patch)
2018-10-31 11:17 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (4.89 KB, patch)
2018-10-31 11:17 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (80.70 KB, patch)
2018-10-31 11:18 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (4.89 KB, patch)
2018-10-31 11:18 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 21596: (follow-up) Handle updated_on part 2 (1.31 KB, patch)
2018-11-05 15:50 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (80.87 KB, patch)
2018-11-05 15:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (4.89 KB, patch)
2018-11-05 15:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (80.87 KB, patch)
2018-11-07 14:35 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (4.89 KB, patch)
2018-11-07 14:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (83.17 KB, patch)
2018-11-30 10:35 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (83.13 KB, patch)
2019-04-10 13:11 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (83.13 KB, patch)
2019-04-10 13:14 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix most Koha QA test tool errors (12.74 KB, patch)
2019-04-11 14:27 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Put database updates in perl script (6.08 KB, patch)
2019-04-12 17:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Put database updates in perl script (8.96 KB, patch)
2019-04-17 15:37 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Add tests and pod for new options param to ModBiblio (6.38 KB, patch)
2019-04-17 15:38 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix categorycode module filter value and rename some presets (4.66 KB, patch)
2019-05-22 14:43 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.10 KB, patch)
2019-06-17 14:16 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (2.46 KB, patch)
2019-06-17 14:17 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (2.46 KB, patch)
2019-06-17 14:27 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (90.45 KB, patch)
2019-06-17 14:27 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (88.92 KB, patch)
2019-06-18 11:04 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Make preset labels translatable (2.96 KB, patch)
2019-06-18 12:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Make preset labels translatable (3.61 KB, patch)
2019-06-18 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Make preset labels translatable (3.83 KB, patch)
2019-06-18 13:17 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (2.46 KB, patch)
2019-09-18 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (88.98 KB, patch)
2019-09-18 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Make preset labels translatable (3.83 KB, patch)
2019-09-18 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Adjust paging option for new version of datatables (997 bytes, patch)
2019-09-18 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix rule javascript validation issue (1.73 KB, patch)
2019-09-18 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Change YUI layout to to Boostrap (1.36 KB, patch)
2019-09-18 12:49 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Remove prefetch of removed relationship (843 bytes, patch)
2019-09-20 14:14 UTC, David Gustafsson
Details | Diff | Splinter Review
Buttons difference (4.91 KB, image/png)
2019-10-02 15:35 UTC, Séverine Queune
Details
Bug 14957: Add icons for confirmation dialog and set preset based on actions configuration (5.26 KB, patch)
2019-10-31 12:20 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Replace filter value with label if one exists (2.09 KB, patch)
2019-10-31 12:42 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Remove usage of deleted Koha::MarcMergeRulesModules (12.81 KB, patch)
2020-03-03 13:15 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: DO NOT PUSH Schema changes (2.51 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (89.03 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Make preset labels translatable (3.88 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Adjust paging option for new version of datatables (1.02 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Fix rule javascript validation issue (1.78 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Change YUI layout to to Boostrap (1.41 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Remove prefetch of removed relationship (891 bytes, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Add icons for confirmation dialog and set preset based on actions configuration (5.31 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Replace filter value with label if one exists (2.13 KB, patch)
2020-03-03 19:37 UTC, David Nind
Details | Diff | Splinter Review
Bug 14957: Remove usage of deleted Koha::MarcMergeRulesModules (12.86 KB, patch)
2020-03-03 19:38 UTC, David Nind
Details | Diff | Splinter Review
Screenshot of option placement in OPAC (99.09 KB, image/png)
2020-03-24 09:05 UTC, Katrin Fischer
Details
Bug 14957: Merge rules system for merging of MARC records (88.91 KB, patch)
2020-09-11 15:32 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Make preset labels translatable (3.87 KB, patch)
2020-09-11 15:32 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Adjust paging option for new version of datatables (1.02 KB, patch)
2020-09-11 15:32 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix rule javascript validation issue (1.77 KB, patch)
2020-09-11 15:33 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Change YUI layout to to Boostrap (1.40 KB, patch)
2020-09-11 15:33 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Remove prefetch of removed relationship (891 bytes, patch)
2020-09-11 15:33 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Add icons for confirmation dialog and set preset based on actions configuration (5.30 KB, patch)
2020-09-11 15:33 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Replace filter value with label if one exists (2.12 KB, patch)
2020-09-11 15:33 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Remove usage of deleted Koha::MarcMergeRulesModules (12.85 KB, patch)
2020-09-11 15:33 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (90.88 KB, patch)
2021-02-10 03:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.46 KB, patch)
2021-02-10 03:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (90.88 KB, patch)
2021-02-10 03:50 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.46 KB, patch)
2021-02-10 03:51 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (90.86 KB, patch)
2021-03-05 12:47 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.45 KB, patch)
2021-03-05 12:47 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.08 KB, patch)
2021-03-05 13:27 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.62 KB, patch)
2021-03-05 13:27 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (3.98 KB, patch)
2021-03-05 13:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.42 KB, patch)
2021-03-05 14:43 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.14 KB, patch)
2021-03-09 12:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.73 KB, patch)
2021-03-09 12:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.54 KB, patch)
2021-03-09 12:41 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.24 KB, patch)
2021-03-28 13:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.79 KB, patch)
2021-03-28 13:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.60 KB, patch)
2021-03-28 13:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Button styling (3.36 KB, patch)
2021-03-28 13:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add tooltips (2.14 KB, patch)
2021-03-28 13:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay' (49.81 KB, patch)
2021-03-28 13:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions (3.49 KB, patch)
2021-03-28 13:40 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: DBIC changes (7.45 KB, patch)
2021-03-28 13:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: (follow-up) marc_merge_rules_modules removed (3.24 KB, patch)
2021-03-28 13:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Clarify 'context' param (8.22 KB, patch)
2021-03-28 13:41 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.14 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.73 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.54 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Button styling (3.42 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add tooltips (2.21 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay' (49.84 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions (3.54 KB, patch)
2021-03-29 09:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: DBIC changes (7.50 KB, patch)
2021-03-29 09:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (follow-up) marc_merge_rules_modules removed (3.30 KB, patch)
2021-03-29 09:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Clarify 'context' param (8.27 KB, patch)
2021-03-29 09:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate (1.45 KB, patch)
2021-03-29 09:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix translation issues (3.68 KB, patch)
2021-03-29 09:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename and move admin link (2.26 KB, patch)
2021-03-29 09:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily (1.39 KB, patch)
2021-03-29 12:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.14 KB, patch)
2021-05-05 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.73 KB, patch)
2021-05-05 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.54 KB, patch)
2021-05-05 15:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Button styling (3.42 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add tooltips (2.21 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay' (49.84 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions (3.54 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: DBIC changes (7.50 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (follow-up) marc_merge_rules_modules removed (3.30 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Clarify 'context' param (8.27 KB, patch)
2021-05-05 15:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate (1.45 KB, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix translation issues (3.68 KB, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename and move admin link (2.26 KB, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily (1.39 KB, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add missing filters (3.46 KB, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 26703 (1.02 KB, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20054 (885 bytes, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20053 (865 bytes, patch)
2021-05-05 15:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 17858 (18.66 KB, patch)
2021-05-05 15:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 27846 (1.40 KB, patch)
2021-05-05 15:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix template indentation (28.21 KB, patch)
2021-05-05 15:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix template indentation (28.13 KB, patch)
2021-05-05 15:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 23271 (1.27 KB, patch)
2021-05-06 07:01 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.14 KB, patch)
2021-05-06 07:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.73 KB, patch)
2021-05-06 07:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.54 KB, patch)
2021-05-06 07:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Button styling (3.42 KB, patch)
2021-05-06 07:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add tooltips (2.21 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay' (49.84 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions (3.54 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: DBIC changes (7.50 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (follow-up) marc_merge_rules_modules removed (3.30 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Clarify 'context' param (8.27 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate (1.45 KB, patch)
2021-05-06 07:28 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix translation issues (3.68 KB, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename and move admin link (2.26 KB, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily (1.39 KB, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add missing filters (3.46 KB, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 26703 (1.02 KB, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20054 (885 bytes, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20053 (865 bytes, patch)
2021-05-06 07:29 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 17858 (18.66 KB, patch)
2021-05-06 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 27846 (1.40 KB, patch)
2021-05-06 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix template indentation (28.13 KB, patch)
2021-05-06 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 23271 (1.27 KB, patch)
2021-05-06 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix script name (2.70 KB, patch)
2021-05-06 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix for jQuery undefined (3.54 KB, patch)
2021-05-06 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Clean controller (3.10 KB, patch)
2021-05-07 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Clean controller (3.14 KB, patch)
2021-05-07 10:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Improve readbility of tests (40.53 KB, patch)
2021-05-07 16:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.05 KB, patch)
2021-09-30 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.73 KB, patch)
2021-09-30 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.54 KB, patch)
2021-09-30 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Button styling (3.42 KB, patch)
2021-09-30 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add tooltips (2.21 KB, patch)
2021-09-30 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay' (49.82 KB, patch)
2021-09-30 13:43 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions (3.54 KB, patch)
2021-09-30 13:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: DBIC changes (7.50 KB, patch)
2021-09-30 13:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (follow-up) marc_merge_rules_modules removed (3.30 KB, patch)
2021-09-30 13:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Clarify 'context' param (8.27 KB, patch)
2021-09-30 13:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate (1.45 KB, patch)
2021-09-30 13:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix translation issues (3.68 KB, patch)
2021-09-30 13:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename and move admin link (2.26 KB, patch)
2021-09-30 13:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily (1.39 KB, patch)
2021-09-30 13:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add missing filters (3.46 KB, patch)
2021-09-30 13:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 26703 (1.02 KB, patch)
2021-09-30 13:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20054 (885 bytes, patch)
2021-09-30 13:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20053 (865 bytes, patch)
2021-09-30 13:45 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 17858 (18.66 KB, patch)
2021-09-30 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 27846 (1.40 KB, patch)
2021-09-30 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix template indentation (28.13 KB, patch)
2021-09-30 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 23271 (1.27 KB, patch)
2021-09-30 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix script name (2.70 KB, patch)
2021-09-30 13:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix for jQuery undefined (3.54 KB, patch)
2021-09-30 13:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Clean controller (3.13 KB, patch)
2021-09-30 13:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Improve readbility of tests (40.53 KB, patch)
2021-09-30 13:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix imports in tests (767 bytes, patch)
2021-09-30 13:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix regression, rules must be refetched on add/edit/deletion (1.54 KB, patch)
2021-10-01 14:18 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Fix imports in marc-overlay-rules.pl (781 bytes, patch)
2021-10-01 14:19 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Exclude leader from saved/updated record comparison in tests (1.52 KB, patch)
2021-10-01 14:48 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: (follow-up) Clarify 'context' param (1.07 KB, patch)
2021-10-18 10:47 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Fix record preview (6.05 KB, patch)
2021-10-18 11:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Fix syspref's values (1.10 KB, patch)
2021-10-18 12:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Fix field order issue (1.89 KB, patch)
2021-10-20 15:18 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 14957: Add tests for subfield order (2.22 KB, patch)
2021-10-21 13:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Tell the cataloguer that a change has been rejected (6.72 KB, patch)
2021-10-21 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: Merge rules system for merging of MARC records (91.05 KB, patch)
2021-10-22 12:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Updated schema files (2.73 KB, patch)
2021-10-22 12:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: fix context for batchmod and batchimport (4.54 KB, patch)
2021-10-22 12:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Button styling (3.42 KB, patch)
2021-10-22 12:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add tooltips (2.21 KB, patch)
2021-10-22 12:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay' (49.82 KB, patch)
2021-10-22 12:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions (3.54 KB, patch)
2021-10-22 12:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: DBIC changes (7.50 KB, patch)
2021-10-22 12:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (follow-up) marc_merge_rules_modules removed (3.30 KB, patch)
2021-10-22 12:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Clarify 'context' param (8.27 KB, patch)
2021-10-22 12:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate (1.45 KB, patch)
2021-10-22 12:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix translation issues (3.68 KB, patch)
2021-10-22 12:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Rename and move admin link (2.26 KB, patch)
2021-10-22 12:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily (1.39 KB, patch)
2021-10-22 12:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Add missing filters (3.53 KB, patch)
2021-10-22 12:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 26703 (1.08 KB, patch)
2021-10-22 12:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20054 (950 bytes, patch)
2021-10-22 12:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 20053 (930 bytes, patch)
2021-10-22 12:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 17858 (18.72 KB, patch)
2021-10-22 12:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 27846 (1.46 KB, patch)
2021-10-22 12:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix template indentation (28.19 KB, patch)
2021-10-22 12:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Update for bug 23271 (1.34 KB, patch)
2021-10-22 12:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix script name (2.76 KB, patch)
2021-10-22 12:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (QA follow-up) Fix for jQuery undefined (3.60 KB, patch)
2021-10-22 12:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Clean controller (3.20 KB, patch)
2021-10-22 12:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Improve readbility of tests (40.59 KB, patch)
2021-10-22 12:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix imports in tests (832 bytes, patch)
2021-10-22 12:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix regression, rules must be refetched on add/edit/deletion (1.60 KB, patch)
2021-10-22 12:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix imports in marc-overlay-rules.pl (846 bytes, patch)
2021-10-22 12:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Exclude leader from saved/updated record comparison in tests (1.58 KB, patch)
2021-10-22 12:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: (follow-up) Clarify 'context' param (1.13 KB, patch)
2021-10-22 12:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix record preview (6.11 KB, patch)
2021-10-22 12:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix syspref's values (1.17 KB, patch)
2021-10-22 12:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix field order issue (1.95 KB, patch)
2021-10-22 12:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Add tests for subfield order (2.28 KB, patch)
2021-10-22 12:40 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 14957: Fix perlcritic error on admin/marc-overlay-rules.pl (839 bytes, patch)
2021-10-27 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14957: DBIC changes - Remove MarcOverlayRulesModule.pm (2.41 KB, patch)
2021-10-27 07:18 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Stenberg 2015-10-05 14:07:53 UTC
Add options for write protecting individual MARC fields based on source of import (Z39.50, stage for import, etc) to avoid overwriting local changes.

This feature is a superset of http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11780
Comment 1 Katrin Fischer 2015-10-05 17:34:58 UTC
+100!
Comment 2 Martin Stenberg 2015-10-27 11:31:38 UTC Comment hidden (obsolete)
Comment 3 Martin Stenberg 2015-10-27 11:31:47 UTC Comment hidden (obsolete)
Comment 4 Martin Stenberg 2015-10-27 11:31:58 UTC Comment hidden (obsolete)
Comment 5 Magnus Enger 2016-04-21 19:36:23 UTC Comment hidden (obsolete)
Comment 6 Eivin Giske Skaaren 2016-05-23 20:53:46 UTC Comment hidden (obsolete)
Comment 7 Eivin Giske Skaaren 2016-05-23 20:59:31 UTC Comment hidden (obsolete)
Comment 8 Eivin Giske Skaaren 2016-05-23 21:01:05 UTC
I have rebased and merged this and it seems to work. I dont have any MARC experience so please test that this works in some real use case.
Comment 9 David Cook 2016-05-24 23:38:09 UTC
Oh I had forgotten about this one... might have to look at this sometime...
Comment 10 Magnus Enger 2016-06-02 09:39:03 UTC Comment hidden (obsolete)
Comment 11 Magnus Enger 2016-06-02 10:13:34 UTC Comment hidden (obsolete)
Comment 12 Magnus Enger 2016-06-02 11:56:29 UTC
When you are on the "MARC field permissions" page, "MARC field permissions" is not shown in the left hand menu.
Comment 13 Andreas Hedström Mace 2016-07-19 20:47:21 UTC
Failed to apply for me (merge conflicts). Needs a rebase again?
Comment 14 delaye 2016-10-12 12:28:50 UTC
i can't test the patch with a sandbox biblibre
i have this message
The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 14957:
<h1>Something went wrong !</h1>Applying: Bug 14957: Database changes
Applying: Bug 14957: Add filters to relevant scripts
.git/rebase-apply/patch:14: trailing whitespace.
        } 
.git/rebase-apply/patch:16: trailing whitespace.
            ModBiblioframework( ,  ); 
warning: 2 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	cataloguing/addbiblio.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
M	misc/migration_tools/bulkmarcimport.pl
M	tools/batch_record_modification.pl
.git/rebase-apply/patch:14: trailing whitespace.
        } 
.git/rebase-apply/patch:16: trailing whitespace.
            ModBiblioframework( ,  ); 
warning: 2 lines add whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging tools/batch_record_modification.pl
CONFLICT (content): Merge conflict in tools/batch_record_modification.pl
Auto-merging misc/migration_tools/bulkmarcimport.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
Auto-merging cataloguing/addbiblio.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14957: Add filters to relevant scripts
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 14957 - Write protecting MARC fields based on source of import

44057 - Bug 14957: Database changes
44058 - Bug 14957: Add filters to relevant scripts
51723 - Bug 14957: Rule based permission for MARC fields
51724 - Bug 14957: Correct parameters for TransformMarcToKoha

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-14957-Add-filters-to-relevant-scripts-7vvvqA.patch .
Comment 15 Aleisha Amohia 2016-10-19 04:23:46 UTC Comment hidden (obsolete)
Comment 16 Aleisha Amohia 2016-10-19 04:28:15 UTC Comment hidden (obsolete)
Comment 17 Josef Moravec 2016-10-19 09:23:03 UTC Comment hidden (obsolete)
Comment 18 Josef Moravec 2016-10-19 09:23:11 UTC Comment hidden (obsolete)
Comment 19 Josef Moravec 2016-10-19 09:23:18 UTC Comment hidden (obsolete)
Comment 20 Josef Moravec 2016-10-19 09:23:29 UTC Comment hidden (obsolete)
Comment 21 Josef Moravec 2016-10-19 09:23:34 UTC Comment hidden (obsolete)
Comment 22 Josef Moravec 2016-10-19 09:23:42 UTC Comment hidden (obsolete)
Comment 23 Josef Moravec 2016-10-19 09:24:34 UTC
Rebased on master

Fixed few issues in the follow-up
Comment 24 Andreas Hedström Mace 2016-11-02 21:32:04 UTC
Doesn't apply for me. Needs yet another rebase?
Comment 25 Josef Moravec 2016-11-03 07:06:07 UTC Comment hidden (obsolete)
Comment 26 Josef Moravec 2016-11-03 07:06:17 UTC
Created attachment 57115 [details] [review]
Bug 14957: Add filters to relevant scripts

Add MARCPermissions filters in scripts which modifies MARC records to enable the
permission system for these scripts.
Comment 27 Josef Moravec 2016-11-03 07:06:51 UTC Comment hidden (obsolete)
Comment 28 Josef Moravec 2016-11-03 07:06:59 UTC Comment hidden (obsolete)
Comment 29 Josef Moravec 2016-11-03 07:07:05 UTC Comment hidden (obsolete)
Comment 30 Josef Moravec 2016-11-03 07:07:11 UTC Comment hidden (obsolete)
Comment 31 Josef Moravec 2016-11-03 07:08:17 UTC
It really needed rebasing again. Done.
Comment 32 Andreas Hedström Mace 2016-12-13 22:42:34 UTC
I started testing this today and after some initial confusion about the rules system (that could use some more documentation!) it seems mostly to work as intended. But am I correct in understanding that Marc field permissions does not currently work with Staged MARC Imports?
Comment 33 David Gustafsson 2017-02-02 12:44:21 UTC Comment hidden (obsolete)
Comment 34 David Gustafsson 2017-02-02 12:49:03 UTC Comment hidden (obsolete)
Comment 35 David Gustafsson 2017-02-02 13:10:51 UTC
The previously posted version of this patch at least for me did not apply cleanly. Rebased against current HEAD of master, and also slightly altered the behavior of "GetMarcPermissionsAction". The previous behaviour struck me as odd since a module with a higher specificity will always override modules with lower, even for the case when a filter argument for a module with lower specificity has an exact match. So for example, if we have a rule with filter 'source' => 'bulkmarcimport', and another for 'borrower' => '*', the later rule will always have precedence, even when source is matching 'bulkmarcimport'. I seems more flexible to have the option to override on a more specific filter match. 

Generally I think this patch is great, and almost exactly what we need, but the whole filter-system I think is in need of some revamping. It would be nice if the modules where more like separate namespaces, so that one set of rules (belonging to a certain module) was applied at once. Right now rules from several modules might be mixed in, and field-specificity has precedence in rules selection. The code could be simplified and optimized if rules where grouped by module instead (and also easier to admin from a user perspective I would believe, since rules precedence would be more obvious. But have made no such changes in the current patch, as wanted to bring this up for discussion before make any large breaking changes.
Comment 36 Andreas Hedström Mace 2017-02-03 09:38:14 UTC
Patch doesn't apply for my (using Kohadevbox). I get the following error:

Traceback (most recent call last):
  File "/usr/local/bin/git-bz", line 2630, in <module>
    do_apply(*args)
  File "/usr/local/bin/git-bz", line 1755, in do_apply
    users = bug.server.get_xmlrpc_proxy().User.get({ 'names': [patch.attacher] })['users']
  File "/usr/lib/python2.7/xmlrpclib.py", line 1233, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1591, in __request
    verbose=self.__verbose
  File "/usr/lib/python2.7/xmlrpclib.py", line 1273, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib/python2.7/xmlrpclib.py", line 1321, in single_request
    response.msg,
xmlrpclib.ProtocolError: <ProtocolError for bugs.koha-community.org/xmlrpc.cgi: 404 Not Found>
Comment 37 Magnus Enger 2017-02-03 09:49:45 UTC
(In reply to Andreas Hedström Mace from comment #36)
> Patch doesn't apply for my (using Kohadevbox). I get the following error:
...
> xmlrpclib.ProtocolError: <ProtocolError for
> bugs.koha-community.org/xmlrpc.cgi: 404 Not Found>

That looks more like an error from git bz than from git itself. 

404 Not Found - are you sure git bz is setup properly and has all the configs it needs?
Comment 38 David Gustafsson 2017-02-03 12:39:06 UTC
I think I might not have followed the guidelines for submitting patches properly. Will try to see if I can generate a patch using the recommended tools (git bz etc).
Comment 39 Magnus Enger 2017-02-03 12:48:25 UTC
(In reply to David Gustafsson from comment #38)
> I think I might not have followed the guidelines for submitting patches
> properly. Will try to see if I can generate a patch using the recommended
> tools (git bz etc).

Indeed, it looks more like a regular diff than a git patch. It seems odd that this should trigger the error that Andreas got, though. But I tested, an I get the same error on this patch while other patches work as they should. A new patch looks like a good place to start, though. :-)
Comment 40 Andreas Hedström Mace 2017-02-03 13:42:16 UTC
(In reply to Magnus Enger from comment #39)
> Indeed, it looks more like a regular diff than a git patch. It seems odd
> that this should trigger the error that Andreas got, though. But I tested,
> an I get the same error on this patch while other patches work as they
> should. A new patch looks like a good place to start, though. :-)

Yeah, as I have never seen such an error either I made sure to check that other patches applied as intended, which they did. Sounds great if you can provide a follow-up David!
Comment 41 David Gustafsson 2017-02-03 14:09:20 UTC Comment hidden (obsolete)
Comment 42 David Gustafsson 2017-02-03 14:13:50 UTC
Ok! One more try. A little bit unsure how to handle attribution, right now looks like the patch is attributed to me which is wrong. But hope it will at least apply now (also hope I did not make any mistake during the rebase, when through the patch several times to check everything adds up, but still might have botched something up).
Comment 43 David Gustafsson 2017-02-27 13:09:39 UTC Comment hidden (obsolete)
Comment 44 David Gustafsson 2017-02-27 13:10:37 UTC
Rebased against master.
Comment 45 David Gustafsson 2017-03-06 09:59:24 UTC Comment hidden (obsolete)
Comment 46 David Gustafsson 2017-03-06 10:00:30 UTC
Screwed up the last merge. Should be fixed now.
Comment 47 David Gustafsson 2017-03-06 10:03:43 UTC
Nope, still managed to screw it up. Should be ok now.
Comment 48 David Gustafsson 2017-03-06 10:04:35 UTC Comment hidden (obsolete)
Comment 49 David Gustafsson 2017-03-09 14:06:35 UTC
Have discovered a lot of weirdness in how permissions are applied, not sure if this is something I have introduces or present in the original patch, but don't merge in this quite yet. Will update the patch some time in the coming days.
Comment 50 David Gustafsson 2017-03-14 13:25:23 UTC Comment hidden (obsolete)
Comment 51 David Gustafsson 2017-03-14 13:32:02 UTC
Made some small adjustments:

- Removed $options-{filter} arguments from ModBiblioMarc in AddBiblio since ApplyMarcPermissions is never run on adding new biblios anyways (since no caller uses it, and marc-xml for current biblionumber will always be empty on save and ApplyMarcPermissions will bail out anyway), and to provide that arguments makes things even more confusing.

- Added source => 'batchimport' for staged import batches. So now ApplyMarcPermissions runs also on staged imports. One thing I did not realize before is that marc permissions are never enforced adding new biblios, this is also the case for staged imports. Only if a match is found permissions will be applied. It would probably be more appropriate to call the current system "Marc merge rules" or something like that, since it is really a framework for defining what happens when two marc records are to be merged (and not "permissions" in the strict sense of the word).
Comment 52 Johan Larsson 2017-03-27 12:00:29 UTC Comment hidden (obsolete)
Comment 53 Johan Larsson 2017-03-27 12:01:57 UTC
corrected spelling error in commit message.
Comment 54 David Gustafsson 2017-03-30 12:24:11 UTC Comment hidden (obsolete)
Comment 55 David Gustafsson 2017-03-30 12:30:45 UTC Comment hidden (obsolete)
Comment 56 David Gustafsson 2017-03-30 12:33:01 UTC
Ops, wrong copy paste. Fixed missing "String::Similarity" include in Biblio.pm, and changed $filter argument in ModBiblio to more flexible $options to avoid future merge conflicts and provide a more stable API.
Comment 57 David Gustafsson 2017-07-04 14:15:50 UTC Comment hidden (obsolete)
Comment 58 David Gustafsson 2017-07-04 15:04:03 UTC
The last change (adding the $options parameter) contained several mistakes, (hopefully) fixed those now. Have run the tests and did some manual testing (which I apparently did not do last time).
Comment 59 Andreas Hedström Mace 2017-08-25 10:30:32 UTC
Wanted to to try and test this again, but patch doesn't apply for me.
Comment 60 David Gustafsson 2018-01-25 17:31:25 UTC
Have discovered several serious issues with the current patch in this issue, so do not recommend that any one uses it or re-roll it against master. My personal view is that the current approach is based on some invalid assumptions (that positions of fields in marc is stable and carry significance for example). Have rewritten most of the core logic, and could perhaps submit it here when cleaned up. But for our needs (Gothenburg University Library) I'm not sure we want to get this in to Koha, but instead refactor and use as a plugin, but because of time-constraints I'm not sure will have time to submit the code for at least a couple of weeks.
Comment 61 Katrin Fischer 2018-01-30 07:20:41 UTC
Hi David, we've always been keen on this one. If you have something to submit, I'd be interested in testing. As we work in a union catalog setting (union catalog is 'master'), Koha's missing ability to protect fields and subfields from being overwritten is sometimes a problem for us.
Comment 62 David Gustafsson 2018-01-30 10:31:53 UTC
Hi! The code can be found here, but there are some parts (help section, tests, user UI) that has not really been adjusted for the new code. Have performed some limited manual testing and seems to work, but it will probably be confusing to configure for someone who has not looked through the code first: https://github.com/ub-digit/Koha/tree/bug_14957

I will try to clean up, make ui more intuitive, and document the functionality, but right now as previously noted there is not enough time for me to do this.
Comment 63 Katrin Fischer 2018-02-22 21:52:39 UTC
Hi David, thx for the feedback. Moving this to Failed QA as it seems more appropriate for now.
Comment 64 David Gustafsson 2018-03-07 10:16:36 UTC Comment hidden (obsolete)
Comment 65 paxed 2018-03-07 10:25:09 UTC
Comment on attachment 72498 [details] [review]
Bug 14957 - Rule based permission for MARC fields

$(this).attr('title', 'Invalid regular expression: ' + e.message);

should be

$(this).attr('title', _("Invalid regular expression: %s").format(e.message));
Comment 66 David Gustafsson 2018-03-07 10:26:36 UTC
Attaching the patch with latest changes even though not yet ready for sign off to not block any further contributions by someone else.

Since the model for how rules work has changed since the "old" version of the patch tests and help pages needs updating to reflect these changes, which has not been done.

I will have time to work some more on this in about a month or so, but since the patch is functional for our library in it's current state it will not get much more love from us until then.
Comment 67 Katrin Fischer 2018-04-01 14:00:55 UTC
Hi David, thx for submitting. Peeking at the code I am not quite sure how it works, but notice it adds a lot of code to C4. Would probably need quite a bit of work.
Comment 68 David Gustafsson 2018-04-25 19:22:47 UTC Comment hidden (obsolete)
Comment 69 David Gustafsson 2018-04-25 19:23:59 UTC
Removed String::Similarity dependency and fixed an issue with the field data hashing function.
Comment 70 David Gustafsson 2018-04-25 19:25:53 UTC
@Katrin Fischer Yes it could use some work, still have not gotten around to updating the help-pages and tests. We are currently running this in production though and has been doing so for a couple of weeks. It could take some time though before have time to fix the above.
Comment 71 David Gustafsson 2018-05-07 12:27:03 UTC Comment hidden (obsolete)
Comment 72 David Gustafsson 2018-05-07 12:30:30 UTC
Rebase against master and remove (probably last of) String::Similarity related code. Have not had time to work on the other issues. We have been running this in production for a couple of weeks now, and have not experienced any issues.
Comment 73 David Gustafsson 2018-05-07 12:33:04 UTC
Which I now saw I already wrote about in previous comment. Still need to fix tests and documentation as the bare minimum before ready for sign-off.
Comment 74 David Gustafsson 2018-06-14 14:11:10 UTC
I'm actively working on fixing the remaining issues and moving relevant things from C4 into the Koha namespace. Will probably submit changes tomorrow or beginning of next week.
Comment 75 David Gustafsson 2018-06-20 12:07:34 UTC Comment hidden (obsolete)
Comment 76 David Gustafsson 2018-06-20 12:10:24 UTC Comment hidden (obsolete)
Comment 77 David Gustafsson 2018-06-20 12:10:30 UTC Comment hidden (obsolete)
Comment 78 David Gustafsson 2018-06-20 12:11:38 UTC Comment hidden (obsolete)
Comment 79 David Gustafsson 2018-06-20 12:11:43 UTC Comment hidden (obsolete)
Comment 80 David Gustafsson 2018-06-20 12:12:31 UTC Comment hidden (obsolete)
Comment 81 David Gustafsson 2018-06-20 12:12:36 UTC Comment hidden (obsolete)
Comment 82 David Gustafsson 2018-06-20 12:15:07 UTC Comment hidden (obsolete)
Comment 83 David Gustafsson 2018-06-20 12:15:16 UTC Comment hidden (obsolete)
Comment 84 David Gustafsson 2018-06-20 12:16:07 UTC
Sorry about the many attachments, had some issues adding the latest patch. Now everything should be in order.
Comment 85 David Gustafsson 2018-06-20 12:23:18 UTC Comment hidden (obsolete)
Comment 86 Katrin Fischer 2018-06-22 05:38:32 UTC
Hi David, thank you for your work on this!
Comment 87 David Gustafsson 2018-06-26 16:05:26 UTC
Thanks. Since this is a rather big change there probably are some minor undiscovered issues with the current patch and we are still running with the prior version in production. The tests are pretty exhaustive though, so I would be surprised if there are any major undiscovered problems. Usability and documentation is probably where there is still most room for improvement and some feedback would be useful to improve on those. But should more or less be ready for sign-off.
Comment 88 Cab Vinton 2018-08-09 13:02:42 UTC
Unable to apply patch in a number of Biblibre sandboxes even after several attempts.
Comment 89 Cab Vinton 2018-08-09 14:33:53 UTC
The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 14957:
<h1>Something went wrong !</h1>Applying: Bug 14957 - Rule based permission for MARC fields
.git/rebase-apply/patch:871: trailing whitespace.
        } 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M       C4/Biblio.pm
M       cataloguing/addbiblio.pl
M       koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
M       koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
M       misc/migration_tools/bulkmarcimport.pl
.git/rebase-apply/patch:871: trailing whitespace.
        } 
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging misc/migration_tools/bulkmarcimport.pl
CONFLICT (content): Merge conflict in misc/migration_tools/bulkmarcimport.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
Auto-merging cataloguing/addbiblio.pl
Auto-merging C4/Biblio.pm
Patch failed at 0001 Bug 14957 - Rule based permission for MARC fields
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
error: Failed to merge in the changes.
Bug 14957 - Write protecting MARC fields based on source of import

76208 - Bug 14957 - Rule based permission for MARC fields
76209 - Bug 14957 - Refactor and fix tests and documentation
76210 - Bug 14957 - Remove exports of removed subroutines

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-14957---Rule-based-permission-for-MARC-fields-eC50px.patch .
Comment 90 David Gustafsson 2018-09-14 13:00:42 UTC Comment hidden (obsolete)
Comment 91 David Gustafsson 2018-09-14 13:00:48 UTC Comment hidden (obsolete)
Comment 92 David Gustafsson 2018-09-14 13:00:53 UTC Comment hidden (obsolete)
Comment 93 David Gustafsson 2018-09-14 13:03:07 UTC
Rebased against master. Patches should now apply. Have not tested that still works though. I will report back once have had the time to test, or if someone else does before me.
Comment 94 Claire Gravely 2018-09-14 18:49:12 UTC
Hi David, sorry the patch doesn't apply :

Applying: Bug 14957 - Rule based permission for MARC fields
Using index info to reconstruct a base tree...
M	help.pl
Falling back to patching base and 3-way merge...
Auto-merging help.pl
CONFLICT (content): Merge conflict in help.pl
Failed to merge in the changes.
Comment 95 Tomás Cohen Arazi 2018-09-19 14:40:13 UTC
I think I might be repeating myself, probably on a different bug. But I think we should add both whitelisting and blacklisting 'MARC record overlay configurations', and then hook them into whatever place we want (i.e. for z39.50, for OAI harvsting, etc). It seems to me that we are duplicating efforts in some places, and solutions don't look generic enough.

'Protecting' is just a way to say you are blacklisting some fields on overlaying.
Comment 96 David Gustafsson 2018-09-25 12:37:49 UTC Comment hidden (obsolete)
Comment 97 David Gustafsson 2018-09-25 12:39:03 UTC Comment hidden (obsolete)
Comment 98 David Gustafsson 2018-09-25 12:39:08 UTC Comment hidden (obsolete)
Comment 99 David Gustafsson 2018-09-25 12:39:15 UTC Comment hidden (obsolete)
Comment 100 David Gustafsson 2018-09-25 12:46:41 UTC
(In reply to Tomás Cohen Arazi from comment #95)
> I think I might be repeating myself, probably on a different bug. But I
> think we should add both whitelisting and blacklisting 'MARC record overlay
> configurations', and then hook them into whatever place we want (i.e. for
> z39.50, for OAI harvsting, etc). It seems to me that we are duplicating
> efforts in some places, and solutions don't look generic enough.
> 
> 'Protecting' is just a way to say you are blacklisting some fields on
> overlaying.

Now rebased again, so should work with latest master. "Protecting" is probably a little bit misleading to reflect what the patch currently does, it has been largely refactored since the first version (which I did not write). I have renamed it to MARC merge rules instead, since it is more of a framework for auto-merging records according to a set or rules. My experience is that is is really tricky to get this right, and if trying to fit it into another abstraction that might not be suited for this exact purpose it could be more trouble than it's worth (https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction).
Comment 101 Tomás Cohen Arazi 2018-09-25 12:56:00 UTC
(In reply to David Gustafsson from comment #100)
> (In reply to Tomás Cohen Arazi from comment #95)
> > I think I might be repeating myself, probably on a different bug. But I
> > think we should add both whitelisting and blacklisting 'MARC record overlay
> > configurations', and then hook them into whatever place we want (i.e. for
> > z39.50, for OAI harvsting, etc). It seems to me that we are duplicating
> > efforts in some places, and solutions don't look generic enough.
> > 
> > 'Protecting' is just a way to say you are blacklisting some fields on
> > overlaying.
> 
> Now rebased again, so should work with latest master. "Protecting" is
> probably a little bit misleading to reflect what the patch currently does,
> it has been largely refactored since the first version (which I did not
> write). I have renamed it to MARC merge rules instead, since it is more of a
> framework for auto-merging records according to a set or rules.

I hadn't noticed that. We should change the bug name to reflect what this actually does.

> My experience is that is is really tricky to get this right, and if trying to
> fit it into another abstraction that might not be suited for this exact
> purpose it could be more trouble than it's worth
> (https://www.sandimetz.com/blog/2016/1/20/the-wrong-abstraction).

While I agree with the assertion, I don't think it is the case. What I'm saying is that we could just add a boolean (whitelist/blacklist) to specify a different behaviour, using the same code and rules. That's something that would belong to a separate bug, of course. I just mentioned here because I thought 'Protect' could be avoided, which you already did :-D
Comment 102 David Gustafsson 2018-10-03 13:52:01 UTC Comment hidden (obsolete)
Comment 103 David Gustafsson 2018-10-03 13:54:02 UTC
Ok :) Fixed an issue where Biblio.pm was missing "use Koha::MarcMergeRules". I missed this since the test works (where file was incluced), but obviously had not tested the code in the staff client.
Comment 104 Cori Lynn Arnold 2018-10-17 13:15:12 UTC
Patch doesn't apply

Bug 14957 - Write protecting MARC fields based on source of import

79368 - Bug 14957 - Rule based permission for MARC fields
79369 - Bug 14957 - Refactor and fix tests and documentation
79370 - Bug 14957 - Remove exports of removed subroutines
79890 - Bug 14957 - Add missing use in Biblio.pm

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14957 - Rule based permission for MARC fields
.git/rebase-apply/patch:871: trailing whitespace.
        }
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M       C4/Biblio.pm
M       cataloguing/addbiblio.pl
M       koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
.git/rebase-apply/patch:871: trailing whitespace.
        }
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
Auto-merging cataloguing/addbiblio.pl
Auto-merging C4/Biblio.pm
CONFLICT (content): Merge conflict in C4/Biblio.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14957 - Rule based permission for MARC fields
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-14957---Rule-based-permission-for-MARC-fields-WiTsgs.patch
Comment 105 David Gustafsson 2018-10-22 11:57:55 UTC Comment hidden (obsolete)
Comment 106 David Gustafsson 2018-10-22 11:58:02 UTC Comment hidden (obsolete)
Comment 107 David Gustafsson 2018-10-22 11:58:08 UTC Comment hidden (obsolete)
Comment 108 David Gustafsson 2018-10-22 11:58:14 UTC Comment hidden (obsolete)
Comment 109 David Gustafsson 2018-10-22 12:01:24 UTC
Rebased against master. I think I was responsible for the conflict this time (bug 19687) :)
Comment 110 David Gustafsson 2018-10-25 15:17:41 UTC Comment hidden (obsolete)
Comment 111 David Gustafsson 2018-10-25 15:18:42 UTC Comment hidden (obsolete)
Comment 112 David Gustafsson 2018-10-25 15:20:46 UTC Comment hidden (obsolete)
Comment 113 David Gustafsson 2018-10-25 15:23:02 UTC
Created attachment 81179 [details] [review]
Bug 14957: Fix control field rules regression and add rule validation
Comment 114 Cori Lynn Arnold 2018-10-30 12:54:42 UTC
Applied fine, but there were runtime errors:

    5. You should see a new link "MARC field permissions" in the "Catalog"
       section under "Koha administrtion". 
	   --> Link Changed to "MARCMergeRules"
    
    7. Next to the page heading ("Manage MARC field permissions") there is a
       button with a question mark, click this r more information about the
       rule system.
	   --> Link labeled "MARC merge rules"
	   --> Page throws error
	   "DBIx::Class::Schema::source(): Can't find source for MarcMergeRule at /home/vagrant/kohaclone/Koha/Objects.pm line 341
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77"
 
   --cori lynn
Comment 115 David Gustafsson 2018-10-30 13:44:36 UTC
(In reply to Cori Lynn Arnold from comment #114)
> Applied fine, but there were runtime errors:
> 
>     5. You should see a new link "MARC field permissions" in the "Catalog"
>        section under "Koha administrtion". 
> 	   --> Link Changed to "MARCMergeRules"
>     
>     7. Next to the page heading ("Manage MARC field permissions") there is a
>        button with a question mark, click this r more information about the
>        rule system.
> 	   --> Link labeled "MARC merge rules"
> 	   --> Page throws error
> 	   "DBIx::Class::Schema::source(): Can't find source for MarcMergeRule at
> /home/vagrant/kohaclone/Koha/Objects.pm line 341
>  at /usr/share/perl5/DBIx/Class/Exception.pm line 77"
>  
>    --cori lynn

Regarding the commit message I'm a little bit confused about how to handle. If I edit the commit message for the first commit it no longer reflect the changes for that commit, on the other hand the "To test" is partly outdated, so perhaps it should have been rewritten in the "Bug 14957 - Refactor and fix tests and documentation"-patch. It seems the local help feature has been removed in Bug  19817, so work is needed to move it into the manual instead? Regarding the "DBIx::Class::Schema::source()" I think you need to run the "misc/devel/update_dbix_class_files.pl" script to regenerate the Schema-files, as those are not to be included in the patch. Perhaps there is a convention to include them an attachment anyway, see if I can find an example of such a bug.
Comment 116 David Gustafsson 2018-10-30 13:50:05 UTC Comment hidden (obsolete)
Comment 117 David Gustafsson 2018-10-30 13:50:49 UTC
Pushed the schema changes and followed the convention in Bug 19482.
Comment 118 David Gustafsson 2018-10-30 13:56:41 UTC
I could also squash all the changes and add a new updated "To test", anyone has some input on this?
Comment 119 Cori Lynn Arnold 2018-10-30 15:15:21 UTC
Thoughts in IRC are that squishing would be fine and to please re-write the tests to make them clearer and easier to follow with correct information.
Comment 120 David Gustafsson 2018-10-30 15:40:42 UTC
Ok, help pages will have to moved to online manual as previously noted.
Comment 121 David Gustafsson 2018-10-31 10:59:53 UTC Comment hidden (obsolete)
Comment 122 David Gustafsson 2018-10-31 11:03:30 UTC Comment hidden (obsolete)
Comment 123 David Gustafsson 2018-10-31 11:17:37 UTC Comment hidden (obsolete)
Comment 124 David Gustafsson 2018-10-31 11:17:42 UTC Comment hidden (obsolete)
Comment 125 David Gustafsson 2018-10-31 11:18:29 UTC Comment hidden (obsolete)
Comment 126 David Gustafsson 2018-10-31 11:18:43 UTC Comment hidden (obsolete)
Comment 127 Cori Lynn Arnold 2018-11-01 19:46:57 UTC

    10. Try creating a rule with tag set to "008" (or other control
        field) and set Appended: Append and Removed: Skip, the other
        options does not matter. Verify that saving this rule produces
        an error message complaining about invalid combination of actions
        for control field.
==Error is generated, but rule still is created. User has to delete the invalid rule. Seems like it shouldn't be created as with the other error conditions. 


    11. With the 245a rule in step 5 in place, edit a bibliographic record,
        change 245a (which should be Title for MARC21) and save.
    12. Verify that the changes has not been saved.
==Changes *are* saved. Attempted to edit a record already in the catalog, and creating a new one via Z39 search. Can you clarify how these rules should be applied for "edit record"?
Comment 128 David Gustafsson 2018-11-05 15:50:40 UTC Comment hidden (obsolete)
Comment 129 David Gustafsson 2018-11-05 15:52:05 UTC Comment hidden (obsolete)
Comment 130 David Gustafsson 2018-11-05 15:52:12 UTC Comment hidden (obsolete)
Comment 131 David Gustafsson 2018-11-05 15:58:05 UTC
(In reply to Cori Lynn Arnold from comment #127)
> 
>     10. Try creating a rule with tag set to "008" (or other control
>         field) and set Appended: Append and Removed: Skip, the other
>         options does not matter. Verify that saving this rule produces
>         an error message complaining about invalid combination of actions
>         for control field.
> ==Error is generated, but rule still is created. User has to delete the
> invalid rule. Seems like it shouldn't be created as with the other error
> conditions. 
> 
> 
>     11. With the 245a rule in step 5 in place, edit a bibliographic record,
>         change 245a (which should be Title for MARC21) and save.
>     12. Verify that the changes has not been saved.
> ==Changes *are* saved. Attempted to edit a record already in the catalog,
> and creating a new one via Z39 search. Can you clarify how these rules
> should be applied for "edit record"?

I accidentally attached another patch, so that comment can be ignored.

The issue where rules are saved also when validation fails should be fixed. The "To test"-instruction regarding 245a was wrong, as only tag expressions are allowed (not subfields). Since even I forgot about that perhaps that should be clarified, or [a-z] not allowed in field patterns.
Comment 132 David Gustafsson 2018-11-07 14:35:53 UTC Comment hidden (obsolete)
Comment 133 David Gustafsson 2018-11-07 14:36:02 UTC Comment hidden (obsolete)
Comment 134 David Gustafsson 2018-11-07 14:36:47 UTC
Fixed too long foreign key length causing troubles with MariaDB.
Comment 135 David Gustafsson 2018-11-30 10:35:32 UTC Comment hidden (obsolete)
Comment 136 David Gustafsson 2018-11-30 10:36:58 UTC
Rebased against master and added granular permission in accordance with Bug 14391.
Comment 137 Katrin Fischer 2019-02-17 12:05:35 UTC
*** Bug 22354 has been marked as a duplicate of this bug. ***
Comment 138 Frank Hansen 2019-03-12 13:24:42 UTC
The patch is tested and works fine. Nice work!
Comment 139 Katrin Fischer 2019-03-12 13:26:29 UTC
Hi Frank! Can you please add a sign-off line to the patch too? This can be done for example by using the sandboxes: https://wiki.koha-community.org/wiki/Sandboxes

If you can't that's ok, we can also fix it in the QA step.
Comment 140 Frank Hansen 2019-03-12 14:20:16 UTC
Katrin, I have already signed off the patches from a sandbox. I'm getting:
koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: needs merge

Guess, I don't have a change to do that. 

The sandbox is here: http://sb1.sandboxes.bywatersolutions.com if somebody want to try.
Comment 141 Katrin Fischer 2019-03-12 14:34:35 UTC
(In reply to Frank Hansen from comment #140)
> Katrin, I have already signed off the patches from a sandbox. I'm getting:
> koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc: needs merge
> 
> Guess, I don't have a change to do that. 
> 
> The sandbox is here: http://sb1.sandboxes.bywatersolutions.com if somebody
> want to try.

Hi Frank, looks like we might need a rebase - master is changing fast and sometimes there are conflicts in files changed by different patches. We will fix it.
Comment 142 David Gustafsson 2019-04-10 13:11:33 UTC Comment hidden (obsolete)
Comment 143 David Gustafsson 2019-04-10 13:14:12 UTC Comment hidden (obsolete)
Comment 144 David Gustafsson 2019-04-10 13:14:32 UTC
Rebased against master.
Comment 145 Katrin Fischer 2019-04-10 23:58:34 UTC
Taking a first view here, starting with the QA script:


1) QA script failures:
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-merge-rules.tt
   FAIL	  filters
		missing_filter at line 376 (                    <input type="checkbox" name="batchremove" value="[% rule.id %]"[% IF rule.removemarked %] checked="checked"[% END %]/>)
		missing_filter at line 364 (                <td>[% rule.filter %]</td>)
		missing_filter at line 372 (                    <a href="?op=remove&id=[% rule.id %]" title="Delete" class="btn btn-mini"><i class="fa fa-trash"></i> Delete</a>)
		missing_filter at line 317 (                                <option value="[% module.id %]">[% module.name %]</option>)
		missing_filter at line 373 (                    <a href="?op=edit&id=[% rule.id %]" title="Edit" class="btn btn-mini"><i class="fa fa-pencil"></i> Edit</a>)
		missing_filter at line 363 (                <td>[% rule.module %]</td>)
		missing_filter at line 308 (            <tr id="[% rule.id %]" class="rule[% IF rule.edit %]-edit[% END %]">)
		missing_filter at line 315 (                                <option value="[% module.id %]" selected="selected">[% module.name %]</option>)
		missing_filter at line 234 (<form action="[% script_name %]" method="GET">)
		missing_filter at line 328 (                            <option value="[% preset %]">[% preset %]</option>)
		missing_filter at line 357 (                    <button class="btn btn-mini" title="Save" id="doedit" value="[% rule.id %]"><i class="fa fa-check"></i> Save</button>)
		missing_filter at line 323 (                <td><input type="text" size="3" name="tag" value="[% rule.tag %]"/></td>)
		wrong_html_filter at line 373 (                    <a href="?op=edit&id=[% rule.id %]" title="Edit" class="btn btn-mini"><i class="fa fa-pencil"></i> Edit</a>)
		missing_filter at line 5 (<link rel="stylesheet" type="text/css" href="[% interface %]/[% theme %]/css/datatables.css" />)
		missing_filter at line 310 (                <td>[% rule.id %]</td>)
		missing_filter at line 385 (<form action="[% script_name %]" method="post">)
		missing_filter at line 365 (                <td>[% rule.tag %]</td>)
		missing_filter at line 241 (<form action="[% script_name %]" method="POST" id="marc-merge-rules-form">)
		wrong_html_filter at line 372 (                    <a href="?op=remove&id=[% rule.id %]" title="Delete" class="btn btn-mini"><i class="fa fa-trash"></i> Delete</a>)
		missing_filter at line 362 (                <td>[% rule.id %]</td>)
		missing_filter at line 263 (                        <option value="[% module.id %]">[% module.name %]</option>)
		missing_filter at line 273 (                        <option value="[% preset %]">[% preset %]</option>)
		missing_filter at line 322 (                <td><input type="text" size="5" name="filter" value="[% rule.filter %]"/></td>)
   FAIL	  forbidden patterns
		forbidden pattern: Do *not* include full path to css files, use the Asset TT plugin (bug 20538) (line 5)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 372)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 373)
		forbidden pattern: Attribute type should not be used for script tags (bug 20053) (line 14)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 357)
		forbidden pattern: Attribute type should not be used for style tags (bug 20054) (line 8)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 302)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 358)
		forbidden pattern: Contains old bootstrap style for buttons (see bug 16239) (line 301)
   OK	  git manipulation
   FAIL	  js_in_body
		A <script> tag found inside head, must be moved to the body (see bug 17858)
   OK	  spelling
   FAIL	  tt_valid
		lines 334, 335, 340, 341, 346, 347, 352, 353, 376
   OK	  valid_template

 FAIL	Koha/MarcMergeRule.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD is missing for 'store'
   OK	  spelling
   OK	  valid

 FAIL	Koha/MarcMergeRules.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD is missing for delete
		POD is missing for find_or_create
		POD is missing for object_class
		POD is missing for operations
		POD is missing for update
		POD is missing for validate
   OK	  spelling
   OK	  valid

 FAIL	Koha/MarcMergeRule.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD is missing for 'store'
   OK	  spelling
   OK	  valid

 FAIL	Koha/MarcMergeRules.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD is missing for delete
		POD is missing for find_or_create
		POD is missing for object_class
		POD is missing for operations
		POD is missing for update
		POD is missing for validate
   OK	  spelling
   OK	  valid


2) Please merge your 3 atomic update SQL files into one .perl file. That will make the RM happy.

3) There is no sign-off line on the patch?
Comment 146 Katrin Fischer 2019-04-11 00:08:15 UTC
Comment on attachment 87711 [details] [review]
Bug 14957: Merge rules system for merging of MARC records

Review of attachment 87711 [details] [review]:
-----------------------------------------------------------------

::: C4/Biblio.pm
@@ +262,3 @@
>  =cut
>  
>  sub ModBiblio {

4) Changes to ModBiblio should be covered by unit tests.

@@ +3615,4 @@
>      return $record;
>  }
>  
> +=head2 ApplyMarcMergeRules

5) New method needs to be covered by unit tests.

::: admin/marc-merge-rules.pl
@@ +15,5 @@
> +# You should have received a copy of the GNU General Public License
> +# along with Koha; if not, see <http://www.gnu.org/licenses>.
> +
> +use strict;
> +use warnings;

6) Should be: Modern::Perl

7) Changes to kohastructure.sql are missing.
Comment 147 Katrin Fischer 2019-04-11 00:15:17 UTC
8) Changes to syspref.sql are missing

9) There are some default values set in the database updat that can't go into kohastructure.sql, might have to go into a new SQL file in the installers.

10) Looking at the setup table I'll also need documentation, pretty please! Right now, I have no clue on how to use this.

Good stuff too: Unit tests pass, new permission is good too.
Comment 148 David Gustafsson 2019-04-11 14:27:29 UTC Comment hidden (obsolete)
Comment 149 David Gustafsson 2019-04-12 17:49:01 UTC Comment hidden (obsolete)
Comment 150 David Gustafsson 2019-04-17 15:37:58 UTC Comment hidden (obsolete)
Comment 151 David Gustafsson 2019-04-17 15:38:03 UTC Comment hidden (obsolete)
Comment 152 David Cook 2019-05-14 05:33:47 UTC
(In reply to David Gustafsson from comment #151)
> https://bugs.koha-community.org/show_bug.cgi?id=14956

I think that URL should probably be https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14957, although I'm not sure why it's there either?

In any case, intrigued to see how these patches progress.
Comment 153 David Gustafsson 2019-05-22 14:43:19 UTC Comment hidden (obsolete)
Comment 154 David Gustafsson 2019-05-22 14:45:17 UTC
Added some documentation here: https://gitlab.com/gnucifer/koha-manual/commits/bug-14957-marc-merge-rules

I could create a pull request, but perhaps better to await some feedback before doing that.
Comment 155 Katrin Fischer 2019-06-02 08:16:50 UTC
Hi David,

1) Documentation

https://gitlab.com/gnucifer/koha-manual/commit/593c5da008dbc4b074cac9dee167cce9a6dfc250#c5e2c75dc923f21c5b95185b2f350df00fec661f

Thanks a lot, this documentation is SO helpful. We really couldn't have continued without as the feature is quite complex.

2) GUI - Terminology and translatability

One thing that bothers me is that the GUI is a big 'rough'. I'd much prefer if it was using better terms/descriptions like in the help instead of 'codes'. 

bachmod = Bach record modification
intranet = Cataloging module (unles it inlcudes batchmod?)
batchimport = Staged MARC import
etc.
batchmarcimport = Cronjob batchmarcimport.pl

(I hope I got them right)

Experience tells that those are likely to get translated strangely or might not be translatable at all. 

3) Over-granularity?

Another thing that bothers me is a bit is the granularity. Being able to limit to specific patrons seems a little overkill? Especially since the interface expects borrowernumbers to be entered? (borrower = patron)

Is there a use case for this?

4) marc_merge_rules_modules

I am still quite suspicious of this one. We should not add descriptions to the database ever (translatability) - and this currently won't work for new installations. Can you please consider moving the modules to the templates instead and checking if this table is really needed as is? Or explain to me?

At the moment the patch set doesn't apply. - I am really interested in the feature, just feel right now that it's an old development that still needs a bit of love.
Comment 156 David Gustafsson 2019-06-12 13:43:30 UTC
Thanks for the feedback. I more or less agree with all of pointed out issues and not that it matters but those design choices, including the somewhat cryptic naming of modules, were not made by me but comes from the original version of the patch. I have not addressed them since just wanted to spend the minimal effort for the patch to get into Koha and have focused on the issues brought up thus far. But at least 4) has been brought up before and I should perhaps have fixed that in the current iteration of the patch since the modules are added in an atomic update and I saw no other instance where data is inserted in kohastructure.sql. Thus if making a fresh Koha installation this data will be missing which is not acceptable. Since the modules are not invoked dynamically in the code, but are hard coded, it makes more sense for them to be defined as constants, which would resolve the database population issue. I think this should be pretty uncomplicated to refactor, by hard coding the modules in the template (to make the label translatable) making that the authoritative source. In my opinion there is a rather urgent need for having translatable strings in Koha without (ab)using the template system, but that is out of the scope of this bug.

About 4) I agree that is to use borrowernumber is not pretty, I guess the reason it is used instead of userid is it is available in scope without having to look up the userid in a database call, but userid (username) would be better. I also agree that it is probably a very rare use-case to need to specify rules per user, but on the other hand, since the cost of providing this context is relatively low, it might also be a little bit presumptuous to remove the option. For example, it is a simple way to allow certain super-users to override all rules by creating a wildcard "allow all" rule without having to reserve a user category for this purpose.
Comment 157 Katrin Fischer 2019-06-12 13:50:32 UTC
3) was more an observation at this point. If you could rebase and take care of 4) I'd love to continue testing. 

This is a bit "too big" overall... and shoudl have been split into smaller bits and pieces...(too late now, I know, but we've seen this with a lot of things) But I hope we can make it move.
Comment 158 David Gustafsson 2019-06-17 14:16:43 UTC Comment hidden (obsolete)
Comment 159 David Gustafsson 2019-06-17 14:17:18 UTC Comment hidden (obsolete)
Comment 160 David Gustafsson 2019-06-17 14:27:34 UTC Comment hidden (obsolete)
Comment 161 David Gustafsson 2019-06-17 14:27:42 UTC Comment hidden (obsolete)
Comment 162 David Gustafsson 2019-06-17 14:33:42 UTC
Fixed most of the issues, rebased and squashed into one commit as things were getting a little bit messy. There are still some things that might need fixing for example I'm not sure if the marc rule presets are translatable as they are defined as js-data. I haven't used the translation tools myself but will try to figure this out.
Comment 163 David Gustafsson 2019-06-17 14:36:45 UTC
Also did a pod change in C4/Biblio.pm without the output so need also to do that.
Comment 164 David Gustafsson 2019-06-18 11:04:54 UTC Comment hidden (obsolete)
Comment 165 David Gustafsson 2019-06-18 12:39:32 UTC Comment hidden (obsolete)
Comment 166 David Gustafsson 2019-06-18 12:43:27 UTC
I managed to find documentation on how to make javascript strings translatable here: https://wiki.koha-community.org/wiki/Coding_Guidelines#JS2:_Enabling_translation_of_language_strings_in_embedded_JavaScript

I'm a little bit unsure if the inline template toolkit strings are translatable though and don't know how to test if they are.

For example:

   [% FOR preset IN [
       'Protect',
       'Overwrite',
       'Add new',
       'Add and append',
       'Protect from deletion'
     ]
  %]
Comment 167 David Gustafsson 2019-06-18 13:00:46 UTC Comment hidden (obsolete)
Comment 168 David Gustafsson 2019-06-18 13:17:34 UTC Comment hidden (obsolete)
Comment 169 David Gustafsson 2019-06-18 14:05:39 UTC
Fixed documentation (borrower changed to userid).

https://gitlab.com/gnucifer/koha-manual/tree/bug-14957-marc-merge-rules
Comment 170 David Gustafsson 2019-09-18 12:49:18 UTC Comment hidden (obsolete)
Comment 171 David Gustafsson 2019-09-18 12:49:23 UTC Comment hidden (obsolete)
Comment 172 David Gustafsson 2019-09-18 12:49:28 UTC Comment hidden (obsolete)
Comment 173 David Gustafsson 2019-09-18 12:49:34 UTC Comment hidden (obsolete)
Comment 174 David Gustafsson 2019-09-18 12:49:39 UTC Comment hidden (obsolete)
Comment 175 David Gustafsson 2019-09-18 12:49:45 UTC Comment hidden (obsolete)
Comment 176 David Gustafsson 2019-09-18 12:52:28 UTC
Rebased against current master and fixed some issues.
Comment 177 David Gustafsson 2019-09-20 14:14:38 UTC Comment hidden (obsolete)
Comment 178 Katrin Fischer 2019-09-22 09:30:47 UTC
Thx for the rebase David, I have a little hope to tackle this at the Marseille Hackfest.
Comment 179 Séverine Queune 2019-10-02 15:31:35 UTC
On step 6 : when I edit the rule, the "Preset" field comes back to the first value "Custom" : I think it's a disturbing and the value registred should be kept.

On step 7 : there are gaps into the buttons, like if icons where missing, and they don't seem to have the same css (can send a screenshot if needed). Behaviours of both buttons are correct.

On step 13 : once the rule is saved, the filter displays "intranet" but when creating the rule, it's "Staff client MARC editor". Maybe it should be uniformised ?

Tested using a sandbox so can't run tests on step 15.

Hope this feedback will be helpfull !
Except these points, the feature works fine and will be very useful !
Comment 180 Séverine Queune 2019-10-02 15:35:31 UTC Comment hidden (obsolete)
Comment 181 Katrin Fischer 2019-10-28 06:58:10 UTC
I think it's too late now for 19.11 (feature slush this Wednesday), but was so happy to see progress here and would love to see it early on in 20.05. 
David, can you provide follow-ups for the problems Severine noted? I think we are close to a sign-off.
Comment 182 David Gustafsson 2019-10-29 15:53:55 UTC
Sorry about the late reply, I'm looking into it.
Comment 183 David Gustafsson 2019-10-31 12:20:18 UTC Comment hidden (obsolete)
Comment 184 David Gustafsson 2019-10-31 12:42:28 UTC Comment hidden (obsolete)
Comment 185 David Gustafsson 2019-10-31 12:43:17 UTC
@Séverine Queune I think most points should now be addressed. Thank you for your feedback!
Comment 186 Katrin Fischer 2020-01-05 12:07:13 UTC
*** Bug 11780 has been marked as a duplicate of this bug. ***
Comment 187 Katrin Fischer 2020-01-05 12:07:56 UTC
*** Bug 11519 has been marked as a duplicate of this bug. ***
Comment 188 Katrin Fischer 2020-01-06 12:55:41 UTC
*** Bug 13213 has been marked as a duplicate of this bug. ***
Comment 189 Katrin Fischer 2020-01-11 23:24:55 UTC
*** Bug 18210 has been marked as a duplicate of this bug. ***
Comment 190 David Nind 2020-02-17 09:48:47 UTC
Had a go at testing but get an error when adding a rule (step 5 of the test plan):

DBIx::Class::Row::delete(): Not in database at /kohadevbox/koha/Koha/Object.pm line 219
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
in DBIx::Class::Exception::throw at /usr/share/perl5/DBIx/Class/Exception.pm line 77
   74:     my $self = { msg => $msg };
   75:     bless $self => $class;
   76: 
   77:     die $self;
   78: }
   79: 
   80: =head2 rethrow
Show function arguments

Can post the full error trace if required.

Notes: The patch applied fine, I ran the database update, and enabled the new system preference.
Comment 191 David Gustafsson 2020-03-02 16:09:27 UTC
Ok, thanks! A regression seems to been introduced in https://github.com/Koha-Community/Koha/commit/abb51797d8d5ebb274856e912e207a921ace478b causing this bug. Very difficult to debug since "delete" is somehow called without appearing in the code, and a stack trace points out a line (where delete is called) that does not exist. But I'm looking into it.
Comment 192 David Gustafsson 2020-03-03 13:15:05 UTC Comment hidden (obsolete)
Comment 193 David Gustafsson 2020-03-03 13:40:28 UTC
I now found the source of the bug and created a new issue: Bug 24788. This problem is currently blocking this patch.
Comment 194 David Nind 2020-03-03 19:37:04 UTC Comment hidden (obsolete)
Comment 195 David Nind 2020-03-03 19:37:11 UTC Comment hidden (obsolete)
Comment 196 David Nind 2020-03-03 19:37:18 UTC Comment hidden (obsolete)
Comment 197 David Nind 2020-03-03 19:37:24 UTC Comment hidden (obsolete)
Comment 198 David Nind 2020-03-03 19:37:30 UTC Comment hidden (obsolete)
Comment 199 David Nind 2020-03-03 19:37:37 UTC Comment hidden (obsolete)
Comment 200 David Nind 2020-03-03 19:37:43 UTC Comment hidden (obsolete)
Comment 201 David Nind 2020-03-03 19:37:49 UTC Comment hidden (obsolete)
Comment 202 David Nind 2020-03-03 19:37:56 UTC Comment hidden (obsolete)
Comment 203 David Nind 2020-03-03 19:38:02 UTC Comment hidden (obsolete)
Comment 204 David Nind 2020-03-03 19:51:27 UTC
(In reply to David Gustafsson from comment #193)
> I now found the source of the bug and created a new issue: Bug 24788. This
> problem is currently blocking this patch.

Have signed off - everything seems to work as per the test plan the tests!

One note for test plan: need to update the database (updatedatabase on koha-testing-docker)
Comment 205 Katrin Fischer 2020-03-24 09:05:52 UTC Comment hidden (obsolete)
Comment 206 Katrin Fischer 2020-03-24 09:10:15 UTC Comment hidden (obsolete)
Comment 207 Katrin Fischer 2020-06-21 16:15:55 UTC
At the moment this is blocked by bug 25539 and also needs a little rebase!
Comment 208 Katrin Fischer 2020-09-03 20:11:36 UTC
*** Bug 26372 has been marked as a duplicate of this bug. ***
Comment 209 Martin Renvoize 2020-09-09 11:05:02 UTC
I've unblocked this by working through and signing off the dependency bug.

Setting to Patch doesn't apply to get attention for the rebase and hopefully we can get this one moving forward again.
Comment 210 David Gustafsson 2020-09-11 15:32:32 UTC Comment hidden (obsolete)
Comment 211 David Gustafsson 2020-09-11 15:32:45 UTC Comment hidden (obsolete)
Comment 212 David Gustafsson 2020-09-11 15:32:57 UTC Comment hidden (obsolete)
Comment 213 David Gustafsson 2020-09-11 15:33:06 UTC Comment hidden (obsolete)
Comment 214 David Gustafsson 2020-09-11 15:33:13 UTC Comment hidden (obsolete)
Comment 215 David Gustafsson 2020-09-11 15:33:22 UTC Comment hidden (obsolete)
Comment 216 David Gustafsson 2020-09-11 15:33:30 UTC Comment hidden (obsolete)
Comment 217 David Gustafsson 2020-09-11 15:33:39 UTC Comment hidden (obsolete)
Comment 218 David Gustafsson 2020-09-11 15:33:47 UTC Comment hidden (obsolete)
Comment 219 David Gustafsson 2020-09-11 15:36:53 UTC
A small conflict in installer/data/mysql/sysprefs.sql, should now be rebased against master.
Comment 220 David Nind 2020-09-18 23:30:57 UTC
Patch(es) no longer apply on latest master 8-( :

root@kohadevbox:koha(bz14957)$ git bz apply 14957
Bug 14957 - Write protecting MARC fields based on source of import

100052 - Bug 14957: DO NOT PUSH Schema changes
109955 - Bug 14957: Merge rules system for merging of MARC records
109956 - Bug 14957: Make preset labels translatable
109957 - Bug 14957: Adjust paging option for new version of datatables
109958 - Bug 14957: Fix rule javascript validation issue
109959 - Bug 14957: Change YUI layout to to Boostrap
109960 - Bug 14957: Remove prefetch of removed relationship
109961 - Bug 14957: Add icons for confirmation dialog and set preset based on actions configuration
109962 - Bug 14957: Replace filter value with label if one exists
109963 - Bug 14957: Remove usage of deleted Koha::MarcMergeRulesModules

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 14957: DO NOT PUSH Schema changes
Applying: Bug 14957: Merge rules system for merging of MARC records
Using index info to reconstruct a base tree...
M	cataloguing/addbiblio.pl
M	installer/data/mysql/kohastructure.sql
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
M	koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
M	misc/migration_tools/bulkmarcimport.pl
M	tools/batch_record_modification.pl
Falling back to patching base and 3-way merge...
Auto-merging tools/batch_record_modification.pl
Auto-merging misc/migration_tools/bulkmarcimport.pl
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/cataloguing.pref
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging cataloguing/addbiblio.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 14957: Merge rules system for merging of MARC records
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-14957-Merge-rules-system-for-merging-of-MARC-r-KugWDA.patch
Comment 221 Katrin Fischer 2020-12-13 13:42:44 UTC
I've added a suggestion for this on the Koha 21.05 Road Map - please sign up if you are interested to finally get this one in :)
https://annuel.framapad.org/p/koha_21.05_roadmap
Comment 222 Aleisha Amohia 2021-02-10 03:50:08 UTC Comment hidden (obsolete)
Comment 223 Aleisha Amohia 2021-02-10 03:50:15 UTC Comment hidden (obsolete)
Comment 224 Aleisha Amohia 2021-02-10 03:50:59 UTC Comment hidden (obsolete)
Comment 225 Aleisha Amohia 2021-02-10 03:51:07 UTC Comment hidden (obsolete)
Comment 226 Andrew Fuerste-Henry 2021-02-10 13:42:20 UTC
I made this rule:
Module: source
Filter: *
Tag: 500
Preset: Protect

When I go to edit a record, there's nothing in the edit interface to suggest my edits won't be saved. I can type in a new 500, click to delete an existing 500, etc. When I save my edits, there's nothing to indicate that my changes were discarded or why. The edits simply aren't saved. 

This lack of feedback seems really confusing.

Meanwhile, if I take a record that has a 500 and put it through batch record modification to remove the 500$a my rule does not prevent that edit as intended.
Comment 227 Martin Renvoize 2021-03-02 16:41:45 UTC
What's the current status of this one..?
Comment 228 Christian Stelzenmüller 2021-03-05 07:32:34 UTC
I got an error at testing on step 4: Click the new link "MARC merge rules" in the "Catalog" section of the Koha administration page.

Error trace:

Could not compile /kohadevbox/koha/admin/marc-merge-rules.pl: Global symbol "$bib_items_added" requires explicit package name (did you forget to declare "my $bib_items_added"?) at /kohadevbox/koha/C4/ImportBatch.pm line 676.
Global symbol "$bib_items_replaced" requires explicit package name (did you forget to declare "my $bib_items_replaced"?) at /kohadevbox/koha/C4/ImportBatch.pm line 676.
Global symbol "$bib_items_errored" requires explicit package name (did you forget to declare "my $bib_items_errored"?) at /kohadevbox/koha/C4/ImportBatch.pm line 676.
Global symbol "$bib_items_added" requires explicit package name (did you forget to declare "my $bib_items_added"?) at /kohadevbox/koha/C4/ImportBatch.pm line 677.
Global symbol "$bib_items_replaced" requires explicit package name (did you forget to declare "my $bib_items_replaced"?) at /kohadevbox/koha/C4/ImportBatch.pm line 678.
Global symbol "$bib_items_errored" requires explicit package name (did you forget to declare "my $bib_items_errored"?) at /kohadevbox/koha/C4/ImportBatch.pm line 679.
syntax error at /kohadevbox/koha/C4/ImportBatch.pm line 684, near "HEAD
                ModAuthority"
Compilation failed in require at /kohadevbox/koha/admin/marc-merge-rules.pl line 33.
BEGIN failed--compilation aborted at /kohadevbox/koha/admin/marc-merge-rules.pl line 33.
 at /usr/share/perl5/CGI/Compile.pm line 132
Comment 229 David Gustafsson 2021-03-05 12:34:36 UTC
Thats probably because of a conflict in that file. I will see if can rebase and fix this.
Comment 230 David Gustafsson 2021-03-05 12:47:32 UTC Comment hidden (obsolete)
Comment 231 David Gustafsson 2021-03-05 12:47:53 UTC Comment hidden (obsolete)
Comment 232 PTFS Europe Sandboxes 2021-03-05 13:27:21 UTC Comment hidden (obsolete)
Comment 233 PTFS Europe Sandboxes 2021-03-05 13:27:27 UTC Comment hidden (obsolete)
Comment 234 David Gustafsson 2021-03-05 13:39:18 UTC Comment hidden (obsolete)
Comment 235 David Gustafsson 2021-03-05 14:00:07 UTC
(In reply to Andrew Fuerste-Henry from comment #226)
> I made this rule:
> Module: source
> Filter: *
> Tag: 500
> Preset: Protect
> 
> When I go to edit a record, there's nothing in the edit interface to suggest
> my edits won't be saved. I can type in a new 500, click to delete an
> existing 500, etc. When I save my edits, there's nothing to indicate that my
> changes were discarded or why. The edits simply aren't saved. 
> 
> This lack of feedback seems really confusing.
> 
> Meanwhile, if I take a record that has a 500 and put it through batch record
> modification to remove the 500$a my rule does not prevent that edit as
> intended.

Had a look at this and ModBiblio context argument for batch-modification was incorrect, an error introduced in one or more of the rebases of this patch. Fixed this and was going to test, but looks like ElasticSearch is broken in latest master, but will try to get around that.

Regarding the lack of user notification I would regard that as a feature request rather than a bug. It's not obvious without being acquainted with the code, but it would actually be quite a lot of work to implement, and since this is not a feature that is prioritize by our library I don't have the resources to do this.
Comment 236 David Gustafsson 2021-03-05 14:43:15 UTC Comment hidden (obsolete)
Comment 237 David Gustafsson 2021-03-05 14:45:29 UTC
Have now verified that batchmod rules should works again.
Comment 238 Martin Renvoize 2021-03-09 08:57:29 UTC
(In reply to Andrew Fuerste-Henry from comment #226)
> 
> This lack of feedback seems really confusing.
> 

I agree feedback is important.. however perhaps we can do that in a followup bug to keep this one moving.
Comment 239 Martin Renvoize 2021-03-09 12:40:53 UTC Comment hidden (obsolete)
Comment 240 Martin Renvoize 2021-03-09 12:41:00 UTC Comment hidden (obsolete)
Comment 241 Martin Renvoize 2021-03-09 12:41:06 UTC Comment hidden (obsolete)
Comment 242 Tomás Cohen Arazi 2021-03-26 16:25:38 UTC
Martin and I are working heavily on this one for QA. Marking as assigned just in case, to avoid overlapping with others.
Comment 243 Tomás Cohen Arazi 2021-03-28 13:39:07 UTC Comment hidden (obsolete)
Comment 244 Tomás Cohen Arazi 2021-03-28 13:39:31 UTC Comment hidden (obsolete)
Comment 245 Tomás Cohen Arazi 2021-03-28 13:39:51 UTC Comment hidden (obsolete)
Comment 246 Tomás Cohen Arazi 2021-03-28 13:40:09 UTC Comment hidden (obsolete)
Comment 247 Tomás Cohen Arazi 2021-03-28 13:40:21 UTC Comment hidden (obsolete)
Comment 248 Tomás Cohen Arazi 2021-03-28 13:40:31 UTC Comment hidden (obsolete)
Comment 249 Tomás Cohen Arazi 2021-03-28 13:40:43 UTC Comment hidden (obsolete)
Comment 250 Tomás Cohen Arazi 2021-03-28 13:41:13 UTC Comment hidden (obsolete)
Comment 251 Tomás Cohen Arazi 2021-03-28 13:41:30 UTC Comment hidden (obsolete)
Comment 252 Tomás Cohen Arazi 2021-03-28 13:41:43 UTC Comment hidden (obsolete)
Comment 253 Tomás Cohen Arazi 2021-03-28 13:50:32 UTC
This patchset has been hanging for too long, and my follow-up patches are my attempt to make it pushable while making it adaptable for future refactorings without major changes.

Missing for my PQA stamp:
- Translatability in the marc-overlay-rules.tt
Comment 254 Tomás Cohen Arazi 2021-03-28 13:51:49 UTC
Ok, I submitted what we have so far so others can add here.

The patchset introduces several 'features' in my opinion, and we could do better with the DB and classes.

In my opinion we would need:

1. A table for rules (done-ish)
2. A table for defining named rule sets
3. A table for assigning 'globally enforced' rule sets to specific places, categories, etc (this is what the patchset does, only, in my opinion).

It is a pity we cannot choose rulesets explicitly when importing, but I know we can do it later.
Comment 255 Martin Renvoize 2021-03-29 09:28:15 UTC Comment hidden (obsolete)
Comment 256 Martin Renvoize 2021-03-29 09:28:22 UTC Comment hidden (obsolete)
Comment 257 Martin Renvoize 2021-03-29 09:28:29 UTC Comment hidden (obsolete)
Comment 258 Martin Renvoize 2021-03-29 09:28:35 UTC Comment hidden (obsolete)
Comment 259 Martin Renvoize 2021-03-29 09:28:42 UTC Comment hidden (obsolete)
Comment 260 Martin Renvoize 2021-03-29 09:28:50 UTC Comment hidden (obsolete)
Comment 261 Martin Renvoize 2021-03-29 09:28:57 UTC Comment hidden (obsolete)
Comment 262 Martin Renvoize 2021-03-29 09:29:04 UTC Comment hidden (obsolete)
Comment 263 Martin Renvoize 2021-03-29 09:29:11 UTC Comment hidden (obsolete)
Comment 264 Martin Renvoize 2021-03-29 09:29:18 UTC Comment hidden (obsolete)
Comment 265 Martin Renvoize 2021-03-29 09:29:25 UTC Comment hidden (obsolete)
Comment 266 Martin Renvoize 2021-03-29 09:29:32 UTC Comment hidden (obsolete)
Comment 267 Martin Renvoize 2021-03-29 09:33:47 UTC Comment hidden (obsolete)
Comment 268 Martin Renvoize 2021-03-29 12:33:49 UTC Comment hidden (obsolete)
Comment 269 Jonathan Druart 2021-04-08 13:24:49 UTC
The QA script raises several errors, please double-check.
Comment 270 Katrin Fischer 2021-04-14 11:43:22 UTC
If yo can, please have a look here - we are getting close to the critical timeframe before release and it would be so great to have this included.
Comment 271 Martin Renvoize 2021-05-05 15:35:39 UTC Comment hidden (obsolete)
Comment 272 Martin Renvoize 2021-05-05 15:35:47 UTC Comment hidden (obsolete)
Comment 273 Martin Renvoize 2021-05-05 15:35:55 UTC Comment hidden (obsolete)
Comment 274 Martin Renvoize 2021-05-05 15:36:03 UTC Comment hidden (obsolete)
Comment 275 Martin Renvoize 2021-05-05 15:36:12 UTC Comment hidden (obsolete)
Comment 276 Martin Renvoize 2021-05-05 15:36:20 UTC Comment hidden (obsolete)
Comment 277 Martin Renvoize 2021-05-05 15:36:28 UTC Comment hidden (obsolete)
Comment 278 Martin Renvoize 2021-05-05 15:36:36 UTC Comment hidden (obsolete)
Comment 279 Martin Renvoize 2021-05-05 15:36:45 UTC Comment hidden (obsolete)
Comment 280 Martin Renvoize 2021-05-05 15:36:54 UTC Comment hidden (obsolete)
Comment 281 Martin Renvoize 2021-05-05 15:37:01 UTC Comment hidden (obsolete)
Comment 282 Martin Renvoize 2021-05-05 15:37:10 UTC Comment hidden (obsolete)
Comment 283 Martin Renvoize 2021-05-05 15:37:17 UTC Comment hidden (obsolete)
Comment 284 Martin Renvoize 2021-05-05 15:37:25 UTC Comment hidden (obsolete)
Comment 285 Martin Renvoize 2021-05-05 15:37:33 UTC Comment hidden (obsolete)
Comment 286 Martin Renvoize 2021-05-05 15:37:41 UTC Comment hidden (obsolete)
Comment 287 Martin Renvoize 2021-05-05 15:37:50 UTC Comment hidden (obsolete)
Comment 288 Martin Renvoize 2021-05-05 15:37:58 UTC Comment hidden (obsolete)
Comment 289 Martin Renvoize 2021-05-05 15:38:07 UTC Comment hidden (obsolete)
Comment 290 Martin Renvoize 2021-05-05 15:40:56 UTC
I fixed the QA issues with follow-ups.. the only remaining ones flagged up are git manipulation errors (and some executable bit errors that are actually resolved by the manipulations anyway)..  I did not squash commits as I didn't want to lose author attribution.. there's been a few people working on this one over the duration of its development life.

I feel this is ready for push myself.. yes it has a few sharp edges and I'd love to see some of the templates indented more nicely for example.. if you agree I could do that indent fix at the least as a follow-up here.

Hoping this could be one of the last ones for this release.
Comment 291 Martin Renvoize 2021-05-05 15:44:14 UTC Comment hidden (obsolete)
Comment 292 Martin Renvoize 2021-05-05 15:49:48 UTC Comment hidden (obsolete)
Comment 293 Martin Renvoize 2021-05-05 15:55:10 UTC Comment hidden (obsolete)
Comment 294 Katrin Fischer 2021-05-05 21:00:25 UTC
Seeing the last status change - what happened?
Comment 295 Martin Renvoize 2021-05-06 07:01:10 UTC Comment hidden (obsolete)
Comment 296 Martin Renvoize 2021-05-06 07:27:34 UTC Comment hidden (obsolete)
Comment 297 Martin Renvoize 2021-05-06 07:27:42 UTC Comment hidden (obsolete)
Comment 298 Martin Renvoize 2021-05-06 07:27:50 UTC Comment hidden (obsolete)
Comment 299 Martin Renvoize 2021-05-06 07:27:58 UTC Comment hidden (obsolete)
Comment 300 Martin Renvoize 2021-05-06 07:28:07 UTC Comment hidden (obsolete)
Comment 301 Martin Renvoize 2021-05-06 07:28:15 UTC Comment hidden (obsolete)
Comment 302 Martin Renvoize 2021-05-06 07:28:23 UTC Comment hidden (obsolete)
Comment 303 Martin Renvoize 2021-05-06 07:28:31 UTC Comment hidden (obsolete)
Comment 304 Martin Renvoize 2021-05-06 07:28:41 UTC Comment hidden (obsolete)
Comment 305 Martin Renvoize 2021-05-06 07:28:49 UTC Comment hidden (obsolete)
Comment 306 Martin Renvoize 2021-05-06 07:28:58 UTC Comment hidden (obsolete)
Comment 307 Martin Renvoize 2021-05-06 07:29:05 UTC Comment hidden (obsolete)
Comment 308 Martin Renvoize 2021-05-06 07:29:13 UTC Comment hidden (obsolete)
Comment 309 Martin Renvoize 2021-05-06 07:29:22 UTC Comment hidden (obsolete)
Comment 310 Martin Renvoize 2021-05-06 07:29:30 UTC Comment hidden (obsolete)
Comment 311 Martin Renvoize 2021-05-06 07:29:38 UTC Comment hidden (obsolete)
Comment 312 Martin Renvoize 2021-05-06 07:29:46 UTC Comment hidden (obsolete)
Comment 313 Martin Renvoize 2021-05-06 07:29:54 UTC Comment hidden (obsolete)
Comment 314 Martin Renvoize 2021-05-06 07:30:02 UTC Comment hidden (obsolete)
Comment 315 Martin Renvoize 2021-05-06 07:30:11 UTC Comment hidden (obsolete)
Comment 316 Martin Renvoize 2021-05-06 07:30:20 UTC Comment hidden (obsolete)
Comment 317 Martin Renvoize 2021-05-06 07:30:28 UTC Comment hidden (obsolete)
Comment 318 Martin Renvoize 2021-05-06 07:30:36 UTC Comment hidden (obsolete)
Comment 319 Martin Renvoize 2021-05-06 07:30:45 UTC Comment hidden (obsolete)
Comment 320 Martin Renvoize 2021-05-06 07:36:08 UTC
(In reply to Katrin Fischer from comment #294)
> Seeing the last status change - what happened?

I broke it with the moving the JS to the bottom of the page.. so whilst I was working to fix that I reset the status to FQA to stop anyone wasting time testing.

I've now fixed it again.. and run through the whole test plan to check I didn't break anything else.
Comment 321 Jonathan Druart 2021-05-07 09:42:32 UTC
Running `dbic` generates a change to Koha/Schema/Result/MarcOverlayRule.pm. It removes a FK to Koha::Schema::Result::MarcOverlayRulesModule.
However this table does not exist. Should I drop the schema changes and regenerate it properly or here is something else missing?
Comment 322 Jonathan Druart 2021-05-07 10:08:21 UTC
Created attachment 120670 [details] [review]
Bug 14957: Clean controller

Too many imports and unecessary complicated init of $rules

There are certainly other things to improve, like the "op" handling,
standardize names of "op", etc.
Comment 323 Jonathan Druart 2021-05-07 10:09:34 UTC
Created attachment 120671 [details] [review]
Bug 14957: Clean controller

Too many imports and unecessary complicated init of $rules

There are certainly other things to improve, like the "op" handling,
standardize names of "op", etc.
Comment 324 Jonathan Druart 2021-05-07 10:31:03 UTC
1. See comment 321

2. We don't hide the admin page if the pref is off, is that expected?

3. Should we link the pref to the admin page?

4. We need a wiki page, or at least more info/examples for the release notes.
There is only 2 very succinct example in the test plan but the feature allows much more.

5. Can you explain me what this rule means?

| id | tag | module | filter   | add | append | remove | delete |
|  1 | 12  | source | batchmod |   1 |      0 |      0 |      0 |

6. How does it interface with the marc modification templates when batch editing bibliographic records?
Comment 325 Jonathan Druart 2021-05-07 10:44:30 UTC
7.
(In reply to Andrew Fuerste-Henry from comment #226)
> I made this rule:
> Module: source
> Filter: *
> Tag: 500
> Preset: Protect
> 
> When I go to edit a record, there's nothing in the edit interface to suggest
> my edits won't be saved. I can type in a new 500, click to delete an
> existing 500, etc. When I save my edits, there's nothing to indicate that my
> changes were discarded or why. The edits simply aren't saved. 
> 
> This lack of feedback seems really confusing.

Still valid.

However a new record can be created with a 500, is that expected?
Comment 326 Martin Renvoize 2021-05-07 11:43:39 UTC
(In reply to Jonathan Druart from comment #321)
> Running `dbic` generates a change to Koha/Schema/Result/MarcOverlayRule.pm.
> It removes a FK to Koha::Schema::Result::MarcOverlayRulesModule.
> However this table does not exist. Should I drop the schema changes and
> regenerate it properly or here is something else missing?

I think drop the dbic patch here and use the regenerated one.. I think the Module bit got removed during the history of this bug but the schema didn't get regenerated.
Comment 327 Martin Renvoize 2021-05-07 11:52:36 UTC
(In reply to Jonathan Druart from comment #324)
> 1. See comment 321

Replied above

> 2. We don't hide the admin page if the pref is off, is that expected?

Whilst I agree with what you're saying here, I think if memory serves this was discussed earlier in the bug and the authors wanted the ability to create their rules and then enable them.. so they wanted to display the page regardless or whether the pref was on or off (the page does warn you at the top if the pref is off).. having said that.. I'd be happy to follow-up to change that.. or, in my opinion in a later bug we could drop the pref and instead have an 'enabled' flag at the rule level.
 
> 3. Should we link the pref to the admin page?

Not sure what you're saying here?

> 4. We need a wiki page, or at least more info/examples for the release notes.
> There is only 2 very succinct example in the test plan but the feature
> allows much more.

I agree, it took me a fair while to wrap my head around the rules.. I think a wiki page would be great and would be happy to start one.

> 5. Can you explain me what this rule means?
> 
> | id | tag | module | filter   | add | append | remove | delete |
> |  1 | 12  | source | batchmod |   1 |      0 |      0 |      0 |

With 'batchmod', only add tag 12 is if exists only in the incoming record.

> 6. How does it interface with the marc modification templates when batch
> editing bibliographic records?

Investigating.. I have a feeling his patchset predates it and I missed this case ! :(
Comment 328 Martin Renvoize 2021-05-07 11:58:59 UTC
OK, I did test this actually.. 

The modification templates are applied prior to save.. so the changes are then blocked by the rules here.

The main thing I don't like about this bug is exactly what Andrew is highlighting.. at the moment it's all rather silent.  It would be great as a followup early next cycle to enhance each of the area's it touched with feedback either during save or prior to it to let the end user know that their changes will not be applied due to the rules in place here.

I felt that the base bug was already large and complicated and shouldn't be blocked any long as a bug in itself.. I'm really hoping that follow-up bugs will be acted on in a timely fashion.. even if it means me lending a hand.
Comment 329 Jonathan Druart 2021-05-07 12:55:55 UTC
(In reply to Martin Renvoize from comment #327)
> (In reply to Jonathan Druart from comment #324)
> > 2. We don't hide the admin page if the pref is off, is that expected?

> so they wanted to display the page regardless or whether the pref was on or off 

Make sense

> (the page does warn you at the top if the pref is off)

That's wrong, there is no warning.

> > 3. Should we link the pref to the admin page?
> 
> Not sure what you're saying here?

Have a link to the admin page, next to the pref.
Comment 330 Jonathan Druart 2021-05-07 14:04:42 UTC
(In reply to Martin Renvoize from comment #327)
> (In reply to Jonathan Druart from comment #324)
> > 5. Can you explain me what this rule means?
> > 
> > | id | tag | module | filter   | add | append | remove | delete |
> > |  1 | 12  | source | batchmod |   1 |      0 |      0 |      0 |
> 
> With 'batchmod', only add tag 12 is if exists only in the incoming record.

I still don't understand, but I now see the manual stuff, will read it (haven't seen it before, sorry!)

Note for myself: One another thing to check, how is working the "preview" for the batchmod.
Comment 331 Jonathan Druart 2021-05-07 15:03:52 UTC
I know librarians can have weird needs but, should we really offer the "custom" preset? I am reading the tests right now and some combinations are doing funky stuffs. Maybe we could simplify the UI if we stick to the only 1 "action" column with the predefined ones? (not considering this question a blocker, but I would be happy to get an answer).
Comment 332 Jonathan Druart 2021-05-07 16:22:07 UTC
Created attachment 120705 [details] [review]
Bug 14957: Improve readbility of tests

Not saying that they were not readable before, but I had to do some
changes to compare the before/after records and it was easier that way.
Feel free to obsolete if you disagree.
Comment 333 Jonathan Druart 2021-05-07 16:25:48 UTC
(In reply to Jonathan Druart from comment #331)
> I know librarians can have weird needs but, should we really offer the
> "custom" preset? I am reading the tests right now and some combinations are
> doing funky stuffs. Maybe we could simplify the UI if we stick to the only 1
> "action" column with the predefined ones? (not considering this question a
> blocker, but I would be happy to get an answer).

See the "warning"s and "fixme"s from the last patch please.
Comment 334 Jonathan Druart 2021-05-11 10:47:10 UTC
I am sorry, I am not feeling confident enough with this one for 21.05.

I would like to get answer to my questions, and improve the user experience.

Keep the work going, it's almost ready! And we push it early in the next cycle.
Comment 335 Jonathan Druart 2021-06-04 09:04:23 UTC
Nobody interested in getting this done?
Comment 336 Katrin Fischer 2021-06-04 09:13:10 UTC
(In reply to Jonathan Druart from comment #335)
> Nobody interested in getting this done?

Martin and I are, but more hands would be useful.
Comment 337 Martin Renvoize 2021-09-29 20:08:32 UTC
Still interested but just haven't been able to commit any more time to it.  Honestly it's a complicated enough patch that I get lost in it for days every time I start to look at it.
Comment 338 David Gustafsson 2021-09-30 11:51:40 UTC
Regarding the level of confidence in this patch regarding stability we have been running this for a pretty large University Library (Gothenburg University Library) in production for a couple of years, about three I think, and not encountered any major issues. We have not merged in the latest upstream changes from this issue, but I don't think much have changed when it comes to the core logic/functionality.
Comment 339 Jonathan Druart 2021-09-30 12:02:31 UTC
There are several questions waiting for answers, see comments 321-333.
Comment 340 Martin Renvoize 2021-09-30 13:43:07 UTC
Created attachment 125514 [details] [review]
Bug 14957: Merge rules system for merging of MARC records

Add a rule based system for merging MARC records to for example
prevent field data from being overwritten.

To test:
    1. Apply this patch.
    2. Log in to staff client.
    3. Enable new syspref MARCMergeRules.
    4. Click the new link "MARC merge rules" in the "Catalog"
       section of the Koha administration page.
    5. Create a new rule:
       Module: source, Filter: *, Tag: 245, Preset: Protect.
    6. Clicking "Edit" should allow you to edit corresponding rule.
    7. Clicking "Delete" should remove corresponding rule after confirmation.
    8. Selecting one or more rules followed by clicking "Delete
       selected" should remove all selected rules after confirmation.
    9. Try creating a rule with tag set to "**", the other options does
       not matter. Verify that saving this rule produces an error
       message complaining about invalid tag regular expression.
    10. Try creating a rule with tag set to "008" (or other control
        field) and set Appended: Append and Removed: Skip, the other
        options does not matter. Verify that saving this rule produces
        an error message complaining about invalid combination of actions
        for control field.
    11. With the 245 rule in step 5 in place, edit a bibliographic record,
        change 245a for example (which should be Title for MARC21) and save.
    12. Verify that the changes has not been saved.
    13. Create a new rule:
        Module: source, Filter: intranet, Tag: 245, Preset: Overwrite.
    14. Repeat step 12, and verify that the changes has now been saved.
    15. Run tests in t/db_dependent/Biblio/MarcMergeRules.t and very
        that all tests pass.

Sponsored-by: Halland County Library
Sponsored-by: Catalyst IT
Sponsored-by: Gothenburg University Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 341 Martin Renvoize 2021-09-30 13:43:16 UTC
Created attachment 125515 [details] [review]
Bug 14957: Updated schema files

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 342 Martin Renvoize 2021-09-30 13:43:26 UTC
Created attachment 125516 [details] [review]
Bug 14957: fix context for batchmod and batchimport

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 343 Martin Renvoize 2021-09-30 13:43:36 UTC
Created attachment 125517 [details] [review]
Bug 14957: (QA follow-up) Button styling

This patch updates the action buttons to include btn-default and
switched from btn-sm to btn-xs to match other similar UI's found in
datatable based admin pages.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 344 Martin Renvoize 2021-09-30 13:43:45 UTC
Created attachment 125518 [details] [review]
Bug 14957: (QA follow-up) Add tooltips

This patch adds an info-tooltip to the field rules in the table header
to expand on their meaning to reduce the need to read the documentation
each time you come to use the configuration tool

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 345 Martin Renvoize 2021-09-30 13:43:57 UTC
Created attachment 125519 [details] [review]
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay'

This patch acknowledges the fact that 'merging' has a different meaning
in Koha than the behavior this great patchset introduces. The more
idiomatic way of describing the behavior is to talk about 'overlay rules'.

This patch also:
- Fixes kohastructure.sql missing table encoding a collation
- Fixes the atomic update completely missing a table
- Moves the syspref entry to 'Importing', probably a rebase issue

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 346 Martin Renvoize 2021-09-30 13:44:07 UTC
Created attachment 125520 [details] [review]
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 347 Martin Renvoize 2021-09-30 13:44:16 UTC
Created attachment 125521 [details] [review]
Bug 14957: DBIC changes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 348 Martin Renvoize 2021-09-30 13:44:25 UTC
Created attachment 125522 [details] [review]
Bug 14957: (follow-up) marc_merge_rules_modules removed

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 349 Martin Renvoize 2021-09-30 13:44:37 UTC
Created attachment 125523 [details] [review]
Bug 14957: (QA follow-up) Clarify 'context' param

This patch renames the (passed through) 'context' param for
'overlay_context'. I propose doing so, because in Koha-land 'context'
has a special meaning, related to C4::Context and it reads ambigous.

The patch itself is pretty trivial.

Tests should pass:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Biblio/MarcOverlayRules.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 350 Martin Renvoize 2021-09-30 13:44:46 UTC
Created attachment 125524 [details] [review]
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 351 Martin Renvoize 2021-09-30 13:44:55 UTC
Created attachment 125525 [details] [review]
Bug 14957: (QA follow-up) Fix translation issues

This patch fixes all the translation issues I could spot in the
marc-overlay-rules template.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 352 Martin Renvoize 2021-09-30 13:45:05 UTC
Created attachment 125526 [details] [review]
Bug 14957: (QA follow-up) Rename and move admin link

This patch renames 'MARC overlay rules' to 'Record overlay rules' and
moves it up the list to just below 'Record matching rules' to make it
more contextually ordered.
Comment 353 Martin Renvoize 2021-09-30 13:45:14 UTC
Created attachment 125527 [details] [review]
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily

It appears this patchset doesn't fully support bulkmarcimport (see bug
25539 for details).

At this time, to get this bug moving I think we should comment out the
ability to define rules that affect bulkmarcimport and then we can fix
and re-enable that feature at a later date.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 354 Martin Renvoize 2021-09-30 13:45:27 UTC
Created attachment 125528 [details] [review]
Bug 14957: (QA follow-up) Add missing filters
Comment 355 Martin Renvoize 2021-09-30 13:45:36 UTC
Created attachment 125529 [details] [review]
Bug 14957: (QA follow-up) Update for bug 26703
Comment 356 Martin Renvoize 2021-09-30 13:45:47 UTC
Created attachment 125530 [details] [review]
Bug 14957: (QA follow-up) Update for bug 20054
Comment 357 Martin Renvoize 2021-09-30 13:45:59 UTC
Created attachment 125531 [details] [review]
Bug 14957: (QA follow-up) Update for bug 20053
Comment 358 Martin Renvoize 2021-09-30 13:46:09 UTC
Created attachment 125532 [details] [review]
Bug 14957: (QA follow-up) Update for bug 17858
Comment 359 Martin Renvoize 2021-09-30 13:46:19 UTC
Created attachment 125533 [details] [review]
Bug 14957: (QA follow-up) Update for bug 27846
Comment 360 Martin Renvoize 2021-09-30 13:46:29 UTC
Created attachment 125534 [details] [review]
Bug 14957: (QA follow-up) Fix template indentation
Comment 361 Martin Renvoize 2021-09-30 13:46:40 UTC
Created attachment 125535 [details] [review]
Bug 14957: (QA follow-up) Update for bug 23271
Comment 362 Martin Renvoize 2021-09-30 13:46:50 UTC
Created attachment 125536 [details] [review]
Bug 14957: (QA follow-up) Fix script name
Comment 363 Martin Renvoize 2021-09-30 13:47:00 UTC
Created attachment 125537 [details] [review]
Bug 14957: (QA follow-up) Fix for jQuery undefined
Comment 364 Martin Renvoize 2021-09-30 13:47:10 UTC
Created attachment 125538 [details] [review]
Bug 14957: Clean controller

Too many imports and unecessary complicated init of $rules

There are certainly other things to improve, like the "op" handling,
standardize names of "op", etc.
Comment 365 Martin Renvoize 2021-09-30 13:47:19 UTC
Created attachment 125539 [details] [review]
Bug 14957: Improve readbility of tests

Not saying that they were not readable before, but I had to do some
changes to compare the before/after records and it was easier that way.
Feel free to obsolete if you disagree.
Comment 366 Martin Renvoize 2021-09-30 13:47:29 UTC
Created attachment 125540 [details] [review]
Bug 14957: Fix imports in tests
Comment 367 Martin Renvoize 2021-09-30 13:48:04 UTC
Another rebase.. another follow-up to fix things that have moved on.

I'm not sure what more I can do... 6 years is too long for an important feature like this to languish.
Comment 368 David Gustafsson 2021-09-30 14:31:25 UTC
(In reply to Jonathan Druart from comment #339)
> There are several questions waiting for answers, see comments 321-333.

I read through those comments again, and the questions where not addressing stability issues but feature requests and refactoring suggestions. Probably one of the reasons this has taken so long is that instead of concentrating on minimal viable functionality there are continuous suggestions for new features and improvements. Some of which are actually pretty complicated to implement (like user feedback). As I said we have been running this in production for years, and it more or less covers our needs (as pretty decently sized university library with 200+ employees.

I did forget about one thing though, that actually is a show-stopper, and that is if this bug is merged without the fix in bug 25539, this will trigger a bug with this current patch. I discovered that bug 25539 was set as depending on this bug when it should have been the other way around which I now fixed.

The reason everything works for us is that we use the fix in bug 25539. We also have large and unruly patch for bulkmarcimport.pl (a which I believe is now deactivated in the current iteration of this patch) so that also source works. Can't remember at the top of my head what the issues in bulkmarcimport.pl was that caused us to disable it, but the fix in bug 25539 will not solve this issue I think (but is still needed to avoid the bug causing rules to be ignored on updates. So I think focusing on sorting out the issue addressed in bug 25539, and possibly address the problems with bulkmarcimport.pl would be reasonable thing to prioritize.
Comment 369 Jonathan Druart 2021-09-30 14:49:24 UTC
I've spent a lost of time on this patch set to make it part of 21.05. Added follow-ups and written test.

Then I had some questions, and didn't get the answers I needed to write the missing bit. I was not asking anybody else to write anything.

I wish I had people asking me to write follow-up for my patches..
Comment 370 Martin Renvoize 2021-09-30 15:06:18 UTC
> I did forget about one thing though, that actually is a show-stopper, and
> that is if this bug is merged without the fix in bug 25539, this will
> trigger a bug with this current patch. I discovered that bug 25539 was set
> as depending on this bug when it should have been the other way around which
> I now fixed.

Pretty sure we reversed the dependencies deliberately to unblock this.. we disable this feature for use with bulkmarcimport in one of the patches.
Comment 371 David Gustafsson 2021-09-30 15:32:12 UTC
(In reply to Martin Renvoize from comment #370)
> > I did forget about one thing though, that actually is a show-stopper, and
> > that is if this bug is merged without the fix in bug 25539, this will
> > trigger a bug with this current patch. I discovered that bug 25539 was set
> > as depending on this bug when it should have been the other way around which
> > I now fixed.
> 
> Pretty sure we reversed the dependencies deliberately to unblock this.. we
> disable this feature for use with bulkmarcimport in one of the patches.

Yes, now I remember. I mistakenly misremembered the bug in bug 25539 appearing when saving using the staff interface. In that case there should be no blockers for this patch (with bulkmarcimport.pl disabled, which would be a show-stopper for us, but we would in case this got merged address with a local patch).
Comment 372 David Gustafsson 2021-09-30 16:50:32 UTC
(In reply to Jonathan Druart from comment #369)
> I've spent a lost of time on this patch set to make it part of 21.05. Added
> follow-ups and written test.
> 
> Then I had some questions, and didn't get the answers I needed to write the
> missing bit. I was not asking anybody else to write anything.
> 
> I wish I had people asking me to write follow-up for my patches..

Sorry I missed some of those questions (i for some reason read comment 221-333 so I was not referring to you regarding requesting new features).

Regarding the question in comment 324:

> 4. We need a wiki page, or at least more info/examples for the release notes.
There is only 2 very succinct example in the test plan but the feature allows much more.

I have written a manual entry, it might now be partly out dated since it was 2 years ago, there are some links to it further up in the thread: https://gitlab.com/gnucifer/koha-manual/-/commit/eb07dd1abcb357de27d294cee49c287c6f5bfe6c


The reason I pointed out that we have been running this in production for a long time is that there appears to be great hesitancy in merging this which is understandable due to the large surface area of changes and and relative high complexity. That serious bugs could lurk is not impossible, but is highly unlikely as we would probably have encountered them by now. There are for example certain contexts that we do not use, like batchmod, z3950 and import_lexile, but it can be good to know that the core functionality appears to be pretty solid. The scariest part of this by far is the potential for unexpected data loss or corruption when records are taken apart and put back together again, and that part appears to be solid. As a side note I don't really get why MergeRules were renamed to OverlayRules, it's not that much worse than merge, but is it really better? To me merge is more intuitive as we basically have to revisions of a record and use certain rules to resolve the diff, merging the two.
Comment 373 David Gustafsson 2021-10-01 14:18:59 UTC
Created attachment 125632 [details] [review]
Bug 14957: Fix regression, rules must be refetched on add/edit/deletion
Comment 374 David Gustafsson 2021-10-01 14:19:11 UTC
Created attachment 125633 [details] [review]
Bug 14957: Fix imports in marc-overlay-rules.pl
Comment 375 David Gustafsson 2021-10-01 14:26:38 UTC
Fixed a regression introduced in commit, "Bug 14957: Clean controller". There was reason for that kludgy code even though it could be somewhat simplified.

Also some imports that needed to be fixed after some subs no longer exported by default.
Comment 376 David Gustafsson 2021-10-01 14:37:58 UTC
Appears to be some failing tests. I think I know why as encountered the same issue with another patch. Part of the leader field is now modified in Koha in ModBiblio, which was not the case before, so records appears to have changed when the data has not as the whole record is now checked, when only specific fields where before. Will have a look at this at monday.
Comment 377 David Gustafsson 2021-10-01 14:48:23 UTC
Created attachment 125636 [details] [review]
Bug 14957: Exclude leader from saved/updated record comparison in tests
Comment 378 David Gustafsson 2021-10-01 14:48:59 UTC
Realized was very easy to fix, now tests should be working.
Comment 379 Jonathan Druart 2021-10-01 14:50:44 UTC
Thanks David for your follow-ups.

Martin and I are ready to help you here. We have enough time before the 21.11 freeze to make this new feature part of the next release :)

Can you reply to the other open questions please? See below.

I am volunteering to write the patches needed.

(In reply to Martin Renvoize from comment #327)
> (In reply to Jonathan Druart from comment #324)
> > 2. We don't hide the admin page if the pref is off, is that expected?
> 
> Whilst I agree with what you're saying here, I think if memory serves this
> was discussed earlier in the bug and the authors wanted the ability to
> create their rules and then enable them.. so they wanted to display the page
> regardless or whether the pref was on or off (the page does warn you at the
> top if the pref is off).. having said that.. I'd be happy to follow-up to
> change that.. or, in my opinion in a later bug we could drop the pref and
> instead have an 'enabled' flag at the rule level.

It seems weird to have the pref off but the link to the admin page visible.

> > 3. Should we link the pref to the admin page?
> 
> Not sure what you're saying here?

I meant, should we have a link to the admin page of the feature in the syspref's description. I can do it.

> > 6. How does it interface with the marc modification templates when batch
> > editing bibliographic records?
> 
> Investigating.. I have a feeling his patchset predates it and I missed this
> case ! :(

Need to be investigated.

(In reply to Jonathan Druart from comment #331)
> I know librarians can have weird needs but, should we really offer the
> "custom" preset? I am reading the tests right now and some combinations are
> doing funky stuffs. Maybe we could simplify the UI if we stick to the only 1
> "action" column with the predefined ones? (not considering this question a
> blocker, but I would be happy to get an answer).

Still waiting for an answer.

See also the "warning"s and "fixme"s from the patch "Improve readbility of tests".
Comment 380 David Gustafsson 2021-10-01 15:05:20 UTC
(And I think it's probably much better to compare the whole record in the tests, you don't get as detailed feedback on failure, but on the other hand catches if there are unexpected modifications of other parts of the record which is more important).
Comment 381 David Gustafsson 2021-10-01 15:25:14 UTC
Hi again!

> It seems weird to have the pref off but the link to the admin page visible.

I have not made any active decision regarding this, it's been like this since before I got involved and have never given it much thought. Don't have any strong opinions either way. Elasticsearch settings seems to work the same way, that is if SearchEngine is Zebra, you still have a "Search engine configuration (Elasticsearch)" link in Administration with a "Elasticsearch is currently disabled" notice at the top. So personally I would probably keep it as it is just to avoid the extra work (even though should be pretty easy to change).

> > 6. How does it interface with the marc modification templates when batch
> > editing bibliographic records?
> 
> Investigating.. I have a feeling his patchset predates it and I missed this
> case ! :(

I don't have time to look at this today but can do on Monday.

> I know librarians can have weird needs but, should we really offer the
> "custom" preset? I am reading the tests right now and some combinations are
> doing funky stuffs. Maybe we could simplify the UI if we stick to the only 1
> "action" column with the predefined ones? (not considering this question a
> blocker, but I would be happy to get an answer).

I see no reason to remove this, I just don't see the harm of having the option if there is some weird scenario where it could be useful. I don't see the "custom" option as complicating the UI, it's just an extra choice in a dropdown.
Comment 382 David Gustafsson 2021-10-01 15:38:41 UTC
And I think there are pedagogical reasons to keep the columns with the action choices and being able to see which ones are selected for each preset. Even though I understand it will be challenging for must librarians to understand the current rules concept, if we where to remove the separate actions (which now also have pretty good explanations), I think it would be much harder for people to understand exactly how rules are applied, and a higher risk for assuming work in some different way.
Comment 383 David Gustafsson 2021-10-05 14:15:15 UTC
> > 6. How does it interface with the marc modification templates when batch
> > editing bibliographic records?
> 
> Investigating.. I have a feeling his patchset predates it and I missed this
> case ! :(

Sorry, never got around to have a look at this yesterday, but now did. From what I can gather from the code I'm pretty sure that if no rules have been defined for the batchmod context, the incoming record simply overwrites the existing record. If there are rules, these are applied on the record after the record has been modified using the specified template. So if the modification template modifies some protected field for example, these changes will not be applied. This is expected behavior as far as I can tell?
Comment 384 Jonathan Druart 2021-10-18 10:43:26 UTC
(In reply to David Gustafsson from comment #383)
> > > 6. How does it interface with the marc modification templates when batch
> > > editing bibliographic records?
> > 
> > Investigating.. I have a feeling his patchset predates it and I missed this
> > case ! :(
> 
> Sorry, never got around to have a look at this yesterday, but now did. From
> what I can gather from the code I'm pretty sure that if no rules have been
> defined for the batchmod context, the incoming record simply overwrites the
> existing record. If there are rules, these are applied on the record after
> the record has been modified using the specified template. So if the
> modification template modifies some protected field for example, these
> changes will not be applied. This is expected behavior as far as I can tell?

Tested right now:
- Protect 500
- Edit bibliographic record, add new 500 field
=> Change not saved (OK)
- Use the batch record modification tool (previously created a MARC modification template that add a 500$a
=> The preview show the 500$a (KO)
- Save
=> The new field is created (KO)
Comment 385 Jonathan Druart 2021-10-18 10:47:57 UTC
Created attachment 126424 [details] [review]
Bug 14957: (follow-up) Clarify 'context' param
Comment 386 Jonathan Druart 2021-10-18 11:05:15 UTC
Created attachment 126425 [details] [review]
Bug 14957: Fix record preview
Comment 387 Jonathan Druart 2021-10-18 12:28:08 UTC
Created attachment 126431 [details] [review]
Bug 14957: Fix syspref's values

It didn't switch off when set to "don't use"
Comment 388 Jonathan Druart 2021-10-18 12:36:35 UTC
Regression found, the order of the subfields (introduced by bug 7882) is not kept.

To recreate:
Turn on MARCOverlayRules
Edit a record
Change the order of subfields
Save
Edit again
=> The order hasn't been saved.
Comment 389 Jonathan Druart 2021-10-18 12:48:06 UTC
(In reply to Jonathan Druart from comment #388)
> Regression found, the order of the subfields (introduced by bug 7882) is not
> kept.
> 
> To recreate:
> Turn on MARCOverlayRules
> Edit a record
> Change the order of subfields
> Save
> Edit again
> => The order hasn't been saved.

diff --git a/Koha/MarcOverlayRules.pm b/Koha/MarcOverlayRules.pm
index 11b33d1928b..70500d00d2f 100644
--- a/Koha/MarcOverlayRules.pm
+++ b/Koha/MarcOverlayRules.pm
@@ -149,7 +149,7 @@ sub merge_records {
     my $hash_field_data = sub {
         my ($field) = @_;
         my $indicators = join("\x1E", map { $field->indicator($_) } (1, 2));
-        return $indicators . "\x1E" . join("\x1E", sort map { join "\x1E", @{$_} } $field->subfields());
+        return $indicators . "\x1E" . join("\x1E", map { join "\x1E", @{$_} } $field->subfields());
     };
 
     my $diff_by_key = sub {

I think it's because of this 'sort', but there are 2 other sort calls in this method, not sure how/why they are useful.
Comment 390 David Gustafsson 2021-10-20 12:52:19 UTC
> I think it's because of this 'sort', but there are 2 other sort calls in
> this method, not sure how/why they are useful.

There might be some other issue that causes this issue, but it's not those sorts if I'm not completely misguided. That's a hash function for comparing field data, and the data sorted will not be written back to the record.
Comment 391 David Gustafsson 2021-10-20 12:59:30 UTC
(In reply to David Gustafsson from comment #390)
> > I think it's because of this 'sort', but there are 2 other sort calls in
> > this method, not sure how/why they are useful.
> 
> There might be some other issue that causes this issue, but it's not those
> sorts if I'm not completely misguided. That's a hash function for comparing
> field data, and the data sorted will not be written back to the record.

Just realized it might actually be the reason, but in a round-about way. If just changing the order of fields without modifying the data, they will still be considered identical so the old value will be kept. This is most likely the cause. If we remove the sort we risk writing an extra entry if for example the order has been changed for a record in Koha, and an external record with the same data in a different order is imported, as it will be considered a new field entry. Will think about how to handle this, hopefully without not complicating the merge algorithm too much.
Comment 392 David Gustafsson 2021-10-20 13:59:22 UTC
Now that looked at this some more, the subfield ordering is one thing, and field ordering another, and has to be handled differently. I think I know how to address the subfield order issue. Will see if can find a way to handle field order correctly.
Comment 393 David Gustafsson 2021-10-20 15:18:01 UTC
Created attachment 126592 [details] [review]
Bug 14957: Fix field order issue
Comment 394 David Gustafsson 2021-10-20 15:22:54 UTC
The fix for subfield ordering actually resolved them both. The change might be a little bit hard to understand so please let me know if have any questions.
Comment 395 David Gustafsson 2021-10-20 15:48:58 UTC
One could also argue that should be able to change order also for non-overwritable field, or to introduce an extra "change order" action.
Comment 396 Katrin Fischer 2021-10-20 18:38:31 UTC
We mostly use "In discussion" only if there is an argument between parties - I don't see that happen here yet, so will switch back to 'needs sign-off' so that this will show for the testers.
Comment 397 Jonathan Druart 2021-10-21 13:52:14 UTC
Created attachment 126666 [details] [review]
Bug 14957: Add tests for subfield order
Comment 398 Jonathan Druart 2021-10-21 14:08:52 UTC
Created attachment 126667 [details] [review]
Bug 14957: Tell the cataloguer that a change has been rejected
Comment 399 Jonathan Druart 2021-10-21 14:10:45 UTC
(In reply to Jonathan Druart from comment #398)
> Created attachment 126667 [details] [review] [review]
> Bug 14957: Tell the cataloguer that a change has been rejected

This patch is pretty ugly but I wanted to know the shortest route to display feedback to the cataloger. That must be tidied to be ready.
Obsoleting and letting who may need it moving it to its own bug report.
Comment 400 Jonathan Druart 2021-10-21 14:11:16 UTC
Comment on attachment 126667 [details] [review]
Bug 14957: Tell the cataloguer that a change has been rejected

This patch needs to be improved and moved to its own bug report.
Comment 401 Martin Renvoize 2021-10-22 12:31:09 UTC
Created attachment 126752 [details] [review]
Bug 14957: Merge rules system for merging of MARC records

Add a rule based system for merging MARC records to for example
prevent field data from being overwritten.

To test:
    1. Apply this patch.
    2. Log in to staff client.
    3. Enable new syspref MARCMergeRules.
    4. Click the new link "MARC merge rules" in the "Catalog"
       section of the Koha administration page.
    5. Create a new rule:
       Module: source, Filter: *, Tag: 245, Preset: Protect.
    6. Clicking "Edit" should allow you to edit corresponding rule.
    7. Clicking "Delete" should remove corresponding rule after confirmation.
    8. Selecting one or more rules followed by clicking "Delete
       selected" should remove all selected rules after confirmation.
    9. Try creating a rule with tag set to "**", the other options does
       not matter. Verify that saving this rule produces an error
       message complaining about invalid tag regular expression.
    10. Try creating a rule with tag set to "008" (or other control
        field) and set Appended: Append and Removed: Skip, the other
        options does not matter. Verify that saving this rule produces
        an error message complaining about invalid combination of actions
        for control field.
    11. With the 245 rule in step 5 in place, edit a bibliographic record,
        change 245a for example (which should be Title for MARC21) and save.
    12. Verify that the changes has not been saved.
    13. Create a new rule:
        Module: source, Filter: intranet, Tag: 245, Preset: Overwrite.
    14. Repeat step 12, and verify that the changes has now been saved.
    15. Run tests in t/db_dependent/Biblio/MarcMergeRules.t and very
        that all tests pass.

Sponsored-by: Halland County Library
Sponsored-by: Catalyst IT
Sponsored-by: Gothenburg University Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 402 Martin Renvoize 2021-10-22 12:31:20 UTC
Created attachment 126753 [details] [review]
Bug 14957: Updated schema files

Signed-off-by: Christian Stelzenmüller <christian.stelzenmueller@bsz-bw.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 403 Martin Renvoize 2021-10-22 12:31:32 UTC
Created attachment 126754 [details] [review]
Bug 14957: fix context for batchmod and batchimport

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 404 Martin Renvoize 2021-10-22 12:31:54 UTC
Created attachment 126755 [details] [review]
Bug 14957: (QA follow-up) Button styling

This patch updates the action buttons to include btn-default and
switched from btn-sm to btn-xs to match other similar UI's found in
datatable based admin pages.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 405 Martin Renvoize 2021-10-22 12:32:29 UTC
Created attachment 126756 [details] [review]
Bug 14957: (QA follow-up) Add tooltips

This patch adds an info-tooltip to the field rules in the table header
to expand on their meaning to reduce the need to read the documentation
each time you come to use the configuration tool

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 406 Martin Renvoize 2021-10-22 12:32:42 UTC
Created attachment 126757 [details] [review]
Bug 14957: (QA follow-up) Rename 'merge' => 'overlay'

This patch acknowledges the fact that 'merging' has a different meaning
in Koha than the behavior this great patchset introduces. The more
idiomatic way of describing the behavior is to talk about 'overlay rules'.

This patch also:
- Fixes kohastructure.sql missing table encoding a collation
- Fixes the atomic update completely missing a table
- Moves the syspref entry to 'Importing', probably a rebase issue

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 407 Martin Renvoize 2021-10-22 12:32:52 UTC
Created attachment 126758 [details] [review]
Bug 14957: Feature hidden in menues if no other 'cagaloguing' permissions

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 408 Martin Renvoize 2021-10-22 12:33:11 UTC
Created attachment 126759 [details] [review]
Bug 14957: DBIC changes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 409 Martin Renvoize 2021-10-22 12:33:46 UTC
Created attachment 126760 [details] [review]
Bug 14957: (follow-up) marc_merge_rules_modules removed

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 410 Martin Renvoize 2021-10-22 12:33:57 UTC
Created attachment 126761 [details] [review]
Bug 14957: (QA follow-up) Clarify 'context' param

This patch renames the (passed through) 'context' param for
'overlay_context'. I propose doing so, because in Koha-land 'context'
has a special meaning, related to C4::Context and it reads ambigous.

The patch itself is pretty trivial.

Tests should pass:
1. Run:
   $ kshell
  k$ prove t/db_dependent/Biblio/MarcOverlayRules.t
=> SUCCESS: Tests pass
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 411 Martin Renvoize 2021-10-22 12:34:13 UTC
Created attachment 126762 [details] [review]
Bug 14957: (QA follow-up) Fix syntax error in atomicupdate

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 412 Martin Renvoize 2021-10-22 12:34:23 UTC
Created attachment 126763 [details] [review]
Bug 14957: (QA follow-up) Fix translation issues

This patch fixes all the translation issues I could spot in the
marc-overlay-rules template.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 413 Martin Renvoize 2021-10-22 12:34:34 UTC
Created attachment 126764 [details] [review]
Bug 14957: (QA follow-up) Rename and move admin link

This patch renames 'MARC overlay rules' to 'Record overlay rules' and
moves it up the list to just below 'Record matching rules' to make it
more contextually ordered.
Comment 414 Martin Renvoize 2021-10-22 12:34:58 UTC
Created attachment 126765 [details] [review]
Bug 14957: (QA follow-up) Comment 'bulkmarcimport' temporarily

It appears this patchset doesn't fully support bulkmarcimport (see bug
25539 for details).

At this time, to get this bug moving I think we should comment out the
ability to define rules that affect bulkmarcimport and then we can fix
and re-enable that feature at a later date.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 415 Martin Renvoize 2021-10-22 12:35:08 UTC
Created attachment 126766 [details] [review]
Bug 14957: (QA follow-up) Add missing filters

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 416 Martin Renvoize 2021-10-22 12:35:21 UTC
Created attachment 126767 [details] [review]
Bug 14957: (QA follow-up) Update for bug 26703

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 417 Martin Renvoize 2021-10-22 12:35:34 UTC
Created attachment 126768 [details] [review]
Bug 14957: (QA follow-up) Update for bug 20054

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 418 Martin Renvoize 2021-10-22 12:36:07 UTC
Created attachment 126769 [details] [review]
Bug 14957: (QA follow-up) Update for bug 20053

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 419 Martin Renvoize 2021-10-22 12:36:17 UTC
Created attachment 126770 [details] [review]
Bug 14957: (QA follow-up) Update for bug 17858

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 420 Martin Renvoize 2021-10-22 12:36:29 UTC
Created attachment 126771 [details] [review]
Bug 14957: (QA follow-up) Update for bug 27846

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 421 Martin Renvoize 2021-10-22 12:36:56 UTC
Created attachment 126772 [details] [review]
Bug 14957: (QA follow-up) Fix template indentation

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 422 Martin Renvoize 2021-10-22 12:37:08 UTC
Created attachment 126773 [details] [review]
Bug 14957: (QA follow-up) Update for bug 23271

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 423 Martin Renvoize 2021-10-22 12:37:29 UTC
Created attachment 126774 [details] [review]
Bug 14957: (QA follow-up) Fix script name

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 424 Martin Renvoize 2021-10-22 12:37:42 UTC
Created attachment 126775 [details] [review]
Bug 14957: (QA follow-up) Fix for jQuery undefined

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 425 Martin Renvoize 2021-10-22 12:38:00 UTC
Created attachment 126776 [details] [review]
Bug 14957: Clean controller

Too many imports and unecessary complicated init of $rules

There are certainly other things to improve, like the "op" handling,
standardize names of "op", etc.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 426 Martin Renvoize 2021-10-22 12:38:32 UTC
Created attachment 126777 [details] [review]
Bug 14957: Improve readbility of tests

Not saying that they were not readable before, but I had to do some
changes to compare the before/after records and it was easier that way.
Feel free to obsolete if you disagree.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 427 Martin Renvoize 2021-10-22 12:38:45 UTC
Created attachment 126778 [details] [review]
Bug 14957: Fix imports in tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 428 Martin Renvoize 2021-10-22 12:38:56 UTC
Created attachment 126779 [details] [review]
Bug 14957: Fix regression, rules must be refetched on add/edit/deletion

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 429 Martin Renvoize 2021-10-22 12:39:17 UTC
Created attachment 126780 [details] [review]
Bug 14957: Fix imports in marc-overlay-rules.pl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 430 Martin Renvoize 2021-10-22 12:39:30 UTC
Created attachment 126781 [details] [review]
Bug 14957: Exclude leader from saved/updated record comparison in tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 431 Martin Renvoize 2021-10-22 12:39:45 UTC
Created attachment 126782 [details] [review]
Bug 14957: (follow-up) Clarify 'context' param

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 432 Martin Renvoize 2021-10-22 12:39:57 UTC
Created attachment 126783 [details] [review]
Bug 14957: Fix record preview

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 433 Martin Renvoize 2021-10-22 12:40:07 UTC
Created attachment 126784 [details] [review]
Bug 14957: Fix syspref's values

It didn't switch off when set to "don't use"

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 434 Martin Renvoize 2021-10-22 12:40:26 UTC
Created attachment 126785 [details] [review]
Bug 14957: Fix field order issue

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 435 Martin Renvoize 2021-10-22 12:40:47 UTC
Created attachment 126786 [details] [review]
Bug 14957: Add tests for subfield order

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 436 Martin Renvoize 2021-10-22 12:56:08 UTC
I worked through the whole set one last time.. must admit brain is hazy on the details with so much water under the bridge again.. but it all appears to be working as expected.

QA scripts spit out some noise, but I'm confident it's all noise due to git manipulations and things.

Passing QA.

Yes, we need the feedback stuff handled in a follow-up bug.
Comment 437 Jonathan Druart 2021-10-26 13:56:17 UTC
Some patches will be squashed when pushed.
Comment 438 Jonathan Druart 2021-10-26 14:41:57 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 439 Jonathan Druart 2021-10-26 14:44:18 UTC
It would be great to have something ready for the manual before 21.11 is released.

By the way, the release notes are saying "NOTE: A follow-up bug is being worked upon to add compatibility with bulkmarcimport.". Which bug is that?
Comment 440 David Gustafsson 2021-10-26 15:18:26 UTC
Great :) Thanks everyone. There is a patch for the manual: https://gitlab.com/gnucifer/koha-manual/commit/593c5da008dbc4b074cac9dee167cce9a6dfc250#c5e2c75dc923f21c5b95185b2f350df00fec661f

But some of the terminology need to change (marc overlay rules instaed of merge rules for example).
Comment 441 David Gustafsson 2021-10-26 15:19:57 UTC
(In reply to Jonathan Druart from comment #439)
> By the way, the release notes are saying "NOTE: A follow-up bug is being
> worked upon to add compatibility with bulkmarcimport.". Which bug is that?

Probably bug 25539
Comment 442 Jonathan Druart 2021-10-27 07:18:16 UTC
Created attachment 126950 [details] [review]
Bug 14957: Fix perlcritic error on admin/marc-overlay-rules.pl

Subroutine "new" called using indirect syntax at line 30, column 13.  See page 349 of PBP.  (Severity: 4)
Comment 443 Jonathan Druart 2021-10-27 07:18:27 UTC
Created attachment 126951 [details] [review]
Bug 14957: DBIC changes - Remove MarcOverlayRulesModule.pm

Where is that coming from??
Comment 444 Jonathan Druart 2021-10-27 07:27:10 UTC
Last two patches pushed to master.
Comment 445 Martin Renvoize 2021-10-27 08:01:18 UTC
(In reply to David Gustafsson from comment #440)
> Great :) Thanks everyone. There is a patch for the manual:
> https://gitlab.com/gnucifer/koha-manual/commit/
> 593c5da008dbc4b074cac9dee167cce9a6dfc250#c5e2c75dc923f21c5b95185b2f350df00fec
> 661f
> 
> But some of the terminology need to change (marc overlay rules instaed of
> merge rules for example).

Could you submit that as a merge request on gitlab now please.. I can do the tidying up if you like.
Comment 446 David Gustafsson 2021-10-27 10:33:39 UTC
Ok, thanks!

https://gitlab.com/gnucifer/koha-manual/-/merge_requests/1
Comment 447 Cab Vinton 2021-10-27 11:03:06 UTC
As someone who periodically uses MarcEdit's integration w/ Koha to download, change, & upload records & I'm curious whether this is one of the included update methods for this enhancement (batchimport, zv39.50, intranet, bulkmarcimport (?), import_lexile, batchmod).

I believe the MarcEdit integration relies on an API, so I'm thinking probably not? (https://blog.reeset.net/archives/2133; also 1123).
Comment 448 David Gustafsson 2021-10-27 12:06:58 UTC
Good point. There is a ModBiblio in svc/bib, there should absolutely be a context module for that, would be easy to add. But I don't know what API MarcEdit is using. There is a swagger-api that I'm totally unfamiliar with, but from what I can gather from there definitions there don't seem to be any support for updating biblios, but it's pretty impenetrable to me as now nothing about swagger and/or that part of Koha. It's also hard to figure out what MarcEdit actually uses since there is no actual code in that post.
Comment 449 David Gustafsson 2021-10-27 12:12:12 UTC
I missed the second blog-post, there it seems like it's the svc-api. So hopefully adding a context for this would in that case make it possible to define rules for MarcEdit as well.
Comment 451 Caroline Cyr La Rose 2022-08-30 16:58:31 UTC
I'm not comfortable enough with gitlab to change the merge request to a merge request to the community manual. Right now it is from David's own project to David's own project.
Comment 452 Caroline Cyr La Rose 2022-08-30 16:59:15 UTC
Ignore my comment! I saw that it was already merged sorry! I removed the Manual keyword.