[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Martin Renvoize  changed:

   What|Removed |Added

 Blocks||22883


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22883
[Bug 22883] Itemtypes table looks strange with multiple translations
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Martin Renvoize  changed:

   What|Removed |Added

 Blocks||22882


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22882
[Bug 22882] ItemType translation modal is not consistent with other modals
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Nick Clemens  changed:

   What|Removed |Added

 Blocks||19671


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19671
[Bug 19671] Circulation wizard / issues_stats.pl does not populate itemtype
descriptions correctly
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Nick Clemens  changed:

   What|Removed |Added

 CC||n...@bywatersolutions.com

--- Comment #58 from Nick Clemens  ---
*** Bug 18464 has been marked as a duplicate of this bug. ***

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #57 from Jonathan Druart  
---
(In reply to Jonathan Druart from comment #56)
> Created attachment 62470 [details] [review]
> Bug 17835: Re-add the use Koha in orderreceive
> 
> It is still used for getitemtypeinfo, at least.

Pushed to master!

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #56 from Jonathan Druart  
---
Created attachment 62470
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62470=edit
Bug 17835: Re-add the use Koha in orderreceive

It is still used for getitemtypeinfo, at least.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #55 from Jonathan Druart  
---
(In reply to Tomás Cohen Arazi from comment #54)
> Created attachment 62370 [details] [review]
> Bug 17835: (followup) Make TestBuilder skip views
> 
> Signed-off-by: Tomas Cohen Arazi 

Pushed to master!

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #54 from Tomás Cohen Arazi  ---
Created attachment 62370
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62370=edit
Bug 17835: (followup) Make TestBuilder skip views

Signed-off-by: Tomas Cohen Arazi 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Katrin Fischer  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 CC||katrin.fisc...@bsz-bw.de
 Status|Pushed to Master|RESOLVED

--- Comment #53 from Katrin Fischer  ---
This won't get ported back to 16.11.x as it is an enhancement.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Kyle M Hall  changed:

   What|Removed |Added

 CC||k...@bywatersolutions.com
 Status|Passed QA   |Pushed to Master

--- Comment #52 from Kyle M Hall  ---
Pushed to master for 17.05, thanks Jonathan, Marcel!

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

 Status|Signed Off  |Passed QA

--- Comment #51 from Marcel de Rooy  ---
QA Comment:
Passing QA on this patch set.
I am not really a fan of the resulting DBIx tric to get this working. Now each
search with localization will need an additional view and an ugly relationship
hack in the schema file. But I have no other solution too. Tested the sub with
no, one or several languages in localization and it works perfect.

Also want to mention that the unit test t/db_dependent/Koha/ItemTypes.t is much
too simplistic. Adding 2/3 English localization is not sufficient to really
prove that the sub works. You cannot test localization with only one language,
and English is a bad example in Koha :) We need to mock getlanguage and put a
few different languages in localization to really see if it works. (I did that
now manually.)

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #50 from Marcel de Rooy  ---
(In reply to Marcel de Rooy from comment #49)
> Another observation: we should also translate the item type description in
> the items table of additem.pl.
> Now you have an English description in the items table and a vernacular
> description in the edited item.

Well, that was too fast. Some refresh pops up the right language now.
So forget this one.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #49 from Marcel de Rooy  ---
Another observation: we should also translate the item type description in the
items table of additem.pl.
Now you have an English description in the items table and a vernacular
description in the edited item.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #48 from Marcel de Rooy  ---
I am not really impressed by the DBIx constructs here. Still trying if we could
simplify a bit here..

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #61969|0   |1
is obsolete||

--- Comment #44 from Marcel de Rooy  ---
Created attachment 62159
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62159=edit
Bug 17835: Remove the subroutine GetItemTypes

At this point the subroutine is no longer in used.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #61971|0   |1
is obsolete||

--- Comment #46 from Marcel de Rooy  ---
Created attachment 62161
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62161=edit
Bug 17835: Mock language pref value

That way if prefs contain other languages, the test will still pass.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #47 from Marcel de Rooy  ---
Created attachment 62162
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62162=edit
Bug 17835: [QA Follow-up] Fix opac-search.pl

Resolves:
Global symbol "$itemtypes_nocategory" requires explicit package name.

Signed-off-by: Marcel de Rooy 
Tested the added line with a debug statement. See itemtype facets in the
search results.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #61970|0   |1
is obsolete||

--- Comment #45 from Marcel de Rooy  ---
Created attachment 62160
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62160=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

Bug 17835: follow-up

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #61968|0   |1
is obsolete||

--- Comment #43 from Marcel de Rooy  ---
Created attachment 62158
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62158=edit
Bug 17835: Replace GetItemTypes with Koha::ItemTypes

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #61967|0   |1
is obsolete||

--- Comment #42 from Marcel de Rooy  ---
Created attachment 62157
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62157=edit
Bug 17835: Do not reselect translated_description if comes from
search_with_localization

If the Koha::ItemType object has been instanciated from a call to
Koha::ItemTypes->search_with_localization, we already have the
translated_description value. So there is no need to fetch it again from
the DB. This is what this trick does: if the translated_description
column exist in the DBIx::Class result source's column list, that means
the value has already been retrieved.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

  Attachment #61966|0   |1
is obsolete||

--- Comment #41 from Marcel de Rooy  ---
Created attachment 62156
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=62156=edit
Bug 17835: Create a ItemtypeLocalization view

To properly move C4::Koha::GetItemTypes to Koha::ItemTypes we need
DBIx::Class to make a join on the localization table to retrieve the
possible translated description of the item types.
To do so there are 2 possibilities. The first one would have been to
rename the localization table to something like itemtype_localization.
That way we could have had a relationship between
itemtype_localization.code and itemtypes.itemtype
That would have meant to create one table per "entity" (here an entity
is itemtype) we allow the translability. There are pros and cons for
this choice, so I opt for another solution.
The other solution is to create a view on top of this localization
table. With this new view we can define the missing relationship.

That sounds like a quite clean solution and easy to implement.
Once we have this relationship, the
Koha::ItemTypes->search_with_localization will join on this view an
return the same result as GetItemTypes( style => 'array' ).
To replace
GetItemtypes( style => 'hash' )
which is the default behavior of this subroutine, we can do something like:
my $itemtypes = Koha::ItemTypes->search_with_localization;
my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed };

This patchset must not introduce big changes but it changes certain
behaviors (which were wrong) in some scripts. Indeed sometimes the
descriptions of the item types were not the translated ones. Moreover it
happens that the item types displayed in a dropdown list were not
ordered by translated description, but by description of code
(itemtypes.itemtype value). These 2 behaviors are what we expect.

Test plan:
Bugs will be hard to catch since these patches change a lot of file, it
will be easier to read the diff and catch possible typos or logic
errors.
However signoffers can focus on modified files and the item types
values.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

Signed-off-by: Marcel de Rooy 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

   Patch complexity|--- |Medium patch
 QA Contact|testo...@bugs.koha-communit |m.de.r...@rijksmuseum.nl
   |y.org   |

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #40 from Marcel de Rooy  ---
QA: Working on this one now.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61700|0   |1
is obsolete||

--- Comment #39 from Lari Taskula  ---
Created attachment 61971
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61971=edit
Bug 17835: Mock language pref value

That way if prefs contain other languages, the test will still pass.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

 CC||lari.task...@jns.fi
 Status|Patch doesn't apply |Signed Off

--- Comment #38 from Lari Taskula  ---
Rebased on top of master (fixed 3 "use" conflicts, 1 per file in
catalogue/detail.pl, opac/opac-detail.pl, opac/opac-user.pl). Switching back to
Signed Off.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61699|0   |1
is obsolete||

--- Comment #37 from Lari Taskula  ---
Created attachment 61970
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61970=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

Bug 17835: follow-up

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61698|0   |1
is obsolete||

--- Comment #36 from Lari Taskula  ---
Created attachment 61969
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61969=edit
Bug 17835: Remove the subroutine GetItemTypes

At this point the subroutine is no longer in used.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61696|0   |1
is obsolete||

--- Comment #34 from Lari Taskula  ---
Created attachment 61967
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61967=edit
Bug 17835: Do not reselect translated_description if comes from
search_with_localization

If the Koha::ItemType object has been instanciated from a call to
Koha::ItemTypes->search_with_localization, we already have the
translated_description value. So there is no need to fetch it again from
the DB. This is what this trick does: if the translated_description
column exist in the DBIx::Class result source's column list, that means
the value has already been retrieved.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61697|0   |1
is obsolete||

--- Comment #35 from Lari Taskula  ---
Created attachment 61968
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61968=edit
Bug 17835: Replace GetItemTypes with Koha::ItemTypes

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Lari Taskula  changed:

   What|Removed |Added

  Attachment #61695|0   |1
is obsolete||

--- Comment #33 from Lari Taskula  ---
Created attachment 61966
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61966=edit
Bug 17835: Create a ItemtypeLocalization view

To properly move C4::Koha::GetItemTypes to Koha::ItemTypes we need
DBIx::Class to make a join on the localization table to retrieve the
possible translated description of the item types.
To do so there are 2 possibilities. The first one would have been to
rename the localization table to something like itemtype_localization.
That way we could have had a relationship between
itemtype_localization.code and itemtypes.itemtype
That would have meant to create one table per "entity" (here an entity
is itemtype) we allow the translability. There are pros and cons for
this choice, so I opt for another solution.
The other solution is to create a view on top of this localization
table. With this new view we can define the missing relationship.

That sounds like a quite clean solution and easy to implement.
Once we have this relationship, the
Koha::ItemTypes->search_with_localization will join on this view an
return the same result as GetItemTypes( style => 'array' ).
To replace
GetItemtypes( style => 'hash' )
which is the default behavior of this subroutine, we can do something like:
my $itemtypes = Koha::ItemTypes->search_with_localization;
my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed };

This patchset must not introduce big changes but it changes certain
behaviors (which were wrong) in some scripts. Indeed sometimes the
descriptions of the item types were not the translated ones. Moreover it
happens that the item types displayed in a dropdown list were not
ordered by translated description, but by description of code
(itemtypes.itemtype value). These 2 behaviors are what we expect.

Test plan:
Bugs will be hard to catch since these patches change a lot of file, it
will be easier to read the diff and catch possible typos or logic
errors.
However signoffers can focus on modified files and the item types
values.

Signed-off-by: Josef Moravec 

Signed-off-by: Lari Taskula 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Marcel de Rooy  changed:

   What|Removed |Added

 Status|Signed Off  |Patch doesn't apply
 CC||m.de.r...@rijksmuseum.nl

--- Comment #32 from Marcel de Rooy  ---
Just a suggestion: If you rebase this one next Thursday, I will resume next
Friday here?

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #29 from Jonathan Druart  
---
Created attachment 61698
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61698=edit
Bug 17835: Remove the subroutine GetItemTypes

At this point the subroutine is no longer in used.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #60048|0   |1
is obsolete||
  Attachment #60049|0   |1
is obsolete||
  Attachment #60914|0   |1
is obsolete||
  Attachment #60915|0   |1
is obsolete||
  Attachment #60916|0   |1
is obsolete||
  Attachment #60917|0   |1
is obsolete||

--- Comment #26 from Jonathan Druart  
---
Created attachment 61695
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61695=edit
Bug 17835: Create a ItemtypeLocalization view

To properly move C4::Koha::GetItemTypes to Koha::ItemTypes we need
DBIx::Class to make a join on the localization table to retrieve the
possible translated description of the item types.
To do so there are 2 possibilities. The first one would have been to
rename the localization table to something like itemtype_localization.
That way we could have had a relationship between
itemtype_localization.code and itemtypes.itemtype
That would have meant to create one table per "entity" (here an entity
is itemtype) we allow the translability. There are pros and cons for
this choice, so I opt for another solution.
The other solution is to create a view on top of this localization
table. With this new view we can define the missing relationship.

That sounds like a quite clean solution and easy to implement.
Once we have this relationship, the
Koha::ItemTypes->search_with_localization will join on this view an
return the same result as GetItemTypes( style => 'array' ).
To replace
GetItemtypes( style => 'hash' )
which is the default behavior of this subroutine, we can do something like:
my $itemtypes = Koha::ItemTypes->search_with_localization;
my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed };

