OFBiz › OFBiz - Dev

[jira] Created: (OFBIZ-435) Re-factor and improve email sending to contact lists

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

11 messages Options

Nicolas Malin (Jira)

[jira] Created: (OFBIZ-435) Re-factor and improve email sending to contact lists

Re-factor and improve email sending to contact lists
----------------------------------------------------

Key: OFBIZ-435
URL: http://issues.apache.org/jira/browse/OFBIZ-435
Project: OFBiz (The Open for Business Project)
Issue Type: Improvement
Components: marketing
Reporter: Si Chen

Re-factor sending of email to contact lists in the following way:

1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".

2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.

3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force

4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")

5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.

6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

Nicolas Malin (Jira)

[jira] Assigned: (OFBIZ-435) Re-factor and improve email sending to contact lists

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=all ]

Si Chen reassigned OFBIZ-435:
-----------------------------

Assignee: Si Chen

> Re-factor and improve email sending to contact lists
> ----------------------------------------------------
>
> Key: OFBIZ-435
> URL: http://issues.apache.org/jira/browse/OFBIZ-435
> Project: OFBiz (The Open for Business Project)
> Issue Type: Improvement
> Components: marketing
> Reporter: Si Chen
> Assigned To: Si Chen
>
> Re-factor sending of email to contact lists in the following way:
> 1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".
> 2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.
> 3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force
> 4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")
> 5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.
> 6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.

--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

Nicolas Malin (Jira)

[jira] Commented: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=comments#action_12448208 ]

Si Chen commented on OFBIZ-435:
-------------------------------

Thinking about it further, I think ContactListCommEventStatus should use a statusId field rather than just a processedTimestamp to denote if something has shipped.

Also, I think instead of adding a logSentCommEvents field, maybe it's better to make it so that comm events to contact lists of the "Newsletter" type are not logged.

Nicolas Malin (Jira)

[jira] Commented: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=comments#action_12449001 ]

Jacques Le Roux commented on OFBIZ-435:
---------------------------------------

Si,

Apart that in 3) I believe it's : "UtilValidate.isEmail(infoString, true)", I think that your proposition is really something that should be in OFBiz !

Nicolas Malin (Jira)

[jira] Updated: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=all ]

Chris Liberty updated OFBIZ-435:
--------------------------------

Attachment: OFBIZ-435_1.patch.zip

Patch is attached.

> Re-factor and improve email sending to contact lists
> ----------------------------------------------------
>
> Key: OFBIZ-435
> URL: http://issues.apache.org/jira/browse/OFBIZ-435
> Project: OFBiz (The Open for Business Project)
> Issue Type: Improvement
> Components: marketing
> Reporter: Si Chen
> Assigned To: Si Chen
> Attachments: OFBIZ-435_1.patch.zip
>
>
> Re-factor sending of email to contact lists in the following way:
> 1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".
> 2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.
> 3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force
> 4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")
> 5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.
> 6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.

Nicolas Malin (Jira)

[jira] Commented: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=comments#action_12451860 ]

Si Chen commented on OFBIZ-435:
-------------------------------

This is most of what I had in mind, except:

1. This line:
+ sendMailParams.put("communicationEventId", communicationEventId);
way at the top block would always put in a communicationEventId, and I think it would make this block moot:
+ if (! contactList.getString("contactListTypeId").equals("NEWSLETTER")) {
+ sendMailParams.put("communicationEventId", communicationEventId);
+ }

Also, shouldn't ! be right before the "c"? Or does that not matter?

