6
\$\begingroup\$

I am developing a script that will act as a surrogate of sorts for a web portal and an email blast program. The web portal is used to create web-to-print files and is very good at creating print-ready PDF files, but currently lacks any email blast or HTML output options for the end user. In order to circumvent this limitation, my script will parse data that is in an email that is automatically sent after each order is placed. In this email are variables from the site that user has submitted as well as images that the user has uploaded.

My code still needs to archive the emails after they have been zipped (to eliminate duplicate files) and send the zip file to the person who ordered the HTML. Despite not being 100%, I was hoping to get some good feedback/criticism on what I have so far. I am not brand new to Python, but I am self taught and figured this is the best place to learn how to refine my code.

Here is my current (expected) workflow:

  1. User logs into web portal to order.
  2. User uploads images, enters text into fields, and receives a proof of their "HTML".
  3. User completes order, checks out. Process is now out of user's hands.
  4. An email is generated and sent to me. This email includes all information necessary to create HTML markup.
  5. Script reads email, downloads attachments, assigns values to variables based on their "markup" in the email. Specific characters are used to identify text strings to be used.
  6. Script creates HTML markup using assigned variables, then adds .html file and all included images to a .zip file fit for distribution.
  7. Script archives email to prevent duplicate zip files being created on next run.
  8. An email is sent to the user with the zip file attached.
  9. User uploads zip file to their own specific email blast program.

Links to my code and supporting files are here:

import email, getpass, imaplib, os, re, csv, zipfile, glob, threading

detach_dir = 'directory' # directory where to save attachments
user = ("username")
pwd = ("password")

# connecting to the gmail imap server
m = imaplib.IMAP4_SSL("imap server")
m.login(user,pwd)
m.select("INBOX") # here you a can choose a mail box like INBOX instead
# use m.list() to get all the mailboxes

resp, items = m.search(None, "ALL") # you could filter using the IMAP rules here (check http://www.example-code.com/csharp/imap-search-critera.asp)
items = items[0].split() # getting the mails id

for emailid in items:
    resp, data = m.fetch(emailid, "(RFC822)") # fetching the mail, "`(RFC822)`" means "get the whole stuff", but you can ask for headers only, etc
    email_body = data[0][1] # getting the mail content
    mail = email.message_from_string(email_body) # parsing the mail content to get a mail object


    #Check if any attachments at all
    if mail.get_content_maintype() != 'multipart':
        continue

    # we use walk to create a generator so we can iterate on the parts and forget about the recursive headache
    for part in mail.walk():
        # each part is a either non-multipart, or another multipart message
        # that contains further parts... Message is organized like a tree
        if part.get_content_type() == 'text/plain':
            content = part.get_payload()
            message = re.compile(r'\%(.+?)\%', re.DOTALL).findall(content)
            message = re.sub(r'=\\r\\', '', str(message))
            message = re.sub(r'\[\'', '', str(message))
            message = re.sub(r'\'\]', '', str(message))
            token = re.compile(r'\$(.+?)\$', re.DOTALL).findall(content)
            token = re.sub(r'\[\'', '', str(token))
            token = re.sub(r'\'\]', '', str(token))
            tag = re.compile(r'\^(.+?)\^', re.DOTALL).findall(content)
            tag = re.sub(r'\[\'', '', str(tag))
            tag = re.sub(r'\'\]', '', str(tag))
            print message
            print token
            print tag
            #print part.get_payload() # prints the raw text
        # multipart are just containers, so we skip them
        if part.get_content_maintype() == 'multipart':
            continue

        # is this part an attachment ?
        if part.get('Content-Disposition') is None:
            continue

        filename = part.get_filename()
        counter = 1

        # if there is no filename, we create one with a counter to avoid duplicates
        if not filename:
            filename = 'part-%03d%s' % (counter, 'bin')
            counter += 1

        att_path = os.path.join(detach_dir, filename)

        #Check if its already there
        if not os.path.isfile(att_path) :
            # finally write the stuff
            fp = open(att_path, 'wb')
            fp.write(part.get_payload(decode=True))
            fp.close()

        path = detach_dir
        os.chdir(path)
        image1 = str(glob.glob('upload-photo1*'))
        image2 = str(glob.glob('upload-photo2*'))
        image3 = str(glob.glob('upload-photo3*'))
        image1 = re.sub(r'\[\'', '', image1)
        image1 = re.sub(r'\'\]', '', image1)
        image2 = re.sub(r'\[\'', '', image2)
        image2 = re.sub(r'\'\]', '', image2)
        image3 = re.sub(r'\[\'', '', image3)
        image3 = re.sub(r'\'\]', '', image3)
        htmlFile = str(token)+'.html'
        #if tag == 'email_blast_demo':
        htmlCode = ('''<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
                    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
                    <html xmlns="http://www.w3.org/1999/xhtml"><head>
                    <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
                    <title>Untitled Document</title></head><body>
                    <table width="554" border="0" cellspacing="0" cellpadding="0" align="center"><tr><td>
                    <img src="'''+image1+'''" width="554" height="186" /></td></tr><tr><td>
                    <p style="font-family:Arial, Helvetica, sans-serif; font-size:11pt; line-height:14pt;">
                    <br />Dear [Fld:FirstName],<br /><br />'''+str(message)+'''<br /><br /><a href="PLACEHOLDER">
                    <img src="'''+image2+'''" width="248" height="38" alt="Opt-in for men\'s health tips now" /></a>
                    <br /><br /><br /><img src="'''+image3+'''" width="167" height="62" align="right" /><br />
                    <p style="font-family:Arial, Helvetica, sans-serif; font-size:10pt;"></td></tr></table>
                    </body></html>''')
        htmlData = open(os.path.join('directory', htmlFile), 'w+')
        htmlData.write(htmlCode)
        print htmlFile+' Complete'
        htmlData.close()
    allFiles = [f for f in os.listdir(path) if not f.endswith('.zip')]
    for file in allFiles:
        archive = zipfile.ZipFile(token+'.zip', mode='a')
        archive.write(file)
        archive.close()
        os.unlink(file)


