moinwiki / moin-1.9

MoinMoin Wiki (1.9, also: 1.5a ... 1.8), stable, for production wikis
https://moinmo.in/
Other
140 stars 51 forks source link

ability to send full page on subscribed change instead of diff for emailnotify #75

Closed bernhardreiter closed 3 years ago

bernhardreiter commented 3 years ago

Currently a diff is send to users subscribing to a PageChangedEvent.

Some users want to get the full page text emailed along with the diff, to easier read the updated text in full.

Ideally a user could chose to have the change be send as diff or full email or both.

(Opening this feature request here, because this is the place were new feature request should go according to http://moinmo.in/FeatureRequests. And I did not find anything seaching the wiki and here about an existing request or patch.)

bernhardreiter commented 3 years ago

Code where the diff is created is in MoinMoin/events/emailnotify.py

bernhardreiter commented 3 years ago

The following patch will send the full raw body of the page added to the bottom of the notification email (always):

--- a/MoinMoin/events/emailnotify.py
+++ b/MoinMoin/events/emailnotify.py
@@ -43,2 +43,5 @@ def prep_page_changed_mail(request, page, comment, email_lang, revisions,
     diff = change['diff']
+    diff += "\n\n== Full page text:\n"
+    diff += page.get_raw_body()
+
     subject = change['subject']
bernhardreiter commented 3 years ago

Here is the full patch, enabling to see the email to be send in the debugging output. (Against current head of today 6fd29487892237b370a89a19a12887c7761c9d0d )

diff --git a/MoinMoin/events/emailnotify.py b/MoinMoin/events/emailnotify.py
index 28f4d4b1..80621d87 100644
--- a/MoinMoin/events/emailnotify.py
+++ b/MoinMoin/events/emailnotify.py
@@ -10,2 +10,4 @@
 """
+from MoinMoin import log
+logging = log.getLogger(__name__)

@@ -43,2 +45,5 @@ def prep_page_changed_mail(request, page, comment, email_lang, revisions,
     diff = change['diff']
+    diff += "\n\n== Full page text:\n"
+    diff += page.get_raw_body()
+
     subject = change['subject']
@@ -82,4 +87,5 @@ def send_notification(request, from_address, emails, data):
     """
-    return sendmail.sendmail(request, emails, data['subject'], data['text'], mail_from=from_address)
-
+    logging.info("Would do sendmail.sendmail() to {!r} subject: {!r} data: {} ".format(emails, data['subject'], data['text']))
+    return 0
+    #return sendmail.sendmail(request, emails, data['subject'], data['text'], mail_from=from_address)

@@ -93,2 +99,3 @@ def handle_page_changed(event):
     """
+    logging.info("handle_page_changed")
     comment = event.comment
@@ -106,2 +113,3 @@ def handle_page_changed(event):
     subscribers = page.getSubscribers(request, return_users=1)
+    logging.info("subscriber: {!r} ".format(subscribers))
     mail_from = page.cfg.mail_from
@@ -191,4 +199,5 @@ def handle(event):

-    if not event.request.cfg.mail_enabled:
-        return
+    ## for develop
+    #if not event.request.cfg.mail_enabled:
+    #    return
bernhardreiter commented 3 years ago

A better version would be to make if configurable. To do so, it would need a branch per user configuration in def handle_page_changed(event) because some users want it and some others don't. Also a way to see and change the setting.

Even better would be to enabled it per user and page, but this would need even more configuration interfacing. :)

bernhardreiter commented 3 years ago

For our workflow it be cool, if the feature could be switched off by wikiconfig_local.py.

And we use a stronger separator like

#################
# Full page text:
#################

not implemented alternatives

Using MIME would be major addition to the current code (should be something for Moinmoin 2.0).

Using -- as separator would hitch-hike on the https://en.wikipedia.org/wiki/Signature_block#Standard_delimiter for signatures and the handling of these are very different in mail user agents. So we'd better not weaken this convention.

bernhardreiter commented 3 years ago

The following patch version makes it more obvious where the diff ends and adds a configuration item to turn on the feature in wikiconfig_local.py

from wikiconfig import LocalConfig

class Config(LocalConfig):
   mail_notification_attach_full_page = True
diff --git a/MoinMoin/events/emailnotify.py b/MoinMoin/events/emailnotify.py
index 28f4d4b1..8eb5618c 100644
--- a/MoinMoin/events/emailnotify.py
+++ b/MoinMoin/events/emailnotify.py
@@ -41,6 +41,11 @@ def prep_page_changed_mail(request, page, comment, email_lang, revisions,
     cfg = request.cfg
     intro = change['text']
     diff = change['diff']
+    if hasattr(cfg, "mail_notification_attach_full_page"
+              ) and cfg.mail_notification_attach_full_page:
+        diff += "\n\n" + "#"*72 + "\n# Full page text:\n" + "#"*72 + "\n"
+        diff += page.get_raw_body()
+
     subject = change['subject']

     if change.has_key('comment'):
mushadow commented 3 years ago

Similar to this, I would like to completely remove any page information from displaying in the subscribe email. This could be another option for an admin to enable. The reason being that some information on the pages might be sensitive and the organization does not want the page information being sent over unencrypted email.

bernhardreiter commented 3 years ago

@mushadow this would better go into a new issue. :)

On the other hand, it seems possible already, just change the notification templates https://github.com/moinwiki/moin-1.9/blob/6fd29487892237b370a89a19a12887c7761c9d0d/docs/CHANGES#L300-L306 (make sure to read the full feature description to change all notification types).

bernhardreiter commented 3 years ago

The patch https://github.com/moinwiki/moin-1.9/issues/75#issuecomment-779273759 works for us - so good enough here, better versions should be targetting moinmoin2.