Request for enhancement. Allow to configure default indicators (eg allow us to impose "1" as the first indicator for an individual authority, because it is the one which is adequate most of the time, and right now, it must be added manually each time.
Not only in the first indicator but also in the second indicator, if the system can let the users put default values in the indicators. This can addressed either: 1) In edit field of the MARC Bibliographic framework, there should be another option for the first indicator and second indicator, or 2) In the subfields where a particular subfield affects the indicators (e.g. When 650$2 is filled up, then the indicator should be 7). In no. 2, there should probably be a mechanism of "if and then" much like the MARC modification template we already have in Koha. No. 2 is kind of difficult to implement unlike 1, where you outright assign the indicators. But no. 1 has some problems too, like assigning indicators that are based on the number of characters (e.g. in 245 indicator 2). These are just my thoughts.
I guess the easiest way, in order not to complicate the MARC framework even more, would be to add a two columns with default values (or just leave them empty to have no default) in the default MARC framework, then in the MARC Framework editing screen add a possibility to set a default value just as for the regular subfields. Checking for some impossible or incorrect combinations (like the previously mentioned indicator 7 if 650$2 exists), could maybe be moved to a general "consistency check" routine that happens after editing the record. Or to a plugin?
Created attachment 52054 [details] [review] Bug 9701 - Configure default indicator This partial patch adds the possibility to define default indicators in the MARC frameworks. It does NOT yet load those defaults in the cataloguing form. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module.
*** Bug 8380 has been marked as a duplicate of this bug. ***
Created attachment 52082 [details] [review] With this second patch, bug 9701 is entirely fixed. The default values are also loaded in the cataloguing form. The database update is now done according to the database update workflow. Previous changes to files that ought not to have changed, done in th Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework.
I'm testing this in the sandbox and I'm getting this error when I go to the MARC structure: Software error: Can't use an undefined value as an ARRAY reference at /usr/local/lib/perl/5.14.2/DBI.pm line 2064. Even the email I received, I got the following: The sandbox you've requested is now ready. The patches attached to bugzilla 9701 have been applied, and the result is: Updatedatabase: Upgrade to 16.05.00.000 done (Koha 16.05) Upgrade to 16.06.00.000 done (Koha 16.06 - starting a new dev line at KohaCon16 in Thessaloniki, Greece! Koha is great!) Upgrade to 16.06.00.001 done (Bug 16200 - 'Hold waiting too long' fee has a translation problem) DEV atomic update: bug_9701-add_default_indicators.sql C4::Installer::load_sql returned the following errors while attempting to load /home/koha/src/installer/data/mysql/atomicupdate/bug_9701-add_default_indicators.sql: So with this, I can't proceed further.
The atomic update file has 'z' instead of 's' in authorised_value You don't need to add the updatedabase.pl changes - the RM does that now: https://wiki.koha-community.org/wiki/Database_updates
Created attachment 52161 [details] [review] Bug 9701 - fixed typo in atomic update file. Sorry for that!
Looks great now! :) I'm signing this one off. I have a question though but might not be related with this enhancement. When the tag is cloned/repeated and the indicator of this cloned tag is not the default indicator, it's not the difficult indicator that appears instead the indicator in the repeated/cloned tag. Well, that I guess is for another bug. But this enhancement does what original poster requested. Thanks a lot Joachim!
Patch tested with a sandbox, by Eugene Jose Espinoza <eugenegf@yahoo.com>
Created attachment 52171 [details] [review] Bug 9701 - Configure default indicator This partial patch adds the possibility to define default indicators in the MARC frameworks. It does NOT yet load those defaults in the cataloguing form. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module.
Created attachment 52172 [details] [review] With this second patch, bug 9701 is entirely fixed. The default values are also loaded in the cataloguing form. The database update is now done according to the database update workflow. Previous changes to files that ought not to have changed, done in th Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework.
Created attachment 52173 [details] [review] Bug 9701 - fixed typo in atomic update file. Sorry for that! Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com>
(In reply to Eugene Espinoza from comment #9) > Looks great now! :) I'm signing this one off. I have a question though but > might not be related with this enhancement. When the tag is cloned/repeated > and the indicator of this cloned tag is not the default indicator, it's not > the difficult indicator that appears instead the indicator in the > repeated/cloned tag. Well, that I guess is for another bug. But this > enhancement does what original poster requested. Thanks a lot Joachim! I'm not sure, that duplication behavior could also be seen as a feature instead of a bug :) . However, one thing is that this patch only creates default indicator settings for bibliographic records. The same for authority records is still a possible future enhancement. Not sure how urgent that would be though, since in my opinion authority records are usually kept pretty minimalistic in most setups.
I'm not sure, that duplication behavior could also be seen as a feature instead of a bug -It should be feature enhancement, my bad! :) And that's another enhancement. But still, good job on this! We just have to scour bugzilla about this. Cheers!
Hi Joachim, I have squashed your 3 original patches to avoid conflicts caused by adding and removing the change to updatedatabase.pl in the first 2 patches. Also fixes the problem that only the last patch was signed off by the sandbox script. I also added a follow-up fixing tabs pointed out by the QA script - please keep in mind that we are using 4 spaces instead of tabs! The patches work really nicely. We are close to getting this through QA, but please add tests for your change to GetMarcStructure. For changes in the core modules, unit tests are mandatory. Please let us know if you need any help with that!
Created attachment 53403 [details] [review] Bug 9701 - Configure default indicator This patch adds the possibility to define default indicators in the MARC frameworks. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module. The default values are also loaded in the cataloguing form. Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework. Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com>
Created attachment 53404 [details] [review] Bug 9701: QA follow-up - Fix indentation and some strings - Fixes tabs pointed out by the QA script. - Changes table headings Indic1 and Indic2 to Ind. 1 and Ind. 2
What is the status of this bug?
We need a default indicator in the framework too when we merge authorities across the authtype border. See bug 14769. Can you get this to work again, Joachim ?
Created attachment 71929 [details] [review] Bug 9701: Configure default indicator This patch adds the possibility to define default indicators in the MARC frameworks. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module. The default values are also loaded in the cataloguing form. Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework. Don't forget to run database and database schema update Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com>
Created attachment 71930 [details] [review] Bug 9701: (QA follow-up) Fix indentation and some strings - Fixes tabs pointed out by the QA script.
Created attachment 71931 [details] [review] Bug 9701: (follow-up) Adjust tests Test plan: prove t/db_dependent/Biblio.t should be green
My git does not like that second patch.. Applying: Bug 9701: (QA follow-up) Fix indentation and some strings *** Error in `git': malloc(): memory corruption: 0x0000000002837340 *** fatal: sha1 information is lacking or useless (C4/Biblio.pm). Repository lacks necessary blobs to fall back on 3-way merge.
Probably my bad, I will try to rebase and re-upload patches, but probably not today, and tomorow I am out, so please be patient...
Created attachment 72010 [details] [review] Bug 9701: Configure default indicator This patch adds the possibility to define default indicators in the MARC frameworks. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module. The default values are also loaded in the cataloguing form. Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework. Don't forget to run database and database schema update Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com>
Created attachment 72011 [details] [review] Bug 9701: (QA follow-up) Fix indentation and some strings - Fixes tabs pointed out by the QA script.
Created attachment 72012 [details] [review] Bug 9701: (follow-up) Adjust tests Test plan: prove t/db_dependent/Biblio.t should be green
Marcel, try it now, it applies cleanly on current master for me now.
Applying: Bug 9701: (QA follow-up) Fix indentation and some strings *** Error in `git': malloc(): memory corruption: 0x0000000000bfc340 *** fatal: sha1 information is lacking or useless (C4/Biblio.pm).
(In reply to Marcel de Rooy from comment #30) > Applying: Bug 9701: (QA follow-up) Fix indentation and some strings > *** Error in `git': malloc(): memory corruption: 0x0000000000bfc340 *** > fatal: sha1 information is lacking or useless (C4/Biblio.pm). I am not understand what is going here - I tried now, and it applied cleanly on current master...
Hey Josef, I tried doing this test: prove t/db_dependent/Biblio.t should be green Before applying the patches I had this: prove t/db_dependent/Biblio.t t/db_dependent/Biblio.t .. ok All tests successful. Files=1, Tests=9, 2 wallclock secs ( 0.05 usr 0.01 sys + 1.91 cusr 0.09 csys = 2.06 CPU) Result: PASS After applying the patches I had this: prove t/db_dependent/Biblio.t t/db_dependent/Biblio.t .. 7/9 DBD::mysql::st execute failed: Unknown column 'ind1_defaultvalue' in 'field list' [for Statement "SELECT tagfield,liblibrarian,libopac,mandatory,repeatable,ind1_defaultvalue,ind2_defaultvalue FROM marc_tag_structure WHERE frameworkcode=? ORDER BY tagfield" with ParamValues: 0=''] at /inlibro/git/koha-master-dev-inlibro/C4/Biblio.pm line 990. DBD::mysql::st fetchrow failed: fetch() without execute() [for Statement "SELECT tagfield,liblibrarian,libopac,mandatory,repeatable,ind1_defaultvalue,ind2_defaultvalue FROM marc_tag_structure WHERE frameworkcode=? ORDER BY tagfield" with ParamValues: 0=''] at /inlibro/git/koha-master-dev-inlibro/C4/Biblio.pm line 993. # Failed test 'expect 6 internals' # at t/db_dependent/Biblio.t line 436. # got: '0' # expected: '6' # Failed test 'check lib' # at t/db_dependent/Biblio.t line 437. # got: '0' # expected: '1' # Failed test 'check tab' # at t/db_dependent/Biblio.t line 438. # got: '0' # expected: '1' # Failed test 'check mandatory' # at t/db_dependent/Biblio.t line 439. # got: '0' # expected: '1' # Failed test 'check repeatable' # at t/db_dependent/Biblio.t line 440. # got: '0' # expected: '1' # Failed test 'check indicator 1 default value' # at t/db_dependent/Biblio.t line 442. # got: '0' # expected: '1' # Failed test 'check indicator 2 default value' # at t/db_dependent/Biblio.t line 443. # got: '0' # expected: '1' # Looks like you failed 7 tests of 8. # Failed test 'IsMarcStructureInternal' # at t/db_dependent/Biblio.t line 444. # Looks like you failed 1 test of 9. t/db_dependent/Biblio.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/9 subtests Test Summary Report ------------------- t/db_dependent/Biblio.t (Wstat: 256 Tests: 9 Failed: 1) Failed test: 8 Non-zero exit status: 1 Files=1, Tests=9, 3 wallclock secs ( 0.05 usr 0.00 sys + 2.01 cusr 0.10 csys = 2.16 CPU) Result: FAIL
(In reply to Maksim Sen from comment #32) > > After applying the patches I had this: > prove t/db_dependent/Biblio.t > t/db_dependent/Biblio.t .. 7/9 DBD::mysql::st execute failed: Unknown column > 'ind1_defaultvalue' in 'field list' did you perform database update?
t/db_dependent/Biblio.t is failing... there is a difference between unsafe and empty call to GetMarcStructure for some reason. Haven't traced it back enough. This is a side issue that is failing this bug.
Created attachment 72986 [details] [review] Bug 9701: Followup to make test pass
Created attachment 73780 [details] [review] Bug 9701: Configure default indicator This patch adds the possibility to define default indicators in the MARC frameworks. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module. The default values are also loaded in the cataloguing form. Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework. Don't forget to run database and database schema update Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 73781 [details] [review] Bug 9701: (QA follow-up) Fix indentation and some strings - Fixes tabs pointed out by the QA script. Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 73782 [details] [review] Bug 9701: (follow-up) Adjust tests Test plan: prove t/db_dependent/Biblio.t should be green Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 73783 [details] [review] Bug 9701: Followup to make test pass Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Hi Marcel, can you please take care of QA here as your patch builds on it?
(In reply to Katrin Fischer from comment #40) > Hi Marcel, can you please take care of QA here as your patch builds on it? Yes I will. They are related but do not really depend on each other.
Comment on attachment 73781 [details] [review] Bug 9701: (QA follow-up) Fix indentation and some strings This patch continually generates malloc errors and other git problems with me. It seems that we do not need it too. I have no qa warnings about tabs.
(In reply to M. Tompsett from comment #34) > t/db_dependent/Biblio.t is failing... there is a difference between unsafe > and empty call to GetMarcStructure for some reason. Haven't traced it back > enough. This is a side issue that is failing this bug. I'm having the impression that the third patch changing forlibrarian to 1 is not needed. We only need to flush the cache once ?
Created attachment 73843 [details] [review] Bug 9701: Configure default indicator This patch adds the possibility to define default indicators in the MARC frameworks. It adds 2 columns in the marc_tag_structure table in the database in order to accomplish this. All files that reference this table have also been updated to reflect these added fields. Test: Add or edit a MARC framework. In the Field list should be 2 extra columns. It should be possible to add default indicators (1 character) in these fields. Nothing else should have changed in the meantime. The default indicator values are not yet visible in the cataloguing module. The default values are also loaded in the cataloguing form. Test: Define default values in some MARC framework. Go to cataloguing and create a new record using this framework. Verify that the defined defaults are visible when set. Verify the default is empty (as before) if no default was set. Verify that if the default is changed, the record is saved with the manually changed value. Verify that upon changing such a new record, the manually set indicator value is used and not the default one from the framework. Don't forget to run database and database schema update Signed-off-by: Eugene Jose Espinoza <eugenegf@yahoo.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 73844 [details] [review] Bug 9701: (follow-up) Adjust tests Test plan: prove t/db_dependent/Biblio.t should be green Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 73845 [details] [review] Bug 9701: (QA follow-up) DBIx schema changes Changes in MarcTagStructure.pm Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Marcel de Rooy from comment #42) > Comment on attachment 73781 [details] [review] [review] > Bug 9701: (QA follow-up) Fix indentation and some strings > > This patch continually generates malloc errors and other git problems with > me. It seems that we do not need it too. I have no qa warnings about tabs. Strange, but I am not attached to it.
(In reply to Katrin Fischer from comment #47) > (In reply to Marcel de Rooy from comment #42) > > Comment on attachment 73781 [details] [review] [review] [review] > > Bug 9701: (QA follow-up) Fix indentation and some strings > > > > This patch continually generates malloc errors and other git problems with > > me. It seems that we do not need it too. I have no qa warnings about tabs. > > Strange, but I am not attached to it. Comment24 for instance
Patches pushed to master for 18.05. Congratulations Joachim for your first patch in!
Enhancement, not backported for 17.11.x