Bug 21232 - Problems when linking a subscription to a non-existing biblionumber
Summary: Problems when linking a subscription to a non-existing biblionumber
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 12921 (view as bug list)
Depends on: 23677
Blocks: 24358
  Show dependency treegraph
 
Reported: 2018-08-16 08:16 UTC by Katrin Fischer
Modified: 2020-01-14 20:59 UTC (History)
8 users (show)

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


Attachments
Bug 21232: Add a client-side check on biblionumber when creating a subscription (1.31 KB, patch)
2019-05-05 14:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21232: Add a client-side check on biblionumber when creating a subscription (3.92 KB, patch)
2019-12-13 11:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21232: Add a client-side check on biblionumber when creating a subscription (3.95 KB, patch)
2019-12-19 15:45 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 21232: Add a client-side check on biblionumber when creating a subscription (4.01 KB, patch)
2019-12-22 17:11 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 Katrin Fischer 2018-08-16 08:16:18 UTC
Appeared in 16.11, for retesting in newer versions:

1) Create a new subscription, for biblionumber enter a numberic high value not existing in your database as a valid biblionumber
2) Verify that you can save (bug 1)
3) Edit the subscription (will show with empty title in search)
4) Correct the link to the record with an existing biblionumber
5) Verify that now the title displays correctly and link to the record works
6) Check "OPAC view" link, it will still show the wrong biblionumber value, 
   leading to nowhere (bug 2)
   Reason: the biblionumber in subscriptionhistory hasn't been updated
7) Check the serial table for your subscription id: there is still the wrong value
   (bug 3) 
   I feel like at least the expected issue should have been updated to the
   correct biblionumber. I am not sure if it could make sense to keep the old
   one for received issues, to keep some history of name changes etc. Maybe 
   worth checking if the serial.biblionumber is used somewhere in a sensible way?
