Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions imio/dms/mail/browser/actionspanel.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ def __init__(self, context, request):
# self.ACCEPTABLE_ACTIONS = ['copy', 'paste', 'delete']
self.ACCEPTABLE_ACTIONS = ["delete", 'approvals']
self.SECTIONS_TO_RENDER += (
"render_duplicate_button",
"render_create_from_template_button",
"render_create_new_message",
"render_send_email",
Expand All @@ -234,6 +235,17 @@ def render_create_from_template_button(self):
return ViewPageTemplateFile("templates/actions_panel_create_from_template.pt")(self)
return ""

def may_duplicate(self):
"""Method that check if special 'duplicate' action has to be displayed."""
if self.member.has_permission("Add portal content", self.portal["outgoing-mail"]):
return True
return False

def render_duplicate_button(self):
if self.may_duplicate():
return ViewPageTemplateFile("templates/actions_panel_duplicate.pt")(self)
return ""

def may_create_new_message(self):
if (
self.context.is_email()
Expand Down
7 changes: 7 additions & 0 deletions imio/dms/mail/browser/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@
class=".views.CreateFromTemplateForm"
/>

<browser:page
name="om-duplicate"
for="imio.dms.mail.dmsmail.IImioDmsOutgoingMail"
permission="zope2.View"
class=".views.OMDuplicateForm"
/>
Comment on lines +22 to +27

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 | 🟠 Major

Permission mismatch: view requires weaker permission than action panel check.

The action panel's may_duplicate() in actionspanel.py checks for "Add portal content" permission, but this browser page is registered with "zope2.View". Users could bypass the UI permission check by directly accessing @@om-duplicate. Since the duplicate operation creates new content in the outgoing-mail folder, the permission should be aligned.

🔒 Proposed fix to align permissions
     <browser:page
         name="om-duplicate"
         for="imio.dms.mail.dmsmail.IImioDmsOutgoingMail"
-        permission="zope2.View"
+        permission="cmf.AddPortalContent"
         class=".views.OMDuplicateForm"
         />
📝 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.

Suggested change
<browser:page
name="om-duplicate"
for="imio.dms.mail.dmsmail.IImioDmsOutgoingMail"
permission="zope2.View"
class=".views.OMDuplicateForm"
/>
<browser:page
name="om-duplicate"
for="imio.dms.mail.dmsmail.IImioDmsOutgoingMail"
permission="cmf.AddPortalContent"
class=".views.OMDuplicateForm"
/>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@imio/dms/mail/browser/configure.zcml` around lines 22 - 27, The page
registration for om-duplicate uses permission "zope2.View" which is weaker than
the permission checked in actionspanel.py::may_duplicate() (which checks "Add
portal content"); update the browser:page entry for name "om-duplicate" (class
".views.OMDuplicateForm") to require the same permission used by may_duplicate()
(i.e., "Add portal content" or the appropriate permission constant) so direct
URL access cannot bypass the action panel's permission check.


<browser:page
name="imiodmsmail-settings"
for="Products.CMFPlone.interfaces.IPloneSiteRoot"
Expand Down
29 changes: 29 additions & 0 deletions imio/dms/mail/browser/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,17 @@ class ISignerRuleSchema(Interface):
# ]
# )

omail_duplicate_fields = SimpleVocabulary(
[
SimpleTerm(value=u"category", title=_(u"Keep classification category")),
SimpleTerm(value=u"folder", title=_(u"Keep classification folder")),
SimpleTerm(value=u"reply_to", title=_(u"Keep reply_to mails")),
SimpleTerm(value=u"dms_files", title=_(u"Keep DMS files")),
SimpleTerm(value=u"annexes", title=_(u"Keep annexes")),
SimpleTerm(value=u"link_to_duplicated", title=_(u"Link to duplicated mail")),
]
)

oemail_sender_email_values = SimpleVocabulary(
[
SimpleTerm(value=u"agent_email", title=_(u"Sender held position email is used")),
Expand Down Expand Up @@ -594,6 +605,8 @@ class IImioDmsMailConfig(model.Schema):
"omail_send_modes",
"omail_post_mailing",
"omail_signer_rules",
"omail_duplicate_display_fields",
"omail_duplicate_true_default_values",
"omail_fields",
"omail_group_encoder",
],
Expand Down Expand Up @@ -678,6 +691,22 @@ class IImioDmsMailConfig(model.Schema):
auto_append=False,
)

omail_duplicate_display_fields = schema.List(
title=_(u"Fields to display when duplicating an outgoing mail"),
required=False,
value_type=schema.Choice(vocabulary=omail_duplicate_fields),
default=[u"category", u"folder", u"reply_to", u"dms_files", u"annexes", u"link_to_duplicated"],
)
# widget("omail_duplicate_display_fields", CheckBoxFieldWidget, multiple="multiple", size=5)

omail_duplicate_true_default_values = schema.List(
title=_(u"Default values to True when duplicating an outgoing mail"),
description=_(u"If checked, the default value will be True."),
required=False,
value_type=schema.Choice(vocabulary=omail_duplicate_fields),
default=[u"category", u"folder", u"annexes"],
)

omail_fields = schema.List(
title=_(u"${type} fields display", mapping={"type": _("Outgoing mail")}),
description=_(u"Configure this carefully. You can order with arrows."),
Expand Down
3 changes: 3 additions & 0 deletions imio/dms/mail/browser/static/copy.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions imio/dms/mail/browser/templates/actions_panel_duplicate.pt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<td class="noPadding" i18n:domain="imio.dms.mail">
<a tal:attributes="href string:${context/absolute_url}/@@om-duplicate"
target="_parent"
class="overlay"
tal:define="dummy view/saveHasActions;">
<input tal:condition="not:view/useIcons" type="button" value="Duplicate" class="apButton apButtonAction apButtonAction_duplicate" i18n:attributes="value" />
<img tal:condition="view/useIcons" i18n:attributes="title" title="Duplicate outgoing mail" tal:attributes="src string: ${view/portal_url}/++resource++imio.dms.mail/copy.svg;"/>
</a>
</td>
<td class="noPadding">
66 changes: 66 additions & 0 deletions imio/dms/mail/browser/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from imio.dms.mail import _
from imio.dms.mail import _tr
from imio.dms.mail import PMH_ENABLED
from imio.dms.mail.browser.settings import IImioDmsMailConfig
from imio.dms.mail.browser.settings import omail_duplicate_fields
from imio.dms.mail.browser.table import ApprovalTable
from imio.dms.mail.browser.table import CKTemplatesTable
from imio.dms.mail.browser.table import PersonnelTable
Expand All @@ -30,6 +32,7 @@
from imio.helpers.fancytree.views import BaseRenderFancyTree
from imio.helpers.workflow import do_transitions
from imio.helpers.xhtml import object_link
from imio.pyutils.utils import safe_encode
from plone import api
from Products.CMFCore.utils import getToolByName
from Products.CMFPlone import utils
Expand All @@ -42,6 +45,10 @@
from Products.PageTemplates.Expressions import SecureModuleImporter
from Products.statusmessages.interfaces import IStatusMessage
from unidecode import unidecode # unidecode_expect_nonascii not yet available in used version
from z3c.form import button
from z3c.form.field import Fields
from z3c.form.form import Form
from zope import schema
from zope.annotation import IAnnotations
from zope.component import getMultiAdapter
from zope.i18n import translate
Expand Down Expand Up @@ -88,6 +95,65 @@ def redirect_url(self, uid):
return "{}/persistent-document-generation?{}".format(url, "&".join(params))


class OMDuplicateForm(Form):

"""Duplicate an outgoing mail."""
label = _(u"Duplicate mail")
ignoreContext = True

def update(self):
"""Handle fields."""
om_fields = api.portal.get_registry_record("omail_fields", IImioDmsMailConfig, [])
om_fields = [dic["field_name"] for dic in om_fields]
matching = {u"category": "IClassificationFolder.classification_categories",
u"folder": "IClassificationFolder.classification_folders",
u"reply_to": "reply_to",
u"link_to_duplicated": "reply_to"}
to_show = api.portal.get_registry_record("omail_duplicate_display_fields", IImioDmsMailConfig, [])
to_true = api.portal.get_registry_record("omail_duplicate_true_default_values", IImioDmsMailConfig, [])
for term in omail_duplicate_fields:
if ((term.value in matching and matching[term.value] not in om_fields) or
(term.value not in to_show and term.value not in to_true)):
continue
self.fields += Fields(
schema.Bool(
__name__=safe_encode(term.value),
title=term.title,
default=term.value in to_true,
))
super(OMDuplicateForm, self).update()

def updateWidgets(self, prefix=None):
super(OMDuplicateForm, self).updateWidgets()

# we hide fields not to show
to_show = api.portal.get_registry_record("omail_duplicate_display_fields", IImioDmsMailConfig, [])
for term in omail_duplicate_fields:
if term.value not in to_show:
self.widgets[term.value].mode = "hidden"
Comment on lines +126 to +133

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 | 🔴 Critical

KeyError when accessing widgets for fields that were skipped in update().

In update(), some terms from omail_duplicate_fields are skipped when they don't meet the conditions (line 115-117). However, updateWidgets() iterates through all terms and unconditionally accesses self.widgets[term.value]. This will raise a KeyError for terms that were skipped and have no corresponding widget.

🐛 Proposed fix to check widget existence before accessing
     def updateWidgets(self, prefix=None):
         super(OMDuplicateForm, self).updateWidgets()

         # we hide fields not to show
         to_show = api.portal.get_registry_record("omail_duplicate_display_fields", IImioDmsMailConfig, [])
         for term in omail_duplicate_fields:
-            if term.value not in to_show:
+            if term.value not in to_show and term.value in self.widgets:
                 self.widgets[term.value].mode = "hidden"
📝 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.

Suggested change
def updateWidgets(self, prefix=None):
super(OMDuplicateForm, self).updateWidgets()
# we hide fields not to show
to_show = api.portal.get_registry_record("omail_duplicate_display_fields", IImioDmsMailConfig, [])
for term in omail_duplicate_fields:
if term.value not in to_show:
self.widgets[term.value].mode = "hidden"
def updateWidgets(self, prefix=None):
super(OMDuplicateForm, self).updateWidgets()
# we hide fields not to show
to_show = api.portal.get_registry_record("omail_duplicate_display_fields", IImioDmsMailConfig, [])
for term in omail_duplicate_fields:
if term.value not in to_show and term.value in self.widgets:
self.widgets[term.value].mode = "hidden"
🧰 Tools
🪛 Ruff (0.15.2)

[warning] 126-126: Unused method argument: prefix

(ARG002)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@imio/dms/mail/browser/views.py` around lines 126 - 133, The updateWidgets
method in OMDuplicateForm assumes every term in omail_duplicate_fields has a
widget, causing KeyError for terms skipped earlier in update; modify
updateWidgets to check for widget existence before accessing it (i.e., verify
term.value exists in self.widgets) and only set self.widgets[term.value].mode =
"hidden" when present; reference the update method behavior and ensure
compatibility with omail_duplicate_fields, self.widgets, and the
api.portal.get_registry_record retrieval of "omail_duplicate_display_fields".


@button.buttonAndHandler(_('Duplicate'), name='duplicate')
def handleApply(self, action):
data, errors = self.extractData()

if errors:
self.status = self.formErrorsMessage
return

# Duplicate the mail
odm_utils = getMultiAdapter((self.context, self.request), name="odm-utils")
duplicated_mail = odm_utils.duplicate(
keep_category=data.get('category', False),
keep_folder=data.get('folder', False),
keep_reply_to=data.get('reply_to', False),
keep_dms_files=data.get('dms_files', False),
keep_annexes=data.get('annexes', False),
link_to_duplicated=data.get('link_to_duplicated', False),
)

self.request.response.redirect(duplicated_mail.absolute_url()+"/edit")


def parse_query(text):
"""Copied from plone.app.vocabularies.catalog.parse_query but cleaned."""
for char in "?-+*()":
Expand Down
4 changes: 4 additions & 0 deletions imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml
Original file line number Diff line number Diff line change
Expand Up @@ -710,6 +710,10 @@ dl.portletWidgetCollection div.portlet_add_icons a {
background-image: url('&dtml-portal_url;/++resource++imio.dms.mail/models_icon.png');
}

.apButtonAction_duplicate {
background-image: url('&dtml-portal_url;/++resource++imio.dms.mail/copy.svg');
}

.apButtonAction_sendemail {
background-image: url('&dtml-portal_url;/++resource++imio.dms.mail/send_email.svg');
background-size: 18px 18px;
Expand Down
10 changes: 10 additions & 0 deletions imio/dms/mail/subscribers.py
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,16 @@ def dmsoutgoingmail_added(mail, event):
zope.event.notify(ObjectModifiedEvent(mail, Attributes(ISigningBehavior, "ISigningBehavior.signers")))


def dmsoutgoingmail_modified(mail, event):
annot = IAnnotations(mail).get('imio.dms.mail', {})
copy_dms_files_from = annot.get('copy_dms_files_from')
if copy_dms_files_from:
del annot['copy_dms_files_from']
original_mail = uuidToObject(copy_dms_files_from, unrestricted=True)
odm_utils = getMultiAdapter((mail, mail.REQUEST), name="odm-utils")
odm_utils.copy_dms_files(original_mail)
Comment on lines +574 to +577

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 | 🟠 Major

Clear copy_dms_files_from only after a successful copy.

At Line 574 the annotation marker is removed before UUID resolution and copy. If uuidToObject fails or copy_dms_files raises, retry information is lost and DMS files will never be copied.

🛠️ Proposed fix
     if copy_dms_files_from:
-        del annot['copy_dms_files_from']
         original_mail = uuidToObject(copy_dms_files_from, unrestricted=True)
+        if original_mail is None:
+            return
         odm_utils = getMultiAdapter((mail, mail.REQUEST), name="odm-utils")
         odm_utils.copy_dms_files(original_mail)
+        annot.pop('copy_dms_files_from', None)
📝 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.

Suggested change
del annot['copy_dms_files_from']
original_mail = uuidToObject(copy_dms_files_from, unrestricted=True)
odm_utils = getMultiAdapter((mail, mail.REQUEST), name="odm-utils")
odm_utils.copy_dms_files(original_mail)
if copy_dms_files_from:
original_mail = uuidToObject(copy_dms_files_from, unrestricted=True)
if original_mail is None:
return
odm_utils = getMultiAdapter((mail, mail.REQUEST), name="odm-utils")
odm_utils.copy_dms_files(original_mail)
annot.pop('copy_dms_files_from', None)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@imio/dms/mail/subscribers.py` around lines 574 - 577, The code currently
deletes the annotation key 'copy_dms_files_from' before resolving the UUID and
copying, which loses retry info on failure; change the sequence in the handler
so you first read copy_dms_files_from from annot, call
uuidToObject(copy_dms_files_from, unrestricted=True), obtain odm_utils via
getMultiAdapter((mail, mail.REQUEST), name="odm-utils") and call
odm_utils.copy_dms_files(original_mail), and only after that call succeeds
remove annot['copy_dms_files_from'] (and persist/save if required); ensure
exceptions from uuidToObject or copy_dms_files do not delete the annotation so
retries remain possible.


Comment on lines +570 to +578

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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n '^def dmsoutgoingmail_modified\(' imio/dms/mail/subscribers.py

Repository: IMIO/imio.dms.mail

Length of output: 155


🏁 Script executed:

# Read context around the first definition at line 475
sed -n '470,490p' imio/dms/mail/subscribers.py

Repository: IMIO/imio.dms.mail

Length of output: 844


🏁 Script executed:

# Read context around the second definition at line 570
sed -n '565,585p' imio/dms/mail/subscribers.py

Repository: IMIO/imio.dms.mail

Length of output: 1085


🏁 Script executed:

# Check for `@subscribe` or event handler registrations around both definitions
# Look at lines before first definition
sed -n '468,476p' imio/dms/mail/subscribers.py
echo "---"
# Look at lines before second definition  
sed -n '563,571p' imio/dms/mail/subscribers.py

Repository: IMIO/imio.dms.mail

Length of output: 860


🏁 Script executed:

# Check further back for decorators (up to 10 lines before each definition)
sed -n '465,476p' imio/dms/mail/subscribers.py
echo "---SECOND---"
sed -n '560,571p' imio/dms/mail/subscribers.py

Repository: IMIO/imio.dms.mail

Length of output: 1045


🏁 Script executed:

# Find ZCML files in the package
find imio/dms/mail -name "*.zcml" -type f

Repository: IMIO/imio.dms.mail

Length of output: 463


🏁 Script executed:

cat imio/dms/mail/subscribers.zcml

Repository: IMIO/imio.dms.mail

Length of output: 13755


🏁 Script executed:

# Find the end of the first dmsoutgoingmail_modified function (line 475)
# Read from line 475 until we hit the next function definition
sed -n '475,570p' imio/dms/mail/subscribers.py | head -95

Repository: IMIO/imio.dms.mail

Length of output: 4349


dmsoutgoingmail_modified is redefined and masks the original subscriber.

The second definition at line 570 overwrites the first at line 475 in Python. When imported and registered in ZCML, only the second function (which copies DMS files) is available. The first function's critical logic—updating signers based on registry rules and calling approval.update_signers()—is lost. This breaks the signer/approval workflow when dmsoutgoingmail_added triggers the ObjectModifiedEvent.

Rename the second definition to _copy_dms_files_from_annotation(mail) and call it from the first function to preserve both functionalities.

🧰 Tools
🪛 Ruff (0.15.2)

[error] 570-570: Redefinition of unused dmsoutgoingmail_modified from line 475: dmsoutgoingmail_modified redefined here

(F811)


[warning] 570-570: Unused function argument: event

(ARG001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@imio/dms/mail/subscribers.py` around lines 570 - 578, The second function
dmsoutgoingmail_modified currently redefines and masks the original; rename the
second definition to _copy_dms_files_from_annotation(mail) and move its logic
there (use IAnnotations, uuidToObject, getMultiAdapter((mail, mail.REQUEST),
name="odm-utils") and odm_utils.copy_dms_files(original_mail)); then update the
original dmsoutgoingmail_modified (the first definition that handles registry
rules and approval.update_signers()) to call
_copy_dms_files_from_annotation(mail) where the previous behavior expected the
copy-from-annotation side effect so both signer/update_signers logic and DMS
file copying run without one definition overriding the other.


def dv_handle_file_creation(obj, event):
"""Intermediate function to avoid converting some files in documentviewer"""
if obj.portal_type in DV_AVOIDED_TYPES:
Expand Down
Loading