Bug 34707

Summary: Abstract intranet-tmpl/includes/*-search.inc
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: Architecture, internals, and plumbingAssignee: Bugs List <koha-bugs>
Status: In Discussion --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, m.de.rooy, martin.renvoize, nick, oleonard, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34624
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 34707: New common/header-search-tabs.inc
Bug 34707: Apply *-search.inc abstraction to cities

Description Pedro Amorim 2023-09-04 16:01:42 UTC
After looking at bug 34624, I thought we could abstract (thus reducing) a lot of code and, making it more maintainable.
Comment 1 Pedro Amorim 2023-09-04 16:04:27 UTC
Created attachment 155202 [details] [review]
Bug 34707: New common/header-search-tabs.inc

Proof of concept of new abstract header-search-tabs.inc to be called everywhere its needed
Comment 2 Pedro Amorim 2023-09-04 16:04:30 UTC
Created attachment 155203 [details] [review]
Bug 34707: Apply *-search.inc abstraction to cities

PROOF OF CONCEPT
I've applied this idea to cities to start discussion around this.
Comment 3 Pedro Amorim 2023-09-04 16:05:13 UTC
I believe we can abstract this even further but, for now, test plan is to apply patches and play around cities. Check that the top search bar works as before, etc.
Comment 4 Jonathan Druart 2023-09-06 07:10:50 UTC
I am not sure, that's a lot of different variables to define. Also some inc files are included from different places, you are going to duplicate this new big block.

Another approach (to test!) would be to define all the 'tabs' in a single include, then pass the idea from the tt?

include would contain all those blocks (here is one):
+    {
+        id          => 'city_search',
+        is_active   => 1,
+        is_allowed  => 1,
+        label       => 'Search cities',    #TODO:translate
+        icon        => 'fa-building',
+        form_action => undef,
+        form_id     => undef,
+        form_method => 'POST',
+        inputs      => [
+            {
+                id          => 'city_name_filter',
+                name        => 'city_name_filter',
+                type        => 'text',
+                value       => city_name_filter,
+                placeholder => 'City search',
+                class       => 'head-searchbox form-control',
+            }
+        ]
+    },

and tt would simply include:
[% INCLUDE header-search-tabs.inc tabs => ['city-search', 'circ_search', 'catalog_search'] %]
Comment 5 Jonathan Druart 2023-09-06 07:11:27 UTC
(In reply to Jonathan Druart from comment #4)
> I am not sure, that's a lot of different variables to define. Also some inc
> files are included from different places, you are going to duplicate this
> new big block.
> 
> Another approach (to test!) would be to define all the 'tabs' in a single
> include, then pass the idea from the tt?
> 
> include would contain all those blocks (here is one):
> +    {
> +        id          => 'city_search',
> +        is_active   => 1,
> +        is_allowed  => 1,
> +        label       => 'Search cities',    #TODO:translate
> +        icon        => 'fa-building',
> +        form_action => undef,
> +        form_id     => undef,
> +        form_method => 'POST',
> +        inputs      => [
> +            {
> +                id          => 'city_name_filter',
> +                name        => 'city_name_filter',
> +                type        => 'text',
> +                value       => city_name_filter,
> +                placeholder => 'City search',
> +                class       => 'head-searchbox form-control',
> +            }
> +        ]
> +    },
> 
> and tt would simply include:
> [% INCLUDE header-search-tabs.inc tabs => ['city-search', 'circ_search',
> 'catalog_search'] %]

s/idea/ids