This patchset must not introduce big changes but it changes certain
behaviors (which were wrong) in some scripts. Indeed sometimes the
descriptions of the item types were not the translated ones. Moreover it
happens that the item types displayed in a dropdown list were not
ordered by translated description, but by description of code
(itemtypes.itemtype value). These 2 behaviors are what we expect.

Test plan:
Bugs will be hard to catch since these patches change a lot of file, it
will be easier to read the diff and catch possible typos or logic
errors.
However signoffers can focus on modified files and the item types
values.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #30 from Jonathan Druart  
---
Created attachment 61699
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61699=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

Bug 17835: follow-up

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #28 from Jonathan Druart  
---
Created attachment 61697
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61697=edit
Bug 17835: Replace GetItemTypes with Koha::ItemTypes

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #31 from Jonathan Druart  
---
Created attachment 61700
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61700=edit
Bug 17835: Mock language pref value

That way if prefs contain other languages, the test will still pass.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #27 from Jonathan Druart  
---
Created attachment 61696
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=61696=edit
Bug 17835: Do not reselect translated_description if comes from
search_with_localization

If the Koha::ItemType object has been instanciated from a call to
Koha::ItemTypes->search_with_localization, we already have the
translated_description value. So there is no need to fetch it again from
the DB. This is what this trick does: if the translated_description
column exist in the DBIx::Class result source's column list, that means
the value has already been retrieved.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 Status|Patch doesn't apply |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Tomás Cohen Arazi  changed:

   What|Removed |Added

 CC||tomasco...@gmail.com
 Status|Signed Off  |Patch doesn't apply

