[Seaside-dev] WAEmailMessage begets WAMultiPartEmailMessage

[Johan Brichau]

Hi Paul,

Did you have a chance to commit these changes?
The repository, I only see your earlier package version.

Thank you!
Johan

On 07 Nov 2013, at 01:30, Paul DeBruicker <pdebruic at gmail.com> wrote:

> Hi Philippe,
> 
> I finally had time to get back to this.  Thanks for the review.  Comments below.
> 
> 
> On Nov 3, 2013, at 12:53 PM, Philippe Marschall <philippe.marschall at gmail.com> wrote:
> 
>> On Sat, Oct 26, 2013 at 2:16 PM, Philippe Marschall
>> <philippe.marschall at gmail.com> wrote:
>>> On Fri, Oct 25, 2013 at 10:53 PM, Paul DeBruicker <pdebruic at gmail.com> wrote:
>>>> I put the packages here:
>>>> 
>>>> 
>>>> MCHttpRepository
>>>>       location: 'http://smalltalkhub.com/mc/Seaside/Seaside30/main'
>>>>       user: ''
>>>>       password: ''
>>> 
>>> Thank you, I'll have a look.
>> 
>> First, thank you again. I'm going to be a bit picky, that's not
>> because your contribution isn't welcome.
>> - Can you use GRPlatfrom current newRandom instead of Random new?
>> (Slime should have picked that one up)
> 
> fixed 
> 
>> - Can you name MyRandom something else? Like BoundaryRandomGenerator
>> or something similar?
> 
> Fixed.  MyRandom was left over from an earlier experiment in generating a random boundary.  I removed it. 
> 
>> - Wan you use WAMimeType instead of Strings?
> 
> Fixed. 
> 
>> - Why does WAEmailBody implement #= and #<=?
>> - Why do you sort the parts?
> 
> I would not be surprised to learn I was wrong about _needing_ to do this but here's why I did: 
> 
> I was following this spec (http://www.w3.org/Protocols/rfc1341/7_2_Multipart.html).  I chose to use the 'multipart/alternative' subtype described in heading 7.2.3 of the spec linked above.  It recommends sorting the parts from plainest to 'richest'.  So thats why I implemented #<= and sort the parts.  I just assume that  when using 'multipart/alternative' it may be problematic for some clients to process emails that have more than one part with the same type, so I use a Set to store the parts and implement #=.
> 
> At some point (maybe now?) WAMultiPartEmailMessage should be changed so the primary Content-Type: of the message is multipart/mixed and any text/html email bodies are included in a subpart of Content-Type: multipart/alternative. That would allow  other useful parts to be included (e.g. attachments, .ics files, etc) in the main message.  Right now, i think,  there is no way to add an attachment alongside the readable email body.
> 
> 
> 
> 
>> - You hard code charset=UTF-8 (as a String) you I don't see you doing
>> any encoding anywhere.
> 
> I was just aping other implementations. But if you leave it off or change it to charset=us-ascii then the email is not processed by Mail.app as a multipart email.  That may be because I'm using multipart/alternative as the main Content-Type: of the email rather than multipart/mixed.  I'll check it out.  
> 
> 
> 
>> - I wonder whether it would make sense to have a WASimpleEmailMessage
>> (subclass of WAEmailMessage) where the body is just a String and get
>> rid of WAStringEmailBody is this case (but we would actually need an
>> encoding in that case as well). Or combine WAMultiPartEmailMessage and
>> WAEmailMessage into a single class.
> 
> Combining them may not be too bad, but the Multipart implementation right now is simple and doesn't include attachments as described above.  I'm not sure whether to combine them now or wait and have a look once the other multipart parts work .  
> 
>> - Can we get rid of #htmlBodyString and #plainTextBodyString and
>> instead delegate more to the parts?
> 
> I moved them to be extensions to the PostMark-Seaside package as that's where I'm using them.  http://postmarkapp.com a service that provides mail deliverability and wants emails in JSON format.  
> 
> 
>> - Can we find a better replacement for the #string selector?
>> 
> 
> I renamed it to #contentString
> 
>> Cheers
>> Philippe
>> _______________________________________________
>> seaside-dev mailing list
>> seaside-dev at lists.squeakfoundation.org
>> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev
> 
> 
> Thanks and let me know what else should get fixed next.  Do you want me to put this revised version somewhere?
> 
> Paul_______________________________________________
> seaside-dev mailing list
> seaside-dev at lists.squeakfoundation.org
> http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev