Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
df35686
Manage file added again to same session, data is updated.
gbastien Mar 6, 2026
caf6fcd
Completed test_add_files_already_exist_is_updated to show that filena…
gbastien Mar 6, 2026
66d8df8
Check added to same session
gbastien Mar 6, 2026
795cc16
Completed test_add_files_already_exist_is_updated regarding session s…
gbastien Mar 6, 2026
19e094f
doc
gbastien Mar 6, 2026
e425dcf
Manage size
gbastien Mar 6, 2026
9eefa2e
Added event to make sure session data (size, title, filename) is upda…
gbastien Mar 9, 2026
f9cef7c
Merge remote-tracking branch 'origin/main' into PARAF-359_file_added_…
gbastien Mar 12, 2026
343e978
Session size is now always correct as it is update on add/edit/remove…
gbastien Mar 12, 2026
a0ff5e9
Added collective.iconifiedcategory to auto-checkout
gbastien Mar 12, 2026
ba0f644
Setup coverage
gbastien Mar 12, 2026
7aef366
Removed double imio.annex
gbastien Mar 12, 2026
70620ab
Removed double get_sessions_for
gbastien Mar 12, 2026
84bd591
Install correct requirements-4.3.txt
gbastien Mar 12, 2026
b0d39a6
Run correct cfg file
gbastien Mar 12, 2026
ee3b6c4
Coverage gha
gbastien Mar 13, 2026
006d4cb
Try to fix coveralls
gbastien Mar 13, 2026
fa8e300
Install liblma-dev but not wheel that was already installed by requir…
gbastien Mar 13, 2026
820ea31
Pinned coverage = 5.3.1
gbastien Mar 13, 2026
c753000
Try to force pip3.8
gbastien Mar 13, 2026
01581a4
Py3
gbastien Mar 13, 2026
5e60ef1
Removed ls command
gbastien Mar 13, 2026
385efb9
Show that without adding it again, data is updated during the modifie…
gbastien Mar 13, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ Changelog
[chris-adam]
- Added external watchers for esign sessions.
[chris-adam, sgeulette]
- Manage file added again to same session, data is updated.
[gbastien]

1.0a2 (2026-02-06)
------------------
Expand Down
29 changes: 29 additions & 0 deletions src/imio/esign/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,35 @@ 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"),
]

annex_uid = self.uids[0]
annex = api.content.get(UID=annex_uid)

sid, session = add_files_to_session(signers, (annex_uid,))
self.assertEqual(len(session["files"]), 1)
self.assertEqual(session["files"][0]["filename"], "annex0.pdf")
self.assertEqual(session["files"][0]["title"], "Annex 0")
# edit annex and add again, still one annex in session and data are updated
annex.file.filename = u"new_filename.pdf"
annex.setTitle('New title')
sid, session = add_files_to_session(signers, (annex_uid,))
self.assertEqual(len(session["files"]), 1)
self.assertEqual(session["files"][0]["filename"], "new_filename.pdf")
self.assertEqual(session["files"][0]["title"], "New title")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Test doesn’t strictly prove “same session” update behavior.

The second call omits session_id, so this can still pass if a new session is created. Also, sid from Line 472 is unused. Force the same-session path and assert identity/count explicitly.

💡 Proposed fix
-        sid, session = add_files_to_session(signers, (annex_uid,))
+        sid2, session = add_files_to_session(signers, (annex_uid,), session_id=sid)
+        self.assertEqual(sid2, sid)
+        self.assertEqual(len(annot["sessions"]), 1)
+        self.assertEqual(annot["uids"][annex_uid], sid)
         self.assertEqual(len(session["files"]), 1)
         self.assertEqual(session["files"][0]["filename"], "new_filename.pdf")
         self.assertEqual(session["files"][0]["title"], "New title")

Based on learnings: In src/imio/esign/utils.py, when an explicit session_id is provided to add_files_to_session(), discriminators are intentionally bypassed.

🧰 Tools
🪛 Ruff (0.15.4)

