PARAF-220: Automate to_print flag and print all to_print files in d-print#138
Open
chris-adam wants to merge 1 commit into
Open
PARAF-220: Automate to_print flag and print all to_print files in d-print#138chris-adam wants to merge 1 commit into
chris-adam wants to merge 1 commit into
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 automatic Changesto_print automation and dashboard print flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant i_annex_added
participant _correct_to_print
participant file_obj
participant categorized_elements
i_annex_added->>_correct_to_print: call with obj
_correct_to_print->>file_obj: compute to_print from portal_type and omail.esign
_correct_to_print->>categorized_elements: check to_be_printed_activated
_correct_to_print->>file_obj: update to_print if changed
_correct_to_print->>categorized_elements: update_categorized_elements
sequenceDiagram
participant DocumentGenerationOMDashboardHelper
participant get_categorized_elements
participant get_print_pages
DocumentGenerationOMDashboardHelper->>get_categorized_elements: collect to_print files
DocumentGenerationOMDashboardHelper->>get_print_pages: build preview pages per file
get_print_pages-->>DocumentGenerationOMDashboardHelper: empty for ODT/image, per-page data otherwise
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
chris-adam
force-pushed
the
PARAF-220/pour_impression
branch
from
32004bc to
6465696
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@CHANGES.rst`:
- Line 16: Fix the typo in the changelog entry by updating the d-print note in
CHANGES.rst so it says “takes” instead of “taks”. Locate the document generation
release note mentioning the to_print attribute and correct the word in that
entry.
In `@imio/dms/mail/adapters.py`:
- Around line 1807-1819: The signing path in `adapters.py` is forcing `to_print`
on GED main files and generated signature PDFs without honoring the
`to_be_printed_activated` gate, which bypasses `subscribers._correct_to_print`.
Update the `pdf_file.to_print` and `orig_fobj.to_print` handling in the
signing-related branches to go through the same gated logic used elsewhere
instead of assigning directly. Also add a regression test covering the signing
flow with `to_be_printed_activated=False` to ensure files do not appear in
`d-print` when printing is disabled.
In `@imio/dms/mail/browser/documentgenerator.py`:
- Around line 370-371: The limit handling in documentgenerator.py treats 0 as
falsy in the file-slicing logic, so get_dms_files(limit=0) returns all files
instead of none. Update the limit check in the file selection block to
explicitly test for a provided value using the relevant file list variable so
that 0 is respected as a valid limit and only None means “no limit”.
- Around line 453-455: The preview-page blob reads in documentgenerator.py leave
each stream open after blob.open().read(), so update the page loop to read the
blob inside a context manager and close it immediately after use. Locate the
page-collection logic in the loop that builds pages and ensure the Blob object
from files["large/dump_%d.%s" % (page, fmt)] is opened with a with block before
appending the page data to pages.
🪄 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: bd386ad4-ba27-423d-931b-de0f2b6ae4e7
📒 Files selected for processing (8)
CHANGES.rstimio/dms/mail/adapters.pyimio/dms/mail/browser/documentgenerator.pyimio/dms/mail/profiles/default/templates/d-print.odtimio/dms/mail/subscribers.pyimio/dms/mail/tests/test_adapters.pyimio/dms/mail/tests/test_documentgenerator.pyimio/dms/mail/tests/test_subscribers.py
Comment on lines
+1807
to
+1819
| # PARAF-220: the generated signature file (from a GED main) is printed; | ||
| # the source GED main is not (its pdf replaces it in the print output). | ||
| pdf_file.to_print = orig_fobj.portal_type == "dmsommainfile" | ||
| if orig_fobj.portal_type == "dmsommainfile" and orig_fobj is not pdf_file: | ||
| orig_fobj.to_print = False | ||
| update_categorized_elements( | ||
| self.context, | ||
| orig_fobj, | ||
| get_category_object(self.context, orig_fobj.content_category), | ||
| limited=True, | ||
| sort=False, | ||
| logging=True, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor to_be_printed_activated in the signing path.
These branches force to_print=True for GED mains / generated signature PDFs without re-checking the category-group gate. That bypasses subscribers._correct_to_print, so a category with printing disabled will start showing up in d-print as soon as the file goes through signing.
Please route these assignments through the same gated logic, and add a regression case with to_be_printed_activated=False.
Also applies to: 1896-1906
🤖 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 `@imio/dms/mail/adapters.py` around lines 1807 - 1819, The signing path in
`adapters.py` is forcing `to_print` on GED main files and generated signature
PDFs without honoring the `to_be_printed_activated` gate, which bypasses
`subscribers._correct_to_print`. Update the `pdf_file.to_print` and
`orig_fobj.to_print` handling in the signing-related branches to go through the
same gated logic used elsewhere instead of assigning directly. Also add a
regression test covering the signing flow with `to_be_printed_activated=False`
to ensure files do not appear in `d-print` when printing is disabled.
afbe2ec to
6b9ab81
Compare
…int" attribute of files
chris-adam
force-pushed
the
PARAF-220/pour_impression
branch
from
6b9ab81 to
dfbbd9c
Compare
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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
1 participant
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.
En résumé, deux changements: activer "to_print" automatiquement à la création des fichiers, et le prendre en compte dans le modèle "Pour impression" même pour les fichiers non-ODT.
@sgeulette Question: Ici, je set
to_printlors de la création du fichier (subscriberi_annex_added). Est-ce qu'il faudrait le set plutôt lors de la transition avant signature (manuscrite) par exemple ?Summary
Two related PARAF-220 changes, delivered together:
1. Automate the
to_printflag on outgoing-mail files (subscribers.py)New
_correct_to_print(file_obj)helper, called fromi_annex_addedwhenever admsommainfileordmsappendixfileis added to an outgoing mail:dmsappendixfile) are never printed →to_print=False.dmsommainfile) is printed when the parent mail is not electronically signed (esignoff), and not printed when it is.to_be_printed_activatedgate is respected:to_printis never forcedTruewhen printing is deactivated for that category group.categorized_elementscache is refreshed (update_categorized_elements) only when the value actually changes.2.
d-printdocument generation now honorsto_print(browser/documentgenerator.py,profiles/default/templates/d-print.odt)get_dms_filesnow returns, for every selected mail, all categorized files (main + appendix) whoseto_printisTrue— viaget_categorized_elements(..., filters={"to_print": True})— ordered main-before-appendix, dashboard selection order preserved. The old per-maillimit=1cap and theis_odt()-only exclusion are gone;limitis now an optional overall cap.collective.documentviewerpreview page images (get_print_pages); image files (png/jpg/…) are inserted at native size (is_image/img_format).print_page_count/needs_blank_after: a blank page follows a file with an odd page count so the next file starts recto. ODT files keep managing their own duplex page break.Tests
test_subscribers.test_correct_to_print— main on/off (esign), appendix always off, and theto_be_printed_activatedgate.test_adapters—create_pdf_filetests extended to assert the generated signature pdf (and its source main) areto_print=False.test_documentgenerator.test_DocumentGenerationOMDashboardHelper—get_dms_filesordering/filtering +limit, and the newis_odt/is_image/img_format/get_print_pages/print_page_count/needs_blank_afterhelpers.Summary by CodeRabbit
Summary