Bug 16895 - Allow writeoffs via SIP2
Summary: Allow writeoffs via SIP2
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 16757
Blocks: 16899
  Show dependency treegraph
 
Reported: 2016-07-11 13:55 UTC by Kyle M Hall
Modified: 2017-12-07 22:16 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16895 - Allow writeoffs via SIP2 (5.84 KB, patch)
2016-07-11 14:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16895 - Allow writeoffs via SIP2 (5.91 KB, patch)
2016-09-16 13:02 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16895 - Allow writeoffs via SIP2 (5.64 KB, patch)
2017-03-21 12:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16895 [QA Followup] - Don't require fee payment to be exact full payment (834 bytes, patch)
2017-04-19 13:29 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16895 - Allow writeoffs via SIP2 (5.74 KB, patch)
2017-04-19 14:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16895 [QA Followup] - Don't require fee payment to be exact full payment (930 bytes, patch)
2017-04-19 14:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16895: [QA Follow-up] Resolve warning on $payment_type_writeoff (1.11 KB, patch)
2017-04-19 14:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2016-07-11 13:55:34 UTC
Many SIP2 services such as those by Comprise Technologies are able to or require that an ILS be able to accept writeoffs via SIP2. The SIP2 protocol specifies that payment type be a two digit number, but does not specify a code for writeoffs. To this end we should allow the write-off code to be specified in the SIP2 config on a per-account basis so that if different vendors use different fixed codes for write-offs we can handle that gracefully.
Comment 1 Kyle M Hall 2016-07-11 14:56:12 UTC
Created attachment 53268 [details] [review]
Bug 16895 - Allow writeoffs via SIP2

Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.

Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
      payment_type_writeoff="06"
  in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
   defined earlier, with a payment amount that is *not* equal to the
   amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
   payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!
Comment 2 Kyle M Hall 2016-09-16 13:02:09 UTC
Created attachment 55614 [details] [review]
[SIGNED-OFF] Bug 16895 - Allow writeoffs via SIP2

Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.

Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
      payment_type_writeoff="06"
  in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
   defined earlier, with a payment amount that is *not* equal to the
   amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
   payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!

Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Comment 3 Marcel de Rooy 2017-03-10 12:32:58 UTC
Any chance to include some kind of unit testing? We have some tests for MsgType..
Can you further explain the use of the pay_type? If I follow the chain, it gets written to the accounts table with a sip prefix? But is that the only thing it does, or do I overlook any specific behavior for sip payment type?

