Bug 32911 - Remove ILL partner_code config from koha-conf.xml and turn it into a system preference
Summary: Remove ILL partner_code config from koha-conf.xml and turn it into a system p...
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Pedro Amorim
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 21548
Blocks: 34842
  Show dependency treegraph
 
Reported: 2023-02-08 16:52 UTC by Pedro Amorim
Modified: 2023-11-30 16:10 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
The partner_code element in the koha-conf.xml was replaced by a new system preference `ILLPartnerCode`. **Sponsored by** *PTFS Europe Ltd*
Version(s) released in:
23.11.00


Attachments
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference (7.96 KB, patch)
2023-02-08 16:59 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 32911: atomicupdate file (1.42 KB, patch)
2023-02-08 16:59 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 32911: atomicupdate file (1.42 KB, patch)
2023-02-08 17:07 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference (8.02 KB, patch)
2023-02-26 11:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32911: atomicupdate file (1.48 KB, patch)
2023-02-26 11:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32911: (follow-up) Move pref from Notifications to Features section (2.11 KB, patch)
2023-02-26 11:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference (8.02 KB, patch)
2023-05-04 14:54 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32911: Database update (1.47 KB, patch)
2023-05-04 14:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32911: (follow-up) Move pref from Notifications to Features section (2.18 KB, patch)
2023-05-04 14:55 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference (8.08 KB, patch)
2023-05-05 13:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: Database update (1.54 KB, patch)
2023-05-05 13:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Move pref from Notifications to Features section (2.24 KB, patch)
2023-05-05 13:15 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Remove a couple of cases from tests (1.54 KB, patch)
2023-05-05 13:25 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Remove partner_code (1.62 KB, patch)
2023-05-05 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 32911: (follow-up) Only move config value from koha-conf.xml if it exists (1.71 KB, patch)
2023-05-05 15:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference (8.08 KB, patch)
2023-05-10 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: Database update (1.54 KB, patch)
2023-05-10 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Move pref from Notifications to Features section (2.24 KB, patch)
2023-05-10 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Remove a couple of cases from tests (1.54 KB, patch)
2023-05-10 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Remove partner_code (1.69 KB, patch)
2023-05-10 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (follow-up) Only move config value from koha-conf.xml if it exists (1.77 KB, patch)
2023-05-10 15:50 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32911: (QA follow-up) Adjust tests (2.56 KB, patch)
2023-09-04 11:52 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Pedro Amorim 2023-02-08 16:52:08 UTC

    
Comment 1 Pedro Amorim 2023-02-08 16:59:22 UTC
Created attachment 146403 [details] [review]
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference

Preparation:
1) apply patch
2) run reset_all
3) enable ILLModule sys preference
4) install a backend (e.g. FreeForm)
5) go into system preferences and confirm that ILLPartnerCode exists and is the default inter-library loan (IL)

Test plan:
1) create an ILL request
2) ensure the patron of category 'IL' has a primary e-mail configured
3) click on "place request with partners"
4) verify that the patron(s) of said category are on the partners list

Upgrade test:
1) Remove the system preference from the database, run query:
delete from systempreferences where variable="ILLPartnerCode";
2) Check or edit the value or partner_code in koha-conf.xml and run atomicupdate:
installer/data/mysql/updatedatabase.pl
3) Confirm the value in koha-conf.xml has been successfully migrated into the system preference, run query:
select * from systempreferences where variable="ILLPartnerCode";

Optional tests:
- Set a patron category in the sys pref and then delete said patron category (needs to not have patrons in it)
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to the empty option
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to a category that has no patrons of its type (ktd examples: Board, or Library)
- Check the about page -> System information and confirm the warning message is correct
Comment 2 Pedro Amorim 2023-02-08 16:59:24 UTC Comment hidden (obsolete)
Comment 3 Pedro Amorim 2023-02-08 17:07:35 UTC
Created attachment 146405 [details] [review]
Bug 32911: atomicupdate file
Comment 4 Katrin Fischer 2023-02-26 11:02:21 UTC
Created attachment 147417 [details] [review]
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference

Preparation:
1) apply patch
2) run reset_all
3) enable ILLModule sys preference
4) install a backend (e.g. FreeForm)
5) go into system preferences and confirm that ILLPartnerCode exists and is the default inter-library loan (IL)

Test plan:
1) create an ILL request
2) ensure the patron of category 'IL' has a primary e-mail configured
3) click on "place request with partners"
4) verify that the patron(s) of said category are on the partners list

