Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Using the variable (http://www.drupal.org/project/variable) module within comment_notify would allow you to:
- Reduce the size of the code using built-in function of the variable module
- Allow users to change the mail's subjects
- Internationalization using the i18n module
I will provide a patch to include variable declaration in the module...
Comment | File | Size | Author |
---|---|---|---|
#22 | translate-comment-notify-1201326-22.patch | 2.62 KB | sano |
#15 | comment_notify-translation-steps-1201326-15.patch | 1.8 KB | juampynr |
#3 | variable_declaration-1201326-3.patch | 26.66 KB | guillaumev |
#2 | variable_declaration-1201326-2.patch | 26.64 KB | guillaumev |
#1 | variable_declaration-1201326-1.patch | 19.01 KB | guillaumev |
Comments
Comment #1
guillaumev CreditAttribution: guillaumev commentedThe patch was tested but obviously some more testing is welcome :-)
Comment #2
guillaumev CreditAttribution: guillaumev commentedThis new patch includes the changes in patch 1 and changes in patch #47 of issue 952072
Comment #3
guillaumev CreditAttribution: guillaumev commentedForgot to delete old variables in .install file...
Comment #4
gregglesI'm concerned about adding a dependency on this since it is not very popular and would make the installation process more complex for the average user.
That said, I appreciate the benefits you laid out and of course am happy to reduce code.
Comment #5
Dave ReidYeah this can and should be done without the dependency.
Comment #6
gregglesThat would remove one of the main reasons to do it: reducing code.
Hmmmm :/
Comment #7
guillaumev CreditAttribution: guillaumev commentedHi,
Is adding a dependency really making the process more complex for the average user ? I think if someone is capable of installing the comment_notify module, he/she should be capable of installing a dependency...
Also remember that on any multilingual Drupal 7 website, you will have variable available, since it is a requirement of the i18n module...
Comment #8
gregglesLet's compare the installation instructions:
1. Download comment_notify
2. Untar it
3. Copy it to the right place
4. Read the installation instructions
5. Enable it
1. Download comment_notify
2. Untar it
3. Copy it to the right place
4. Read the installation instructions
5. Download something else
6. Untar it
7. Copy it to the right place
8. Read the installation instructions
9. Enable both
Yes, quite a bit more complex and I'm leaving out a lot of steps. I'm sure installing a module including dependencies is easy for you, but I'm concerned about keeping this module "lightweight" (which is one of the stated goals) and adding a dependency reduces the lightweight nature.
Comment #9
guillaumev CreditAttribution: guillaumev commentedUp to you, you're the module maintainer... :-)
But I'm pretty sure there are other users of comment_notify who would like to change the mail's subjects and have i18n integrated...
Comment #11
gregglesMarking "works as designed" until/unless this can be done as Dave says:
I agree it would be awesome to let people modify the mail subject and have i18n more integrated, but don't want to add a dependency.
Comment #12
juampynr CreditAttribution: juampynr commentedHow about using hook_variable_info() as described at http://drupal.org/node/1113374?
Comment #13
juampynr CreditAttribution: juampynr commentedDoh! hook_variable_info() is part of variable module. Please ignore this.
I have seen that the template is passed through t() at the following piece of the module's source code:
So I guess that the approach is to send a test email so the template gets into the core translation system so then it can be translated using the translation interface. Is this correct? If so, adding a mention to this at the INSTALL.txt or at the module's documentation page would be helpful for module users.
Comment #14
gregglesIf there is some way for comment_notify to work even with variable disabled and have added functionality for people who use the variable module then that seems great to me.
Yes, that's correct and I'd love a patch to do that.
Comment #15
juampynr CreditAttribution: juampynr commentedHere it is.
Comment #16
juampynr CreditAttribution: juampynr commentedMaybe it is simply easier to add the default templates to the Translation system automatically.
Is it OK to use the translation API to see if the default templates exist and, if not (and Locale module is enabled), add them? If so, let me know and I will work out a patch.
Comment #17
gregglesAwesome, thanks juampy - committed at http://drupalcode.org/project/comment_notify.git/commit/7207254
Comment #19
Pafla CreditAttribution: Pafla commentedHi
If i do this https://www.drupal.org/files/comment_notify-translation-steps-1201326-15...
I can translate:
1 subject can be translated
1 body can be translated
But there are 2 subject & 2 body
How to fix that?
Pafla
Comment #20
drupalfan81 CreditAttribution: drupalfan81 commentedThanks for the great work! Giving this a try tonight! I agree that having an email with the body translated but not the subject is kind of a deal breaker to send out to users. It would be like a user receiving an email from their english website with the subject line in Japanese and then the body in English. Looks bad.
Comment #21
Pafla CreditAttribution: Pafla commentedFor not to be mistaken.
Notification for Author:
Subject can be translated
Body can be translated
Notification for commenter:
Subject can NOT be translated
Body can NOT be translated
In "comment_notify.inc" Line 320
$variables['watcher_subject'] = '[site:name] :: new comment on [comment:node:title]';
to
$variables['watcher_subject'] = t('[site:name] :: new comment on [comment:node:title]');
Then Subject for commenter can be translated
For subject I suggest this:
Line 314 "$variables['author_subject'] = t('[site:name] :: new comment for your post.');"
To "$variables['author_subject'] = t('[site:name] :: New comment on [comment:node:title]');"
Line 320 "$variables['watcher_subject'] = '[site:name] :: new comment on [comment:node:title]';"
To "$variables['watcher_subject'] = t('[site:name] :: New comment on [comment:node:title]');"
But still not possible to translate body for commenter
Any solution?
Pafla
Comment #22
sano CreditAttribution: sano as a volunteer commentedattached patch should fix all issues mentioned by @Pafla.
Comment #23
sano CreditAttribution: sano as a volunteer commentedsee a related issue https://www.drupal.org/project/comment_notify/issues/3004933.
That issue contains the same patch as the one I attached above.