--- Comment #25 from Tomás Cohen Arazi  ---
Patches don't apply cleanly

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #60051|0   |1
is obsolete||

--- Comment #22 from Jonathan Druart  
---
Created attachment 60915
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60915=edit
Bug 17835: Remove the subroutine GetItemTypes

At this point the subroutine is no longer in used.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #60052|0   |1
is obsolete||

--- Comment #23 from Jonathan Druart  
---
Created attachment 60916
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60916=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

Bug 17835: follow-up

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #60053|0   |1
is obsolete||

--- Comment #24 from Jonathan Druart  
---
Created attachment 60917
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60917=edit
Bug 17835: Mock language pref value

That way if prefs contain other languages, the test will still pass.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #60050|0   |1
is obsolete||

--- Comment #21 from Jonathan Druart  
---
Created attachment 60914
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60914=edit
Bug 17835: Replace GetItemTypes with Koha::ItemTypes

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17835
Bug 17835 depends on bug 17627, which changed state.

Bug 17627 Summary: Move C4::Koha::GetItemTypesByCategory to Koha::ItemTypes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17627

   What|Removed |Added

 Status|Pushed to Master|RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #60046|0   |1
is obsolete||

--- Comment #20 from Josef Moravec  ---
Created attachment 60053
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60053=edit
Bug 17835: Mock language pref value