Please clarify.
Comment 4 Kyle M Hall 2017-03-10 13:11:53 UTC
(In reply to Marcel de Rooy from comment #3)
> Any chance to include some kind of unit testing? We have some tests for
> MsgType..
> Can you further explain the use of the pay_type? 

Yes, it is a legacy parameter that simply declares this payment was made via SIP.

> If I follow the chain, it
> gets written to the accounts table with a sip prefix? But is that the only
> thing it does, or do I overlook any specific behavior for sip payment type?

Nope, that is literally all it does, it is existing behavior that we are continuing to use.
Comment 5 Colin Campbell 2017-03-10 15:50:36 UTC
Yes pay type is just recorded. We are finding it useful though in reconciling when cashing up units that handle rental fees. Hopefully this will be included in a future submission
Comment 6 Marcel de Rooy 2017-03-17 07:14:33 UTC
Depends on a PNA
Comment 7 Kyle M Hall 2017-03-21 12:51:37 UTC
Created attachment 61377 [details] [review]
Bug 16895 - Allow writeoffs via SIP2

Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.

Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
      payment_type_writeoff="06"
  in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
   defined earlier, with a payment amount that is *not* equal to the
   amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
   payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!

Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>
Comment 8 Marcel de Rooy 2017-03-24 12:33:51 UTC
Question raised while testing:

Why does sub pay in C4/SIP/ILS/Transaction/FeePayment.pm contain this check:
if ( $fee && $fee->amountoutstanding == $amt ) {

If I am paying a partial amount in the interface, this is allowed too. But note that it is not possible to writeoff a partial amount in the interface.
I tried a partial writeoff in testing this patch and I wondered why I got a N in the response. But the cause is this test. I think we should print a message why we refused the payment.

While I understand that you refuse it for a writeoff, I do not understand it for another payment type. And thats where I can tie it to this report: Should this check be applied only to the writeoffs ?

In summary I propose two changes:
[1] Send a message why you refuse a partial payment for writeoffs.
[2] Accept a partial payment for other types.

Changing status
Comment 9 Kyle M Hall 2017-04-19 13:29:37 UTC
Created attachment 62364 [details] [review]
Bug 16895 [QA Followup] - Don't require fee payment to be exact full payment
Comment 10 Kyle M Hall 2017-04-19 13:31:14 UTC
In retrospect that kind of artificial limit seems silly for an API. There is absolutely no reason to require even a writeoff to be the same amount just because the interface requires it ( for the time being ). It makes more sense for the client to make those decisions. Limit removed!

(In reply to Marcel de Rooy from comment #8)
> Question raised while testing:
> 
> Why does sub pay in C4/SIP/ILS/Transaction/FeePayment.pm contain this check:
> if ( $fee && $fee->amountoutstanding == $amt ) {
> 
> If I am paying a partial amount in the interface, this is allowed too. But
> note that it is not possible to writeoff a partial amount in the interface.
> I tried a partial writeoff in testing this patch and I wondered why I got a
> N in the response. But the cause is this test. I think we should print a
> message why we refused the payment.
> 
> While I understand that you refuse it for a writeoff, I do not understand it
> for another payment type. And thats where I can tie it to this report:
> Should this check be applied only to the writeoffs ?
> 
> In summary I propose two changes:
> [1] Send a message why you refuse a partial payment for writeoffs.
> [2] Accept a partial payment for other types.
> 
> Changing status
Comment 11 Marcel de Rooy 2017-04-19 14:38:36 UTC
Created attachment 62377 [details] [review]
Bug 16895 - Allow writeoffs via SIP2

Many SIP2 services such as those by Comprise Technologies are able to or
require that an ILS be able to accept writeoffs via SIP2. The SIP2
protocol specifies that payment type be a two digit number, but does not
specify a code for writeoffs. To this end we should allow the write-off
code to be specified in the SIP2 config on a per-account basis so that
if different vendors use different fixed codes for write-offs we can
handle that gracefully.

Test Plan:
1) Apply this patch
2) Modify your SIP2 config to include
      payment_type_writeoff="06"
  in the login portion of the account you will be using for the test.
3) Restart your SIP2 server
4) Create a fee for a patron
5) Send a SIP2 fee paid message specifying the payment type code we
   defined earlier, with a payment amount that is *not* equal to the
   amount outstanding for the fee.
6) Note the fee paid response indicates the payment failed
7) Repeat step 5, but this time send the amount outstanding as the
   payment amount
8) Note that the fee paid response indicates a successful payment
9) Note in Koha that the fee has been written off!

Signed-off-by: Rhonda Kuiper <kuiper@roundrocktexas.gov>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2017-04-19 14:38:41 UTC
Created attachment 62378 [details] [review]
Bug 16895 [QA Followup] - Don't require fee payment to be exact full payment

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2017-04-19 14:38:45 UTC
Created attachment 62379 [details] [review]
Bug 16895: [QA Follow-up] Resolve warning on $payment_type_writeoff

Resolves:
  Use of uninitialized value $payment_type_writeoff in string eq at C4/SIP/Sip/MsgType.pm line 1072, <STDIN> line 2.
Would be raised if this parameter is not found in SIPconfig.xml.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Kyle M Hall 2017-04-21 15:01:56 UTC
Pushed to master for 17.05!
Comment 15 Katrin Fischer 2017-04-21 23:15:10 UTC
This won't get ported back to 16.11.x as it is an enhancement.