Description
Katrin Fischer
2018-08-16 08:16:18 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? 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 *** Bug 12921 has been marked as a duplicate of this bug. *** 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 (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. Created attachment 89357 [details] [review] Bug 21232: Add a client-side check on biblionumber when creating a subscription (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 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! 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> 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> Nice work everyone! Pushed to master for 20.05 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? 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 Pushed to 19.11.x branch for 19.11.02 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? (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. 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. (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). backported to 19.05.x for 19.05.07 (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? Thanks Jonathan :) 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. |