That way if prefs contain other languages, the test will still pass.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #58563|0   |1
is obsolete||

--- Comment #17 from Josef Moravec  ---
Created attachment 60050
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60050=edit
Bug 17835: Replace GetItemTypes with Koha::ItemTypes

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #58564|0   |1
is obsolete||

--- Comment #18 from Josef Moravec  ---
Created attachment 60051
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60051=edit
Bug 17835: Remove the subroutine GetItemTypes

At this point the subroutine is no longer in used.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #58562|0   |1
is obsolete||

--- Comment #16 from Josef Moravec  ---
Created attachment 60049
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60049=edit
Bug 17835: Do not reselect translated_description if comes from
search_with_localization

If the Koha::ItemType object has been instanciated from a call to
Koha::ItemTypes->search_with_localization, we already have the
translated_description value. So there is no need to fetch it again from
the DB. This is what this trick does: if the translated_description
column exist in the DBIx::Class result source's column list, that means
the value has already been retrieved.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #60033|0   |1
is obsolete||

--- Comment #19 from Josef Moravec  ---
Created attachment 60052
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60052=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

Bug 17835: follow-up

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

 Status|Needs Signoff   |Signed Off

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

  Attachment #58561|0   |1
is obsolete||

--- Comment #15 from Josef Moravec  ---
Created attachment 60048
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60048=edit
Bug 17835: Create a ItemtypeLocalization view

