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 <jonathan.field@ptfs-europe.com>

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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;
-                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;
+                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->{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);
                 }
             }
         );

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

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

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 <http://www.gnu.org/licenses>.
+
+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;

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

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

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}":

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';

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
 

koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsShow.vue

@@ -251,6 +251,43 @@
                             </div>
                         </div>
                     </li>
+
+                    <li v-if="agreement.documents.length">
+                        <label>{{ $t("Documents") }}</label>
+                        <div id="agreement_documents">
+                            <ul>
+                                <li
+                                    v-for="document in agreement.documents"
+                                    v-bind:key="document.document_id"
+                                >
+                                    <div v-if="document.file_name">
+                                        <span v-if="document.file_description"
+                                            >{{ document.file_description }} -
+                                        </span>
+                                        <a
+                                            download
+                                            :href="`/api/v1/erm/agreements/${agreement.agreement_id}/documents/${document.document_id}/file/content`"
+                                        >
+                                            {{ document.file_name }}
+                                            <i class="fa fa-download"></i>
+                                        </a>
+                                        ({{ document.file_type }}) Uploaded on:
+                                        {{ format_date(document.uploaded_on) }}
+                                    </div>
+                                    <div v-if="document.physical_location">
+                                        {{ $t("Physical location") }}:
+                                        {{ document.physical_location }}
+                                    </div>
+                                    <div v-if="document.uri">
+                                        {{ $t("URI") }}: {{ document.uri }}
+                                    </div>
+                                    <div v-if="document.notes">
+                                        {{ $t("Notes") }}: {{ document.notes }}
+                                    </div>
+                                </li>
+                            </ul>
+                        </div>
+                    </li>
                 </ol>
             </fieldset>
             <fieldset class="action">
@@ -333,7 +370,11 @@ export default {
     font-size: 11px;
 }
 #agreement_relationships,
-#agreement_packages {
+#agreement_packages,
+#agreement_documents {
     padding-left: 10rem;
 }
-</style>
+#agreement_documents ul {
+    padding-left: 0;
+}
+</style>