From: Jonathan Druart Date: Wed, 20 Jul 2022 10:01:05 +0000 (+0200) Subject: Bug 32030: ERM - Agreement documents (FIXED) X-Git-Tag: v22.11.00~372 X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=commitdiff_plain;h=64a838ce32f8ad6451729a4a7739d3353da8efcb;p=koha-ffzg.git Bug 32030: ERM - Agreement documents (FIXED) Document - don't send file_content when fetching a document apt install libmojolicious-plugin-renderfile-perl Signed-off-by: Jonathan Field Signed-off-by: Martin Renvoize Signed-off-by: Kyle M Hall Signed-off-by: Tomas Cohen Arazi --- diff --git a/Koha/ERM/Agreement.pm b/Koha/ERM/Agreement.pm index f199589759..817d57b10f 100644 --- a/Koha/ERM/Agreement.pm +++ b/Koha/ERM/Agreement.pm @@ -17,6 +17,9 @@ package Koha::ERM::Agreement; use Modern::Perl; +use MIME::Base64 qw( decode_base64 ); +use MIME::Types; + use Koha::Database; use Koha::DateUtils qw( dt_from_string ); @@ -186,13 +189,47 @@ sub documents { my $schema = $self->_result->result_source->schema; $schema->txn_do( sub { - $self->documents->delete; + my $existing_documents = $self->documents; + + # FIXME Here we are not deleting all the documents before recreating them, like we do for other related resources. + # As we do not want the content of the documents to transit over the network we need to use the document_id (and allow it in the API spec) + # to distinguish from each other + # Delete all the documents that are not part of the PUT request + my $modified_document_ids = [ map { $_->{document_id} || () } @$documents ]; + $self->documents->search( + { + @$modified_document_ids + ? ( + document_id => { + '-not_in' => $modified_document_ids + } + ) + : () + } + )->delete; + for my $document (@$documents) { - if ( $document->{file_content} ) { - $document->{file_type} = 'unknown'; # FIXME How to detect file type from base64? - $document->{uploaded_on} //= dt_from_string; + if ( $document->{document_id} ) { + # The document already exists in DB + $existing_documents->find( $document->{document_id} ) + ->set( + { + file_description => $document->{file_description}, + physical_location => $document->{physical_location}, + uri => $document->{uri}, + notes => $document->{notes}, + } + )->store; + } + else { + # Creating a whole new document + my $file_content = decode_base64( $document->{file_content} ); + my $mt = MIME::Types->new(); + $document->{file_type} = $mt->mimeTypeOf( $document->{file_name} ); + $document->{uploaded_on} //= dt_from_string; + $document->{file_content} = $file_content; + $self->_result->add_to_erm_agreement_documents( $document); } - $self->_result->add_to_erm_agreement_documents($document); } } ); diff --git a/Koha/ERM/Agreement/Document.pm b/Koha/ERM/Agreement/Document.pm index 41cae26ac7..d83a1c7b71 100644 --- a/Koha/ERM/Agreement/Document.pm +++ b/Koha/ERM/Agreement/Document.pm @@ -31,6 +31,17 @@ Koha::ERM::Agreement::Document - Koha Agreement Document Object class =cut +=head3 to_api_mapping + +=cut + +sub to_api_mapping { + return { + # Do not expose file_content to prevent the content to be fetch from the DB and sent over the network when embeded + file_content => undef, + }; +} + =head2 Internal methods =head3 _type diff --git a/Koha/REST/V1.pm b/Koha/REST/V1.pm index 79d73bbb87..13bbcfcf20 100644 --- a/Koha/REST/V1.pm +++ b/Koha/REST/V1.pm @@ -98,6 +98,8 @@ sub startup { route => $self->routes->under('/api/v1')->to('Auth#under'), } ); + + $self->plugin('RenderFile'); } catch { # Validation of the complete spec failed. Resort to validation one-by-one diff --git a/Koha/REST/V1/ERM/Documents.pm b/Koha/REST/V1/ERM/Documents.pm new file mode 100644 index 0000000000..47594f58b9 --- /dev/null +++ b/Koha/REST/V1/ERM/Documents.pm @@ -0,0 +1,61 @@ +package Koha::REST::V1::ERM::Documents; + +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . + +use Modern::Perl; + +use Mojo::Base 'Mojolicious::Controller'; + +use Koha::ERM::Agreement::Documents; + +use Scalar::Util qw( blessed ); +use Try::Tiny qw( catch try ); + +=head1 API + +=head2 Methods + +=head3 get + +Controller function that handles retrieving a single Koha::ERM::Agreement object + +=cut + +sub get { + my $c = shift->openapi->valid_input or return; + + return try { + my $agreement_id = $c->validation->param('agreement_id'); + my $document_id = $c->validation->param('document_id'); + + # Do not use $c->objects->find here, we need the file_content + my $document = Koha::ERM::Agreement::Documents->find($document_id); + + if ( !$document || $document->agreement_id != $agreement_id) { + return $c->render( + status => 404, + openapi => { error => "Document not found" } + ); + } + + $c->render_file('data' => $document->file_content, 'filename' => $document->file_name); + } + catch { + $c->unhandled_exception($_); + }; +} + +1; diff --git a/api/v1/swagger/definitions/erm_agreement_document.yaml b/api/v1/swagger/definitions/erm_agreement_document.yaml index d4f9e294b2..fddef432be 100644 --- a/api/v1/swagger/definitions/erm_agreement_document.yaml +++ b/api/v1/swagger/definitions/erm_agreement_document.yaml @@ -4,7 +4,6 @@ properties: document_id: type: integer description: internally assigned identifier - readOnly: true agreement_id: type: integer description: Internal agreement identifier @@ -17,7 +16,8 @@ properties: type: - string - "null" - description: Name of the file + readOnly: true + description: Type of the file file_description: type: - string @@ -32,13 +32,9 @@ properties: type: - string - "null" + readOnly: true format: date-time description: Datetime of the upload - file_type: - type: - - string - - "null" - description: Name of the file physical_location: type: - string diff --git a/api/v1/swagger/paths/erm_documents.yaml b/api/v1/swagger/paths/erm_documents.yaml new file mode 100644 index 0000000000..0dc73a3052 --- /dev/null +++ b/api/v1/swagger/paths/erm_documents.yaml @@ -0,0 +1,51 @@ +--- +"/erm/agreements/{agreement_id}/documents/{document_id}/file/content": + get: + x-mojo-to: ERM::Documents#get + operationId: downloadErmAgreementDocument + tags: + - document + summary: Download agreement document + produces: + - application/octet-stream + parameters: + - description: Case insensitive search on agreement agreement_id + in: path + name: agreement_id + required: true + type: integer + - description: Case insensitive search on agreement document_id + in: path + name: document_id + required: true + type: integer + responses: + 200: + description: Anagreement + schema: + type: file + 401: + description: Authentication required + schema: + $ref: "../swagger.yaml#/definitions/error" + 403: + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + 404: + description: Ressource not found + schema: + $ref: "../swagger.yaml#/definitions/error" + 500: + description: |- + Internal server error. Possible `error_code` attribute values: + * `internal_server_error` + schema: + $ref: "../swagger.yaml#/definitions/error" + 503: + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + erm: 1 diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 34873839dd..0bb636ea78 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -171,6 +171,8 @@ paths: $ref: ./paths/erm_agreements.yaml#/~1erm~1agreements "/erm/agreements/{agreement_id}": $ref: "./paths/erm_agreements.yaml#/~1erm~1agreements~1{agreement_id}" + "/erm/agreements/{agreement_id}/documents/{document_id}/file/content": + $ref: "./paths/erm_documents.yaml#/~1erm~1agreements~1{agreement_id}~1documents~1{document_id}~1file~1content" "/erm/eholdings/{provider}/titles": $ref: ./paths/erm_eholdings_titles.yaml#/~1erm~1eholdings~1{provider}~1titles "/erm/eholdings/{provider}/titles/{title_id}": diff --git a/cpanfile b/cpanfile index 259c854e2f..bd02675b62 100644 --- a/cpanfile +++ b/cpanfile @@ -80,6 +80,7 @@ requires 'Module::CPANfile', '1.1000'; requires 'Mojo::JWT', '0.08'; requires 'Mojolicious', '8.12'; requires 'Mojolicious::Plugin::OpenAPI', '5.05'; +requires 'Mojolicious::Plugin::RenderFile', '0.12'; requires 'Net::CIDR', '0.17'; requires 'Net::Netmask', '1.9022'; requires 'Net::Stomp', '0.57'; diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsFormAdd.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsFormAdd.vue index 0aef2ba742..9229d8c1e4 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsFormAdd.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsFormAdd.vue @@ -348,7 +348,7 @@ export default { agreement.agreement_relationships = agreement.agreement_relationships.map(({ related_agreement, ...keepAttrs }) => keepAttrs) - agreement.documents = agreement.documents.map(({ document_id, ...keepAttrs }) => keepAttrs) + agreement.documents = agreement.documents.map(({ file_type, uploaded_on, ...keepAttrs }) => keepAttrs) delete agreement.agreement_packages diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsShow.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsShow.vue index b065b7f9cd..240da65d50 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsShow.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsShow.vue @@ -251,6 +251,43 @@ + +
  • + +
    +
      +
    • +
      + {{ document.file_description }} - + + + {{ document.file_name }} + + + ({{ document.file_type }}) Uploaded on: + {{ format_date(document.uploaded_on) }} +
      +
      + {{ $t("Physical location") }}: + {{ document.physical_location }} +
      +
      + {{ $t("URI") }}: {{ document.uri }} +
      +
      + {{ $t("Notes") }}: {{ document.notes }} +
      +
      • +
    +
    +
    • @@ -333,7 +370,11 @@ export default { font-size: 11px; } #agreement_relationships, -#agreement_packages { +#agreement_packages, +#agreement_documents { padding-left: 10rem; } - \ No newline at end of file +#agreement_documents ul { + padding-left: 0; +} +