To properly move C4::Koha::GetItemTypes to Koha::ItemTypes we need
DBIx::Class to make a join on the localization table to retrieve the
possible translated description of the item types.
To do so there are 2 possibilities. The first one would have been to
rename the localization table to something like itemtype_localization.
That way we could have had a relationship between
itemtype_localization.code and itemtypes.itemtype
That would have meant to create one table per "entity" (here an entity
is itemtype) we allow the translability. There are pros and cons for
this choice, so I opt for another solution.
The other solution is to create a view on top of this localization
table. With this new view we can define the missing relationship.

That sounds like a quite clean solution and easy to implement.
Once we have this relationship, the
Koha::ItemTypes->search_with_localization will join on this view an
return the same result as GetItemTypes( style => 'array' ).
To replace
GetItemtypes( style => 'hash' )
which is the default behavior of this subroutine, we can do something like:
my $itemtypes = Koha::ItemTypes->search_with_localization;
my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed };

This patchset must not introduce big changes but it changes certain
behaviors (which were wrong) in some scripts. Indeed sometimes the
descriptions of the item types were not the translated ones. Moreover it
happens that the item types displayed in a dropdown list were not
ordered by translated description, but by description of code
(itemtypes.itemtype value). These 2 behaviors are what we expect.

Test plan:
Bugs will be hard to catch since these patches change a lot of file, it
will be easier to read the diff and catch possible typos or logic
errors.
However signoffers can focus on modified files and the item types
values.

Signed-off-by: Josef Moravec 

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #14 from Jonathan Druart  
---
It failed if opaclanguages contained something else than just 'en'.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #13 from Jonathan Druart  
---
Created attachment 60046
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60046=edit
Bug 17835: Mock language pref value

