Added red/green icon next to signers to show if not signed or signed. - #44
Conversation
In info viewlet, display `signed` in green. See #PARAF-476
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis pull request adds visual status indicators for signers in the signer list, displaying conditional red/green icon styling based on signed status with HTML-escaped translated tooltips, alongside supporting translation infrastructure updates and changelog documentation. ChangesSigner status visual indicators with i18n tooltips
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 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 unit tests (beta)
Comment |
Coverage Report for CI Build 27603847520Coverage decreased (-0.2%) to 85.962%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/imio/esign/locales/en/LC_MESSAGES/imio.esign.po (1)
521-527: ⚡ Quick winProvide English translations for new status title keys.
The new msgids
status_title_not_signedandstatus_title_signedhave emptymsgstrvalues. English users will see the raw msgid keys (e.g., "status_title_not_signed") instead of user-friendly text.📝 Suggested English translations
#: ../browser/table.py msgid "status_title_not_signed" -msgstr "" +msgstr "This signer has not yet signed the session" #: ../browser/table.py msgid "status_title_signed" -msgstr "" +msgstr "This signer has signed the session"🤖 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/locales/en/LC_MESSAGES/imio.esign.po` around lines 521 - 527, Update the English .po entries for the two new msgids by filling their msgstr values so users see friendly text: set msgid "status_title_not_signed" msgstr to something like "Not signed" and set msgid "status_title_signed" msgstr to "Signed" (or other agreed-upon English phrases) in imio.esign.po to replace the empty strings.src/imio/esign/browser/table.py (2)
20-20: ⚡ Quick winRemove redundant import.
The
htmlmodule is imported here, butescapeis already imported fromhtmlat line 4. Line 117 callshtml.escape()when it could simply useescape()directly.♻️ Simplify by removing line 20 and using the imported function directly
-import htmlThen at line 117, change:
- html.escape( + escape(🤖 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/table.py` at line 20, Remove the redundant top-level "import html" and use the already-imported escape function instead of html.escape(); specifically delete the "import html" statement and replace all usages of html.escape(...) (e.g., in the code path that renders table cells) with escape(...) so the module relies on the existing "from html import escape" import.
113-113: ⚡ Quick winApply consistent encoding to email field.
The
safe_encode(), whilefullname(line 111) andposition(line 112) are. This inconsistency could lead to encoding issues if the email contains special characters.♻️ Wrap email in safe_encode for consistency
- s.get("email"), + safe_encode(s.get("email", "")),🤖 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/table.py` at line 113, The email field is not encoded consistently with fullname and position; wrap s.get("email") with safe_encode() just like fullname and position to prevent encoding issues, i.e., replace s.get("email") with safe_encode(s.get("email")) in the same code block (look for safe_encode, fullname, position and s.get("email") in the table generation function in table.py).
🤖 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/table.py`:
- Line 112: The call to safe_encode(s.get("position")) can receive None if
"position" is missing; update the code that builds the position value to provide
a safe default (e.g., use s.get("position", "") or otherwise coerce to an empty
string) before calling safe_encode so safe_encode never receives None; locate
the safe_encode(s.get("position")) usage and replace the get(...) call with a
defaulted variant.
---
Nitpick comments:
In `@src/imio/esign/browser/table.py`:
- Line 20: Remove the redundant top-level "import html" and use the
already-imported escape function instead of html.escape(); specifically delete
the "import html" statement and replace all usages of html.escape(...) (e.g., in
the code path that renders table cells) with escape(...) so the module relies on
the existing "from html import escape" import.
- Line 113: The email field is not encoded consistently with fullname and
position; wrap s.get("email") with safe_encode() just like fullname and position
to prevent encoding issues, i.e., replace s.get("email") with
safe_encode(s.get("email")) in the same code block (look for safe_encode,
fullname, position and s.get("email") in the table generation function in
table.py).
In `@src/imio/esign/locales/en/LC_MESSAGES/imio.esign.po`:
- Around line 521-527: Update the English .po entries for the two new msgids by
filling their msgstr values so users see friendly text: set msgid
"status_title_not_signed" msgstr to something like "Not signed" and set msgid
"status_title_signed" msgstr to "Signed" (or other agreed-upon English phrases)
in imio.esign.po to replace the empty strings.
🪄 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: e6832a0b-5384-4db7-bd53-6a759844035b
📒 Files selected for processing (7)
CHANGES.rstsrc/imio/esign/browser/table.pysrc/imio/esign/browser/templates/macros.ptsrc/imio/esign/locales/en/LC_MESSAGES/imio.esign.posrc/imio/esign/locales/fr/LC_MESSAGES/imio.esign.posrc/imio/esign/locales/imio.esign.potsrc/imio/esign/locales/manual.pot
| "<li>%s, %s%s (%s)</li>" % ( | ||
| "<li>%s, %s (%s) %s</li>" % ( | ||
| safe_encode(s.get("fullname", "")), | ||
| safe_encode(s.get("position")), |
There was a problem hiding this comment.
Provide default value for position field.
The position field uses .get("position") without a default value. If the key is missing, None will be passed to safe_encode(), which may raise an error or produce unexpected output.
🛡️ Add empty string default
- safe_encode(s.get("position")),
+ safe_encode(s.get("position", "")),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| safe_encode(s.get("position")), | |
| safe_encode(s.get("position", "")), |
🤖 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/table.py` at line 112, The call to
safe_encode(s.get("position")) can receive None if "position" is missing; update
the code that builds the position value to provide a safe default (e.g., use
s.get("position", "") or otherwise coerce to an empty string) before calling
safe_encode so safe_encode never receives None; locate the
safe_encode(s.get("position")) usage and replace the get(...) call with a
defaulted variant.
In info viewlet, display
signedin green.See #PARAF-476
Summary by CodeRabbit
Release Notes
New Features
Localization