You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@roller.apache.org by sn...@apache.org on 2013/04/06 15:02:45 UTC
svn commit: r1465236 -
/roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java
Author: snoopdave
Date: Sat Apr 6 13:02:44 2013
New Revision: 1465236
URL: http://svn.apache.org/r1465236
Log:
Fix for https://issues.apache.org/jira/browse/ROL-1926
Comment notifcation emails being sent before comment approval
Modified:
roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java
Modified: roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java
URL: http://svn.apache.org/viewvc/roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java?rev=1465236&r1=1465235&r2=1465236&view=diff
==============================================================================
--- roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java (original)
+++ roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java Sat Apr 6 13:02:44 2013
@@ -294,31 +294,35 @@ public class MailUtil {
// build list of email addresses to send notification to
Set subscribers = new TreeSet();
- // If we are to notify subscribers, then...
- if (notifySubscribers) {
- log.debug("Sending notification email to all subscribers");
-
- // Get all the subscribers to this comment thread
- List comments = entry.getComments(true, true);
- for (Iterator it = comments.iterator(); it.hasNext();) {
- WeblogEntryComment comment = (WeblogEntryComment) it.next();
- if (!StringUtils.isEmpty(comment.getEmail())) {
- // If user has commented twice,
- // count the most recent notify setting
- if (comment.getNotify().booleanValue()) {
- // only add those with valid email
- if (comment.getEmail().matches(EMAIL_ADDR_REGEXP)) {
- subscribers.add(comment.getEmail());
- }
- } else {
- // remove user who doesn't want to be notified
- subscribers.remove(comment.getEmail());
- }
- }
- }
- } else {
- log.debug("Sending notification email only to weblog owner");
- }
+ // If we are to notify subscribers, then...
+ if (notifySubscribers) {
+ log.debug("Sending notification email to all subscribers");
+
+ // Get all the subscribers to this comment thread
+ List comments = entry.getComments(true, true);
+ for (Iterator it = comments.iterator(); it.hasNext();) {
+ WeblogEntryComment comment = (WeblogEntryComment) it.next();
+ if (!StringUtils.isEmpty(comment.getEmail())) {
+ // If user has commented twice,
+ // count the most recent notify setting
+ if (commentObject.getApproved()) {
+ if (comment.getNotify().booleanValue()) {
+ // only add those with valid email
+ if (comment.getEmail().matches(EMAIL_ADDR_REGEXP)) {
+ log.info("Add to subscribers list : " + comment.getEmail());
+ subscribers.add(comment.getEmail());
+ }
+ } else {
+ // remove user who doesn't want to be notified
+ log.info("Remove from subscribers list : " + comment.getEmail());
+ subscribers.remove(comment.getEmail());
+ }
+ }
+ }
+ }
+ } else {
+ log.debug("Sending notification email only to weblog owner");
+ }
// Form array of commenter addrs
String[] commenterAddrs = (String[])subscribers.toArray(new String[0]);
@@ -357,7 +361,8 @@ public class MailUtil {
msg.append((escapeHtml) ? "\n" : "<br />");
// Build link back to comment
- String commentURL = WebloggerFactory.getWeblogger().getUrlStrategy().getWeblogCommentsURL(weblog, null, entry.getAnchor(), true);
+ String commentURL = WebloggerFactory.getWeblogger()
+ .getUrlStrategy().getWeblogCommentsURL(weblog, null, entry.getAnchor(), true);
if (escapeHtml) {
msg.append(commentURL);
@@ -377,7 +382,8 @@ public class MailUtil {
for (Iterator it = messages.getMessages(); it.hasNext();) {
RollerMessage rollerMessage = (RollerMessage)it.next();
ownermsg.append((escapeHtml) ? "" : "<li>");
- ownermsg.append(MessageFormat.format(resources.getString(rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
+ ownermsg.append(MessageFormat.format(resources.getString(
+ rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
ownermsg.append((escapeHtml) ? "\n\n" : "</li>");
}
if (messages.getMessageCount() > 0) {
@@ -394,7 +400,8 @@ public class MailUtil {
for (Iterator it = messages.getErrors(); it.hasNext();) {
RollerMessage rollerMessage = (RollerMessage)it.next();
ownermsg.append((escapeHtml) ? "" : "<li>");
- ownermsg.append(MessageFormat.format(resources.getString(rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
+ ownermsg.append(MessageFormat.format(resources.getString(
+ rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
ownermsg.append((escapeHtml) ? "\n\n" : "</li>");
}
if (messages.getErrorCount() > 0) {
@@ -542,7 +549,8 @@ public class MailUtil {
StringBuffer msg = new StringBuffer();
msg.append(resources.getString("email.comment.commentApproved"));
msg.append("\n\n");
- msg.append(WebloggerFactory.getWeblogger().getUrlStrategy().getWeblogCommentsURL(weblog, null, entry.getAnchor(), true));
+ msg.append(WebloggerFactory.getWeblogger().getUrlStrategy()
+ .getWeblogCommentsURL(weblog, null, entry.getAnchor(), true));
// send message to author of approved comment
try {
@@ -663,7 +671,8 @@ public class MailUtil {
// Extract the remaining potentially good addresses
remainingAddresses=ex.getValidUnsentAddresses();
}
- } while (remainingAddresses!=null && remainingAddresses.length>0 && remainingAddresses.length!=nAddresses);
+ } while (remainingAddresses!=null && remainingAddresses.length>0
+ && remainingAddresses.length!=nAddresses);
} finally {
transport.close();
Re: Uh Oh... (Re: svn commit: r1465236 - /roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java)
Posted by Dave <sn...@gmail.com>.
Yeah, my commit did not fix the problem. I just committed a new fix that
replaces all tabs with spaces and I switched my Roller Netbeans projects
over to use "project specific settings" and 4-spaces instead of tabs.
- Dave
On Sat, Apr 6, 2013 at 9:37 AM, Glen Mazza <gl...@gmail.com> wrote:
> At least in Eclipse, if you create a separate workspace for
> space-delimited Apache projects, you can dictate spaces for tabs in that
> particular workspace. (I can provide more config info if needed.)
>
> I'm not sure your commit fixed that problem -- it just shows one line
> being changed, but according to the svn commit mail the entire "if
> (notifySubscribers) {" block is tab-delimited.
>
> Glen
>
>
> On 04/06/2013 09:33 AM, Dave wrote:
>
>> On Sat, Apr 6, 2013 at 9:11 AM, Glen Mazza <gl...@gmail.com> wrote:
>>
>> Dave, please update your IDE (and fix this commit) to replace tabs with
>>> four spaces...the spacing below in your latest change indicates actual
>>> tab
>>> characters are being used.
>>>
>>> Yuck, tabs! Unfortunately, we use tabs where I work. Just committed a
>> fix
>> for this.
>>
>> - Dave
>>
>>
>
Re: Uh Oh... (Re: svn commit: r1465236 - /roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java)
Posted by Glen Mazza <gl...@gmail.com>.
At least in Eclipse, if you create a separate workspace for
space-delimited Apache projects, you can dictate spaces for tabs in that
particular workspace. (I can provide more config info if needed.)
I'm not sure your commit fixed that problem -- it just shows one line
being changed, but according to the svn commit mail the entire "if
(notifySubscribers) {" block is tab-delimited.
Glen
On 04/06/2013 09:33 AM, Dave wrote:
> On Sat, Apr 6, 2013 at 9:11 AM, Glen Mazza <gl...@gmail.com> wrote:
>
>> Dave, please update your IDE (and fix this commit) to replace tabs with
>> four spaces...the spacing below in your latest change indicates actual tab
>> characters are being used.
>>
> Yuck, tabs! Unfortunately, we use tabs where I work. Just committed a fix
> for this.
>
> - Dave
>
Re: Uh Oh... (Re: svn commit: r1465236 - /roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java)
Posted by Dave <sn...@gmail.com>.
On Sat, Apr 6, 2013 at 9:11 AM, Glen Mazza <gl...@gmail.com> wrote:
> Dave, please update your IDE (and fix this commit) to replace tabs with
> four spaces...the spacing below in your latest change indicates actual tab
> characters are being used.
>
Yuck, tabs! Unfortunately, we use tabs where I work. Just committed a fix
for this.
- Dave
Uh Oh... (Re: svn commit: r1465236 - /roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java)
Posted by Glen Mazza <gl...@gmail.com>.
Dave, please update your IDE (and fix this commit) to replace tabs with
four spaces...the spacing below in your latest change indicates actual
tab characters are being used.
Glen
On 04/06/2013 09:02 AM, snoopdave@apache.org wrote:
> Author: snoopdave
> Date: Sat Apr 6 13:02:44 2013
> New Revision: 1465236
>
> URL: http://svn.apache.org/r1465236
> Log:
> Fix for https://issues.apache.org/jira/browse/ROL-1926
> Comment notifcation emails being sent before comment approval
>
> Modified:
> roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java
>
> Modified: roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java
> URL: http://svn.apache.org/viewvc/roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java?rev=1465236&r1=1465235&r2=1465236&view=diff
> ==============================================================================
> --- roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java (original)
> +++ roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java Sat Apr 6 13:02:44 2013
> @@ -294,31 +294,35 @@ public class MailUtil {
> // build list of email addresses to send notification to
> Set subscribers = new TreeSet();
>
> - // If we are to notify subscribers, then...
> - if (notifySubscribers) {
> - log.debug("Sending notification email to all subscribers");
> -
> - // Get all the subscribers to this comment thread
> - List comments = entry.getComments(true, true);
> - for (Iterator it = comments.iterator(); it.hasNext();) {
> - WeblogEntryComment comment = (WeblogEntryComment) it.next();
> - if (!StringUtils.isEmpty(comment.getEmail())) {
> - // If user has commented twice,
> - // count the most recent notify setting
> - if (comment.getNotify().booleanValue()) {
> - // only add those with valid email
> - if (comment.getEmail().matches(EMAIL_ADDR_REGEXP)) {
> - subscribers.add(comment.getEmail());
> - }
> - } else {
> - // remove user who doesn't want to be notified
> - subscribers.remove(comment.getEmail());
> - }
> - }
> - }
> - } else {
> - log.debug("Sending notification email only to weblog owner");
> - }
> + // If we are to notify subscribers, then...
> + if (notifySubscribers) {
> + log.debug("Sending notification email to all subscribers");
> +
> + // Get all the subscribers to this comment thread
> + List comments = entry.getComments(true, true);
> + for (Iterator it = comments.iterator(); it.hasNext();) {
> + WeblogEntryComment comment = (WeblogEntryComment) it.next();
> + if (!StringUtils.isEmpty(comment.getEmail())) {
> + // If user has commented twice,
> + // count the most recent notify setting
> + if (commentObject.getApproved()) {
> + if (comment.getNotify().booleanValue()) {
> + // only add those with valid email
> + if (comment.getEmail().matches(EMAIL_ADDR_REGEXP)) {
> + log.info("Add to subscribers list : " + comment.getEmail());
> + subscribers.add(comment.getEmail());
> + }
> + } else {
> + // remove user who doesn't want to be notified
> + log.info("Remove from subscribers list : " + comment.getEmail());
> + subscribers.remove(comment.getEmail());
> + }
> + }
> + }
> + }
> + } else {
> + log.debug("Sending notification email only to weblog owner");
> + }
>
> // Form array of commenter addrs
> String[] commenterAddrs = (String[])subscribers.toArray(new String[0]);
> @@ -357,7 +361,8 @@ public class MailUtil {
> msg.append((escapeHtml) ? "\n" : "<br />");
>
> // Build link back to comment
> - String commentURL = WebloggerFactory.getWeblogger().getUrlStrategy().getWeblogCommentsURL(weblog, null, entry.getAnchor(), true);
> + String commentURL = WebloggerFactory.getWeblogger()
> + .getUrlStrategy().getWeblogCommentsURL(weblog, null, entry.getAnchor(), true);
>
> if (escapeHtml) {
> msg.append(commentURL);
> @@ -377,7 +382,8 @@ public class MailUtil {
> for (Iterator it = messages.getMessages(); it.hasNext();) {
> RollerMessage rollerMessage = (RollerMessage)it.next();
> ownermsg.append((escapeHtml) ? "" : "<li>");
> - ownermsg.append(MessageFormat.format(resources.getString(rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
> + ownermsg.append(MessageFormat.format(resources.getString(
> + rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
> ownermsg.append((escapeHtml) ? "\n\n" : "</li>");
> }
> if (messages.getMessageCount() > 0) {
> @@ -394,7 +400,8 @@ public class MailUtil {
> for (Iterator it = messages.getErrors(); it.hasNext();) {
> RollerMessage rollerMessage = (RollerMessage)it.next();
> ownermsg.append((escapeHtml) ? "" : "<li>");
> - ownermsg.append(MessageFormat.format(resources.getString(rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
> + ownermsg.append(MessageFormat.format(resources.getString(
> + rollerMessage.getKey()), (Object[])rollerMessage.getArgs()) );
> ownermsg.append((escapeHtml) ? "\n\n" : "</li>");
> }
> if (messages.getErrorCount() > 0) {
> @@ -542,7 +549,8 @@ public class MailUtil {
> StringBuffer msg = new StringBuffer();
> msg.append(resources.getString("email.comment.commentApproved"));
> msg.append("\n\n");
> - msg.append(WebloggerFactory.getWeblogger().getUrlStrategy().getWeblogCommentsURL(weblog, null, entry.getAnchor(), true));
> + msg.append(WebloggerFactory.getWeblogger().getUrlStrategy()
> + .getWeblogCommentsURL(weblog, null, entry.getAnchor(), true));
>
> // send message to author of approved comment
> try {
> @@ -663,7 +671,8 @@ public class MailUtil {
> // Extract the remaining potentially good addresses
> remainingAddresses=ex.getValidUnsentAddresses();
> }
> - } while (remainingAddresses!=null && remainingAddresses.length>0 && remainingAddresses.length!=nAddresses);
> + } while (remainingAddresses!=null && remainingAddresses.length>0
> + && remainingAddresses.length!=nAddresses);
>
> } finally {
> transport.close();
>
>