[Koha-bugs] [Bug 14385] Extend OpacHiddenItems to allow specifying exempt borrower categories

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14385

M. Tompsett <mtompset at hotmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Patch doesn't apply         |ASSIGNED

--- Comment #143 from M. Tompsett <mtompset at hotmail.com> ---
(In reply to Josef Moravec from comment #142)
> these patchset doesn't apply.

Thanks for looking and noticing this.


> So just few comments from quick reading of the
> code:
> 
> You don't have to change signature of GetHiddenItemnumbers, you can ask for
> patron category here... And you can than leave the $search_context as is.

I can't recall the reason off the top of my head, and even after rebasing this
locally -- I have yet to test and put it back up. But I've got a feeling it had
something to so with the nesting of some search functions which were not easily
determined to be opac or intranet otherwise.


> Also the "opac" param in GetMarcBiblio and EmbedItemsInMarcBiblio is not
> needed, you can use C4::Context->interface when you need to know the
> actual interface.

It is better to pass it in my opinion to refactor the parameters list, so that
test functions don't require special mocking to get the C4::Context function to
return the correct value in different cases (like shell script utilities). And
this makes maintenance for other changes in the future easier for others, and
brings the code up to preferred coding style
(https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL16:_Hashrefs_should_be_used_as_arguments)


> And some squashing would be nice here ;)

I was tweaking, testing, fixing, repeating multiple times. Also, smaller
patches are easier to test individually. I will squash as much as possible. I
do, however, not want to lose the fact that Aleisha Amohia put in a massive
effort which helped me see how to solve my C4::Search mess problem that I had
hit while working on originally in bug 10589.

-- 
You are receiving this mail because:
You are watching all bug changes.