When suggesting an existing title for purchase, we should copy bib information from the record into the suggestion. This used to happen, but has broken in master and 21.05 (I think 21.05.04 and later). Maybe due to bug 28941? That's a shot in the dark, but is the last update to opac-suggestions.pl.
Yes, caused by commit 586bed1319592e05f5dc3acf64a1dba8cae69d6b Bug 28941: Filter suggestion inputs at the OPAC
Created attachment 127347 [details] [review] Bug 29416: (bug 28941 follow-up) Restore suggestion prefill From commit 586bed1319592e05f5dc3acf64a1dba8cae69d6b Bug 28941: Filter suggestion inputs at the OPAC But later in the script we send { suggestion => %$suggestion } to the template and so erase the variable previously passed. Test plan: Login at the OPAC Go to the bibliographic detail page Suggest for purchase => The form must be prefilled!
QA: Looking here
Out of scope, just seeing this: { last unless ($op eq 'add'); # ETC } Normally we only use last in a loop (for, while). Perl allows this and says: "Note that a block by itself is semantically identical to a loop that executes once. Thus last can be used to effect an early exit out of such a block." But surely this should just have been: if( $op eq 'add' ) { # ETC }
Created attachment 127354 [details] [review] Bug 29416: (bug 28941 follow-up) Restore suggestion prefill From commit 586bed1319592e05f5dc3acf64a1dba8cae69d6b Bug 28941: Filter suggestion inputs at the OPAC But later in the script we send { suggestion => %$suggestion } to the template and so erase the variable previously passed. Test plan: Login at the OPAC Go to the bibliographic detail page Suggest for purchase => The form must be prefilled! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested, seems to work ;) Can be backported as a minimal fix now. But surely this code should get more attention in terms of quality. Advanced hacking.
Created attachment 127356 [details] [review] Bug 29416: (bug 28941 follow-up) Restore suggestion prefill From commit 586bed1319592e05f5dc3acf64a1dba8cae69d6b Bug 28941: Filter suggestion inputs at the OPAC But later in the script we send { suggestion => %$suggestion } to the template and so erase the variable previously passed. Test plan: Login at the OPAC Go to the bibliographic detail page Suggest for purchase => The form must be prefilled! Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
(In reply to Marcel de Rooy from comment #4) > Out of scope, just seeing this: > > { > last unless ($op eq 'add'); > # ETC > } > > Normally we only use last in a loop (for, while). Perl allows this and says: > "Note that a block by itself is semantically identical to a loop that > executes once. Thus last can be used to effect an early exit out of such a > block." > > But surely this should just have been: > > if( $op eq 'add' ) { > # ETC > } Please add a bug report for that.
Pushed to master for 21.11, thanks to everybody involved!
Pushed to 21.05.x for 21.05.05
Pushed to 20.11.x for 20.11.12
Backported: Pushed to 20.05.x branch for 20.05.18
Not backporting to 19.11.x - LTS version will just backport security patches