Added session info size#24
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds session-size display to the quick-look UI: FilesColumn now computes max session size from registry and renders size with a warning threshold; SessionFilesView records files_count; session_files.pt displays a paragraph with the session element count; changelog and i18n entries added. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Table as FilesColumn
participant Registry as Registry\n(get_esign_registry_max_session_size)
participant View as SessionFilesView
participant Template as session_files.pt
User->>Table: open quick-look
Table->>Registry: get_esign_registry_max_session_size()
Table->>View: read item / session context
View->>View: set files_count = len(files)
Table->>Template: renderQuickLook(size, max, warning, files_count)
Template->>User: return rendered quick-look
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 the current code and only fix it if needed.
Inline comments:
In `@src/imio/esign/browser/templates/session_files.pt`:
- Around line 2-7: The template sets i18n:translate="session_files_count" which
doesn't match the PO msgid; change the element to use i18n:translate="" so the
element's text becomes the msgid that matches the catalog (keep the existing
tal:condition="python: view.files_count >= 10" and the <strong
tal:content="view/files_count" i18n:name="count">N</strong> as-is) — replace the
explicit message id with an empty i18n:translate to let the literal text "This
session contains ${count} element(s)." be used for translation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e3a470e-a04d-4085-a94e-9c8e3ec4f0d6
📒 Files selected for processing (7)
CHANGES.rstsrc/imio/esign/browser/table.pysrc/imio/esign/browser/templates/session_files.ptsrc/imio/esign/browser/views.pysrc/imio/esign/locales/en/LC_MESSAGES/imio.esign.posrc/imio/esign/locales/fr/LC_MESSAGES/imio.esign.posrc/imio/esign/locales/imio.esign.pot
chris-adam
force-pushed
the
PARAF-364/quick_look_size
branch
3 times, most recently
from
6a4b17f to
101681b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/imio/esign/browser/views.py (1)
95-103:⚠️ Potential issue | 🟡 MinorCount only the files that will actually be rendered.
files_countis taken from the raw session payload before Lines 98-103 drop unresolved entries. That can make the UI report more files than the list shows, or even show a count when nothing is renderable.🧮 Suggested fix
def __call__(self): session_id = int(self.request.get("session_id")) session = self.get_session(session_id) - self.files_count = len(session["files"]) files = [] for f in session["files"]: ctx = uuidToObject(f["context_uid"]) obj = uuidToObject(f["uid"]) if obj and ctx: files.append((ctx, obj)) self.files = files + self.files_count = len(files) return self.index()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/imio/esign/browser/views.py` around lines 95 - 103, The code sets self.files_count from the raw session payload before filtering unresolved entries; change the order to resolve entries first and count only renderable files: call session = self.get_session(session_id), build the files list by resolving each entry with uuidToObject for f["context_uid"] and f["uid"] (as done in the loop using ctx and obj), assign self.files = files, and then set self.files_count = len(self.files) so files_count reflects only successfully resolved/renderable items (adjust code in the block around get_session, uuidToObject, self.files, and self.files_count).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/imio/esign/browser/table.py`:
- Around line 117-126: The size label in renderQuickLook is not localized and
uses "Mb"; update the code that builds size_label (in function renderQuickLook)
to use the translation machinery (e.g. translate/_) for the full user-facing
string and change the unit to "MB" (uppercase), passing size_mb and max_size_mb
as interpolation parameters so translators can reorder/format them; ensure the
translated string is used when constructing the returned HTML span (preserving
size_style and escaping/interpolating values appropriately).
---
Outside diff comments:
In `@src/imio/esign/browser/views.py`:
- Around line 95-103: The code sets self.files_count from the raw session
payload before filtering unresolved entries; change the order to resolve entries
first and count only renderable files: call session =
self.get_session(session_id), build the files list by resolving each entry with
uuidToObject for f["context_uid"] and f["uid"] (as done in the loop using ctx
and obj), assign self.files = files, and then set self.files_count =
len(self.files) so files_count reflects only successfully resolved/renderable
items (adjust code in the block around get_session, uuidToObject, self.files,
and self.files_count).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35b3993c-5916-45d8-a5ee-5c251d8be040
📒 Files selected for processing (7)
CHANGES.rstsrc/imio/esign/browser/table.pysrc/imio/esign/browser/templates/session_files.ptsrc/imio/esign/browser/views.pysrc/imio/esign/locales/en/LC_MESSAGES/imio.esign.posrc/imio/esign/locales/fr/LC_MESSAGES/imio.esign.posrc/imio/esign/locales/imio.esign.pot
🚧 Files skipped from review as they are similar to previous changes (4)
- src/imio/esign/browser/templates/session_files.pt
- src/imio/esign/locales/imio.esign.pot
- src/imio/esign/locales/en/LC_MESSAGES/imio.esign.po
- src/imio/esign/locales/fr/LC_MESSAGES/imio.esign.po
chris-adam
force-pushed
the
PARAF-364/quick_look_size
branch
from
101681b to
745ff80
Compare
Coverage Report for CI Build 25096885729Coverage increased (+0.2%) to 76.753%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
chris-adam
force-pushed
the
PARAF-364/quick_look_size
branch
from
4f6d669 to
50956b8
Compare
chris-adam
force-pushed
the
PARAF-364/quick_look_size
branch
from
65bfa6a to
5b313a9
Compare
chris-adam
force-pushed
the
PARAF-364/quick_look_size
branch
from
5b313a9 to
9452a0b
Compare
| <div tal:replace="structure view/render_table"></div> | ||
| <div style="text-align: right;"> | ||
| <a tal:attributes="href view/get_parapheo_link" target="_blank"> | ||
| <button class="apButton apButtonAction apButtonAction_parapheo"> |
There was a problem hiding this comment.
cela me semblerait mieux d'en faire un simple lien et centré sous le tableau afin que les utilisateurs ne pensent pas qu'il faut utiliser exclusivement ce bouton, qui est trop mis en évidence niveau UI par rapport aux éléments du tableau.
There was a problem hiding this comment.
Ok, j'ai changé ça mais je trouve pas ça très beau ^^'
| </tal:none> | ||
| <p i18n:translate=""> | ||
| This session contains | ||
| <strong tal:content="view/files_count" i18n:name="count">N</strong> |
There was a problem hiding this comment.
Je mettrais cette information directement visible dans la cellule, avant l'information de taille car cela me semble pertinent aussi de visualiser sans ouvrir le nombre de fichiers à signer
|
@chris-adam des détails, mais ne mettrions nous pas: "Ouvrir la plateforme de signature électronique"? A priori on met "signature électronique" partout pour ne pas avoir parfois eSign, parfois e-signature, parfois "signature digitale", ... |
@gbastien On peut voir ça cette après-midi après les ateliers :) |
4 participants
Summary by CodeRabbit
New Features
Documentation
Chores