#  This script will access a set email account, parse the text and attachments of each email, create HTML markup
#  and zip the files together. This script assumes a set template for the HTML. I will most likely have to change
#  this in order to incorporate multiple templates. The HTML markup for each template will be sent in the email
#  and be parsed in the ame fashion as the `token` and `tag` variables above.
#
#  What still needs to be done:
#  1) Archive email after being zipped so that duplicates are not created
#  2) Email .zip file to requestor (person who ordered)
\$\endgroup\$
3
  • \$\begingroup\$ Just curious, is there a (possibly business) reason everything has to be done through email? It sounds like you're generating email templates for something like MailChimp or Campaign Monitor, in which case, why don't you just store the files on your server (or a third-party storage SaaS) rather than sending through email? This just seems like a roundabout way of doing something fairly simple. \$\endgroup\$
    – Johntron
    May 15, 2013 at 4:31
  • \$\begingroup\$ @JohnSyrinek It's definitely business and I do not know the specific email blast program the client is using. It's my understanding that there may be multiple programs in use and so the client wants something "universal." Also, the client specified that the files be sent via email to the end-user. One of the big selling points for the client was the automation process that the web portal provides, and I am trying to maintain that by having the files be sent directly to the end-user. \$\endgroup\$ May 15, 2013 at 16:29
  • \$\begingroup\$ @JohnSyrinek (cont.) You couldn't be more right about this being a roundabout way of doing things, but in order to circumvent the web portal's limitations while still incorporating with the portal's functionality, it may be necessary. \$\endgroup\$ May 15, 2013 at 16:32

1 Answer 1

2
\$\begingroup\$

PEP8

You're not following PEP8, Python's official coding style guide, for example:

  • Don't put multiple imports on the same line
  • Put a space after commas in parameter lists, so m.login(user, pwd) instead of m.login(user,pwd)
  • There shouldn't be whitespace after :
  • Use at least 2 spaces before inline comments (before each # comment)
  • Put a space after # when starting comments, so # comment instead of #comment
  • Some lines are too long (longer than 79 characters)

Bad practices

  • Remove unused imports: getpass, csv, threading
  • No need for the parens in user = ("username"), and it may get confused as a tuple (which it isn't)
  • The variable m is poorly named: judging by the name I might guess it's a "message", when it's actually a mailbox
  • At the point where you do str(message), message might not have been assigned yet
  • At the point where you do os.listdir(path), path might not have been assigned yet

In this code:

resp, items = m.search(None, "ALL")
items = items[0].split()  # getting the mails id

for emailid in items:
    resp, data = m.fetch(emailid, "(RFC822)")

The initialization of resp is pointless, because it will be reassigned anyway in the loop. Effectively you throw away the first assignment of resp. In such cases a common practice is to use _ as the variable name, like this:

_, items = m.search(None, "ALL")

On closer look, I see that after the 2nd assignment too, resp is not used again. So it could be replaced with _ there too.


message = re.compile(r'\%(.+?)\%', re.DOTALL).findall(content)

This is the same as the above but simpler:

message = re.findall(r'\%(.+?)\%', content, re.DOTALL)

But since you have this code inside a for loop, it would be best to compile the pattern once before the loop, and reuse it inside, like this:

re_between_percent = re.compile(r'\%(.+?)\%', re.DOTALL)
for part in mail.walk():
    # ...
    message = re_between_percent.findall(content)

Do the same for the other statements too where you do re.compile(...).findall(...).


Instead of this:

    htmlData = open(os.path.join('directory', htmlFile), 'w+')
    htmlData.write(htmlCode)
    print(htmlFile + ' Complete')
    htmlData.close()

The Pythonic way to work with files:

    with open(os.path.join('directory', htmlFile), 'w+') as htmlData:
        htmlData.write(htmlCode)
        print(htmlFile + ' Complete')
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.