2. With contactListCommStatusRecord.getString("statusId").equals("COM_COMPLETE"), it is better to write as "COM_COMPLETE".equals(...getString('statusId")) in case there are any null values. Otherwise sooner or later you'd get an NPE

This actually brings up a question: should this service fail when some of the emails fail? If so, running it as async would cause it to repeat itself over and over again if some of the email addresses could not be sent.

I think the idea is sound - IN_PROGRESS emails get sent over and over again, but there should be a max-retry on the service so eventually we'd give up rather than blow up the server?

3. David -- are you OK with the UtilValidate changes?

> Re-factor and improve email sending to contact lists
> ----------------------------------------------------
>
> Key: OFBIZ-435
> URL: http://issues.apache.org/jira/browse/OFBIZ-435
> Project: OFBiz (The Open for Business Project)
> Issue Type: Improvement
> Components: marketing
> Reporter: Si Chen
> Assigned To: Si Chen
> Attachments: OFBIZ-435_1.patch.zip
>
>
> Re-factor sending of email to contact lists in the following way:
> 1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".
> 2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.
> 3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force
> 4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")
> 5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.
> 6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.

Scott Gray

Re: [jira] Commented: (OFBIZ-435) Re-factor and improve email sending to contact lists

Si Chen (JIRA) wrote:
> 2. With contactListCommStatusRecord.getString("statusId").equals("COM_COMPLETE"), it is better to write as "COM_COMPLETE".equals(...getString('statusId")) in case there are any null values. Otherwise sooner or later you'd get an NPE
>
Hi Si

I did a quick search for .equals(" and there are 812 occurrences in the
code, is this something we need to tidy up at some point?

Regards
Scott

Si Chen-2

Re: [jira] Commented: (OFBIZ-435) Re-factor and improve email sending to contact lists

Yeah, generally I think so, and maybe make a note of it somewhere
prominent for new contributors.

On Nov 21, 2006, at 10:19 PM, Scott Gray wrote:

> Si Chen (JIRA) wrote:
>> 2. With contactListCommStatusRecord.getString("statusId").equals
>> ("COM_COMPLETE"), it is better to write as "COM_COMPLETE".equals
>> (...getString('statusId")) in case there are any null values.
>> Otherwise sooner or later you'd get an NPE
>>
> Hi Si
>
> I did a quick search for .equals(" and there are 812 occurrences in
> the code, is this something we need to tidy up at some point?
>
> Regards
> Scott

Best Regards,

Si
[hidden email]

Nicolas Malin (Jira)

[jira] Updated: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=all ]

Chris Liberty updated OFBIZ-435:
--------------------------------

Attachment: OFBIZ-435_v2.patch.zip

Si,

1 - Quite right, thanks for catching that. Patch v2 is attached which resolves that issue.

2 - Whitespace makes no difference unless we're working in Python :)

> Re-factor and improve email sending to contact lists
> ----------------------------------------------------
>
> Key: OFBIZ-435
> URL: http://issues.apache.org/jira/browse/OFBIZ-435
> Project: OFBiz (The Open for Business Project)
> Issue Type: Improvement
> Components: marketing
> Reporter: Si Chen
> Assigned To: Si Chen
> Attachments: OFBIZ-435_1.patch.zip, OFBIZ-435_v2.patch.zip
>
>
> Re-factor sending of email to contact lists in the following way:
> 1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".
> 2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.
> 3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force
> 4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")
> 5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.
> 6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.

Nicolas Malin (Jira)

[jira] Commented: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=comments#action_12452355 ]

Chris Liberty commented on OFBIZ-435:
-------------------------------------

Oops, missed your 2nd point:

the NPE check is accomplished by the first evaluation in the else if - the full line is:

} else if (contactListCommStatusRecord.get("statusId") != null && contactListCommStatusRecord.getString("statusId").equals("COM_COMPLETE")) {

but your notation is definitely more concise and this can be changed if you wish.

> Re-factor and improve email sending to contact lists
> ----------------------------------------------------
>
> Key: OFBIZ-435
> URL: http://issues.apache.org/jira/browse/OFBIZ-435
> Project: OFBiz (The Open for Business Project)
> Issue Type: Improvement
> Components: marketing
> Reporter: Si Chen
> Assigned To: Si Chen
> Attachments: OFBIZ-435_1.patch.zip, OFBIZ-435_v2.patch.zip
>
>
> Re-factor sending of email to contact lists in the following way:
> 1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".
> 2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.
> 3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force
> 4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")
> 5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.
> 6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.

Nicolas Malin (Jira)

[jira] Closed: (OFBIZ-435) Re-factor and improve email sending to contact lists

In reply to this post by Nicolas Malin (Jira)

[ http://issues.apache.org/jira/browse/OFBIZ-435?page=all ]

Si Chen closed OFBIZ-435.
-------------------------

Resolution: Fixed

r 478902.

> Re-factor and improve email sending to contact lists
> ----------------------------------------------------
>
> Key: OFBIZ-435
> URL: http://issues.apache.org/jira/browse/OFBIZ-435
> Project: OFBiz (The Open for Business Project)
> Issue Type: Improvement
> Components: marketing
> Reporter: Si Chen
> Assigned To: Si Chen
> Attachments: OFBIZ-435_1.patch.zip, OFBIZ-435_v2.patch.zip
>
>
> Re-factor sending of email to contact lists in the following way:
> 1. A separate service will be used to send comm event when there is a contactListId on the commevent and the commevent type is "Email".
> 2. The new sendEmailToContactList will not use a global transaction but rather will use a separate transaction for each address on the contact list. If there is a service error from any particular sendMail, it will skip that email and move on.
> 3. UtilValidate.isEmail will be re-factored into two methods, isEmail(String email) which just calls isEmail(email, false) and isEmail(String email, boolean requireDot) which is the current isEmail code, except that the block for checking "." will be turned on if requireDot is true. where requireDot will force
> 4. Each email will be cleaned up with a .trim() first and then validated using UtilValidate.isEmail(infoString, ".")
> 5. A new entity ContactListCommStatus will be created with keys contactListId, commEventId, contactMechId, processedTimestamp to record when an email has been sent to a contactMechId. Before any email is sent, we will check if there is a not null processedTimestamp for this email address (contactMechId) for the commEventId and contactListId. If so, then it will no longer be sent. After an email is sent, a record (by direct delegator.create to speed things up) will be created in ContactListCommStatus. This will make contact lists "transactional" in the sense that it can recover in the middle of a list.
> 6. A new field ContactList.logSentCommEvents (indicator) will be created. If it is set to "N", then communicationEventId will be passed into the sendMail service so that the storeCommEventAsEmail service will not be triggered. This will prevent mass marketing emails from being stored for all recipients and clogging up the database, while still allowing important emails (recall notifications, etc.) to be stored.