Skip to content

Display files table in DMS mail edit mode#109

Open
chris-adam wants to merge 2 commits intomasterfrom
DMS-1038/view_annexes_in_edit
Open

Display files table in DMS mail edit mode#109
chris-adam wants to merge 2 commits intomasterfrom
DMS-1038/view_annexes_in_edit

Conversation

@chris-adam
Copy link
Copy Markdown
Contributor

@chris-adam chris-adam commented Mar 27, 2026

ça a été demandé par Lasne et Seraing. Mais je vois que cette table a été expressément masquée en mode édition. Je me demande donc quelles étaient les raisons de l'avoir masquée à la base ?

Summary by CodeRabbit

  • Style

    • Adjusted vertical spacing in the document edit view's versions area for improved layout.
    • Removed an unnecessary top offset in edit mode for a cleaner appearance.
  • Bug Fixes

    • Ensure the versions table is visible in edit mode.
    • Hide action and icon columns while editing to simplify the versions list and reduce clutter.

@chris-adam chris-adam requested a review from sgeulette March 27, 2026 15:22
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 27, 2026

📝 Walkthrough

Walkthrough

CSS adjustments to edit-mode spacing and removal of a rule hiding the versions table, plus logic in the versions table to detect edit views and remove action/icon-clickable columns when editing.

Changes

Cohort / File(s) Summary
CSS Styling Updates
imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml
Replaced padding-top: 4em; with margin-top: 5em; for .template-dmsdocument-edit #fieldset-versions``, set .template-dmsdocument-edit #DV-container` { margin-top: 0; }`, and removed the rule that hid `#fieldset-versions table`.
Table behavior (Python)
imio/dms/mail/browser/table.py
Added BaseVersionsTable.is_edit_mode() (inspects last segment of request['ACTUAL_URL']) and overridden BaseVersionsTable.setUpColumns() to filter out "action-column" and IconClickableColumn instances when in edit mode.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nudge a margin where padding used to play,
I tuck away icons when editors stay,
CSS lines shift and columns quietly hide,
A rabbit hops through code and style with pride. 🐇

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: displaying the files table in DMS mail edit mode, which aligns with both the CSS and Python modifications that remove edit-mode hiding rules and add column filtering logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch DMS-1038/view_annexes_in_edit

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Member

@sgeulette sgeulette left a comment

Choose a reason for hiding this comment

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

Ok pour montrer le tableau mais il faut alors enlever tout ce qui est colonne inutile ou d'édition. Soit on ne laisse que le titre, soit on laisse les iconified (mais en lecture seule) et les icones de view et téléchargement des actions

.template-dmsdocument-edit #fieldset-versions {
width: 65% !important;
padding-top: 4em;
margin-top: 6em;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

5em
.template-dmsdocument-edit #DV-container {margin-top: 0;}

@chris-adam chris-adam force-pushed the DMS-1038/view_annexes_in_edit branch from 5b0bed0 to b8d18c3 Compare April 14, 2026 13:09
@chris-adam
Copy link
Copy Markdown
Contributor Author

chris-adam commented Apr 14, 2026

@sgeulette J'ai fait les changements que tu as demandé. Mais je ne suis pas certain que c'est la bonne manière d masquer les colonnes en mode edit
94fae02

@chris-adam chris-adam requested a review from sgeulette April 14, 2026 13:11
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

♻️ Duplicate comments (1)
imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml (1)

159-161: ⚠️ Potential issue | 🟠 Major

Move the #DV-container margin override to the effective rule block.

margin-top: 0 here is overridden later by the same selector at Line 238 (margin-top: 3em), so this change does not take effect in edit mode.

Proposed fix
 .template-dmsdocument-edit `#DV-container` {
-    margin-top: 0;
+    margin-top: 0;
 }
@@
 .template-dmsdocument-edit `#DV-container` {
     width: 100% !important;
-    margin-top: 3em;
+    margin-top: 0;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml` around lines 159 -
161, The rule setting margin-top: 0 on the selector .template-dmsdocument-edit
`#DV-container` is being overridden later by the same selector (margin-top: 3em);
move or consolidate the margin-top: 0 into the effective rule block where
.template-dmsdocument-edit `#DV-container` is defined later (the block around the
later occurrence) so the edit-mode override takes effect — update the later rule
to use margin-top: 0 for edit mode or remove the duplicate earlier rule and keep
the desired margin-top in the single effective selector
(.template-dmsdocument-edit `#DV-container`).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml`:
- Around line 159-161: The rule setting margin-top: 0 on the selector
.template-dmsdocument-edit `#DV-container` is being overridden later by the same
selector (margin-top: 3em); move or consolidate the margin-top: 0 into the
effective rule block where .template-dmsdocument-edit `#DV-container` is defined
later (the block around the later occurrence) so the edit-mode override takes
effect — update the later rule to use margin-top: 0 for edit mode or remove the
duplicate earlier rule and keep the desired margin-top in the single effective
selector (.template-dmsdocument-edit `#DV-container`).

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 10e3e4a1-bb1d-44f4-9042-ff46741d7f7e

📥 Commits

Reviewing files that changed from the base of the PR and between 5b0bed0 and b8d18c3.

📒 Files selected for processing (2)
  • imio/dms/mail/browser/table.py
  • imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml

@chris-adam chris-adam force-pushed the DMS-1038/view_annexes_in_edit branch from b8d18c3 to 94fae02 Compare April 14, 2026 13:15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 `@imio/dms/mail/browser/table.py`:
- Around line 127-129: The is_edit_mode function can miss edit views when
ACTUAL_URL has trailing slashes or query/fragment parts; update is_edit_mode to
normalize the URL before extracting the view name by first removing query
strings/fragments and trimming trailing slashes (e.g. split on '?' and '#', then
rstrip('/')), then take the last path segment into view_name and check it
against ('edit', '@@edit'); modify the is_edit_mode implementation accordingly
to use this normalized view_name so URLs like "/foo/edit/", "/foo/@@edit?x=1" or
"/foo/@@edit/#frag" are handled.
🪄 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: 892c8d3c-ff41-4261-9b5b-b678fabbcd52

📥 Commits

Reviewing files that changed from the base of the PR and between b8d18c3 and 94fae02.

📒 Files selected for processing (2)
  • imio/dms/mail/browser/table.py
  • imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml

Comment on lines +127 to +129
def is_edit_mode(self):
view_name = self.request.get('ACTUAL_URL', '').split('/')[-1]
return view_name in ('edit', '@@edit')
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Harden edit-mode detection against URL suffix variations.

Line 128 can fail for URLs ending with / or containing query params, which would skip edit-mode column filtering.

🔧 Proposed fix
 def is_edit_mode(self):
-    view_name = self.request.get('ACTUAL_URL', '').split('/')[-1]
+    actual_url = self.request.get('ACTUAL_URL', '')
+    view_name = actual_url.split('?', 1)[0].rstrip('/').split('/')[-1]
     return view_name in ('edit', '@@edit')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@imio/dms/mail/browser/table.py` around lines 127 - 129, The is_edit_mode
function can miss edit views when ACTUAL_URL has trailing slashes or
query/fragment parts; update is_edit_mode to normalize the URL before extracting
the view name by first removing query strings/fragments and trimming trailing
slashes (e.g. split on '?' and '#', then rstrip('/')), then take the last path
segment into view_name and check it against ('edit', '@@edit'); modify the
is_edit_mode implementation accordingly to use this normalized view_name so URLs
like "/foo/edit/", "/foo/@@edit?x=1" or "/foo/@@edit/#frag" are handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants