From Koha administration: Patron categories .. / cgi-bin / koha / admin / categorie.pl? & op = add_form CategoryCode = [^] and Authorized values / cgi-bin/koha/admin/authorised_values.pl? op = & id = add_form and Patron attribute types gi-bin/koha/admin/patron-attr-types.pl? edit_attribute_type = op & code = [^] and Item types / cgi-bin/koha/admin/itemtypes.pl? op = add_form & itemtype = link each code to a branchcode list. Then the administrator can choose to link a code (category, attribute, value allowed) to a specific branchcode Interest: each librarie will see that its codes. Each librarie will be able to secure the access and the use of its codes in a network. The filter should apply to the following screens: MEMBERS .. / cgi-bin / koha / members / members-home.pl [^] .. / cgi-bin / koha / members / memberentry.pl? op = add & CategoryCode = [^] .. / cgi-bin / koha / members / memberentry.pl? op = add & CategoryCode = [^] PERIODICAL ... / cgi-bin / koha / serials / subscription-add.pl? op = & mod = SubscriptionId ... ACQUISITIONS ... / cgi-bin / koha / admin / aqbudgets.pl? & op = add_form budget_parent_id = 131 & budget_period_id = .. ... / cgi-bin / koha / acquisition / neworderempty.pl? OrderNumber = 26 & booksellerid = 2 & basketno = ... TOOLS .. / cgi-bin / koha / tools / modborrowers.pl SORT1, SORT2, borrower_attribute, CategoryCode, branchcode ... / cgi-bin / koha / tools / batchMod.p ==> Items.location ==> Items.notforloan ==> items.ccode CATALOGUING / cgi-bin/koha/cataloguing/additem.pl? biblionumber = / cgi-bin/koha/cataloguing/addbiblio.pl? biblionumber = 37202 & op = OPAC .. / cgi-bin / koha / opac-search.pl
Created attachment 8939 [details] [review] Bug 7919 : Display of values depending on the connexion librairy In a librairies network, we would like to declare specific values just for one (or more) librairy. Here we implement the ability to associate categories, patron attributes types and/or authorised_values with librairies (branches). This patch adds 3 new association tables: - categories_branches ( association table between categories and branches ) - authorised_values_branches ( association table between authorised_values and branches ) - borrower_attribute_types_branches (association table between borrower_attribute_types and branches ) Plan test: - Create (or modify) categories, patron attributes and authorised_values and link it with one (or more) library. - Set one of these librairies - Go to one of the multiple pages where this specific value must be displayed and check that it does appear. - Set a librairy not concerned. - Check on the same pages this value is doest not appear. A page list: cataloguing/addbiblio.pl cataloguing/additems.pl members/members-home.pl members/memberentry.pl acqui/neworderempty.pl tools/import_borrowers.pl and others ? Please say me if filters don't work on some pages. Take no account of filters on item types. They are not impletemented by this development
i have this error message when i go in ..cgi-bin/koha/members/members-home.pl Software error: Can't use an undefined value as an ARRAY reference at /home/koha/src/C4/Category.pm line 86.
Created attachment 9170 [details] [review] Bug 7919 :Bug 7919 : Display of values depending on the connexion librairy Display of values depending on the connexion librairy Rebased patch Stephane, I don't understand your error. Could you check if your database contains the 3 association tables added by this patch?
I do not have the 3 tables(categories_branches,authorised_values_branches,borrower_attribute_types_branches) in my database after apply the patch in a sandbox ; so I can not test. Can you give me the script mysql to add the tables in the database?
Created attachment 9973 [details] [review] Bug 7919 : Display of values depending on the connexion librairy Display of values depending on the connexion librairy Rebased patch
Patch tested with a sandbox, by Delaye Stephane <stephane.delaye@biblibre.com>
Created attachment 9984 [details] [review] Bug 7919 : Display of values depending on the connexion librairy In a librairies network, we would like to declare specific values just for one (or more) librairy. Here we implement the ability to associate categories, patron attributes types and/or authorised_values with librairies (branches). This patch adds 3 new association tables: - categories_branches ( association table between categories and branches ) - authorised_values_branches ( association table between authorised_values and branches ) - borrower_attribute_types_branches (association table between borrower_attribute_types and branches ) Plan test: - Create (or modify) categories, patron attributes and authorised_values and link it with one (or more) library. - Set one of these librairies - Go to one of the multiple pages where this specific value must be displayed and check that it does appear. - Set a librairy not concerned. - Check on the same pages this value is doest not appear. A page list: cataloguing/addbiblio.pl cataloguing/additems.pl members/members-home.pl members/memberentry.pl acqui/neworderempty.pl tools/import_borrowers.pl and others :) Please say me if filters don't work on some pages. Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com>
I'm all for having more granular control of system features on the branch level, but this patch seems to be doing it in a very ad-hoc, bolt-on way for only certain features. There are lots of other things that would be good to control on a per-branch level, and I'd like to see clean, well thought out approach that can handle these use cases and many others with one approach. While it's certainly more work to do it this way, it would lead to more uniform, clean and understandable code, with lower maintenance costs and better extensibility.
(In reply to comment #8) Hi Ian, Yes you are right, it is not very clean. But... it is Koha and I cannot find 10 days to rewrite completely an Authorised Values module. I don't know how filter authorised values functions of the branch level without adding a join on each SELECT query. The authorised value management is, for me, one of the most dirty part of Koha and it could be difficult to make something clean. Likewise, I do not know how it is possible to filter elements simply (here AV, borrower attributes, categories, what else do you think about ?), with the current code of Koha. > I'd like to see clean, well thought out approach that can handle these use cases and many others with one approach How do you see this approach ? Thanks for your review :)
I agree with Jonathan's comment here : it is as clean as possible with the current code imo. Rewritting it "from scratch" is another thing. Maybe, as we spoke of the Koha:: during the hackfest, that's something that could emerge here. But it's another topic. I propose to Switch back to "signed off"
Created attachment 10424 [details] [review] Bug 8272 - OPAC : date expiration format Adding TT date filter to output of expiration date on OPAC user page. Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com>
Comment on attachment 9973 [details] [review] Bug 7919 : Display of values depending on the connexion librairy Display of values depending on the connexion librairy obsoleted by signed off patch
Extensive testing this patch that has not been QAed yet, signed-off since 1 month and a half. QA Comments 1 Patch applies with a small conflict I've solved 2 Usability comment: If there is a library limit on a given value, there's nothing specific displayed on lists in admin. It would be worth to have a column with applicable branches for each value -when there are-. Not worth failing QA though 3 tested = updated a category only visible to librarians from library B. Being connected as librarian from library A, reaching members pages, and checked I can't see this category. 4 tested = adding a limit to afr authorised_values LANG, connected to 101$a (language in UNIMARC). In addbiblio.pl, all the lists appear as empty !!! switching back to master it work well. Seems it's because of the SQL that is built, that returns nothing: SELECT id,authorised_value, lib FROM authorised_values RIGHT JOIN authorised_values_branches ON ( id = av_id AND ( branchcode = 'BDP' OR branchcode IS NULL ) ) WHERE category = 'LANG' GROUP BY lib ORDER BY lib, lib_opac; 5 perlcritic, valid-templates, translatable-templates OK Without point 4, this would have earned QA, but with point 4, it's failed QA, sorry
Created attachment 11163 [details] [review] Bug 7919: Followup: using LEFT JOIN instead of JOIN for 2 files
Please test last patch on pages cataloguing/addbiblio.pl and tools/batchMod.pl
Created attachment 11168 [details] [review] Bug 7919: Followup: Add a column "Branches limitations" for the 3 pages
Sandbox test result: Something went wrong ! Applying: Bug 7919 : Display of values depending on the connexion librairy Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging C4/Budgets.pm Auto-merging C4/Input.pm Auto-merging C4/Koha.pm Auto-merging C4/Members.pm Auto-merging C4/Members/AttributeTypes.pm Auto-merging C4/Members/Attributes.pm Auto-merging admin/authorised_values.pl Auto-merging admin/categorie.pl CONFLICT (content): Merge conflict in admin/categorie.pl Auto-merging admin/patron-attr-types.pl Auto-merging cataloguing/addbiblio.pl Auto-merging cataloguing/additem.pl Auto-merging installer/data/mysql/kohastructure.sql Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt Failed to merge in the changes. Patch failed at 0001 Bug 7919 : Display of values depending on the connexion librairy When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Bug 7919 - Display of values depending on the connexion library Bug 7919 : Display of values depending on the connexion librairy Apply? [yn] Patch left in /tmp/Bug-7919--Display-of-values-depending-on-the-conne-hVli22.patch
Created attachment 11373 [details] [review] Bug 7919 : Display of values depending on the connexion library Rebased and squashed patch
test with a sandbox : a few points about this patch : 1. In tables patron category administration, authorized values,,patron attributes types, when appears "1 branches limitations" there's a link which point on the actual page : either it doesn't point on the good URL or it wasn't usefull. 2.In the table Patron attribute types, there's an inversion between column actions et column Branches limitations 3.In page members/memberentry.pl, there's no Drop-down menu for sort1 and sort2, so restrictions on Bsort1 and Bsort2 in authorized values can't be apply 4.In additem.pl, i try a restriction on LOST, it works but the value isn't saved 5.In acqui/neworderempty.pl, restrictions about localisation don't work 6.There's no Drop-down menu in tools/import_borrowers.pl Everything is fine in tools/batchMod.pl and with authorised_values LANG in page cataloguing/addbiblio.pl
Created attachment 11751 [details] [review] Bug 7919: Followup FIX several behaviours Hi Sonia, (In reply to comment #20) > test with a sandbox : > a few points about this patch : > 1. In tables patron category administration, authorized values,,patron > attributes types, when appears "1 branches limitations" there's a link which > point on the actual page : either it doesn't point on the good URL or it > wasn't usefull. Ok, I replaced the "a href" link with a "span" to avoid ambiguity. The limitations appear with a mouseover. > 2.In the table Patron attribute types, there's an inversion between column > actions et column Branches limitations Done. > 3.In page members/memberentry.pl, there's no Drop-down menu for sort1 and > sort2, so restrictions on Bsort1 and Bsort2 in authorized values can't be > apply A bug is corrected. It would work now. > 4.In additem.pl, i try a restriction on LOST, it works but the value isn't > saved Hum... It works for me. > 5.In acqui/neworderempty.pl, restrictions about localisation don't work Done. > 6.There's no Drop-down menu in tools/import_borrowers.pl Normaly you can filter on Patron Categories. Now you have Sort1 and 2 too. Thank you for testing !
Hi Jonathan, > 6.There's no Drop-down menu in tools/import_borrowers.pl Normaly you can filter on Patron Categories. Now you have Sort1 and 2 too. -> Can you just check with my sandbox, but for me, there's no drop-down menu to choose some value in this page http://pro.salers.biblibre.com/cgi-bin/koha/tools/import_borrowers.pl But, I can miss somthing. Thanks !!
(In reply to comment #22) > -> Can you just check with my sandbox, but for me, there's no drop-down menu > to choose some value in this page > http://pro.salers.biblibre.com/cgi-bin/koha/tools/import_borrowers.pl > But, I can miss somthing. ho, sorry ! It is not import_borrowers.pl but tools/modborrowers.pl :-/
For point 4 "In additem.pl, i try a restriction on LOST, it works but the value isn't saved", it's the same this another patch, it must be a problem of settings. Ok for the page tools/modborrowers.pl So everything is ok for me !
Changing target version so this bug report will show up in the list of 3.12-targeted features and doesn't get lost in the shuffle during feature freeze.
QA comment: * from comment 27 I understand that Koha Team Lyon 3 sign-off the patch, I add their signature * squashed the main patch and the follow-up * koha-qa.pl complain because of POD in CA/Members/Attributes.pm, but the problem was already here. I fixed it during QA * koha-qa.pl complain because of tabs in admin/categorie.pl, but they were already here before, not failing QA for that * koha-qa complain because of "my" variable $sth masks earlier declaration in same scope "my" variable $dbh masks earlier declaration in same scope in admin/categorie.pl, that's added byt this patch, I've fixed that too * checked that things worked, could not find anything wrong * If I had written it myself, I would have added a 'branch' column, with a '*' for default, and used the same rule as for circulation rules, but the way it's done is OK. It will just make caching harder to develop (but that's another topic) * in templates, I feel the span with explanations could be styled a little bit better, but won't fail QA for that, someone (Owen ?) will for sure improve the look if needed Passed QA !
Created attachment 13736 [details] [review] Bug 7919 : Display of values depending on the connexion library In a librairies network, we would like to declare specific values just for one (or more) library. Here we implement the ability to associate categories, patron attributes types and/or authorised_values with librairies (branches). This patch adds 3 new association tables: - categories_branches ( association table between categories and branches ) - authorised_values_branches ( association table between authorised_values and branches ) - borrower_attribute_types_branches (association table between borrower_attribute_types and branches ) Plan test: - Create (or modify) categories, patron attributes and authorised_values and link it with one (or more) library. - Set one of these librairies - Go to one of the multiple pages where this specific value must be displayed and check that it does appear. - Set a library not concerned. - Check on the same pages this value is doest not appear. A page list: cataloguing/addbiblio.pl cataloguing/additems.pl members/members-home.pl members/memberentry.pl acqui/neworderempty.pl tools/import_borrowers.pl and others :) Please say me if filters don't work on some pages. Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com> Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Could I please get another signoff on this patch? It touches some rather core functionality so I'd feel better if it had a little more testing, since I don't use the Acquisitions or Patrons modules very heavily in my day-to-day work.
Hi, I tried to test this patch, but I don't think it's working. After applying the patch, I went to Patron Categories and choose to edit an existing patron category. Under "Branch Limitation," I clicked on one branch name, then clicked save. But when the Patron Category Administration page came back up, it still shows no branch limitation. I then went to Patron Attribute Types and chose to edit an existing attribute. The same thing happened. After choosing a branch and clicking on save, the Patron Attribute Types page reloaded with a message: Modified patron attribute type "HBR" but it still says no branch limitation in the column below. Next, I clicked on Patrons in the top menu bar and got this software error: Can't use an undefined value as an ARRAY reference at /home/melia/kohaclone/C4/Category.pm line 89. For help, please send mail to the webmaster (webmaster@ByWater-Test), giving this error message and the time and date of the error. Hope that helps! Thanks, Melia
(In reply to comment #29) > Hi, > > I tried to test this patch, but I don't think it's working. Are you sure your database had been updated ? This patch add the following tables: categories_branches authorised_values_branches borrower_attribute_types_branches
Hi Melia, Thanks for testing. I tested again and it works for me. Could you please check if the 3 queries of the updatedatabase (3 new tables: borrower_attribute_types_branches, categories_branches and authorised_values_branches) are correctly executed ? When you save your changes, there is certainly an error in your Koha log, could you please copy it here ?
Ok, thanks! Updatedatabase was the problem. I've run ./installer/data/mysql/updatedatabase.pl and things are looking better now. Here is what I have done to test: 1. Modified two existing patron categories ("Board" and "Kid"). I limited "Board" to Centerville branch and "Kid" to Fairview branch. 2. Modified existing patron attributes ("Driver's License" and "Resident"). I limited "Driver's License" to Midway Branch and "Resident" to Centerville branch. 3. Created two new authorized values (one for the MARC 504 field, and a new collection code) and limited them both to Centerville branch. Then I set the library as Centerville branch. - I went to cataloging to add a new bib record and verified that I can see my new MARC 504 field authorized value in the dropdown menu there. - I searched the catalog for an existing Centerville item to edit and verified that I can see my new collection code authorized value there. - I added a new patron and verified that I can see "Board" as a possible patron category but cannot see "Kid" as a possible patron category - I edited an existing Centerville patron record and verified that I can see "Resident" under Patron Attributes but cannot see "Driver's License" - I went to Acquisitions and created a new order, but I'm not sure what to look at on this page, sorry... - I went to Tools > Import Patrons, but I'm not sure what to look for on this page either. I can see patron_attributes as a choice here, but it doesn't list the specific attributes, so I can't verify whether the one that is limited to this branch shows up or not. Then I set the library as Midway branch. - I went to cataloging to add a new bib record verified that I CANNOT see my new MARC 504 field authorized value there (correct - it was limited to Centerville branch). - I searched the catalog for an existing Midway item to edit and verified that I CANNOT see my new collection code authorized value there (correct - it was limited to Centerville branch). - I added a new patron and verified that I CANNOT see "Board" or "Kid" as possible patron categories (correct - "Board" is limited to Centerville branch, and "Kid" is limited to Fairview branch.) - I edited an existing Midway patron record and verified that I CANNOT see "Resident" under Patron Attributes but can see "Driver's License" (correct - "Resident" is limited to Centerville, but "Driver's License" is limited to Midway) For the pages I was able to test (cataloguing and members), this seems to be working well. I will add my sign off, with the understanding that I did not test acqui/neworderempty.pl or tools/import_borrowers.pl. Thanks, Melia
Created attachment 13862 [details] [review] [Signed off] Bug 7919 : Display of values depending on the connexion library In a librairies network, we would like to declare specific values just for one (or more) library. Here we implement the ability to associate categories, patron attributes types and/or authorised_values with librairies (branches). This patch adds 3 new association tables: - categories_branches ( association table between categories and branches ) - authorised_values_branches ( association table between authorised_values and branches ) - borrower_attribute_types_branches (association table between borrower_attribute_types and branches ) Plan test: - Create (or modify) categories, patron attributes and authorised_values and link it with one (or more) library. - Set one of these librairies - Go to one of the multiple pages where this specific value must be displayed and check that it does appear. - Set a library not concerned. - Check on the same pages this value is doest not appear. A page list: cataloguing/addbiblio.pl cataloguing/additems.pl members/members-home.pl members/memberentry.pl acqui/neworderempty.pl tools/import_borrowers.pl and others :) Please say me if filters don't work on some pages. Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com> Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
Oh also, one tiny thing - if you choose to limit to just one branch, it shows up as "1 branches limitations," which should be "1 branch limitation."
(In reply to comment #32) > For the pages I was able to test (cataloguing and members), this seems to be > working well. I will add my sign off, with the understanding that I did not > test acqui/neworderempty.pl or tools/import_borrowers.pl. It is not tools/import_borrowers.pl but tools/modborrowers.pl. I did not update the commit message, sorry. In acqui/neworderempty you can test the statistics (1 and 2) values after selecting a fund (this one has to be linked to 1 or 2 authorized value(s)). (In reply to comment #34) > Oh also, one tiny thing - if you choose to limit to just one branch, it > shows up as "1 branches limitations," which should be "1 branch limitation." A patch is coming.
Created attachment 13867 [details] [review] Bug 7919 : Display of values depending on the connexion library In a librairies network, we would like to declare specific values just for one (or more) library. Here we implement the ability to associate categories, patron attributes types and/or authorised_values with librairies (branches). This patch adds 3 new association tables: - categories_branches ( association table between categories and branches ) - authorised_values_branches ( association table between authorised_values and branches ) - borrower_attribute_types_branches (association table between borrower_attribute_types and branches ) Plan test: - Create (or modify) categories, patron attributes and authorised_values and link it with one (or more) library. - Set one of these librairies - Go to one of the multiple pages where this specific value must be displayed and check that it does appear. - Set a library not concerned. - Check on the same pages this value is doest not appear. A page list: cataloguing/addbiblio.pl cataloguing/additems.pl members/members-home.pl members/memberentry.pl acqui/neworderempty.pl tools/modborrowers.pl and others :) Please say me if filters don't work on some pages. Signed-off-by: Delaye Stephane <stephane.delaye@biblibre.com> Signed-off-by: Koha Team Lyon 3 <koha@univ-lyon3.fr> Signed-off-by: Paul Poulain <paul.poulain@biblibre.com> Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
I updated the commit message in the last patch
Created attachment 13868 [details] [review] Bug 7919: Followup: Fix plural and GetAuthvalueDropbox Remove plural if there is just one branch limitation (authorised_values.tt, categorie.tt and patron-attr-types.tt) FIX a bug in GetAuthvalueDropbox (called by acqui/fetch_sort_dropbox.pl)
The last patch needs a SO. There was a bug in the neworderempty page, fixed. Plurals in 3 pages, fixed.
Thanks, Jonathan! I have now tested tools/modborrowers.pl, and the patron categories on that page look good. I also applied your follow up and tested acqui/neworderempty.pl using the Asort1 and Asort2 for Statistics. This works perfectly as well. And thanks for the "1 branch limitation" correction. I can now call this one completely signed off!
Created attachment 13891 [details] [review] [Signed off] Bug 7919: Followup: Fix plural and GetAuthvalueDropbox Remove plural if there is just one branch limitation (authorised_values.tt, categorie.tt and patron-attr-types.tt) FIX a bug in GetAuthvalueDropbox (called by acqui/fetch_sort_dropbox.pl) Signed-off-by: Melia Meggs <melia@bywatersolutions.com>
passed QA again, the follow-up is small, not a big deal from QA POV
I will be pushing these patches shortly, since they work great, and add a useful feature. However, I'd like to suggest a follow-up. Right now the branch limitations are based on the staff user's branch, but it seems to me that it would be more helpful if the limitations were based on the branch of the object being edited.
This patch has been pushed to master.
(In reply to comment #43) > I will be pushing these patches shortly, since they work great, and add a > useful feature. However, I'd like to suggest a follow-up. Right now the > branch limitations are based on the staff user's branch, but it seems to me > that it would be more helpful if the limitations were based on the branch of > the object being edited. I've been told that comment was unclear. By "staff user's branch," I mean "branch currently set by the staff user," not "staff user's home branch."
(In reply to comment #45) > (In reply to comment #43) > > I will be pushing these patches shortly, since they work great, and add a > > useful feature. However, I'd like to suggest a follow-up. Right now the > > branch limitations are based on the staff user's branch, but it seems to me > > that it would be more helpful if the limitations were based on the branch of > > the object being edited. > > I've been told that comment was unclear. By "staff user's branch," I mean > "branch currently set by the staff user," not "staff user's home branch." Are you saying we should change the behaviour or add a syspref to let the library choose ? The library that sponsored this development wanted that way, and I think most of our libraries will be happy to have it that way. OTOH, I think that, it's like the HomeOrHoldingBranch syspref, some libraries could be interested by the other behaviour (ie= AV depends on the object library, not staff user's branch. I'm not sure I see one of libary needing it this way, but why not. (just to be clear: I don't thin we will do that in a predictable time, as none of our library is requesting it for now)
(In reply to comment #46) > (In reply to comment #45) > > (In reply to comment #43) > > > I will be pushing these patches shortly, since they work great, and add a > > > useful feature. However, I'd like to suggest a follow-up. Right now the > > > branch limitations are based on the staff user's branch, but it seems to me > > > that it would be more helpful if the limitations were based on the branch of > > > the object being edited. > > > > I've been told that comment was unclear. By "staff user's branch," I mean > > "branch currently set by the staff user," not "staff user's home branch." > > Are you saying we should change the behaviour or add a syspref to let the > library choose ? The library that sponsored this development wanted that > way, and I think most of our libraries will be happy to have it that way. Probably a syspref would be useful. As I said, this is not something to block the feature going in, just a potentially useful addition. > OTOH, I think that, it's like the HomeOrHoldingBranch syspref, some > libraries could be interested by the other behaviour (ie= AV depends on the > object library, not staff user's branch. I'm not sure I see one of libary > needing it this way, but why not. > (just to be clear: I don't thin we will do that in a predictable time, as > none of our library is requesting it for now)
Jenkins caught a problem in t/Members_AttributeTypes.t that I missed (this will teach me to check my PERL5LIB environment variable!). Could I please get a follow-up that fixes this test?
Created attachment 13901 [details] [review] Bug 7919: Fix unit test (Members_AttributeTypes.t) - Reintroduce the class column in a select statement (was present before this development) - FIX UT: If there are no arg passed in parameter, the routine calls C4::Context->userenv which call an execute statement. It consumes the mocked resultset
Created attachment 13952 [details] [review] Bug 7919: Fix unit test (Members_AttributeTypes.t) - Reintroduce the class column in a select statement (was present before this development) - FIX UT: If there are no arg passed in parameter, the routine calls C4::Context->userenv which call an execute statement. It consumes the mocked resultset Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 13953 [details] [review] Bug 7919: Fix unit test (Members_AttributeTypes.t) [Signed-Off] [Passed QA] - Reintroduce the class column in a select statement (was present before this development) - FIX UT: If there are no arg passed in parameter, the routine calls C4::Context->useren Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Elliott Davis <elliott@test.bywatersolutions.com> All tests successful. Files=1, Tests=9, 0 wallclock secs ( 0.02 usr 0.01 sys + 0.16 cusr 0.06 csys = 0.25 CPU) Result: PASS
Thanks for the follow-up. Pushed to master.
This has broken the "new patron" menu on members-home.pl. categorycodes are not being passed to the template, so although all the category descriptions are listed, each link is the same: /cgi-bin/koha/members/memberentry.pl?op=add&categorycode=
Display of patron categories in patron search results (member.pl) is now broken as well.
I confirm Owen's two bugs.
Created attachment 14162 [details] [review] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation).
Created attachment 14163 [details] [review] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation).
Created attachment 14184 [details] [review] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation). Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 14287 [details] [review] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation). same for the GetBorrowercategoryList routine (some category codes are undefined)
I found another place where the problem exists. I increase the priority to critical
Created attachment 14350 [details] [review] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation). same for the GetBorrowercategoryList routine (some category codes are undefined) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 14372 [details] [review] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation). same for the GetBorrowercategoryList routine (some category codes are undefined) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Liz Rea <liz@catalyst.net.nz> Signed off for also fixing problems with patron searching noted by Katrin.
Created attachment 14377 [details] [review] [SIGNED-OFF] Bug 7919: FIX the "all" categories method The C4::Category->all method must return fields from the categories table only. Without this patch, there is 2 "categorycode" values, the second one is sometimes undef (if no branch limitation). same for the GetBorrowercategoryList routine (some category codes are undefined) Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Liz Rea <liz@catalyst.net.nz> Signed off for also fixing problems with patron searching noted by Katrin. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Problems fixed by this patch include: - on patron search result list the category description is shown again - it's possible to limit a patron search to a patron category again All tests pass.
Doesn't apply for 3.10.x but I don't think it is needed, if it is please submit a patch formatted for 3.10.x