files: fix preview button for file name containing dots

* Adds tests to support accents in file names.
* Preview links are based on mime types.
* Fixes crash when a document without extensions is submitted.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>

sonar/config_sonar.py

@@ -540,10 +540,17 @@
              'ExportSchemaV1'),
 }
 
-SONAR_APP_FILE_PREVIEW_EXTENSIONS = [
-    'jpeg', 'jpg', 'gif', 'png', 'pdf', 'json', 'xml', 'csv', 'zip', 'md'
+SONAR_APP_FILE_PREVIEW_MIMETYPES = [
+    'application/pdf',
+    'image/jpeg',
+    'image/png',
+    'application/octet-stream',
+    'image/gif',
+    'text/csv',
+    'application/json',
+    'application/xml'
 ]
-"""List of extensions for which files can be previewed."""
+"""List of the mimetype for which files can be previewed."""
 
 SONAR_APP_WEBDAV_HEG_HOST = 'https://share.rero.ch/HEG'
 SONAR_APP_WEBDAV_HEG_USER = None

sonar/modules/documents/utils.py

@@ -19,7 +19,7 @@
 
 from __future__ import absolute_import, print_function
 
-import re
+import os
 from datetime import datetime
 
 from flask import current_app, request
@@ -93,17 +93,19 @@ def get_file_links(file, record):
         links['external'] = file['external_url']
         return links
 
-    match = re.search(r'\.(.*)$', file['key'])
-    if not match:
+    if not file.get('mimetype'):
         return links
 
     links['download'] = '/documents/{pid}/files/{key}'.format(
         pid=record['pid'], key=file['key'])
 
-    if not match.group(1) in current_app.config.get(
-            'SONAR_APP_FILE_PREVIEW_EXTENSIONS', []):
+    if file['mimetype'] not in current_app.config.get(
+            'SONAR_APP_FILE_PREVIEW_MIMETYPES', []):
         return links
-
+    # only markdown is supported
+    if file['mimetype'] == 'application/octet-stream':
+        if os.path.splitext(file['key'])[-1] != '.md':
+            return links
     links['preview'] = '/documents/{pid}/preview/{key}'.format(
         pid=record['pid'], key=file['key'])
     return links

sonar/modules/utils.py

@@ -18,6 +18,7 @@
 """Utils functions for application."""
 
 import datetime
+import os
 import re
 
 import requests
@@ -68,16 +69,16 @@ def change_filename_extension(filename, extension):
     Additionally, the original extension is appended to the filename, to avoid
     conflict with other files having the same name (without extension).
     """
-    matches = re.search(r'(.*)\.(.*)$', filename)
+    basename, ext = os.path.splitext(filename)
 
-    if matches is None:
-        raise Exception(
-            '{filename} is not a valid filename'.format(filename=filename))
+    if not basename:
+        raise Exception(f'{filename} is not a valid filename')
 
-    return '{name}-{source_extension}.{extension}'.format(
-        name=matches.group(1),
-        source_extension=matches.group(2),
-        extension=extension)
+    if not ext:
+        return f'{basename}.{extension}'
+    # remove dot
+    ext = ext.replace('.', '')
+    return f'{basename}-{ext}.{extension}'
 
 
 def send_email(recipients, subject, template, ctx=None, html=True, lang='en'):

tests/api/documents/test_documents_files_rest.py

@@ -75,7 +75,7 @@ def test_get_metadata(app, client, document_with_file):
 def test_put_delete(app, client, document, pdf_file):
     """Test create and delete a file."""
     app.config.update(SONAR_APP_DISABLE_PERMISSION_CHECKS=True)
-    file_name = 'test.pdf'
+    file_name = 'testé.pdf'
 
     # upload the file
     url_file_content = url_for(

tests/ui/documents/test_documents_utils.py

@@ -326,7 +326,11 @@ def test_get_file_restriction(app, organisation, admin, monkeypatch,
 def test_get_file_links(app):
     """Test getting links for a file."""
     document = {'pid': 1, 'external_url': True}
-    file = {'key': 'test.pdf', 'restriction': {'restricted': True}}
+    file = {
+        'key': 'test.pdf',
+        'restriction': {'restricted': True},
+        'mimetype': 'application/pdf'
+    }
 
     # File is restricted
     assert utils.get_file_links(file, document) == {
@@ -347,29 +351,56 @@ def test_get_file_links(app):
     # File key has no extension, no preview possible
     file['key'] = 'test'
     file['external_url'] = None
+    del file['mimetype']
     assert utils.get_file_links(file, document) == {
         'download': None,
         'external': None,
         'preview': None
     }
 
     # Preview not possible
-    file['key'] = 'test.unknown'
+    file['key'] = 'test.tiff'
     file['external_url'] = None
+    file['mimetype'] = 'image/tiff'
     assert utils.get_file_links(file, document) == {
-        'download': '/documents/1/files/test.unknown',
+        'download': '/documents/1/files/test.tiff',
         'external': None,
         'preview': None
     }
 
-    # Preview OK
-    file['key'] = 'test.pdf'
+    # File key has no extension, no preview possible
+    file['key'] = 'test.foo'
+    file['external_url'] = None
+    file['mimetype'] = 'application/octet-stream'
     assert utils.get_file_links(file, document) == {
-        'download': '/documents/1/files/test.pdf',
+        'download': f'/documents/1/files/test.foo',
         'external': None,
-        'preview': '/documents/1/preview/test.pdf'
+        'preview': None
     }
 
+    # Preview OK
+    mimetypes = [
+        'application/pdf',
+        'image/jpeg',
+        'image/png',
+        'application/octet-stream',
+        'image/gif',
+        'text/csv',
+        'application/json',
+        'application/xml'
+    ]
+    for mimetype in mimetypes:
+        ext = mimetype.split('/')[1]
+        if ext == 'octet-stream':
+            ext = 'md'
+        file['key'] = f'test.{ext}'
+        file['mimetype'] = mimetype
+        assert utils.get_file_links(file, document) == {
+            'download': f'/documents/1/files/test.{ext}',
+            'external': None,
+            'preview': f'/documents/1/preview/test.{ext}'
+        }
+
 
 def test_get_thumbnail():
     """Test get the thumbnail for a file."""

tests/ui/test_utils.py

@@ -30,10 +30,11 @@
 def test_change_filename_extension(app):
     """Test change filename extension."""
     with pytest.raises(Exception) as e:
-        change_filename_extension('test', 'txt')
-    assert str(e.value) == 'test is not a valid filename'
+        change_filename_extension('', 'txt')
+    assert str(e.value) == ' is not a valid filename'
 
     assert change_filename_extension('test.pdf', 'txt') == 'test-pdf.txt'
+    assert change_filename_extension('test', 'txt') == 'test.txt'
 
 
 def test_create_thumbnail_from_file():