|
Description
Martin Renvoize (ashimema)
2020-07-24 10:31:48 UTC
Created attachment 110476 [details] [review] Bug 26057: Add datecancelled field to branchtransfers Created attachment 110477 [details] [review] Bug 26057: Add 'cancel' method to Koha::Item::Transfer This patch adds the 'cancel' method to Koha::Item::Transfer which sets the transfer as cancelled by updating the datecancelled field. We also update Koha::Item->get_transfer here to accomodate for the new resolution available for a transfer. Test plan: 1/ Run the included unit tests additions (t/db_dependent/Koha/Items.t, t/db_dependent/Koha/Item/Transfer.t I couldn't get this one to work - the patch applies, and I rebuilt the schema, CSS and restarted services. I tried two methods: 1. - Place a hold - Find item and put it into transit - Cancel hold and transfer 2. - Manually transfer an item to a library - Manually transfer the same item to a different library In the branchtransfers table, datecancelled as a field shows up - but it doesn't populate; the datearrived field still populates. I'll add a screenshot of the branchtransfers table. Created attachment 111279 [details]
A screenshot of the branchtransfers table after some transfers are performed.
This is my mistake I'm afraid.. for not adding a test plan. This patch only adds the underlying code to handle cancellations but does not update anywhere to call it as yet. My apologies there Sally.. I'll get back to this tree shortly and add proper test plans to the bugs that expose the functionality properly. Test plan 1/ Run the included tests Created attachment 116554 [details] [review] Bug 26057: Add datecancelled, cancellation_reason fields This patch adds the datecancelled and cancellation_reason fields to the branchtransfers table to record when and why transfers have been cancelled. To test 1/ Run the database update 2/ Check the database for the 'datecancelled' and 'cancellation_reason' fields 3/ Signoff Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 116555 [details] [review] Bug 26057: Add 'cancel' method to Koha::Item::Transfer This patch adds the 'cancel' method to Koha::Item::Transfer which sets the transfer as cancelled by updating the datecancelled field. We also update Koha::Item->get_transfer here to accomodate for the new resolution available for a transfer. Test plan: 1/ Run the included unit tests additions (t/db_dependent/Koha/Items.t, t/db_dependent/Koha/Item/Transfer.t Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 116556 [details] [review] Bug 26057: (QA follow-up) Drop test for 'reason' We decided this test was superflous to requirements and as such I've removed it as requested. Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 117585 [details] [review] Bug 26057: Add datecancelled, cancellation_reason fields This patch adds the datecancelled and cancellation_reason fields to the branchtransfers table to record when and why transfers have been cancelled. To test 1/ Run the database update 2/ Check the database for the 'datecancelled' and 'cancellation_reason' fields 3/ Signoff Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 117586 [details] [review] Bug 26057: Add 'cancel' method to Koha::Item::Transfer This patch adds the 'cancel' method to Koha::Item::Transfer which sets the transfer as cancelled by updating the datecancelled field. We also update Koha::Item->get_transfer here to accomodate for the new resolution available for a transfer. Test plan: 1/ Run the included unit tests additions (t/db_dependent/Koha/Items.t, t/db_dependent/Koha/Item/Transfer.t Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 117587 [details] [review] Bug 26057: (QA follow-up) Drop test for 'reason' We decided this test was superflous to requirements and as such I've removed it as requested. Signed-off-by: Kathleen Milne <kathleen.milne@cne-siar.gov.uk> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Created attachment 117588 [details] [review] Bug 26057: (QA follow-up) Fix Indexer tests The addition of cancellationdate meant that our test objects in the Indexer tests now longer matched expectations and as such the tests failed. Pushed to master for 21.05, thanks to everybody involved! Created attachment 117632 [details] [review] Bug 26057: (QA follow-up) Fix Biblios test for datecancellation With the introduction of datecancellation we now have two fields to check to varify if a transfer is 'complete' or not. I forgot to update this test at the time to ensure the datecancellation field was not set when adding the mock transfer. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117633 [details] [review] Bug 26057: (QA follow-up) Fix Biblios and Reserves tests With the introduction of datecancellation we now have two fields to check to varify if a transfer is 'complete' or not. I forgot to update these two tests at the time to ensure the datecancellation field was not set when adding the mock transfer. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 117635 [details] [review] Bug 26057: (QA follow-up) Fix Biblios, Reserves, Z3950Responder and XSLT tests With the introduction of datecancellation we now have two fields to check to varify if a transfer is 'complete' or not. I forgot to update these tests at the time to ensure the datecancellation field was not set when adding the mock transfer. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> (In reply to Martin Renvoize from comment #17) > Created attachment 117635 [details] [review] [review] > Bug 26057: (QA follow-up) Fix Biblios, Reserves, Z3950Responder and XSLT > tests > > With the introduction of datecancellation we now have two fields to > check to varify if a transfer is 'complete' or not. I forgot to update > these tests at the time to ensure the datecancellation field was not set > when adding the mock transfer. > > Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> follow-up pushed to master Enhancement not pushed to 20.11.x |