Upgrade test:
1) Remove the system preference from the database, run query:
delete from systempreferences where variable="ILLPartnerCode";
2) Check or edit the value or partner_code in koha-conf.xml and run atomicupdate:
installer/data/mysql/updatedatabase.pl
3) Confirm the value in koha-conf.xml has been successfully migrated into the system preference, run query:
select * from systempreferences where variable="ILLPartnerCode";

Optional tests:
- Set a patron category in the sys pref and then delete said patron category (needs to not have patrons in it)
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to the empty option
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to a category that has no patrons of its type (ktd examples: Board, or Library)
- Check the about page -> System information and confirm the warning message is correct

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2023-02-26 11:02:24 UTC
Created attachment 147418 [details] [review]
Bug 32911: atomicupdate file

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2023-02-26 11:02:26 UTC
Created attachment 147419 [details] [review]
Bug 32911: (follow-up) Move pref from Notifications to Features section

The new preference appeared under Notifications, but is better
suited for the Features section on the ILL tab in system
preferences.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Pedro Amorim 2023-02-26 13:19:02 UTC
(In reply to Katrin Fischer from comment #6)
> The new preference appeared under Notifications, but is better
> suited for the Features section on the ILL tab in system
> preferences.

Thank you Katrin, I wasn't sure where this should be placed.
Comment 8 Pedro Amorim 2023-04-11 17:04:28 UTC
Kyle, can we move this back to "Signed-off"?
Comment 9 Martin Renvoize 2023-04-11 17:09:27 UTC
Yes 🙂
Comment 10 Marcel de Rooy 2023-05-04 13:40:55 UTC
Applying: Bug 32911: (follow-up) Move pref from Notifications to Features section
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/interlibrary_loans.pref).
Comment 11 Katrin Fischer 2023-05-04 14:24:42 UTC
I'll try to fix.
Comment 12 Katrin Fischer 2023-05-04 14:54:59 UTC
Created attachment 150672 [details] [review]
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference

Preparation:
1) apply patch
2) run reset_all
3) enable ILLModule sys preference
4) install a backend (e.g. FreeForm)
5) go into system preferences and confirm that ILLPartnerCode exists and is the default inter-library loan (IL)

Test plan:
1) create an ILL request
2) ensure the patron of category 'IL' has a primary e-mail configured
3) click on "place request with partners"
4) verify that the patron(s) of said category are on the partners list

Upgrade test:
1) Remove the system preference from the database, run query:
delete from systempreferences where variable="ILLPartnerCode";
2) Check or edit the value or partner_code in koha-conf.xml and run atomicupdate:
installer/data/mysql/updatedatabase.pl
3) Confirm the value in koha-conf.xml has been successfully migrated into the system preference, run query:
select * from systempreferences where variable="ILLPartnerCode";

Optional tests:
- Set a patron category in the sys pref and then delete said patron category (needs to not have patrons in it)
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to the empty option
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to a category that has no patrons of its type (ktd examples: Board, or Library)
- Check the about page -> System information and confirm the warning message is correct

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2023-05-04 14:55:01 UTC
Created attachment 150673 [details] [review]
Bug 32911: Database update

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2023-05-04 14:55:04 UTC
Created attachment 150674 [details] [review]
Bug 32911: (follow-up) Move pref from Notifications to Features section

The new preference appeared under Notifications, but is better
suited for the Features section on the ILL tab in system
preferences.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Martin Renvoize 2023-05-05 13:15:17 UTC
Created attachment 150750 [details] [review]
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference

Preparation:
1) apply patch
2) run reset_all
3) enable ILLModule sys preference
4) install a backend (e.g. FreeForm)
5) go into system preferences and confirm that ILLPartnerCode exists and is the default inter-library loan (IL)

Test plan:
1) create an ILL request
2) ensure the patron of category 'IL' has a primary e-mail configured
3) click on "place request with partners"
4) verify that the patron(s) of said category are on the partners list

Upgrade test:
1) Remove the system preference from the database, run query:
delete from systempreferences where variable="ILLPartnerCode";
2) Check or edit the value or partner_code in koha-conf.xml and run atomicupdate:
installer/data/mysql/updatedatabase.pl
3) Confirm the value in koha-conf.xml has been successfully migrated into the system preference, run query:
select * from systempreferences where variable="ILLPartnerCode";

Optional tests:
- Set a patron category in the sys pref and then delete said patron category (needs to not have patrons in it)
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to the empty option
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to a category that has no patrons of its type (ktd examples: Board, or Library)
- Check the about page -> System information and confirm the warning message is correct

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2023-05-05 13:15:20 UTC
Created attachment 150751 [details] [review]
Bug 32911: Database update

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2023-05-05 13:15:23 UTC
Created attachment 150752 [details] [review]
Bug 32911: (follow-up) Move pref from Notifications to Features section

