Paraf 445/max session files#38
Open
sgeulette wants to merge 5 commits into
Open
Conversation
📝 WalkthroughWalkthroughAdds max files-per-session registry; refactors add_files_to_session to split batches by size and file-count and return multiple (session_id, session) tuples; moves ZCML profile/upgrade declarations to separate files; updates callers and tests to consume the new return shape. ChangesSession file-count limit enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/imio/esign/utils.py`:
- Around line 167-170: The code assumes session["files"][0]["scan_id"] is a
non-None string before slicing; guard that value first and handle the None case
to avoid TypeError: check scan_id = session["files"][0].get("scan_id") (or
similar), and if scan_id is truthy and a string use session["client_id"] =
scan_id[:7] else set session["client_id"] to a safe fallback (e.g. formatted
from session_id or a fixed sentinel) before constructing session["sign_id"] =
"{}{:05d}".format(session["client_id"], session_id); update references to
session["files"][0]["scan_id"][0:7] accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71d9ba18-3c36-4e9e-aeba-8ce4ba5cff85
📒 Files selected for processing (11)
src/imio/esign/browser/actions.pysrc/imio/esign/browser/settings.pysrc/imio/esign/config.pysrc/imio/esign/configure.zcmlsrc/imio/esign/profiles.zcmlsrc/imio/esign/profiles/default/metadata.xmlsrc/imio/esign/tests/test_browser_views.pysrc/imio/esign/tests/test_services.pysrc/imio/esign/tests/test_utils.pysrc/imio/esign/upgrades.zcmlsrc/imio/esign/utils.py
Comment on lines
+167
to
+170
| if session["client_id"] is None: | ||
| # FIXME what if scan_id is None ? | ||
| session["client_id"] = session["files"][0]["scan_id"][0:7] | ||
| session["sign_id"] = "{}{:05d}".format(session["client_id"], session_id) |
There was a problem hiding this comment.
Guard scan_id before slicing to prevent dispatch crashes.
Line 169 assumes scan_id is always a string. If it is None, this raises TypeError and aborts the whole add flow.
Proposed fix
if session["client_id"] is None:
- # FIXME what if scan_id is None ?
- session["client_id"] = session["files"][0]["scan_id"][0:7]
+ scan_id = session["files"][0].get("scan_id")
+ if not scan_id:
+ logger.error(
+ "Missing scan_id for uid %s while initializing session %s client_id",
+ uid,
+ session_id,
+ )
+ scan_id = get_suid_from_uuid(uid).replace("-", "")
+ session["client_id"] = scan_id[0:7]
session["sign_id"] = "{}{:05d}".format(session["client_id"], session_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/imio/esign/utils.py` around lines 167 - 170, The code assumes
session["files"][0]["scan_id"] is a non-None string before slicing; guard that
value first and handle the None case to avoid TypeError: check scan_id =
session["files"][0].get("scan_id") (or similar), and if scan_id is truthy and a
string use session["client_id"] = scan_id[:7] else set session["client_id"] to a
safe fallback (e.g. formatted from session_id or a fixed sentinel) before
constructing session["sign_id"] = "{}{:05d}".format(session["client_id"],
session_id); update references to session["files"][0]["scan_id"][0:7]
accordingly.
Coverage Report for CI Build 26155201448Coverage increased (+0.2%) to 84.76%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
gbastien
reviewed
May 19, 2026
| watchers=self.get_watchers(), | ||
| discriminators=self.get_discriminators(), | ||
| ) | ||
| )[-1] |
Member
There was a problem hiding this comment.
Salut @sgeulette
en regardant vite une première fois depuis le garage (changement de pneus ;-)) je vois ceci:
- le [-1] systématique partout, pas plus simple d'avoir un paramètre genre "return_last_session=True" par défaut pour utils.add_files_to_session? Puisque c'est le comportement qu'on veut la plupart du temps...
- le terme "complete" peut-être pas idéal, changer en "draft_complete" par exemple pour que ce soit plus clair et traduit en "en création (complet)"? à voir
Pour le reste je vais tester vraiment le code cet aprem je te dirai (sauf si Alain B vient à l'atelier pour moi...)
…oSessionView action
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/imio/esign/browser/actions.py`:
- Line 58: The success message is joining integer session_id values which raises
a TypeError; update the call to self._finished so the session_ids values are
converted to strings before joining (e.g. map/str conversion of [tup[0] for tup
in session_ids]) when building the mapping for the "session_ids" key so the u",
".join succeeds; locate the join in actions.py where
self._finished(mapping={"session_ids": u", ".join([tup[0] for tup in
session_ids])}) is used and cast each tup[0] to str.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f0188411-5eee-4c1a-bd3c-686f691bd75c
📒 Files selected for processing (4)
src/imio/esign/browser/actions.pysrc/imio/esign/browser/table.pysrc/imio/esign/tests/test_utils.pysrc/imio/esign/utils.py
| # audit("add_to_session", "session={} context={} files={} signers={}".format( | ||
| # session_id, self.context.UID(), ",".join(files_uids), len(signers))) | ||
| self._finished() | ||
| self._finished(mapping={"session_ids": u", ".join([tup[0] for tup in session_ids])}) |
There was a problem hiding this comment.
Fix session ID join type mismatch in success message.
Line 58 joins integer session_id values directly; this raises TypeError (expected str instance) on Python 3 and breaks redirect flow after add.
Proposed fix
- self._finished(mapping={"session_ids": u", ".join([tup[0] for tup in session_ids])})
+ self._finished(mapping={"session_ids": u", ".join([safe_unicode(tup[0]) for tup in session_ids])})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/imio/esign/browser/actions.py` at line 58, The success message is joining
integer session_id values which raises a TypeError; update the call to
self._finished so the session_ids values are converted to strings before joining
(e.g. map/str conversion of [tup[0] for tup in session_ids]) when building the
mapping for the "session_ids" key so the u", ".join succeeds; locate the join in
actions.py where self._finished(mapping={"session_ids": u", ".join([tup[0] for
tup in session_ids])}) is used and cast each tup[0] to str.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes / Behavior
Chores
UX