Merge pull request #21 from IMIO/PARAF-359_file_added_to_same_session_again

Manage file added again to same session, data is updated.

Author: gbastien

.github/workflows/main.yml

@@ -26,7 +26,7 @@ jobs:
       - name: Install dependencies
         run: |
           sudo apt-get update
-          sudo apt-get install -y libjpeg-dev libbz2-dev
+          sudo apt-get install -y libjpeg8-dev libbz2-dev liblzma-dev
       - name: Set up pyenv and Python
         uses: "gabrielfalcao/pyenv-action@v18"
         with:
@@ -50,6 +50,56 @@ jobs:
       - name: test
         run: |
           bin/test -t !robot
+  coverage:
+    runs-on: ubuntu-24.04
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - python: 2.7.18
+            plone: 4.3
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v5
+      - name: Setup Env
+        run: |
+          sudo apt-get update -qqy
+          sudo apt-get install -y libjpeg8-dev libbz2-dev liblzma-dev libreadline-dev
+      - name: Set up pyenv and Python
+        uses: "gabrielfalcao/pyenv-action@v18"
+        with:
+          default: 2.7.18
+          versions: 3.8.16
+          command: pyenv -v
+      - name: Setup Env
+        run: |
+          sudo apt-get update
+          pip install -r requirements-4.3.txt coverage==5.3.1
+      - name: Cache eggs
+        uses: actions/cache@v4
+        env:
+          cache-name: cache-eggs
+        with:
+          path: ~/buildout-cache/eggs
+          key: ${{ runner.os }}-coverage-${{ env.cache-name }}
+          restore-keys: ${{ runner.os }}-coverage-${{ env.cache-name }}
+      - name: buildout
+        run: |
+          sed -ie "s#test.cfg#test-${{matrix.plone}}.cfg#" gha.cfg
+          buildout -c gha.cfg
+      - name: test coverage
+        run: |
+          bin/coverage run bin/test
+          bin/coverage report
+      - name: Install Coveralls
+        run: |
+          pip3 install -U pip setuptools --no-cache-dir
+          pip3 install -U "coveralls>=3.0.0" coverage==5.3.1 --no-cache-dir
+      - name: Publish to Coveralls
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          coveralls --service=github
 #  coverage:
 #    runs-on: ubuntu-24.04
 #    strategy:

CHANGES.rst

@@ -31,11 +31,18 @@ Changelog
   [chris-adam, sgeulette]
 - Added possibility to have elements of the same context to belong to different sessions.
   [chris-adam]
+- Manage file added again to same session, data is updated.
+  [gbastien]
 - Configured the `@@remove-from-esign-session` the same way as `@@remove-item-from-esign-session`
   so relying on an `available` method to show it only if a context is in a session.
   [gbastien]
 - Fixed faceted viewlet broken because sessions format changed from list to OrderedDict.
   [gbastien]
+- Turned file info in session annotation from `dict` to `PersistentMapping`.
+  Entire annotation structure is now persistent, remove `_p_changed`.
+  [gbastien]
+- Use `@CachedProperty` for `FacetedSessionInfoViewlet.sessions` and `ItemSessionInfoViewlet.sessions`.
+  [gbastien]
 
 1.0a2 (2026-02-06)
 ------------------

checkouts.cfg

@@ -1,6 +1,7 @@
 [buildout]
 always-checkout = force
 auto-checkout +=
+    collective.iconifiedcategory
     imio.annex
     imio.helpers
     imio.pyutils
@@ -19,6 +20,7 @@ collective = https://github.com/collective
 collective_push = git@github.com:collective
 
 [sources]
+collective.iconifiedcategory = git ${remotes:collective}/collective.iconifiedcategory.git pushurl=${remotes:collective_push}/collective.iconifiedcategory.git
 imio.annex = git ${remotes:imio}/imio.annex.git pushurl=${remotes:imio_push}/imio.annex.git
 imio.helpers = git ${remotes:imio}/imio.helpers.git pushurl=${remotes:imio_push}/imio.helpers.git
 imio.pyutils = git ${remotes:imio}/imio.pyutils.git pushurl=${remotes:imio_push}/imio.pyutils.git

src/imio/esign/browser/actions.py

