Bug 16157 - C4::Koha::GetAuthorisedValues should not handle the selected option
Summary: C4::Koha::GetAuthorisedValues should not handle the selected option
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 16158 17300
  Show dependency treegraph
 
Reported: 2016-03-29 15:59 UTC by Jonathan Druart
Modified: 2017-12-07 22:17 UTC (History)
3 users (show)

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


Attachments
Bug 16157: Move the selected flag from GetAuthorisedValues to the templates (38.45 KB, patch)
2016-03-29 16:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16157: Move the selected flag from GetAuthorisedValues to the templates (38.67 KB, patch)
2016-03-30 08:29 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 16157: Move the selected flag from GetAuthorisedValues to the templates (38.69 KB, patch)
2016-04-03 14:55 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-03-29 15:59:22 UTC
From C4::Koha::GetAuthorisedValues

    # TODO: the "selected" feature should be replaced by a utility function
    # somewhere else, it doesn't belong in here. For starters it makes
    # caching much more complicated. Or just let the UI logic handle it, it's
    # what it's for.
Comment 1 Jonathan Druart 2016-03-29 16:05:14 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2016-03-30 08:29:00 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2016-04-03 14:55:36 UTC
Created attachment 49825 [details] [review]
[PASSED QA] Bug 16157: Move the selected flag from GetAuthorisedValues to the templates

From C4::Koha::GetAuthorisedValues

    # TODO: the "selected" feature should be replaced by a utility function
    # somewhere else, it doesn't belong in here. For starters it makes
    # caching much more complicated. Or just let the UI logic handle it, it's
    # what it's for.

Indeed, it's not a job for a subroutine, the template should take care of that.
Note that a perf gain could be won with this patch \o/

Test plan:
- Edit an itemtype and check the value of the "Search category" dropdown list
- Edit a patron attribute type and check the value of the "Class" dropdown list
- Detail for a catalogue record, the Status column should be correctly
  populated if items are damaged and/or lost
- Item details for a catalogue record, the lost, damaged and withdrawn
  value should be correctly displayed
- Edit a patron, the "street type" should be correctly selected
- Create a patron attribute type linked to an authorised value list.
- Edit a patron, set a value for this attribute, edit it again. The
  correct value should be selected.
- Search for subscriptions. The 'Location' dropdown list should behave
  correctly (select the entry you have choosen before, etc.)
- Edit a subscription, the location dropdown list should select the
  correct value.
- Edit and view a suggestion with a 'reason for suggestion' set (you
  should have at least 1 OPAC_SUG AV defined)

Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Brendan Gallagher 2016-04-07 00:17:20 UTC
Pushed to Master - Should be in the May 2016 release.  Thanks!