Bug 17847

Summary: Move C4::Koha::GetAuthvalueDropbox to Koha::AuthorisedValues
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: katrin.fischer, kyle, m.de.rooy
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 15779, 15799, 18573    
Attachments: Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues
Bug 17847: Remove C4::Koha::GetAuthvalueDropbox
Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues
Bug 17847: Remove C4::Koha::GetAuthvalueDropbox
Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues
Bug 17847: Remove C4::Koha::GetAuthvalueDropbox

Description Jonathan Druart 2017-01-04 14:28:10 UTC

    
Comment 1 Jonathan Druart 2017-01-04 14:41:58 UTC
Created attachment 58589 [details] [review]
Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues

The C4::Koha::GetAuthvalueDropbox subroutine does the same job as
Koha::AuthorisedValues->search
We should then replace the different calls to this subroutine to finally
remove it.
There were 2 calls to this subroutine:
- from the AuthorisedValues TT plugin (called from av-build-dropbox.inc
and members/housebound.tt)
- from the acqui/ajax-getauthvaluedropbox.pl ajax script

To make sure that this patchset does not introduce regressions, we will have
to test that the TT plugin and the ajax script still behave as before.

Test plan:
1/ Test acqui/ajax-getauthvaluedropbox.pl
- Link a fund to an authorised value category
- Create a new order
=> When you select a fund linked to AV category, the sort1 (and/or
sort2, depending on what you set) should be replaced with a dropdown
list populated with the authorised values
2/ Test av-build-dropbox.inc
- Create some authorised values for Bsort1
- Edit a patron
=> The sort1 should be a dropdown list populated with the Bsort1 AV
3/ Test members/housebound.tt
- Enable the housebound module (pref HouseboundModule)
- On the patron detail page, click on the "Housebound" tab
=> The frequency dropdown list should be populated with the different
HSBND_FREQ AV
Comment 2 Jonathan Druart 2017-01-04 14:42:02 UTC
Created attachment 58590 [details] [review]
Bug 17847: Remove C4::Koha::GetAuthvalueDropbox
Comment 3 Josef Moravec 2017-02-08 15:15:31 UTC
Created attachment 60019 [details] [review]
Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues

The C4::Koha::GetAuthvalueDropbox subroutine does the same job as
Koha::AuthorisedValues->search
We should then replace the different calls to this subroutine to finally
remove it.
There were 2 calls to this subroutine:
- from the AuthorisedValues TT plugin (called from av-build-dropbox.inc
and members/housebound.tt)
- from the acqui/ajax-getauthvaluedropbox.pl ajax script

To make sure that this patchset does not introduce regressions, we will have
to test that the TT plugin and the ajax script still behave as before.

Test plan:
1/ Test acqui/ajax-getauthvaluedropbox.pl
- Link a fund to an authorised value category
- Create a new order
=> When you select a fund linked to AV category, the sort1 (and/or
sort2, depending on what you set) should be replaced with a dropdown
list populated with the authorised values
2/ Test av-build-dropbox.inc
- Create some authorised values for Bsort1
- Edit a patron
=> The sort1 should be a dropdown list populated with the Bsort1 AV
3/ Test members/housebound.tt
- Enable the housebound module (pref HouseboundModule)
- On the patron detail page, click on the "Housebound" tab
=> The frequency dropdown list should be populated with the different
HSBND_FREQ AV

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 4 Josef Moravec 2017-02-08 15:15:36 UTC
Created attachment 60020 [details] [review]
Bug 17847: Remove C4::Koha::GetAuthvalueDropbox

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Marcel de Rooy 2017-03-10 10:25:35 UTC
Created attachment 60982 [details] [review]
Bug 17847: Replace C4::Koha::GetAuthvalueDropbox with Koha::AuthorisedValues

The C4::Koha::GetAuthvalueDropbox subroutine does the same job as
Koha::AuthorisedValues->search
We should then replace the different calls to this subroutine to finally
remove it.
There were 2 calls to this subroutine:
- from the AuthorisedValues TT plugin (called from av-build-dropbox.inc
and members/housebound.tt)
- from the acqui/ajax-getauthvaluedropbox.pl ajax script

To make sure that this patchset does not introduce regressions, we will have
to test that the TT plugin and the ajax script still behave as before.

Test plan:
1/ Test acqui/ajax-getauthvaluedropbox.pl
- Link a fund to an authorised value category
- Create a new order
=> When you select a fund linked to AV category, the sort1 (and/or
sort2, depending on what you set) should be replaced with a dropdown
list populated with the authorised values
2/ Test av-build-dropbox.inc
- Create some authorised values for Bsort1
- Edit a patron
=> The sort1 should be a dropdown list populated with the Bsort1 AV
3/ Test members/housebound.tt
- Enable the housebound module (pref HouseboundModule)
- On the patron detail page, click on the "Housebound" tab
=> The frequency dropdown list should be populated with the different
HSBND_FREQ AV

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2017-03-10 10:25:39 UTC
Created attachment 60983 [details] [review]
Bug 17847: Remove C4::Koha::GetAuthvalueDropbox

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2017-03-10 10:26:33 UTC
acqui/ajax-getauthvaluedropbox.pl
We should actually use a html template here and move the html stuff outside the script.
Yes, it was already there, but this is/was an opportunity?

Passed QA
Comment 8 Kyle M Hall 2017-03-31 10:53:32 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 9 Katrin Fischer 2017-04-02 16:16:34 UTC
This won't get ported back to 16.11.x as it is an enhancement.