Bug 17387: (follow-up) address review feedback
- Fix permission description to explicitly mention bibliographic records and items (not just 'records') - Fix date filter to use correct DB column per table: deletedbiblio uses 'timestamp', deleteditems uses 'deleted_on' - Remove leftover CAN_user_editcatalogue_edit_catalogue condition from the Batch editing section in cat-menu.inc; a user with only edit_catalogue would see an empty section heading - Wrap biblio and item restore operations in DB transactions so that a failed store cannot leave a record inserted without being removed from the deleted table (or vice-versa) - Catch Koha::Exceptions::Object::DuplicateID in the item restore controller and return a human-readable 409 instead of a generic 500 - When 'Restore bibliographic record' is clicked in the deleted-bib warning modal, fetch the biblio and open the full restore modal with the originally clicked item pre-selected, rather than restoring immediately without item selection Signed-off-by: Michaela <michaela.sieber@kit.edu> Sponsored-by: ByWater Solutions Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize
Lucas Gass
parent
3a83da117e
commit
06f4abbf9a
|
|
@ -181,35 +181,42 @@ sub restore {
|
|||
|
||||
$params //= {};
|
||||
|
||||
my $patron = $params->{patron};
|
||||
my $item_ids = $params->{item_ids};
|
||||
my $restore_all = $params->{restore_all} // 0;
|
||||
|
||||
my $biblionumber = $self->biblionumber;
|
||||
my @restored_items;
|
||||
my @skipped_items;
|
||||
|
||||
my $new_biblio = Koha::Database->schema->txn_do(
|
||||
sub {
|
||||
my $biblio_data = $self->unblessed;
|
||||
my $biblioitem = $self->biblioitem;
|
||||
my $biblioitem_data = $biblioitem->unblessed;
|
||||
my $metadata = $self->metadata;
|
||||
my $metadata_data = $metadata->unblessed;
|
||||
|
||||
my $new_biblio = Koha::Biblio->new($biblio_data)->store;
|
||||
my $biblio = Koha::Biblio->new($biblio_data)->store;
|
||||
|
||||
$biblioitem_data->{biblionumber} = $new_biblio->biblionumber;
|
||||
$biblioitem_data->{biblioitemnumber} = $new_biblio->biblionumber;
|
||||
$biblioitem_data->{biblionumber} = $biblio->biblionumber;
|
||||
$biblioitem_data->{biblioitemnumber} = $biblio->biblionumber;
|
||||
Koha::Biblioitem->new($biblioitem_data)->store;
|
||||
|
||||
delete $metadata_data->{id};
|
||||
$metadata_data->{biblionumber} = $new_biblio->biblionumber;
|
||||
$metadata_data->{biblionumber} = $biblio->biblionumber;
|
||||
Koha::Biblio::Metadata->new($metadata_data)->store;
|
||||
|
||||
$metadata->delete;
|
||||
$biblioitem->delete;
|
||||
$self->delete;
|
||||
|
||||
my $patron = $params->{patron};
|
||||
my $item_ids = $params->{item_ids};
|
||||
my $restore_all = $params->{restore_all} // 0;
|
||||
|
||||
my @restored_items;
|
||||
my @skipped_items;
|
||||
return $biblio;
|
||||
}
|
||||
);
|
||||
|
||||
if ( $restore_all || $item_ids ) {
|
||||
my $deleted_items = $self->items;
|
||||
my $deleted_items = Koha::Old::Items->search( { biblionumber => $biblionumber } );
|
||||
while ( my $deleted_item = $deleted_items->next ) {
|
||||
my $should_restore = 0;
|
||||
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ use C4::Context;
|
|||
use C4::Log qw( logaction );
|
||||
use Koha::Biblio;
|
||||
use Koha::Biblios;
|
||||
use Koha::Database;
|
||||
use Koha::Exceptions;
|
||||
use Koha::Item;
|
||||
use Koha::Old::Biblios;
|
||||
|
|
@ -78,13 +79,18 @@ sub restore {
|
|||
Koha::Exceptions::ObjectNotFound->throw("Bibliographic record not found for item")
|
||||
unless $biblio;
|
||||
|
||||
my $new_item = Koha::Database->schema->txn_do(
|
||||
sub {
|
||||
my $item_data = $self->unblessed;
|
||||
delete $item_data->{deleted_on};
|
||||
|
||||
my $new_item = Koha::Item->new($item_data)->store;
|
||||
|
||||
my $item = Koha::Item->new($item_data)->store;
|
||||
$self->delete;
|
||||
|
||||
return $item;
|
||||
}
|
||||
);
|
||||
|
||||
my $indexer = Koha::SearchEngine::Indexer->new( { index => $Koha::SearchEngine::BIBLIOS_INDEX } );
|
||||
$indexer->index_records( $new_item->biblionumber, "specialUpdate", "biblioserver" );
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ use Modern::Perl;
|
|||
use Mojo::Base 'Mojolicious::Controller';
|
||||
|
||||
use Koha::Exceptions;
|
||||
use Koha::Exceptions::Object;
|
||||
use Koha::Old::Items;
|
||||
|
||||
use Scalar::Util qw( blessed );
|
||||
|
|
@ -110,6 +111,12 @@ sub restore {
|
|||
}
|
||||
);
|
||||
}
|
||||
if ( blessed $_ && $_->isa('Koha::Exceptions::Object::DuplicateID') ) {
|
||||
return $c->render(
|
||||
status => 409,
|
||||
openapi => { error => "Cannot restore item: another item with the same barcode already exists." }
|
||||
);
|
||||
}
|
||||
$c->unhandled_exception($_);
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -133,7 +133,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES
|
|||
(13, 'records_batchmod', 'Perform batch modification of records (biblios or authorities)'),
|
||||
(13, 'marc_modification_templates', 'Manage marc modification templates'),
|
||||
(13, 'records_batchdel', 'Perform batch deletion of records (bibliographic or authority)'),
|
||||
(13, 'records_restore', 'Restore deleted records'),
|
||||
(13, 'records_restore', 'Restore deleted bibliographic records and items'),
|
||||
(13, 'access_files', 'Access to the files stored on the server'),
|
||||
(13, 'upload_general_files', 'Upload any file'),
|
||||
(13, 'upload_manage', 'Manage uploaded files'),
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@
|
|||
</ul>
|
||||
[% END %]
|
||||
|
||||
[% IF ( CAN_user_tools_items_batchmod || CAN_user_tools_items_batchdel || CAN_user_tools_records_batchmod || CAN_user_tools_records_batchdel || CAN_user_tools_records_restore || CAN_user_tools_marc_modification_templates || CAN_user_editcatalogue_edit_catalogue ) %]
|
||||
[% IF ( CAN_user_tools_items_batchmod || CAN_user_tools_items_batchdel || CAN_user_tools_records_batchmod || CAN_user_tools_records_batchdel || CAN_user_tools_records_restore || CAN_user_tools_marc_modification_templates ) %]
|
||||
<h5>Batch editing</h5>
|
||||
<ul>
|
||||
[% IF ( CAN_user_tools_items_batchmod ) %]
|
||||
|
|
|
|||
|
|
@ -464,7 +464,7 @@
|
|||
<span class="sub_permission records_batchmod_subpermission"> Perform batch modification of records (biblios or authorities) </span>
|
||||
<span class="permissioncode">([% name | html %])</span>
|
||||
[%- CASE 'records_restore' -%]
|
||||
<span class="sub_permission records_restore_subpermission"> Restore deleted records </span>
|
||||
<span class="sub_permission records_restore_subpermission"> Restore deleted bibliographic records and items </span>
|
||||
<span class="permissioncode">([% name | html %])</span>
|
||||
[%- CASE 'rotating_collections' -%]
|
||||
<span class="sub_permission rotating_collections_subpermission"> Manage rotating collections </span>
|
||||
|
|
|
|||
|
|
@ -216,7 +216,7 @@
|
|||
}
|
||||
}, 100);
|
||||
|
||||
function buildApiUrl(baseUrl) {
|
||||
function buildApiUrl(baseUrl, dateField) {
|
||||
var from = $("#from").val();
|
||||
var to = $("#to").val();
|
||||
|
||||
|
|
@ -226,15 +226,15 @@
|
|||
|
||||
var query = {};
|
||||
if (from && to) {
|
||||
query["me.timestamp"] = {
|
||||
query[dateField] = {
|
||||
"-between": [from + "T00:00:00Z", to + "T23:59:59Z"],
|
||||
};
|
||||
} else if (from) {
|
||||
query["me.timestamp"] = {
|
||||
query[dateField] = {
|
||||
">=": from + "T00:00:00Z",
|
||||
};
|
||||
} else if (to) {
|
||||
query["me.timestamp"] = {
|
||||
query[dateField] = {
|
||||
"<=": to + "T23:59:59Z",
|
||||
};
|
||||
}
|
||||
|
|
@ -245,7 +245,7 @@
|
|||
// Deleted biblios DataTable
|
||||
var biblios_table = $("#deleted_biblios_table").kohaTable({
|
||||
ajax: {
|
||||
url: buildApiUrl("/api/v1/deleted/biblios"),
|
||||
url: buildApiUrl("/api/v1/deleted/biblios", "me.timestamp"),
|
||||
},
|
||||
embed: "items",
|
||||
order: [[3, "desc"]],
|
||||
|
|
@ -313,7 +313,7 @@
|
|||
// Deleted items DataTable
|
||||
var items_table = $("#deleted_items_table").kohaTable({
|
||||
ajax: {
|
||||
url: buildApiUrl("/api/v1/deleted/items"),
|
||||
url: buildApiUrl("/api/v1/deleted/items", "me.deleted_on"),
|
||||
},
|
||||
embed: "biblio",
|
||||
order: [[5, "desc"]],
|
||||
|
|
@ -405,30 +405,20 @@
|
|||
var modal_items_table = null;
|
||||
var modal_items_table_api = null;
|
||||
|
||||
// Restore biblio handler
|
||||
$("#deleted_biblios_table").on("click", ".restore-biblio", function (e) {
|
||||
e.preventDefault();
|
||||
var button = $(this);
|
||||
var biblio_id = button.data("biblio-id");
|
||||
var title = button.data("title");
|
||||
var row = biblios_table_api.row(button.closest("tr")).data();
|
||||
|
||||
// Populate modal with biblio info
|
||||
// Populate and show the restore biblio modal.
|
||||
// preselectedItemId: optional item_id to pre-check in the items list.
|
||||
function showRestoreBiblioModal(biblio_id, title, author, items, preselectedItemId) {
|
||||
$("#restore-modal-biblio-id").text(biblio_id);
|
||||
$("#restore-modal-biblio-title").text(title || _("(No title)"));
|
||||
$("#restore-modal-biblio-author").text(row.author || _("(No author)"));
|
||||
$("#restore-modal-biblio-author").text(author || _("(No author)"));
|
||||
|
||||
// Check if there are deleted items
|
||||
var items = row.items || [];
|
||||
if (items.length > 0) {
|
||||
$("#items-section").show();
|
||||
|
||||
// Destroy existing DataTable if it exists
|
||||
if (modal_items_table_api) {
|
||||
modal_items_table_api.destroy();
|
||||
}
|
||||
|
||||
// Initialize kohaTable with the items data
|
||||
modal_items_table = $("#deleted-items-list").kohaTable({
|
||||
data: items,
|
||||
paging: false,
|
||||
|
|
@ -439,7 +429,8 @@
|
|||
if (type === "display") {
|
||||
var canEdit = canEditItem(row.home_library_id);
|
||||
var disabled = canEdit ? '' : ' disabled';
|
||||
var checkbox = '<input type="checkbox" class="item-checkbox" data-item-id="' + row.item_id + '"' + disabled + '>';
|
||||
var preselected = (preselectedItemId && row.item_id == preselectedItemId && canEdit) ? ' checked' : '';
|
||||
var checkbox = '<input type="checkbox" class="item-checkbox" data-item-id="' + row.item_id + '"' + disabled + preselected + '>';
|
||||
|
||||
if (!canEdit) {
|
||||
return '<span title="' + _("You do not have permission to restore items from this library") + '">' + checkbox + '</span>';
|
||||
|
|
@ -501,12 +492,21 @@
|
|||
$("#items-section").hide();
|
||||
}
|
||||
|
||||
// Store biblio data
|
||||
$("#restore-biblio-with-items-button").data("biblio-id", biblio_id);
|
||||
$("#restore-biblio-with-items-button").data("restore-button", button);
|
||||
|
||||
var modal = new bootstrap.Modal(document.getElementById("restoreBiblioModal"));
|
||||
modal.show();
|
||||
}
|
||||
|
||||
// Restore biblio handler (from deleted biblios table)
|
||||
$("#deleted_biblios_table").on("click", ".restore-biblio", function (e) {
|
||||
e.preventDefault();
|
||||
var button = $(this);
|
||||
var biblio_id = button.data("biblio-id");
|
||||
var row = biblios_table_api.row(button.closest("tr")).data();
|
||||
|
||||
$("#restore-biblio-with-items-button").data("restore-button", button);
|
||||
showRestoreBiblioModal(biblio_id, row.title, row.author, row.items || []);
|
||||
});
|
||||
|
||||
// Restore item handler
|
||||
|
|
@ -561,30 +561,37 @@
|
|||
});
|
||||
});
|
||||
|
||||
// Restore biblio from modal
|
||||
// "Restore bibliographic record" button in the deleted-bib warning modal:
|
||||
// fetch the biblio (with items) and open the full restore modal with the
|
||||
// originally clicked item pre-selected.
|
||||
$("#restore-biblio-button").on("click", function () {
|
||||
var button = $(this);
|
||||
var biblio_id = button.data("biblio-id");
|
||||
var item_id = button.data("item-id");
|
||||
var original_item_id = button.data("item-id");
|
||||
var item_button = button.data("item-button");
|
||||
|
||||
button.prop("disabled", true);
|
||||
|
||||
$.ajax({
|
||||
url: "/api/v1/deleted/biblios/" + biblio_id,
|
||||
type: "PUT",
|
||||
type: "GET",
|
||||
headers: {
|
||||
"x-koha-request-id": Math.random(),
|
||||
"x-koha-embed": "items",
|
||||
},
|
||||
success: function (data) {
|
||||
showMessage(_("Bibliographic record %s restored successfully").format(biblio_id), "success");
|
||||
success: function (biblio) {
|
||||
bootstrap.Modal.getInstance(document.getElementById("deletedBiblioModal")).hide();
|
||||
biblios_table_api.ajax.reload();
|
||||
items_table_api.ajax.reload();
|
||||
$("#restore-biblio-with-items-button").data("restore-button", item_button);
|
||||
showRestoreBiblioModal(
|
||||
biblio.biblio_id,
|
||||
biblio.title,
|
||||
biblio.author,
|
||||
biblio.items || [],
|
||||
original_item_id
|
||||
);
|
||||
button.prop("disabled", false);
|
||||
},
|
||||
error: function (xhr) {
|
||||
var error_msg = _("Error restoring bibliographic record %s").format(biblio_id);
|
||||
var error_msg = _("Error loading bibliographic record %s").format(biblio_id);
|
||||
if (xhr.responseJSON && xhr.responseJSON.error) {
|
||||
error_msg += ": " + xhr.responseJSON.error;
|
||||
}
|
||||
|
|
@ -678,8 +685,8 @@
|
|||
});
|
||||
|
||||
$("#filter_table").on("click", function () {
|
||||
biblios_table_api.ajax.url(buildApiUrl("/api/v1/deleted/biblios")).load();
|
||||
items_table_api.ajax.url(buildApiUrl("/api/v1/deleted/items")).load();
|
||||
biblios_table_api.ajax.url(buildApiUrl("/api/v1/deleted/biblios", "me.timestamp")).load();
|
||||
items_table_api.ajax.url(buildApiUrl("/api/v1/deleted/items", "me.deleted_on")).load();
|
||||
});
|
||||
|
||||
$("#clear_filters").on("click", function () {
|
||||
|
|
|
|||
Loading…
Reference in New Issue