You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@roller.apache.org by Glen Mazza <gl...@gmail.com> on 2013/04/06 15:11:50 UTC
Uh Oh... (Re: svn commit: r1465236 - /roller/trunk/weblogger-business/src/main/java/org/apache/roller/weblogger/util/MailUtil.java)
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();
>
>
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