Bug 8775 - Add collection column to built in 'Items lost' report
Summary: Add collection column to built in 'Items lost' report
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 22468
  Show dependency treegraph
 
Reported: 2012-09-17 12:24 UTC by Nicole C. Engard
Modified: 2019-05-30 12:15 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8775: Add collection code to lost report (1.78 KB, patch)
2019-02-22 14:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8775: Add collection code to lost report (2.42 KB, patch)
2019-03-01 16:27 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8775: Add collection code to lost report (2.47 KB, patch)
2019-03-01 19:57 UTC, Bin Wen
Details | Diff | Splinter Review
Bug 8775: (follow-up) Change column header to "Collection" (1.19 KB, patch)
2019-03-06 15:26 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 8775: Add collection code to lost report (2.54 KB, patch)
2019-03-06 18:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8775: (follow-up) Change column header to "Collection" (1.27 KB, patch)
2019-03-06 18:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8775: Use the AV mapped with items.ccode instead of CCODE (1.59 KB, patch)
2019-03-06 18:23 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2012-09-17 12:24:06 UTC
The canned lost report shows the shelving location, but not collection code, it should show both if it shows one.

Nicole
Comment 1 Katrin Fischer 2016-08-14 18:58:17 UTC
Still valid.
Comment 2 Owen Leonard 2019-02-22 14:06:50 UTC Comment hidden (obsolete)
Comment 3 Séverine Queune 2019-02-27 09:15:09 UTC
Hi Owen,

On /cgi-bin/koha/admin/columns_settings.pl page, 'collection code' is not present in 'id=lostitems-table', is it normal ?

Except that point , behaviour is correct on items lost report, and the column can be hibben using the "Comulns" button on the /cgi-bin/koha/reports/itemslost.pl page.
Comment 4 Owen Leonard 2019-03-01 16:27:47 UTC Comment hidden (obsolete)
Comment 5 Bin Wen 2019-03-01 19:57:40 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2019-03-04 22:45:37 UTC
Hi Bin, please make sure to change the status to "Signed Off" when signing off.
Comment 7 Katrin Fischer 2019-03-06 07:17:37 UTC
The sequence of columns appears a little strange:

- Library (homelibrary)
- Item type
- Collection code
- Current location (holdinglibrary)
- Location

Out of scope here, but can we change "Collection code" to be "Collection"? We are not showing the code, but the description :)
Comment 8 Owen Leonard 2019-03-06 15:26:19 UTC Comment hidden (obsolete)
Comment 9 Jonathan Druart 2019-03-06 16:36:25 UTC
We should use the more flexible way to retrieve an AV's value:

AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => item.ccode )

That way, if items.ccode is not bind to an AV, or bind to a different one than CCODE, it will display the correct value. I know it's not how we did in all of the places they are displayed, but it should be the correct way to do :)
Comment 10 Katrin Fischer 2019-03-06 16:41:31 UTC
(In reply to Jonathan Druart from comment #9)
> We should use the more flexible way to retrieve an AV's value:
> 
> AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode',
> authorised_value => item.ccode )
> 
> That way, if items.ccode is not bind to an AV, or bind to a different one
> than CCODE, it will display the correct value. I know it's not how we did in
> all of the places they are displayed, but it should be the correct way to do
> :)

What happens if it's not mapped? I know the normal plugin outputs the content of the field. 

If you mapped another category, you are breaking a lot of things. I am not sure if we should start officially supporting this.
Comment 11 Jonathan Druart 2019-03-06 16:47:17 UTC
(In reply to Katrin Fischer from comment #10)
> (In reply to Jonathan Druart from comment #9)
> > We should use the more flexible way to retrieve an AV's value:
> > 
> > AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode',
> > authorised_value => item.ccode )
> > 
> > That way, if items.ccode is not bind to an AV, or bind to a different one
> > than CCODE, it will display the correct value. I know it's not how we did in
> > all of the places they are displayed, but it should be the correct way to do
> > :)
> 
> What happens if it's not mapped? I know the normal plugin outputs the
> content of the field. 

It should output the value, yes.

> If you mapped another category, you are breaking a lot of things. I am not
> sure if we should start officially supporting this.

Not supporting, but keeping it in mind for new additions.
Comment 12 Owen Leonard 2019-03-06 16:51:55 UTC
(In reply to Jonathan Druart from comment #9)
> We should use the more flexible way to retrieve an AV's value:
> 
> AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode',
> authorised_value => item.ccode )

Is this really a blocker for this bug? There are many existing instances of using AuthorisedValues.GetByCode for ccode. If this is a new best practice they could all be fixed in a separate bug.
Comment 13 Jonathan Druart 2019-03-06 17:38:30 UTC
It's a one line fix, I can provide a follow-up later.
Comment 14 Jonathan Druart 2019-03-06 18:23:05 UTC
Created attachment 86200 [details] [review]
Bug 8775: Add collection code to lost report

This patch adds a column for collection code to the lost items report.

To test, apply the patch and run the lost items report. You should see a
collection code column.

Test column configuration for this table: Go to Administration ->
Configure columns. Confirm that the "collection_code" column can be
configured and that configuration choices are applied correctly.

Signed-off-by: Bin Wen <bin.wen@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2019-03-06 18:23:18 UTC
Created attachment 86201 [details] [review]
Bug 8775: (follow-up) Change column header to "Collection"

This patch changes the table heading for ccode to be "Collection"
instead of "Collection code." Current templates vary, but "Collection"
is more logical because we show the description, not the code.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Jonathan Druart 2019-03-06 18:23:31 UTC
Created attachment 86202 [details] [review]
Bug 8775: Use the AV mapped with items.ccode instead of CCODE

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Nick Clemens 2019-03-16 00:40:04 UTC
(In reply to Jonathan Druart from comment #16)
> Created attachment 86202 [details] [review] [review]
> Bug 8775: Use the AV mapped with items.ccode instead of CCODE
> 
> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

The newer method (GetDescriptionBYKOhaField) is nice because it caches, however, it does not currently return a value when the code does not match an authorised value. 

This patch also repeats the code and description when a description is found:
[% item.ccode%][% AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.ccode', authorised_value => item.ccode ) | html %]

Obsoleting the last patch here, please address using the new method and returning the code if no value found on another bug.
Comment 18 Nick Clemens 2019-03-16 01:10:53 UTC
Awesome work all!

Pushed to master for 19.05
Comment 19 Martin Renvoize 2019-03-19 11:00:57 UTC
Enhancement will not be backported to 18.11.x series.