Comment 1 Jonathan Druart 2018-08-17 14:42:56 UTC
Certainly a duplicate of bug 20351.
Comment 2 Katrin Fischer 2018-08-30 06:09:27 UTC
(In reply to Jonathan Druart from comment #1)
> Certainly a duplicate of bug 20351.

On first glance, I don't think it is. It's catching invalid subscriptionids, while this is about an invalid biblionumber?
Comment 3 Katrin Fischer 2018-08-30 06:16:28 UTC
Updating to master, there is still no check in place for linking subscriptions
to non-existing biblionumbers. I think this should be checked on saving or
maybe even when moving from the first part of the subscription form to the
second
Comment 4 Katrin Fischer 2019-02-10 22:03:12 UTC
*** Bug 12921 has been marked as a duplicate of this bug. ***
Comment 5 Jonathan Druart 2019-02-17 16:09:22 UTC
Now failing with 

Can't call method "title" on an undefined value at /home/vagrant/kohaclone/Koha/Subscription.pm line 165
        Koha::Subscription::get_sharable_info('Koha::Subscription', 1) called at /home/vagrant/kohaclone/Koha/SharedContent.pm line 123
        Koha::SharedContent::prepare_entity_data('', 51, 1, 'subscription') called at /home/vagrant/kohaclone/Koha/SharedContent.pm line 82
        Koha::SharedContent::send_entity('', 51, 1, 'subscription') called at /home/vagrant/kohaclone/serials/subscription-add.pl line 376
        CGI::Compile::ROOT::home_vagrant_kohaclone_serials_subscription_2dadd_2epl::redirect_add_subscription at /home/vagrant/kohaclone/serials/subscription-add.pl line 178


It's time to add the missing FK
Comment 6 Jonathan Druart 2019-05-05 14:18:55 UTC
(In reply to Jonathan Druart from comment #5)
> Now failing with 
> 
> Can't call method "title" on an undefined value at
> /home/vagrant/kohaclone/Koha/Subscription.pm line 165
>         Koha::Subscription::get_sharable_info('Koha::Subscription', 1)
> called at /home/vagrant/kohaclone/Koha/SharedContent.pm line 123
>         Koha::SharedContent::prepare_entity_data('', 51, 1, 'subscription')
> called at /home/vagrant/kohaclone/Koha/SharedContent.pm line 82
>         Koha::SharedContent::send_entity('', 51, 1, 'subscription') called
> at /home/vagrant/kohaclone/serials/subscription-add.pl line 376
>        
> CGI::Compile::ROOT::home_vagrant_kohaclone_serials_subscription_2dadd_2epl::
> redirect_add_subscription at
> /home/vagrant/kohaclone/serials/subscription-add.pl line 178
> 
> 
> It's time to add the missing FK

After bug 22849 the subscription will be created without warning/error and inserted with an invalid biblionumber.
Comment 7 Jonathan Druart 2019-05-05 14:19:46 UTC
Created attachment 89357 [details] [review]
Bug  21232: Add a client-side check on biblionumber when creating a subscription
Comment 8 Jonathan Druart 2019-05-05 14:21:16 UTC
(In reply to Jonathan Druart from comment #7)
> Created attachment 89357 [details] [review] [review]
> Bug  21232: Add a client-side check on biblionumber when creating a
> subscription

Something like that could work, but much more is missing:
- JS code is horrible there
- Biblio's title could be retrieved and displayed
- server-side code is missing
Comment 9 Jonathan Druart 2019-12-13 11:45:35 UTC
Created attachment 96245 [details] [review]
Bug 21232: Add a client-side check on biblionumber when creating a subscription

This patch adds an AJAX call to the REST API (/api/v1/biblios/) to
retrieve and display the biblio's title.
On clicking the "next" button a check is done to make sure the title
exists, which means the biblionumber we manually entered is valid (can
we assume a title is mandatory?)

Test plan:
- Create or edit a new subscription
- Enter an invalid biblionumber in the input
=> A friendly note is telling you that the biblio does not exist
- Try to switch to the next screen
=> You get an alert
- Enter a valid biblionumber in the input
=> The title is displayed
=> Try to switch to the next screen
=> It works!
Comment 10 ByWater Sandboxes 2019-12-19 15:45:10 UTC
Created attachment 96489 [details] [review]
Bug 21232: Add a client-side check on biblionumber when creating a subscription

This patch adds an AJAX call to the REST API (/api/v1/biblios/) to
retrieve and display the biblio's title.
On clicking the "next" button a check is done to make sure the title
exists, which means the biblionumber we manually entered is valid (can
we assume a title is mandatory?)

Test plan:
- Create or edit a new subscription
- Enter an invalid biblionumber in the input
=> A friendly note is telling you that the biblio does not exist
- Try to switch to the next screen
=> You get an alert
- Enter a valid biblionumber in the input
=> The title is displayed
=> Try to switch to the next screen
=> It works!

Signed-off-by: hc <hc@interleaf.ie>
Comment 11 Katrin Fischer 2019-12-22 17:11:51 UTC
Created attachment 96576 [details] [review]
Bug 21232: Add a client-side check on biblionumber when creating a subscription

This patch adds an AJAX call to the REST API (/api/v1/biblios/) to
retrieve and display the biblio's title.
On clicking the "next" button a check is done to make sure the title
exists, which means the biblionumber we manually entered is valid (can
we assume a title is mandatory?)

Test plan:
- Create or edit a new subscription
- Enter an invalid biblionumber in the input
=> A friendly note is telling you that the biblio does not exist
- Try to switch to the next screen
=> You get an alert
- Enter a valid biblionumber in the input
=> The title is displayed
=> Try to switch to the next screen
=> It works!

Signed-off-by: hc <hc@interleaf.ie>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Martin Renvoize 2019-12-23 11:08:54 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 13 Joy Nelson 2020-01-06 21:30:46 UTC
I've been attempting to push this to 19.11.x and I can apply the patch, but it does not perform as expected.  I am not warned that my biblionumber is invalid.  I'm not stopped from completing the addition of the serial linked to a 'bad' bibnumber.  Any ideas?
Comment 14 Joy Nelson 2020-01-06 21:40:49 UTC
Must have been a cache issue.  A bit of playing with FF and the settings and few hard cache clears and all's well.  It's working now. :)

joy
Comment 15 Joy Nelson 2020-01-06 21:43:32 UTC
Pushed to 19.11.x branch for 19.11.02
Comment 16 Katrin Fischer 2020-01-06 21:46:43 UTC
Comment on attachment 96576 [details] [review]
Bug 21232: Add a client-side check on biblionumber when creating a subscription

Review of attachment 96576 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/js/subscription-add.js
@@ +60,5 @@
>      }
>  
> +    var bib_exists = $("input[name='title']").val().length;
> +
> +    if (!bib_exists) alert(_("Bibliographic record does not exist!"));

This won't translate - got missed before. Can we fix it in a follow-up?
Comment 17 Jonathan Druart 2020-01-06 22:08:51 UTC
(In reply to Katrin Fischer from comment #16)
> Comment on attachment 96576 [details] [review] [review]
> Bug 21232: Add a client-side check on biblionumber when creating a
> subscription
> 
> Review of attachment 96576 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: koha-tmpl/intranet-tmpl/prog/js/subscription-add.js
> @@ +60,5 @@
> >      }
> >  
> > +    var bib_exists = $("input[name='title']").val().length;
> > +
> > +    if (!bib_exists) alert(_("Bibliographic record does not exist!"));
> 
> This won't translate - got missed before. Can we fix it in a follow-up?

Sorry about that, see bug 24358.
Comment 18 Hayley Mapley 2020-01-14 04:01:36 UTC
Is this a candidate to be backported to 18.11.x?

I am testing this at the moment, and each existing biblionumber I put in the Record field I get the 'This bibliographic record does not exist' message and and blocked from going to the next page.

Can anyone think of any reason the API call would be failing for me? I may be missing something.
Comment 19 Jonathan Druart 2020-01-14 08:33:50 UTC
(In reply to Hayley Mapley from comment #18)
> Is this a candidate to be backported to 18.11.x?
> 
> I am testing this at the moment, and each existing biblionumber I put in the
> Record field I get the 'This bibliographic record does not exist' message
> and and blocked from going to the next page.
> 
> Can anyone think of any reason the API call would be failing for me? I may
> be missing something.

Hi Hayley, the API routes for biblios do not exist in 18.11 (added by bug 23677).
Comment 20 Lucas Gass 2020-01-14 15:52:31 UTC
backported to 19.05.x for 19.05.07
Comment 21 Katrin Fischer 2020-01-14 20:26:11 UTC
(In reply to Lucas Gass from comment #20)
> backported to 19.05.x for 19.05.07

Hi Luke, it looks like the dependency is not in 19.05 - did you test if this works?
Comment 22 Hayley Mapley 2020-01-14 20:46:48 UTC
Thanks Jonathan :)
Comment 23 Lucas Gass 2020-01-14 20:59:48 UTC
Kait, 

Im sorry, I am missing that dependency. I have reverted this patch set. This is NOT in 19.05.x

Sorry for the confusion.