Skip to content

Conversation

chris-adam
Copy link
Contributor

No description provided.

@chris-adam chris-adam force-pushed the DMS-888/duplicate_omail branch 2 times, most recently from c2cc8ae to 62464cd Compare July 31, 2025 15:16
@chris-adam chris-adam force-pushed the DMS-888/duplicate_omail branch 2 times, most recently from 0ee1081 to 3e49ec0 Compare August 1, 2025 13:16
@chris-adam chris-adam force-pushed the DMS-888/duplicate_omail branch from 3e49ec0 to b2c75dd Compare August 1, 2025 14:16
@chris-adam chris-adam requested a review from Copilot August 1, 2025 14:21
@chris-adam chris-adam marked this pull request as ready for review August 1, 2025 14:21
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a duplicate action for outgoing mails, allowing users to create copies of existing outgoing mails with configurable options for what elements to preserve from the original.

Key changes include:

  • Implementation of a duplicate form with various configuration options (keep categories, folders, linked mails, DMS files, annexes, and linking to original)
  • Addition of configuration settings for default values in the duplication process
  • UI integration with action panel button and CSS styling

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
imio/dms/mail/tests/test_views.py Comprehensive test suite for the duplicate functionality covering all configuration options
imio/dms/mail/browser/views.py Core duplicate form implementation with schema and business logic
imio/dms/mail/browser/settings.py Configuration schema for default duplication values
imio/dms/mail/browser/actionspanel.py Integration of duplicate button in the actions panel
imio/dms/mail/browser/templates/actions_panel_duplicate.pt Template for the duplicate button
imio/dms/mail/browser/configure.zcml Browser view registration for the duplicate form
imio/dms/mail/skins/imio_dms_mail/imiodmsmail.css.dtml CSS styling for the duplicate button
imio/dms/mail/locales/*.po Localization files with French translations and empty English strings

self.assertEqual(len(brains), 1)
duplicated_mail = brains[0].getObject()
self.assertEqual(len(duplicated_mail.reply_to), 1)
self.assertNotEqual(self.omail2, duplicated_mail.reply_to[0].to_object)
Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

This assertion appears to be incorrect. When keep_linked_mails is False but link_to_original is True, the reply_to should contain only the original mail (self.omail), not self.omail2. The assertion should check that duplicated_mail.reply_to[0].to_object equals self.omail, not that it's not equal to self.omail2.

Suggested change
self.assertNotEqual(self.omail2, duplicated_mail.reply_to[0].to_object)
self.assertEqual(self.omail, duplicated_mail.reply_to[0].to_object)

Copilot uses AI. Check for mistakes.

self.assertEqual(len(brains), 1)
duplicated_mail = brains[0].getObject()
self.assertEqual(len(duplicated_mail.reply_to), 1)
self.assertNotEqual(self.omail, duplicated_mail.reply_to[0].to_object)
Copy link
Preview

Copilot AI Aug 1, 2025

Choose a reason for hiding this comment

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

This assertion is incorrect. When link_to_original is False but keep_linked_mails is True, the reply_to should contain only the previously linked mail (self.omail2), not the original mail. The assertion should check that duplicated_mail.reply_to[0].to_object equals self.omail2, not that it's not equal to self.omail.

Suggested change
self.assertNotEqual(self.omail, duplicated_mail.reply_to[0].to_object)
self.assertEqual(self.omail2, duplicated_mail.reply_to[0].to_object)

Copilot uses AI. Check for mistakes.

@chris-adam chris-adam force-pushed the DMS-888/duplicate_omail branch from b2c75dd to 13c936d Compare August 1, 2025 14:28
@chris-adam chris-adam requested a review from sgeulette August 1, 2025 14:28
@IMIO IMIO deleted a comment from Copilot AI Aug 4, 2025
Copy link
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.

j'ai pas relu les tests pour le moment...

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="Dupliquer" tal:attributes="src string: ${view/portal_url}/++resource++imio.dms.mail/copy.svg;"/>
Copy link
Member

Choose a reason for hiding this comment

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

title="duplicate outgoing mail"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

C'est le texte du bouton, ça va faire très grand si on met tout ce texte

Copy link
Member

Choose a reason for hiding this comment

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

on fera plus court dans la traduction mais au moins c'est plus explicite


def updateWidgets(self):
super(DuplicateForm, self).updateWidgets()
self.widgets["keep_category"].value = ['selected'] if api.portal.get_registry_record("omail_duplicate_default_keep_category", IImioDmsMailConfig, True) else []
Copy link
Member

Choose a reason for hiding this comment

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

Il faudrait aussi tester si le champ "en relation" est affiché, à savoir dans la config des champs affichés. En effet, pas besoin de proposer l'option keep_folder si les folders sont pas utilisés... Et cela, il y a moyen de le gérer via une autre méthode je pense...celles qui gère les fields du form

self.widgets["keep_annexes"].value = ['selected'] if api.portal.get_registry_record("omail_duplicate_default_keep_annexes", IImioDmsMailConfig, True) else []
self.widgets["link_to_original"].value = ['selected'] if api.portal.get_registry_record("omail_duplicate_default_link_to_original", IImioDmsMailConfig, True) else []

navtree_props = getToolByName(api.portal.get(), 'portal_properties').navtree_properties
Copy link
Member

Choose a reason for hiding this comment

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

c'est pas la bonne façon de faire. Voir commentaire précédent...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je pensais que c'était ça la config des champs affichés, et ici la méthode qui gère les fields du form...


# Duplicate the mail
parent = self.context.aq_parent
clipboard = parent.manage_copyObjects([self.context.getId()])
Copy link
Member

Choose a reason for hiding this comment

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

dangereux un copy/paste car les susbcribers vont se déclencher sur du contenu qui va être modifié ou effacé.
Il serait préférable de créer de nouveaux objets à partir des données qu'on veut.
Et copier le sous-contenu le cas échéant. ET là encore, on ne peut pas copier tel quel un document généré car le CB dedans ne peut pas être dupliqué. Il faudrait plutôt regénérer le même modèle: on a les infos pour le faire...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, et où on peut retrouver les infos de comment ont été générés les main files ? Et qu'est-ce qui se passe si le pod template a changé entre temps ?

Copy link
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.

j'ai mis quelques remarques pour handleApply en diagonale

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="Dupliquer" tal:attributes="src string: ${view/portal_url}/++resource++imio.dms.mail/copy.svg;"/>
Copy link
Member

Choose a reason for hiding this comment

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

on fera plus court dans la traduction mais au moins c'est plus explicite

# duplicated_mail.mail_date = None
original_mail = self.context
i = 0
while True:
Copy link
Member

Choose a reason for hiding this comment

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

c'est préférable de gérer l'id indépendamment de tester la création qui est "lourde".
Tu peux utiliser la méthode get_correct_id de imio.helpers.transmogrifier en repartant de l'id de l'objet dupliqué.

else:
break

if data['keep_category'] and hasattr(original_mail, 'classification_categories') and original_mail.classification_categories:
Copy link
Member

Choose a reason for hiding this comment

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

comme tout n'est plus forcément dans data, essaie data.get(xxx, False)

duplicated_mail.reply_to = original_mail.reply_to[:]

if data['keep_dms_files']:
# FIXME do not use clipboard
Copy link
Member

Choose a reason for hiding this comment

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

de fait ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Je ne comprends pas très bien comment regénérer les fichiers DMS. Je ne trouve pas les informations concernant le modèle utilisé. Si le modèle a changé entre temps, ça ne fonctionne pas. Si l'utilisateur a changé le contenu du fichier, ça ne fonctionne pas non plus

Copy link
Member

@sgeulette sgeulette Aug 19, 2025

Choose a reason for hiding this comment

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

Pour info, quand on génère depuis un modèle, on stocke dans une annotation du fichier généré les infos sur le modèle (documentgenerator)...
En fait pour les fichiers (ged), il y aura toujours une problématique.
Si on reprend l'original, on va reprendre un CB incorrect que l'utilisateur ne saura pas changer.
Si le destinataire n'est pas le même ce n'est pas bon non plus.
Au mieux, on pourrait dupliquer la fiche et après édition d'une duplication, regénérer le même document avec les infos de la fiche qui viennent d'être adaptée.
Mais pour le corps de texte, cela reste problématique car il se trouve uniquement dans l'odt original...
Je pense que c'est le mieux qu'on puisse faire...

@chris-adam chris-adam force-pushed the DMS-888/duplicate_omail branch from 70719dd to 2cec978 Compare August 22, 2025 14:44
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