Created attachment 109533 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Created attachment 109547 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

We're missing unit tests here I'm afraid.  I beleive there is prior art for such tests that we need added for the Koha::Patron change here.

Failing QA

Created attachment 113579 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Created attachment 113580 [details] [review]
Bug 26352: Add unit test

I update the doc in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks

Damn.. I thought I switched this to PQA already :(

There's an undocumented feature here: When 'autoMemberNum' is enabled there's a second plugin hook available called 'barcode_generate'. 

I'm wondering if this needs to be an explicit method of it's own as currently implemented or it could just be the case for when an empty or undefined cardnumber is passed to the patron_barcode_transform hook?

(In reply to Martin Renvoize from comment #8)
> There's an undocumented feature here: When 'autoMemberNum' is enabled
> there's a second plugin hook available called 'barcode_generate'. 
> 
> I'm wondering if this needs to be an explicit method of it's own as
> currently implemented or it could just be the case for when an empty or
> undefined cardnumber is passed to the patron_barcode_transform hook?

I like the simplicity of your latter idea. I'll submit a followup to make that change!

Created attachment 113880 [details] [review]
Bug 26352: Replace call to barcode_generate with call to patron_barcode_transform

(In reply to Fridolin Somers from comment #6)
> I update the doc in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks

Thanks Frido!

Created attachment 113885 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 113886 [details] [review]
Bug 26352: Add unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 113887 [details] [review]
Bug 26352: Replace call to barcode_generate with call to patron_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 113888 [details] [review]
Bug 26352: (QA follow-up) Add additional tests

This patch adds additional unit tests for autoMemberNum handling and
corrects the implimentation there.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Works well and is now fully tested.

Passing QA

Created attachment 115167 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115168 [details] [review]
Bug 26352: Add unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115169 [details] [review]
Bug 26352: Replace call to barcode_generate with call to patron_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115170 [details] [review]
Bug 26352: (QA follow-up) Add additional tests

This patch adds additional unit tests for autoMemberNum handling and
corrects the implimentation there.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 115171 [details] [review]
Bug 26352: Switch from using call() to call_recursive()

Created attachment 118691 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 118692 [details] [review]
Bug 26352: Add unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 118693 [details] [review]
Bug 26352: Replace call to barcode_generate with call to patron_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 118694 [details] [review]
Bug 26352: (QA follow-up) Add additional tests

This patch adds additional unit tests for autoMemberNum handling and
corrects the implimentation there.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 118695 [details] [review]
Bug 26352: Switch from using call() to call_recursive()

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Tested and passed the final followup patch.

Blocked by bug 28026.

Created attachment 122428 [details] [review]
Bug 26352: Add plugin hooks to transform patron barcodes

Some of our partners have unusual barcode requirements that have
required us to transform scanned barcodes using javascript. This is not
the most reliable method. It would make more sense to have Koha
transform the barcodes on the backend using a plugin. We should add
hooks to transform and generate new item and patron barcodes.

Test Plan:
1) Apply this patch
2) Download and install the Barcode Transformer plugin
   https://github.com/bywatersolutions/koha-plugin-barcode-transformer/releases/download/v1.0.1/koha-plugin-barcode-transformer-v1.0.1.kpz
3) Go to the plugin configuration page, set the configuration to the example configuration from the same page
4) In the patron search and anywhere else you can scan a patron
   cardnumber, type in some cardnumbers but prefix them with A and postfix
   them with A or B, e.g. A12345A, A12345B
5) Note the letters are removed by Koha!
6) Try a cardnumber like X123456Y
7) Note Koha converts it to Z13456Z

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 122429 [details] [review]
Bug 26352: Add unit test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 122430 [details] [review]
Bug 26352: Replace call to barcode_generate with call to patron_barcode_transform

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 122431 [details] [review]
Bug 26352: (QA follow-up) Add additional tests

This patch adds additional unit tests for autoMemberNum handling and
corrects the implimentation there.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 122432 [details] [review]
Bug 26352: Switch from using call() to call_recursive()

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Pushed to master for 21.11, thanks to everybody involved!

This is great.
I've edited "First version" in https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks

(In reply to Fridolin Somers from comment #35)
> This is great.
> I've edited "First version" in
> https://wiki.koha-community.org/wiki/Koha_Plugin_Hooks

Thanks Frido!

This no longer seems to work unless I haven't set something up right. I added an A as a suffix and prefix to Bug 29637 in the patron search. The plugin installed okay (I used the latest version) and I copied the configuration in.

I was testing Bug 29637 - which indicates this bug broke AutoSwitchPatron.

If you used plugin koha-plugin-barcode-transformer, I confirm it does not work.
I've opened an issue :
https://github.com/bywatersolutions/koha-plugin-barcode-transformer/issues/5

I've created a pull-request on KitchenSink to add an easy example for this two hooks :
https://github.com/bywatersolutions/dev-koha-plugin-kitchen-sink/pull/17

Those hooks use a ref to value which is not easy to understand, we have to document it well.

(In reply to Fridolin Somers from comment #38)
> If you used plugin koha-plugin-barcode-transformer, I confirm it does not
> work.

Thanks Fridolin! I had used that plugin.

I find this plugin really confusing, since it's called in a wide variety of different contexts...

Perhaps it would've made sense to break it up into patron_barcode_transform and patron_generate_cardnumber...

(In reply to Martin Renvoize from comment #8)
> There's an undocumented feature here: When 'autoMemberNum' is enabled
> there's a second plugin hook available called 'barcode_generate'. 
> 
> I'm wondering if this needs to be an explicit method of it's own as
> currently implemented or it could just be the case for when an empty or
> undefined cardnumber is passed to the patron_barcode_transform hook?

Apparently this was already discussed.

I would've preferred a second hook for sure. I thought maybe it would be safe to create a new cardnumber when it's empty/undefined, but Koha::Patron::store() has some oddities. Like "patron_barcode_transform" gets called twice if you're using autoMemberNum... and fixup_cardnumber() assumes the output of autoMemberNum is an integer, which seems presumptuous especially when the original use case talked about prefixing/suffixing cardnumbers with letters...

(In reply to David Cook from comment #41)
> (In reply to Martin Renvoize from comment #8)
> > There's an undocumented feature here: When 'autoMemberNum' is enabled
> > there's a second plugin hook available called 'barcode_generate'. 
> > 
> > I'm wondering if this needs to be an explicit method of it's own as
> > currently implemented or it could just be the case for when an empty or
> > undefined cardnumber is passed to the patron_barcode_transform hook?
> 
> Apparently this was already discussed.
> 
> I would've preferred a second hook for sure. I thought maybe it would be
> safe to create a new cardnumber when it's empty/undefined, but
> Koha::Patron::store() has some oddities. Like "patron_barcode_transform"
> gets called twice if you're using autoMemberNum... and fixup_cardnumber()
> assumes the output of autoMemberNum is an integer, which seems presumptuous
> especially when the original use case talked about prefixing/suffixing
> cardnumbers with letters...

I've developed a plugin to create an alphanumeric cardnumber if there's no cardnumber passed into "patron_barcode_transform", and yeah it doesn't work if you're using "autoMemberNum".

If you're using "autoMemberNum", the cardnumber referenced in "patron_barcode_transform" must be a positive integer, or you'll run into strife.