When a patron makes a suggestion from an existing bib, the patron is allowed to edit the bibliographic information in the suggestion. This leads to confusion, since the suggestion is still tied to the original bib. To recreate: - find a record in the OPAC - click Suggest for Purchase - delete the title, author, ISBN, etc that have been pre-populated from the original bib - fill out a suggestion for an entirely different book - submit - go to your purchase suggestion in your account on the OPAC. See the title you entered, observe it links to the bib you started from - go over to the staff side and Accept your suggestion - put your suggestion into a basket - observe your order is for the original bib but your suggestion still shows the new title The simplest answer I see here is to just not allowing editing of those fields when making a suggestion from an existing title.
Created attachment 133120 [details] [review] Bug 29311: Do not allow editing of bibliographic information when entering suggestion from existing bibs This patch changes the behavior of the suggestion form in OPAC. The suggestion form for an existing record has its total of inputs limited, otherwise the inputs are all available. Test plan: 1)OPAC: Login to your OPAC account 2)Choose a record and get onto its details page 3)Click 'suggest for purchase' 4)Notice: all the fields can be modified 5)Apply patch 6)Repeat 2) and 3) or refresh 7)You see that the fields about bibliographic informations are disabled now
Created attachment 134531 [details] [review] Bug 29311: Do not allow editing of bibliographic information when entering suggestion from existing bibs This patch changes the behavior of the suggestion form in OPAC. The suggestion form for an existing record has its total of inputs limited, otherwise the inputs are all available. Test plan: 1)OPAC: Login to your OPAC account 2)Choose a record and get onto its details page 3)Click 'suggest for purchase' 4)Notice: all the fields can be modified 5)Apply patch 6)Repeat 2) and 3) or refresh 7)You see that the fields about bibliographic informations are disabled now Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Thank you Jérémy, everything seems to work as expected, I didn't notice strange behaviours Maybe just a tiny modification could be done when using OPACSuggestionMandatoryFields syspref : as field can't be modified, could the 'Required' mention be hidden ?
Looking here
Hi Jeremy, Why do we not disable the form fields in the template instead of relying on the JavaScript do do it? And I think that would also work with Severine's request, as we then could and shoudl remove the class="required" in the form. + [% IF ( biblionumber ) %] + <label for="publishercode" class="required">Publisher:</label> + [% publishercode | html %] + [% ELSIF ( publishercode_required ) %] I feel like class="required" should be removed here?
Created attachment 146849 [details] [review] Bug 29311: Forbid editing biblio info when creating suggestion for biblio When creating a suggestion for an existing biblio, biblio info like title or author should not be editable. Test plan: 1. Enable system preference 'suggestion' 2. Log in at OPAC 3. Go to a biblio record detail page 4. Click 'suggest for purchase' 5. Notice that all fields can be modified (title, author, ...) 6. Apply patch 7. Repeat steps 2 and 3 or refresh the page 8. Verify that the biblio fields cannot be modified now. The only fields that should be editable are: quantity, item type, library, reason for suggestion, and notes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
(In reply to Katrin Fischer from comment #5) > Why do we not disable the form fields in the template instead of relying on > the JavaScript do do it? You're right. The javascript code was completely unnecessary. It was probably the remains of the first version of this patch :) > And I think that would also work with Severine's request, as we then could > and shoudl remove the class="required" in the form. Lazy copy/paste. Fixed in the latest patch
Created attachment 148776 [details] [review] Bug 29311: Forbid editing biblio info when creating suggestion for biblio When creating a suggestion for an existing biblio, biblio info like title or author should not be editable. Test plan: 1. Enable system preference 'suggestion' 2. Log in at OPAC 3. Go to a biblio record detail page 4. Click 'suggest for purchase' 5. Notice that all fields can be modified (title, author, ...) 6. Apply patch 7. Repeat steps 2 and 3 or refresh the page 8. Verify that the biblio fields cannot be modified now. The only fields that should be editable are: quantity, item type, library, reason for suggestion, and notes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com>
Created attachment 148922 [details] [review] Bug 29311: Forbid editing biblio info when creating suggestion for biblio When creating a suggestion for an existing biblio, biblio info like title or author should not be editable. Test plan: 1. Enable system preference 'suggestion' 2. Log in at OPAC 3. Go to a biblio record detail page 4. Click 'suggest for purchase' 5. Notice that all fields can be modified (title, author, ...) 6. Apply patch 7. Repeat steps 2 and 3 or refresh the page 8. Verify that the biblio fields cannot be modified now. The only fields that should be editable are: quantity, item type, library, reason for suggestion, and notes Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Laura Escamilla <laura.escamilla@bywatersolutions.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 148923 [details] [review] Bug 29311: (QA follow-up) Fix an existing spelling typo
Created attachment 148924 [details] [review] Bug 29311: (QA follow-up) Fix filter
Pushed to master for 23.05. Nice work everyone, thanks!
It looks like this breaks a selenium test: t/db_dependent/selenium/regressions.t .. 7/8 STRACE: /usr/share/perl5/Try/Tiny.pm:123 in Selenium::Remote::Driver::catch {...} /usr/share/perl5/Selenium/Remote/Driver.pm:361 in Try::Tiny::try (eval 549):1 in Selenium::Remote::Driver::__ANON__ (eval 551):2 in Selenium::Remote::Driver::__ANON__ /usr/share/perl5/Selenium/Remote/Driver.pm:1340 in Selenium::Remote::Driver::_execute_command /usr/share/perl5/Selenium/Remote/Driver.pm:1340 in (eval) t/db_dependent/selenium/regressions.t:358 in Selenium::Remote::Driver::find_element /usr/share/perl/5.32/Test/Builder.pm:334 in main::__ANON__ /usr/share/perl/5.32/Test/Builder.pm:334 in (eval) /usr/share/perl/5.32/Test/More.pm:809 in Test::Builder::subtest t/db_dependent/selenium/regressions.t:372 in Test::More::subtest # No tests run! t/db_dependent/selenium/regressions.t .. 8/8 # Failed test 'No tests run for subtest "OPAC - Suggest for purchase"' # at t/db_dependent/selenium/regressions.t line 372. Error while executing command: no such element: Unable to locate element: //input[@id="title"] at /usr/share/perl5/Selenium/Remote/Driver.pm line 411. at /usr/share/perl5/Selenium/Remote/Driver.pm line 356. # Looks like your test exited with 11 just after 8. The test looks for an input field, but with this patch applied, there is no longer any input, just this: <label>Title:</label> <a href="/cgi-bin/koha/opac-detail.pl?biblionumber=1">E Street shuffle :</a>
Created attachment 149391 [details] [review] Bug 29311: (follow-up) Add id to span elements The input fields in the form have ids, these are useful for styling, but also for the Selenium tests. This patch adds the ids from the former inputs to the now spans.
Created attachment 149392 [details] [review] Bug 29311: (follow-up) Fix t/db_dependent/selenium/regressions.t The test was checking for the value of an input field. But with this patch set, there is no longer an input, but a link element with the title. This adjusts the patch to look for the correct element.
Last two patches pushed to master.
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Backported to 22.05.x for upcoming 22.05.13.
applied to 21.11 for 21.11.21