That way if prefs contain other languages, the test will still pass.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #12 from Josef Moravec  ---
(In reply to Jonathan Druart from comment #11)
> (In reply to Josef Moravec from comment #8)
> > test t/db_dependent/Koha/ItemTypes.t is failing:
> > #   Failed test 'item types should be sorted by translated description'
> > #   at t/db_dependent/Koha/ItemTypes.t line 120.
> > #  got: 'description'
> > # expected: 'a translated itemtype desc'
> > # Looks like you failed 1 test of 20.
> 
> Weird, I not recreate, it passes for me...
> 

I figured it out, the translation is done for "en" language, but when
subroutine C4::Languages::getlanguage returns something else (it could happen -
I have installed two additional languages for now), the original description is
selected and test fails.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #11 from Jonathan Druart  
---
(In reply to Josef Moravec from comment #8)
> test t/db_dependent/Koha/ItemTypes.t is failing:
> #   Failed test 'item types should be sorted by translated description'
> #   at t/db_dependent/Koha/ItemTypes.t line 120.
> #  got: 'description'
> # expected: 'a translated itemtype desc'
> # Looks like you failed 1 test of 20.

Weird, I not recreate, it passes for me...

> and in the fifth patch in
> koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt line 1202 is typo I
> think: extra "XX" on the ond of line

Yes indeed, fixed in the last patch (and I have squashed the last two ones).

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

  Attachment #58565|0   |1
is obsolete||
  Attachment #58600|0   |1
is obsolete||

--- Comment #10 from Jonathan Druart  
---
Created attachment 60033
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=60033=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

Bug 17835: follow-up

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #9 from Josef Moravec  ---
but otherwise looks good for me ;) good job

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Josef Moravec  changed:

   What|Removed |Added

 CC||josef.mora...@gmail.com

--- Comment #8 from Josef Moravec  ---
test t/db_dependent/Koha/ItemTypes.t is failing:
#   Failed test 'item types should be sorted by translated description'
#   at t/db_dependent/Koha/ItemTypes.t line 120.
#  got: 'description'
# expected: 'a translated itemtype desc'
# Looks like you failed 1 test of 20.

and in the fifth patch in
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt line 1202 is typo I
think: extra "XX" on the ond of line

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #7 from Jonathan Druart  
---
Created attachment 58600
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58600=edit
Bug 17835: follow-up

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 Status|Failed QA   |Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Owen Leonard  changed:

   What|Removed |Added

 Status|Needs Signoff   |Failed QA

--- Comment #6 from Owen Leonard  ---
I tested this by looking at all the pages modified and making sure the item
type information or form fields displayed correctly on each. I found two
problems:

- No item types display in the dropdown on reports/reserves_stats.pl
- Error on suggestion/suggestion.pl: "Undefined subroutine
::GetAuthorisedValues"

Everywhere else I could see no problems.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 Blocks||17843


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17843
[Bug 17843] Move C4::Koha::getitemtypeinfo to Koha::ItemTypes
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #2 from Jonathan Druart  
---
Created attachment 58562
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58562=edit
Bug 17835: Do not reselect translated_description if comes from
search_with_localization

If the Koha::ItemType object has been instanciated from a call to
Koha::ItemTypes->search_with_localization, we already have the
translated_description value. So there is no need to fetch it again from
the DB. This is what this trick does: if the translated_description
column exist in the DBIx::Class result source's column list, that means
the value has already been retrieved.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #5 from Jonathan Druart  
---
Created attachment 58565
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58565=edit
Bug 17835: Add an additional LEFT JOIN condition using DBIx::Class

The previous query was wrong. If an item type did not contain the
translation in the interface's language, the ->search_with_localization
did not return it at all.

What we need is definitely to add a second condition on the join.

For reference:
http://search.cpan.org/dist/DBIx-Class/lib/DBIx/Class/Relationship/Base.pm#condition
https://blog.afoolishmanifesto.com/posts/dbix-class-parameterized-relationships/

That sounds hacky but seems to be the DBIx::Class path to follow.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #4 from Jonathan Druart  
---
Created attachment 58564
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58564=edit
Bug 17835: Remove the subroutine GetItemTypes

At this point the subroutine is no longer in used.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #3 from Jonathan Druart  
---
Created attachment 58563
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58563=edit
Bug 17835: Replace GetItemTypes with Koha::ItemTypes

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

--- Comment #1 from Jonathan Druart  
---
Created attachment 58561
  -->
https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=58561=edit
Bug 17835: Create a ItemtypeLocalization view

To properly move C4::Koha::GetItemTypes to Koha::ItemTypes we need
DBIx::Class to make a join on the localization table to retrieve the
possible translated description of the item types.
To do so there are 2 possibilities. The first one would have been to
rename the localization table to something like itemtype_localization.
That way we could have had a relationship between
itemtype_localization.code and itemtypes.itemtype
That would have meant to create one table per "entity" (here an entity
is itemtype) we allow the translability. There are pros and cons for
this choice, so I opt for another solution.
The other solution is to create a view on top of this localization
table. With this new view we can define the missing relationship.

That sounds like a quite clean solution and easy to implement.
Once we have this relationship, the
Koha::ItemTypes->search_with_localization will join on this view an
return the same result as GetItemTypes( style => 'array' ).
To replace
GetItemtypes( style => 'hash' )
which is the default behavior of this subroutine, we can do something like:
my $itemtypes = Koha::ItemTypes->search_with_localization;
my %itemtypes = map { $_->{itemtype} => $_ } @{ $itemtypes->unblessed };

This patchset must not introduce big changes but it changes certain
behaviors (which were wrong) in some scripts. Indeed sometimes the
descriptions of the item types were not the translated ones. Moreover it
happens that the item types displayed in a dropdown list were not
ordered by translated description, but by description of code
(itemtypes.itemtype value). These 2 behaviors are what we expect.

Test plan:
Bugs will be hard to catch since these patches change a lot of file, it
will be easier to read the diff and catch possible typos or logic
errors.
However signoffers can focus on modified files and the item types
values.

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 Status|ASSIGNED|Needs Signoff

-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 Depends on||17627


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17627
[Bug 17627] Move C4::Koha::GetItemTypesByCategory to Koha::ItemTypes
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

[Koha-bugs] [Bug 17835] Move C4::Koha::GetItemTypes to Koha::ItemTypes

[bugzilla-daemon]

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

Jonathan Druart  changed:

   What|Removed |Added

 Blocks||15779


Referenced Bugs:

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15779
[Bug 15779] Remove unnecessary parts of C4::Koha
-- 
You are receiving this mail because:
You are watching all bug changes.
___
Koha-bugs mailing list
Koha-bugs@lists.koha-community.org
http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/