The new preference appeared under Notifications, but is better
suited for the Features section on the ILL tab in system
preferences.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2023-05-05 13:16:48 UTC
Discussed with Katrin, I have permission to QA some of the ILL bugs in the queue even though they're written by our team.  There are few people in a position to properly review and QA ILL so she's trusting me to be unbiased.

As such, this one is a pass, code works, it's a nice cleanup and the QA scripts are all happy too.

Passing QA
Comment 19 Martin Renvoize 2023-05-05 13:25:45 UTC
Created attachment 150753 [details] [review]
Bug 32911: (follow-up) Remove a couple of cases from tests

This patch simply removes the partner_code defaults from some tests
where it is no longer required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 20 Martin Renvoize 2023-05-05 13:26:44 UTC
I find it really odd that Illrequest::Config.pm has partner_code as a 'setter' as well as getter.. it's never actually used in that way, but it is tested for.

I reckon we can drop that functionality at a later date.
Comment 21 Martin Renvoize 2023-05-05 14:41:04 UTC
Doh.. I missed something (that Pedro asking a question just highlighted to me)

We also need to remove it from the installer configs.. i.e debian/templates and /etc
Comment 22 Pedro Amorim 2023-05-05 15:09:07 UTC
Created attachment 150768 [details] [review]
Bug 32911: (follow-up) Remove partner_code

from debian and /etc koha-conf.xml files
Comment 23 Pedro Amorim 2023-05-05 15:09:09 UTC
Created attachment 150769 [details] [review]
Bug 32911: (follow-up) Only move config value from koha-conf.xml if it exists
Comment 24 Martin Renvoize 2023-05-10 15:50:37 UTC
Created attachment 151011 [details] [review]
Bug 32911: Remove ILL partner_code config from koha-conf.xml and turn it into a system preference

Preparation:
1) apply patch
2) run reset_all
3) enable ILLModule sys preference
4) install a backend (e.g. FreeForm)
5) go into system preferences and confirm that ILLPartnerCode exists and is the default inter-library loan (IL)

Test plan:
1) create an ILL request
2) ensure the patron of category 'IL' has a primary e-mail configured
3) click on "place request with partners"
4) verify that the patron(s) of said category are on the partners list

Upgrade test:
1) Remove the system preference from the database, run query:
delete from systempreferences where variable="ILLPartnerCode";
2) Check or edit the value or partner_code in koha-conf.xml and run atomicupdate:
installer/data/mysql/updatedatabase.pl
3) Confirm the value in koha-conf.xml has been successfully migrated into the system preference, run query:
select * from systempreferences where variable="ILLPartnerCode";

Optional tests:
- Set a patron category in the sys pref and then delete said patron category (needs to not have patrons in it)
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to the empty option
- Check the about page -> System information and confirm the warning message is correct

- Set the sys pref to a category that has no patrons of its type (ktd examples: Board, or Library)
- Check the about page -> System information and confirm the warning message is correct

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 25 Martin Renvoize 2023-05-10 15:50:40 UTC
Created attachment 151012 [details] [review]
Bug 32911: Database update

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2023-05-10 15:50:43 UTC
Created attachment 151013 [details] [review]
Bug 32911: (follow-up) Move pref from Notifications to Features section

The new preference appeared under Notifications, but is better
suited for the Features section on the ILL tab in system
preferences.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2023-05-10 15:50:46 UTC
Created attachment 151014 [details] [review]
Bug 32911: (follow-up) Remove a couple of cases from tests

This patch simply removes the partner_code defaults from some tests
where it is no longer required.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2023-05-10 15:50:48 UTC
Created attachment 151015 [details] [review]
Bug 32911: (follow-up) Remove partner_code

from debian and /etc koha-conf.xml files

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2023-05-10 15:50:51 UTC
Created attachment 151016 [details] [review]
Bug 32911: (follow-up) Only move config value from koha-conf.xml if it exists

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Tomás Cohen Arazi 2023-09-01 14:44:34 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 31 Tomás Cohen Arazi 2023-09-04 11:52:22 UTC
Created attachment 155195 [details] [review]
Bug 32911: (QA follow-up) Adjust tests

This patch adjusts tests so they acknowledge the new syspref and the
fact `partner_code` will be skipped if found on the config file.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 32 Fridolin Somers 2023-09-08 05:57:23 UTC
Enhancement not pushed to 23.05.x
Comment 33 Jonathan Druart 2023-09-22 12:54:27 UTC
Looks like there are many wrong things here, please have a look at bug 34842.
Comment 34 Jonathan Druart 2023-09-22 13:01:01 UTC
(In reply to Jonathan Druart from comment #33)
> Looks like there are many wrong things here, please have a look at bug 34842.

s/many/a couple of/ ;)