1
\$\begingroup\$

Please review this:

class KoolDailyReminderAfterWeek(webapp2.RequestHandler):
    def get(self):
        logging.info('sending reminders')
        timeline = datetime.now () - timedelta (days = 7)
        edge = datetime.now () - timedelta (days = 8)
        ads = Ad.all().filter("published =", True).filter("added <", timeline).filter("url IN", ['www.koolbusiness.com']).filter("added >", edge).fetch(99999)
        for ad in ads:
            logging.info('sending reminder to %s', ad.name)
            if ad.title:
                subject= ad.title
            else:
                subject = 'Reminder'
            message = mail.EmailMessage(sender='Kool Business <[email protected]>', subject=subject)
            message.body = """
            Hello!<br>Now your ad <a href="http://www.koolbusiness.com/vi/%d.html">%s</a> has been out for a week. If you already have sold your product you can remove your ad.<br><br>Some advice to promote your ad:<br>Change + Renew the ad and it will be on top of the list. You can also change text and price.<br>Change the ad if you only want to lower the price or change the ad text<br>Renew so that the ad will be on top if the list. <br><br>Best regards,<br>Koolbusiness.com
            """  % (ad.key().id(),ad.title)
            message.html = """
            <html><head></head><body>
            Hello!<br>Now your ad <a href="http://www.koolbusiness.com/vi/%d.html">%s</a> has been out for a week. If you already have sold your product you can remove your ad.<br><br>Some advice to promote your ad:<br>Change + Renew the ad and it will be on top of the list. You can also change text and price.<br>Change the ad if you only want to lower the price or change the ad text<br>Renew so that the ad will be on top if the list. <br><br>Best regards,<br>Koolbusiness.com
            </body></html>
            """  % (ad.key().id(),ad.title)
            message.to=ad.email
            message.send()
            logging.info('sent email to: ' +ad.email)

YAML

 - description: daily mailout
   url: /report/dailyafterweek
   schedule: every 24 hours

Background

Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

I'm not familiar with that framework, so I can only provide some general points:

  • You can rewrite the if statement to subject = ad.title or 'Reminder' (see here).
  • You are a bit inconsistent with spacings (before and after =, ,, (), etc.), and string formatting (your second and third logging.info)
  • I'd recomment spreading both those chained filters and overly long strings over multiple lines.
  • Maybe you could use some helper method to derive the HTML version of the message from the plain-text version or vice versa?
Improve this answer
\$\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.