@@ -50,12 +50,12 @@ def get_signers(self):
         """
         try:
             signers = ISignable(self.context).get_signers()
-        except ValueError, msg:
+        except ValueError as msg:
             signers = []
             api.portal.show_message(
                 _(
                     "Problem getting signers: \"${error}\")!",
-                    mapping={"error": msg},
+                    mapping={"error": str(msg)},
                 ),
                 request=self.request,
                 type="warning",

src/imio/esign/browser/views.py

@@ -30,6 +30,7 @@
 from Products.Five import BrowserView
 from Products.PageTemplates.Expressions import SecureModuleImporter
 from zope.browserpage.viewpagetemplatefile import ViewPageTemplateFile
+from zope.cachedescriptors.property import CachedProperty
 from zope.component import getMultiAdapter
 from zope.i18n import translate
 from zope.interface import implementer
@@ -213,11 +214,8 @@ def render(self):
             return self.sessions_listing_view(self.context, self.request).render_table()
         return ""
 
-    @property
+    @CachedProperty
     def sessions(self):
-        # caching
-        if hasattr(self, "_cached_session"):
-            return self._cached_session
         session_id = self.request.form.get("esign_session_id[]", None)
         try:
             session_id = int(session_id)
@@ -227,8 +225,6 @@ def sessions(self):
         session_info = get_session_info(session_id)
         if session_info:
             session = {session_id: session_info}
-        # caching
-        self._cached_session = session
         return session
 
     def get_table_rows(self, column):
@@ -271,7 +267,7 @@ def render(self):
             return self.index()
         return ""
 
-    @property
+    @CachedProperty
     def sessions(self):
         """Return all sessions that contain files from this context."""
         return get_sessions_for(self.context.UID())

src/imio/esign/configure.zcml

@@ -8,6 +8,7 @@
 
   <i18n:registerTranslations directory="locales" />
 
+  <include file="events.zcml" />
   <include file="permissions.zcml" />
 
   <include package=".browser" />

src/imio/esign/events.py

@@ -0,0 +1,65 @@
+# -*- coding: utf-8 -*-
+
+from imio.esign.utils import get_file_info
+from imio.esign.utils import get_sessions_for
+from imio.helpers.transmogrifier import get_correct_id
+from os import path
+
+
+def on_categorized_annex_updated(annex, event):
+    '''When an annex is modified, update check if need to update esign session.'''
+    old_values = event.old_values
+    # we are creating a new annex, not in a session
+    if not old_values:
+        return
+
+    sessions = get_sessions_for(event.parent.UID(), readonly=False)
+    if not sessions:
+        return
+
+    # make sure annex_uid is in a session
+    annex_uid = annex.UID()
+    file_infos = []
+    for session_id in sessions:
+        file_info = get_file_info(session_id, annex_uid)
+        if file_info:
+            file_infos.append(file_info)
+    if not file_infos:
+        return
+
+    # here we are sure that annex is in a session, we need to update data
+    # if something usefull changed, we will update the session
+    new_values = event.new_values
+    update = False
+    checked_keys = ['title', 'filesize', 'relative_url']
+    for checked_key in checked_keys:
+        if new_values[checked_key] != old_values[checked_key]:
+            update = True
+            break
+    # check scan_id and filename
+    if update is False:
+        for file_info in file_infos:
+            if file_info and (annex.scan_id != file_info['scan_id'] or \
+               annex.file.filename != file_info['filename']):
+                update = True
+                break
+
+    if update is True:
+        for session_id, session in sessions.items():
+            # size
+            size_diff = new_values['filesize'] - old_values['filesize']
+            session['size'] += size_diff
+            # title and filename
+            for file_data in session['files']:
+                if file_data['uid'] == annex_uid:
+                    file_data['title'] = new_values['title']
+                    file_data['scan_id'] = annex.scan_id
+                    # filename changed, need to make sure new filename is unique
+                    if annex.file.filename != file_data['filename']:
+                        existing_files = [path.splitext(f["filename"])[0]
+                                          for f in session["files"]]
+                        filename, ext = path.splitext(annex.file.filename)
+                        new_filename = get_correct_id(existing_files, filename)
+                        file_data['filename'] = new_filename + ext
+                    # file_uid is only there one time per session
+                    break

src/imio/esign/events.zcml

@@ -0,0 +1,8 @@
+<configure
+    xmlns="http://namespaces.zope.org/zope">
+
+ <subscriber for="imio.annex.content.annex.IAnnex
+                  collective.iconifiedcategory.interfaces.ICategorizedElementUpdatedEvent"
+             handler=".events.on_categorized_annex_updated" />
+
+</configure>

src/imio/esign/tests/test_utils.py

@@ -11,6 +11,7 @@
 from imio.esign.utils import add_files_to_session
 from imio.esign.utils import create_external_session
 from imio.esign.utils import get_file_download_url
+from imio.esign.utils import get_file_info
 from imio.esign.utils import get_filesize
 from imio.esign.utils import get_max_download_date
 from imio.esign.utils import get_session_annotation
@@ -30,6 +31,8 @@
 from plone.namedfile.file import NamedBlobFile
 from plone.namedfile.file import NamedBlobImage
 from zope.annotation import IAnnotations
+from zope.event import notify
+from zope.lifecycleevent import ObjectModifiedEvent
 
 import collective.iconifiedcategory
 import json
@@ -452,6 +455,77 @@ def test_add_files_with_duplicate_filenames(self):
         self.assertIn("same_filename-1.pdf", filenames)
         self.assertIn("same_filename-2.pdf", filenames)
 
+    def test_add_files_already_exist_is_updated(self):
+        """When adding a file to the same session, it is updated."""
+        annot = get_session_annotation()
+        self.assertEqual(len(annot["sessions"]), 0)
+
+        signers = [
+            ("user1", "user1@sign.com", "User 1", "Position 1"),
+        ]
+
+        annex0_uid = self.uids[0]
+        annex0 = api.content.get(UID=annex0_uid)
+
+        sid, session = add_files_to_session(signers, (annex0_uid,))
+        self.assertEqual(sid, 0)
+        self.assertEqual(len(session["files"]), 1)
+        self.assertEqual(session["files"][0]["filename"], "annex0.pdf")
+        self.assertEqual(session["files"][0]["title"], "Annex 0")
+        self.assertEqual(session["size"], 6968)
+        # edit annex and add again, still one annex in session and data are updated
+        annex0.file.filename = u"new_annex0.pdf"
+        annex0.setTitle('New Annex 0')
+        sid, session = add_files_to_session(signers, (annex0_uid,))
+        # same session_id
+        self.assertEqual(sid, 0)
+        self.assertEqual(len(session["files"]), 1)
+        self.assertEqual(session["files"][0]["filename"], "new_annex0.pdf")
+        self.assertEqual(session["files"][0]["title"], "New Annex 0")
+        self.assertEqual(session["size"], 6968)
+        # add again exact same file
+        sid, session = add_files_to_session(signers, (annex0_uid,))
+        self.assertEqual(sid, 0)
+        self.assertEqual(len(session["files"]), 1)
+        self.assertEqual(session["files"][0]["filename"], "new_annex0.pdf")
+        self.assertEqual(session["files"][0]["title"], "New Annex 0")
+        self.assertEqual(session["size"], 6968)
+        # add second file 2 times
+        annex1_uid = self.uids[1]
+        annex1 = api.content.get(UID=annex1_uid)
+        sid, session = add_files_to_session(signers, (annex1_uid,))
+        self.assertEqual(sid, 0)
+        self.assertEqual(len(session["files"]), 2)
+        self.assertEqual(session["files"][1]["filename"], "annex1.pdf")
+        self.assertEqual(session["files"][1]["title"], "Annex 1")
+        self.assertEqual(session["size"], 13982)
+        # edit and add again
+        annex1.setTitle('New Annex 1')
+        # edit file, filename and content so size changed
+        with open(os.path.join(os.path.dirname(__file__), "annex1.pdf"), "rb") as f:
+            annex1.file = NamedBlobFile(data=f.read(), filename=u"new_annex1.pdf", contentType="application/pdf")
+        notify(ObjectModifiedEvent(annex1))
+        # data were already updated
+        self.assertEqual(len(session["files"]), 2)
+        self.assertEqual(session["files"][1]["filename"], "new_annex1.pdf")
+        self.assertEqual(session["files"][1]["title"], "New Annex 1")
+        self.assertEqual(session["files"][1]["scan_id"], "012345600000001")
+        self.assertEqual(session["size"], 13936)
+        # change file but add a filename already used, change scan_id as well
+        with open(os.path.join(os.path.dirname(__file__), "annex1.pdf"), "rb") as f:
+            annex1.file = NamedBlobFile(data=f.read(), filename=u"new_annex0.pdf", contentType="application/pdf")
+        annex1.scan_id = "012345600000002"
+        notify(ObjectModifiedEvent(annex1))
+        self.assertEqual(session["files"][1]["filename"], "new_annex0-1.pdf")
+        self.assertEqual(session["files"][1]["scan_id"], "012345600000002")
+        # edit annex out of any session
+        annex2_uid = self.uids[2]
+        annex2 = api.content.get(UID=annex2_uid)
+        notify(ObjectModifiedEvent(annex2))
+        # just to check, remove annex0
+        remove_files_from_session((annex0_uid,))
+        self.assertEqual(session["size"], 6968)
+
     def test_remove_context_from_session(self):
         """Test removing a context from a session."""
         annot = get_session_annotation()
@@ -528,6 +602,30 @@ def test_get_sessions_for(self):
         sessions[0]['watchers'] = ["watcher@sign.com"]
         self.assertEqual(get_session_info(0)['watchers'], ["watcher@sign.com"])
 
+    def test_get_file_info(self):
+        """Test getting infos for a given file."""
+        annex0_uid = self.uids[0]
+        annex1_uid = self.uids[1]
+        # no session
+        self.assertIsNone(get_file_info(0, annex0_uid))
+        # create session
+        signers = [("user1", "user1@sign.com", "User 1", "Position 1")]
+        sid, session = add_files_to_session(signers, (annex0_uid,))
+        self.assertEqual(get_file_info(0, annex0_uid)['uid'], annex0_uid)
+        self.assertIsNone(get_file_info(0, annex1_uid))
+        # readonly=True by default
+        file_info = get_file_info(0, annex0_uid)
+        file_info['title'] = u'New title annex 0'
+        # not changed in the annotation
+        self.assertEqual(
+            get_session_annotation()['sessions'][0]['files'][0]['title'], u'Annex 0')
+        # readonly=False
+        file_info = get_file_info(0, annex0_uid, readonly=False)
+        file_info['title'] = u'New title annex 0'
+        # changed in the annotation
+        self.assertEqual(
+            get_session_annotation()['sessions'][0]['files'][0]['title'], u'New title annex 0')
+
     def test_get_file_download_url(self):
         """Test generating file download URL from UID."""
         uid = "f40682caafc045b4b81973bd82ea9ab6"