[warning] 472-472: Unpacked variable sid is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/imio/esign/tests/test_utils.py` around lines 465 - 475, The test
currently calls add_files_to_session(signers, (annex_uid,)) twice without
supplying the existing session_id, so it may create a new session instead of
updating the same one; modify the test to capture the returned sid from the
first call, pass that sid back as the session_id argument on the second
add_files_to_session(...) invocation, then assert that the returned sid is
unchanged (identity) and that session["files"] still has length 1 with updated
filename and title; reference add_files_to_session and the session_id parameter
when making the change.

# add again exact same file
sid, session = add_files_to_session(signers, (annex_uid,))
self.assertEqual(len(session["files"]), 1)
self.assertEqual(session["files"][0]["filename"], "new_filename.pdf")
self.assertEqual(session["files"][0]["title"], "New title")

def test_remove_context_from_session(self):
"""Test removing a context from a session."""
annot = get_session_annotation()
Expand Down
13 changes: 11 additions & 2 deletions src/imio/esign/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ def add_files_to_session(
annex = uuidToObject(uuid=uid, unrestricted=True)
context_uid_provider = getAdapter(annex, IContextUidProvider)
context_uid = context_uid_provider.get_context_uid()
# update data if adding same file to same session
if annot['uids'].get(uid, -1) == session_id:
logger.info('File with UID %s is already in session_id %s and data were updated!', uid, session_id)
# remove old filename to avoid filename being renamed
old_filename = path.splitext([fn for fn in session["files"]
if fn['uid'] == uid][0]['filename'])[0]
existing_files.remove(old_filename)
remove_files_from_session([uid], remove_empty_session=False)
Comment on lines +93 to +96
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Prevent crash when UID mapping is stale.

This lookup assumes the UID always exists in session["files"]; if annotations drift, ...[0] raises and aborts the operation. Guard the lookup before dereferencing.

💡 Proposed fix
-            old_filename = path.splitext([fn for fn in session["files"]
-                                          if fn['uid'] == uid][0]['filename'])[0]
+            old_file = next((fn for fn in session["files"] if fn["uid"] == uid), None)
+            if old_file is None:
+                logger.error(
+                    "File UID %s mapped to session %s but missing from session files.",
+                    uid, session_id,
+                )
+                continue
+            old_filename = path.splitext(old_file["filename"])[0]
🧰 Tools
🪛 Ruff (0.15.4)

[warning] 95-96: Prefer next(...) over single element slice

Replace with next(...)

(RUF015)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/imio/esign/utils.py` around lines 95 - 98, The code assumes a matching
entry for uid exists in session["files"] and directly indexes [0], causing a
crash if the mapping is stale; update the block that computes old_filename (the
list comprehension using session["files"] and uid) to safely search for the file
(e.g., use next(...) with a default None or check length of the filtered list)
and only call existing_files.remove(old_filename) and
remove_files_from_session([uid], ...) when a match is found; if no match is
found, skip removing old_filename and either log a warning or proceed to call
remove_files_from_session with the uid as needed to avoid raising an IndexError.


filename, ext = path.splitext(annex.file.filename or "no_filename.pdf")
new_filename = get_correct_id(existing_files, filename)
session["files"].append(
Expand Down Expand Up @@ -353,7 +362,7 @@ def remove_context_from_session(context_uids):
remove_files_from_session(list(c_uids[context_uid]))


def remove_files_from_session(files_uids):
def remove_files_from_session(files_uids, remove_empty_session=True):
"""Remove files from their corresponding sessions.

:param files_uids: list of file UIDs to remove
Expand Down Expand Up @@ -386,7 +395,7 @@ def remove_files_from_session(files_uids):
continue

del session["files"][i]
if not session["files"]:
if not session["files"] and remove_empty_session:
del sessions[session_id]
else:
session["last_update"] = datetime.now()
Expand Down
5 changes: 5 additions & 0 deletions test-4.3.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -150,3 +150,8 @@ Pygments = 2.5.2
# beautifulsoup
soupsieve = 1.9.2
backports.functools-lru-cache = 1.5

# Required by:
# mock
mock = 3.0.5
funcsigs = 1.0.2