This would be handy, as right now it can only be set through an API request.
+1 that would be handy! Also, tomas forgot to mention, the api may set the record source upon creation of a record (which is great) but there is no way to change it (even with the api) once it's created. Then the only way to change it is through a direct access to the database. In some use-cases it may be needed that a librarian "unlock" a record to let a fellow edit the record.
Created attachment 166161 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface
Isn't it a done by Bug 35919 ?
(In reply to Fridolin Somers from comment #3) > Isn't it a done by Bug 35919 ? it is not, Bug 35919 provides all the logic but no interface. Which this bug does (based on the logic provided by bug 35919).
Created attachment 168054 [details] [review] Bug 36372 : trigger a record indexing when the record source is set
Created attachment 168195 [details] [review] Bug 36372 : Cypress tests
Created attachment 168338 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface Test plan : - apply Cypress patch - running cypress patch in ktd will fail - apply both code patches, cypress tests will pass - open any record in the staff interface - with superlibrarian user, click on the "Edit" button group. - you should be able to click on the "Set record source" below the "Edit record button". - you will be given a form to choose between any defined record source. If none defined there would be a link to configure the record sources. - change a users permissions to have cataloguing permissions but without the "set_record_source" permissions. - login as this forementionned user and go to any record view. - the "Set record source" button should now be greyed out and not possible to click.#
Created attachment 168339 [details] [review] Bug 36372: trigger a record indexing when the record source is set
Created attachment 168340 [details] [review] Bug 36372: Cypress tests
Created attachment 168430 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface Test plan : - apply Cypress patch - running cypress patch in ktd will fail - apply both code patches, cypress tests will pass - open any record in the staff interface - with superlibrarian user, click on the "Edit" button group. - you should be able to click on the "Set record source" below the "Edit record button". - you will be given a form to choose between any defined record source. If none defined there would be a link to configure the record sources. - change a users permissions to have cataloguing permissions but without the "set_record_source" permissions. - login as this forementionned user and go to any record view. - the "Set record source" button should now be greyed out and not possible to click.#
Created attachment 168510 [details] [review] Bug 36372: Cypress tests Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 168511 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface Test plan : - apply Cypress patch - running cypress patch in ktd will fail - apply both code patches, cypress tests will pass - open any record in the staff interface - with superlibrarian user, click on the "Edit" button group. - you should be able to click on the "Set record source" below the "Edit record button". - you will be given a form to choose between any defined record source. If none defined there would be a link to configure the record sources. - change a users permissions to have cataloguing permissions but without the "set_record_source" permissions. - login as this forementionned user and go to any record view. - the "Set record source" button should now be greyed out and not possible to click.# Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 168512 [details] [review] Bug 36372: trigger a record indexing when the record source is set Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Post test remarks: 0) Works as declared. 1) To make cypress test happy for the second run, I had to manually delete Poop source created in the first run. 2) It was necessary to apply interactively "trigger a record indexing when the record source is set" only after "provide an interface to set the source of a record from the staff interface" was already applied. Otherwise an error results. 3) QA scripts unhappy.
Created attachment 168897 [details] [review] Bug 36372: Cypress tests
Created attachment 168898 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface Test plan : - apply Cypress patch - running cypress patch in ktd will fail - apply both code patches, cypress tests will pass - open any record in the staff interface - with superlibrarian user, click on the "Edit" button group. - you should be able to click on the "Set record source" below the "Edit record button". - you will be given a form to choose between any defined record source. If none defined there would be a link to configure the record sources. - change a users permissions to have cataloguing permissions but without the "set_record_source" permissions. - login as this forementionned user and go to any record view. - the "Set record source" button should now be greyed out and not possible to click.#
Created attachment 168899 [details] [review] Bug 36372: trigger a record indexing when the record source is set
Created attachment 168900 [details] [review] Bug 36372: (QA follow-up) perltidy cataloguing/record_source.pl, fix permissions issues, add html filters to new template#
Hi, I have re-uploaded the patch in the correct order + made a followup patch to fix the issues from the qa scripts. Can anyone make signoff again?
Created attachment 168902 [details] [review] Bug 36372: (QA follow-up) perltidy cataloguing/record_source.pl, fix permissions issues, add html filters to new template#
Arthur, it does not work for me. After clicking the button "Set record source" from /cgi-bin/koha/cataloguing/record_source.pl?biblionumber=111, I'm getting: /cgi-bin/koha/cataloguing/record_source.pl?biblionumber=&record_source_id=4&save=Set+record+source Note, without biblionumber. And in record_source.pl you are not checking if the $biblionumber is present and correct nor if Koha::Biblios->find gave you a valid object. As a result my ktd exploded with: Can't call method "metadata" on an undefined value at /kohadevbox/koha/cataloguing/record_source.pl line 45
After the recent correction of 2024-07-12 13:01 UTC it seems to work. But there is a serious issue with deleting the record source--when you delete a record source already used in some records, it seem delete also biblio_metadata rows linked with that biblio_metadata.record_source_id. The biblio and biblioitems rows remain, but biblio_metadata is gone. Could you confirm it please? Is it universal problem or is it connected only with some local circumstances?
Created attachment 169268 [details] [review] Bug 36372: Cypress tests Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 169269 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface Test plan : - apply Cypress patch - running cypress patch in ktd will fail - apply both code patches, cypress tests will pass - open any record in the staff interface - with superlibrarian user, click on the "Edit" button group. - you should be able to click on the "Set record source" below the "Edit record button". - you will be given a form to choose between any defined record source. If none defined there would be a link to configure the record sources. - change a users permissions to have cataloguing permissions but without the "set_record_source" permissions. - login as this forementionned user and go to any record view. - the "Set record source" button should now be greyed out and not possible to click.# Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 169270 [details] [review] Bug 36372: trigger a record indexing when the record source is set Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 169271 [details] [review] Bug 36372: (QA follow-up) perltidy cataloguing/record_source.pl, fix permissions issues, add html filters to new template# Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Signed-off, but still some issues with tidiness: WARN cataloguing/record_source.pl WARN tidiness The file is less tidy than before (bad/messy lines before: 0, now: 2)
Arthur, just another remark. It seems that the template cataloguing/record_source.tt was prepared to display the current record source if already set. But it is not shown because the current record source (current_source.record_source_id) is not passed in the template call. The following change would be a possible fix: diff --git a/cataloguing/record_source.pl b/cataloguing/record_source.pl index 1005e1aa1f..61579ef5d6 100755 --- a/cataloguing/record_source.pl +++ b/cataloguing/record_source.pl @@ -62,6 +62,7 @@ my $record_sources = Koha::RecordSources->search(); $template->param( biblio => $biblio, record_sources => $record_sources, + current_source => { record_source_id => $biblio->metadata->record_source_id }, );
Created attachment 169542 [details] [review] Bug 36372: (QA followup) pass current record-source to template while editing
Created attachment 169831 [details] [review] Bug 36372: (QA followup) pass current record-source to template while editing Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 172369 [details] [review] Bug 36372: Cypress tests Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 172370 [details] [review] Bug 36372: provide an interface to set the source of a record from the staff interface Test plan : - apply Cypress patch - running cypress patch in ktd will fail - apply both code patches, cypress tests will pass - open any record in the staff interface - with superlibrarian user, click on the "Edit" button group. - you should be able to click on the "Set record source" below the "Edit record button". - you will be given a form to choose between any defined record source. If none defined there would be a link to configure the record sources. - change a users permissions to have cataloguing permissions but without the "set_record_source" permissions. - login as this forementionned user and go to any record view. - the "Set record source" button should now be greyed out and not possible to click.# Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Created attachment 172371 [details] [review] Bug 36372: Fix style issues in dropdown Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
A few remarks: * I don't think we should introduce a new set of .pl+.tt this should be handled in a simple modal. * The new page style is weird. Maybe a side-effect of the move to Bootstrap 5, cannot tell. * The form for setting the record source is sending a GET (should be post) * The form for setting the record source is not passing a CSRF token It otherwise works as expected. My follow-up just makes it look the same as the rest of the dropdown (again, bootstrap 5 change side-effect).