Org: Avoid generating redundant file links for linked general files

This also updates WebTest to the newest version, since we need it in
order to test multi-file uploads.

TYPE: Bugfix
LINK: OGC-1967

Author: Daverball

Diff for: setup.cfg

@@ -162,7 +162,7 @@ install_requires =
     webcolors
     webob
     websockets
-    webtest < 3.0.1
+    webtest
     werkzeug
     wtforms
     xlrd

Diff for: src/onegov/form/fields.py

@@ -453,7 +453,8 @@ def populate_obj(self, obj: object, name: str) -> None:
             dummy = _DummyFile()
             dummy.file = file
             field.populate_obj(dummy, 'file')
-            if dummy.file is not None:
+            # avoid generating multiple links to the same file
+            if dummy.file is not None and dummy.file not in output:
                 output.append(dummy.file)
                 if (
                     dummy.file is not file

Diff for: src/onegov/org/upgrade.py

@@ -389,3 +389,41 @@ def remove_stored_contact_html_and_opening_hours_html(
 
         if 'contact_html' in obj.content:
             del obj.content['contact_html']
+
+
+@upgrade_task('Remove redundant page to general file links')
+def remove_redundant_page_to_general_file_links(
+    context: UpgradeContext
+) -> None:
+
+    if not context.has_table('files_for_pages_files'):
+        return
+
+    duplicate_pairs = [
+        {'file_id': file_id, 'pages_id': pages_id}
+        for file_id, pages_id in context.session.execute("""
+            SELECT file_id, pages_id FROM (
+                SELECT COUNT(*) as cnt, file_id, pages_id
+                  FROM files_for_pages_files
+                 GROUP BY file_id, pages_id
+            ) AS t
+            WHERE t.cnt > 1
+        """)
+    ]
+
+    if not duplicate_pairs:
+        return
+
+    # delete all the links with duplicate entries
+    context.session.execute("""
+        DELETE
+          FROM files_for_pages_files
+         WHERE file_id = :file_id
+           AND pages_id = :pages_id
+    """, duplicate_pairs)
+
+    # then reinsert a single link per duplicate entry
+    context.session.execute("""
+        INSERT INTO files_for_pages_files (file_id, pages_id)
+        VALUES (:file_id, :pages_id)
+    """, duplicate_pairs)

Diff for: tests/onegov/feriennet/test_views.py

@@ -2380,7 +2380,7 @@ def test_send_email_with_link_and_attachment(client, scenario):
     client.login_admin()
 
     page = client.get('/files')
-    page.form['file'] = Upload('Test.txt', b'File content.')
+    page.form['file'] = [Upload('Test.txt', b'File content.')]
     page.form.submit()
 
     file_id = FileCollection(scenario.session).query().one().id

Diff for: tests/onegov/gazette/test_views_notice_attachments.py

@@ -35,23 +35,23 @@ def test_view_notice_attachments(gazette_app, temporary_path, pdf_1, pdf_2):
         assert "Keine Anhänge." in manage
 
         # Try to upload an invalid file
-        manage.form['file'] = Upload(
+        manage.form['file'] = [Upload(
             'fake.pdf', 'PDF'.encode('utf-8'), 'application/pdf'
-        )
+        )]
         manage.form.submit(status=415)
 
         # Upload two attachment (with the same name!)
         with open(pdf_1, 'rb') as file:
             content_1 = file.read()
-        manage.form['file'] = Upload('1.pdf', content_1, 'application/pdf')
+        manage.form['file'] = [Upload('1.pdf', content_1, 'application/pdf')]
         manage = manage.form.submit().maybe_follow()
         assert "Anhang hinzugefügt" in manage
         assert "1.pdf" in manage
         assert manage.click('1.pdf').content_type == 'application/pdf'
 
         with open(pdf_2, 'rb') as file:
             content_2 = file.read()
-        manage.form['file'] = Upload('1.pdf', content_2, 'application/pdf')
+        manage.form['file'] = [Upload('1.pdf', content_2, 'application/pdf')]
         manage = manage.form.submit().maybe_follow()
         assert "Anhang hinzugefügt" in manage
         assert "1.pdf" in manage

Diff for: tests/onegov/org/test_extensions.py

@@ -7,11 +7,13 @@
 from markupsafe import Markup
 from tempfile import TemporaryDirectory
 from uuid import UUID
+from webtest import Upload
 
 from onegov.core.orm.abstract import MoveDirection
 from onegov.core.utils import Bunch
 from onegov.form import Form
 from onegov.form.extensions import Extendable
+from onegov.org.models import Topic
 from onegov.org.models.extensions import (
     PersonLinkExtension, ContactExtension, AccessExtension, HoneyPotExtension,
     SidebarLinksExtension, PeopleShownOnMainPageExtension,
@@ -557,9 +559,8 @@ def test_general_file_link_extension(client):
             "*Experts say it's the fact that Govikon does not really exist.*"
         )
         filename = os.path.join(td, 'simple.pdf')
-        pdf = create_pdf(filename)
-        new_page.form.fields['files'][-1].value = [filename]
-        new_page.files = pdf
+        create_pdf(filename)
+        new_page.form.fields['files'][-1].value = [Upload(filename)]
         new_page.form['show_file_links_in_sidebar'] = True
         page = new_page.form.submit().follow()
 
@@ -576,6 +577,51 @@ def test_general_file_link_extension(client):
         assert 'simple.pdf' not in page
 
 
+def test_general_file_link_extension_deduplication(client):
+    client.login_admin()
+
+    with TemporaryDirectory() as td:
+
+        root_page = client.get('/topics/themen')
+        new_page = root_page.click('Thema')
+
+        assert 'files' in new_page.form.fields
+
+        new_page.form['title'] = "Living in Govikon is Swell"
+        new_page.form['text'] = (
+            "## Living in Govikon is Really Great\n"
+            "*Experts say it's the fact that Govikon does not really exist.*"
+        )
+        filename = os.path.join(td, 'simple.pdf')
+        create_pdf(filename)
+        new_page.form.fields['files'][-1].value = [
+            Upload(filename),
+            Upload(filename)
+        ]
+        new_page.form['show_file_links_in_sidebar'] = True
+        page = new_page.form.submit().follow()
+
+        assert 'Living in Govikon is Swell' in page
+        assert 'Dokumente' in page
+        assert 'simple.pdf' in page
+
+        session = client.app.session()
+        topic = session.query(Topic).filter(
+            Topic.title == "Living in Govikon is Swell").one()
+        assert len(topic.files) == 1
+
+        pages_id = topic.id
+        file_id = topic.files[0].id
+
+        count, = session.execute("""
+            SELECT COUNT(*)
+              FROM files_for_pages_files
+             WHERE pages_id = :pages_id
+               AND file_id = :file_id
+        """, {'pages_id': pages_id, 'file_id': file_id}).fetchone()
+        assert count == 1
+
+
 def test_sidebar_links_extension(session):
 
     class Topic(SidebarLinksExtension):

Diff for: tests/onegov/org/test_views_directory.py

@@ -848,7 +848,7 @@ def test_directory_explicitly_link_referenced_files(client):
     path = module_path('tests.onegov.org', 'fixtures/sample.pdf')
     with open(path, 'rb') as f:
         page = client.get('/files')
-        page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf')
+        page.form['file'] = [Upload('Sample.pdf', f.read(), 'application/pdf')]
         page.form.submit()
 
     pdf_url = (

Diff for: tests/onegov/org/test_views_event.py

@@ -251,9 +251,8 @@ def test_view_occurrences_event_documents(client):
         client.login_admin()
         settings = client.get('/event-settings')
         filename_1 = os.path.join(td, 'zoo-programm-saison-2024.pdf')
-        pdf_1 = create_pdf(filename_1)
-        settings.form.fields['event_files'][-1].value = [filename_1]
-        settings.files = [pdf_1]
+        create_pdf(filename_1)
+        settings.form.fields['event_files'][-1].value = [Upload(filename_1)]
         settings = settings.form.submit().follow()
         assert settings.status_code == 200
 

Diff for: tests/onegov/org/test_views_files.py

@@ -10,7 +10,7 @@ def test_view_files(client):
 
     assert "Noch keine Dateien hochgeladen" in files_page
 
-    files_page.form['file'] = Upload('Test.txt', b'File content.')
+    files_page.form['file'] = [Upload('Test.txt', b'File content.')]
     files_page.form.submit()
 
     files_page = client.get('/files')

Diff for: tests/onegov/org/test_views_forms.py

@@ -367,7 +367,7 @@ def test_forms_explicitly_link_referenced_files(client):
     path = module_path('tests.onegov.org', 'fixtures/sample.pdf')
     with open(path, 'rb') as f:
         page = admin.get('/files')
-        page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf')
+        page.form['file'] = [Upload('Sample.pdf', f.read(), 'application/pdf')]
         page.form.submit()
 
     pdf_url = (

Diff for: tests/onegov/org/test_views_images.py

@@ -12,10 +12,10 @@ def test_view_images(client):
 
     assert "Noch keine Bilder hochgeladen" in images_page
 
-    images_page.form['file'] = Upload('Test.txt', b'File content')
+    images_page.form['file'] = [Upload('Test.txt', b'File content')]
     assert images_page.form.submit(expect_errors=True).status_code == 415
 
-    images_page.form['file'] = Upload('Test.jpg', create_image().read())
+    images_page.form['file'] = [Upload('Test.jpg', create_image().read())]
     images_page.form.submit()
 
     images_page = client.get('/images')

Diff for: tests/onegov/org/test_views_pages.py

@@ -79,7 +79,7 @@ def test_pages(client):
     admin.login_admin()
 
     images = admin.get('/images')
-    images.form['file'] = Upload('Test.jpg', create_image().read())
+    images.form['file'] = [Upload('Test.jpg', create_image().read())]
     images.form.submit()
     img_url = admin.get('/images').pyquery('.image-box a').attr('href')
 
@@ -147,7 +147,7 @@ def test_pages_explicitly_link_referenced_files(client):
     path = module_path('tests.onegov.org', 'fixtures/sample.pdf')
     with open(path, 'rb') as f:
         page = admin.get('/files')
-        page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf')
+        page.form['file'] = [Upload('Sample.pdf', f.read(), 'application/pdf')]
         page.form.submit()
 
     pdf_url = (
@@ -200,7 +200,7 @@ def test_pages_person_link_extension(client):
     admin.login_admin()
 
     images = admin.get('/images')
-    images.form['file'] = Upload('Test.jpg', create_image().read())
+    images.form['file'] = [Upload('Test.jpg', create_image().read())]
     images.form.submit()
     img_url = admin.get('/images').pyquery('.image-box a').attr('href')
 
@@ -262,7 +262,7 @@ def test_delete_pages(client):
     )
     # we add a file attachment to ensure we can delete a page, even if
     # it contains file attachments
-    new_page.form.fields['files'][-1] = Upload('test.txt')
+    new_page.form.fields['files'][-1] = [Upload('test.txt')]
     page = new_page.form.submit().follow()
     delete_link = page.pyquery('a[ic-delete-from]')[0].attrib['ic-delete-from']
 

Diff for: tests/onegov/org/test_views_photoalbum.py

@@ -23,7 +23,7 @@ def test_manage_album(client):
     assert "noch keine Bilder" in album
 
     images = albums.click("Bilder verwalten")
-    images.form['file'] = Upload('test.jpg', create_image().read())
+    images.form['file'] = [Upload('test.jpg', create_image().read())]
     images.form.submit()
 
     select = album.click("Bilder auswählen")

Diff for: tests/onegov/org/test_views_resources.py

@@ -156,7 +156,7 @@ def test_resources_explicitly_link_referenced_files(client):
     path = module_path('tests.onegov.org', 'fixtures/sample.pdf')
     with open(path, 'rb') as f:
         page = admin.get('/files')
-        page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf')
+        page.form['file'] = [Upload('Sample.pdf', f.read(), 'application/pdf')]
         page.form.submit()
 
     pdf_url = (

Diff for: tests/onegov/org/test_views_search.py

@@ -201,7 +201,7 @@ def test_search_publication_files(client_with_es):
     path = module_path('tests.onegov.org', 'fixtures/sample.pdf')
     with open(path, 'rb') as f:
         page = client.get('/files')
-        page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf')
+        page.form['file'] = [Upload('Sample.pdf', f.read(), 'application/pdf')]
         page.form.submit()
 
     client.app.es_indexer.process()

Diff for: tests/onegov/org/test_views_signing.py

@@ -17,7 +17,7 @@ def test_sign_document(client):
     path = module_path('tests.onegov.org', 'fixtures/sample.pdf')
     with open(path, 'rb') as f:
         page = client.get('/files')
-        page.form['file'] = Upload('Sample.pdf', f.read(), 'application/pdf')
+        page.form['file'] = [Upload('Sample.pdf', f.read(), 'application/pdf')]
         page.form.submit()
 
     pdf = FileCollection(client.app.session()).query().one()

Diff for: tests/onegov/swissvotes/test_views_page.py

@@ -69,11 +69,11 @@ def test_view_page_attachments(swissvotes_app, page_attachments):
     assert "No attachments." in manage
 
     # Upload two attachment (en_US, de_CH)
-    manage.form['file'] = Upload(
+    manage.form['file'] = [Upload(
         '1.pdf',
         page_attachments['en_US']['CODEBOOK'].reference.file.read(),
         'application/pdf'
-    )
+    )]
     manage = manage.form.submit().maybe_follow()
     assert manage.status_code == 200
 
@@ -84,11 +84,11 @@ def test_view_page_attachments(swissvotes_app, page_attachments):
     client.get('/locale/de_CH').follow()
     manage = client.get('/page/about').click("Anhänge verwalten")
 
-    manage.form['file'] = Upload(
+    manage.form['file'] = [Upload(
         '2.pdf',
         page_attachments['de_CH']['CODEBOOK'].reference.file.read(),
         'application/pdf'
-    )
+    )]
     manage = manage.form.submit().maybe_follow()
     assert manage.status_code == 200
 
@@ -162,11 +162,11 @@ def test_view_page_slider_images(swissvotes_app, slider_images):
     assert 'No attachments.' in manage
 
     # Upload image
-    manage.form['file'] = Upload(
+    manage.form['file'] = [Upload(
         '2.1-x.png',
         slider_images['2.1-x'].reference.file.read(),
         'image/png'
-    )
+    )]
     assert manage.form.submit().maybe_follow().status_code == 200
 
     manage = client.get('/page/about').click('Manage slider images')

Diff for: tests/onegov/town6/test_views_event.py

@@ -9,6 +9,7 @@
 from onegov.event import Event
 from tests.onegov.town6.common import step_class
 from unittest.mock import patch
+from webtest import Upload
 
 from tests.shared.utils import create_pdf
 
@@ -282,9 +283,8 @@ def test_view_occurrences_event_documents(client):
         client.login_admin()
         settings = client.get('/event-settings')
         filename_1 = os.path.join(td, 'zoo-programm-saison-2024.pdf')
-        pdf_1 = create_pdf(filename_1)
-        settings.form.fields['event_files'][-1].value = [filename_1]
-        settings.files = [pdf_1]
+        create_pdf(filename_1)
+        settings.form.fields['event_files'][-1].value = [Upload(filename_1)]
         settings = settings.form.submit().follow()
         assert settings.status_code == 200
 

Diff for: tests/onegov/town6/test_views_images.py

@@ -12,10 +12,10 @@ def test_view_images(client):
 
     assert "Noch keine Bilder hochgeladen" in images_page
 
-    images_page.form['file'] = Upload('Test.txt', b'File content')
+    images_page.form['file'] = [Upload('Test.txt', b'File content')]
     assert images_page.form.submit(expect_errors=True).status_code == 415
 
-    images_page.form['file'] = Upload('Test.jpg', create_image().read())
+    images_page.form['file'] = [Upload('Test.jpg', create_image().read())]
     images_page.form.submit()
 
     images_page = client.get('/images')