-
Notifications
You must be signed in to change notification settings - Fork 0
Straw man pull request: structure notifications #1
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,17 @@ <h1>EVEMail-Email Relay Configuration</h1> | |
<dd><input name="rcpt_org" value="{{ rcpt_org }}" /> (ID or Name)</dd> | ||
<dt>Recipient Organization #2 From Which to Relay EVEMail (Optional)</dt> | ||
<dd><input name="rcpt_org2" value="{{ rcpt_org2 }}" /> (ID or Name)</dd> | ||
<dt>Types of Notifications To Relay</dt> | ||
<dd><select name="notify_types" multiple="multiple" size="8"> | ||
{% for t in notify_descriptions %} | ||
{% if t.type_id in notify_types %} | ||
<option value="{{ t.type_id }}" selected="selected">{{ t.description }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DRY - |
||
</option> | ||
{% else %} | ||
<option value="{{ t.type_id }}">{{ t.description }}</option> | ||
{% endif %} | ||
{% endfor %} | ||
</select></dd> | ||
<dt>Destination Email Address</dt> | ||
<dd><input name="dest_email" value="{{ dest_email }}" /></dd> | ||
</dl> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import datetime | ||
import logging | ||
import os | ||
import re | ||
|
||
|
@@ -10,7 +11,7 @@ | |
|
||
import evelink | ||
from evelink import appengine as elink_appengine | ||
from models import Configuration, SeenMail | ||
from models import Configuration, SeenMail, SeenNotification, NotificationTypes | ||
|
||
jinja_environment = jinja2.Environment( | ||
loader=jinja2.FileSystemLoader(os.path.dirname(__file__))) | ||
|
@@ -19,6 +20,8 @@ class HomeHandler(webapp2.RequestHandler): | |
|
||
def get(self): | ||
config = memcache.get('config') or Configuration.all().get() | ||
notify_descriptions = (memcache.get('ndesc') or | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can be more descriptive with the cache key than 5 characters There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will fix. |
||
NotificationTypes.all()) | ||
|
||
if config: | ||
template_values = { | ||
|
@@ -28,6 +31,8 @@ def get(self): | |
'rcpt_org': config.rcpt_org, | ||
'rcpt_org2': config.rcpt_org2 or '', | ||
'dest_email': config.dest_email, | ||
'notify_types': config.notify_types, | ||
'notify_descriptions': notify_descriptions, | ||
} | ||
else: | ||
template_values = { | ||
|
@@ -37,6 +42,8 @@ def get(self): | |
'rcpt_org': '', | ||
'rcpt_org2': '', | ||
'dest_email': '', | ||
'notify_types': [], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this a list There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because multiple notification types can be requested. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (As opposed to a set.) |
||
'notify_descriptions': notify_descriptions, | ||
} | ||
|
||
template = jinja_environment.get_template('index.html') | ||
|
@@ -48,6 +55,7 @@ def post(self): | |
rcpt_char = self.request.get('rcpt_char') | ||
rcpt_org = self.request.get('rcpt_org') | ||
rcpt_org2 = self.request.get('rcpt_org2') | ||
notify_types = [int(x) for x in self.request.get_all('notify_types')] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can potentially crash |
||
dest_email = self.request.get('dest_email') | ||
|
||
if not (key_id and vcode and rcpt_org and dest_email): | ||
|
@@ -92,6 +100,7 @@ def post(self): | |
rcpt_char=rcpt_char, | ||
rcpt_org=rcpt_org, | ||
rcpt_org2=rcpt_org2, | ||
notify_types=notify_types, | ||
dest_email=dest_email, | ||
) | ||
else: | ||
|
@@ -101,6 +110,7 @@ def post(self): | |
config.rcpt_org = rcpt_org | ||
if rcpt_org2: | ||
config.rcpt_org2 = rcpt_org2 | ||
config.notify_types = notify_types | ||
config.dest_email = dest_email | ||
|
||
config.put() | ||
|
@@ -125,6 +135,9 @@ class CronHandler(webapp2.RequestHandler): | |
|
||
def get(self): | ||
config = memcache.get('config') or Configuration.all().get() | ||
notify_descriptions = (memcache.get('ndesc') or | ||
NotificationTypes.all()) | ||
|
||
if not config: | ||
# We haven't set up our configuration yet, so don't try to do anything | ||
return | ||
|
@@ -133,6 +146,12 @@ def get(self): | |
elink_char = evelink.char.Char(config.rcpt_char, api=elink_api) | ||
elink_eve = evelink.eve.EVE(api=elink_api) | ||
|
||
self.send_emails(config, elink_api, elink_char, elink_eve) | ||
self.send_notifications(config, elink_api, elink_char, elink_eve, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these two names are misleading - send_notifications also sends emails There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. send_emails should be send_messages instead. |
||
notify_descriptions) | ||
|
||
def send_emails(self, config, elink_api, elink_char, elink_eve): | ||
|
||
recips = set([config.rcpt_org]) | ||
if config.rcpt_org2: | ||
recips.add(config.rcpt_org2) | ||
|
@@ -154,7 +173,7 @@ def get(self): | |
memcache.set('seen-%s' % m_id, True) | ||
|
||
if not message_ids_to_relay: | ||
self.response.out.write("No pending messages.") | ||
self.response.out.write("No pending messages.<br/>") | ||
return | ||
|
||
bodies = elink_char.message_bodies(message_ids_to_relay) | ||
|
@@ -175,12 +194,75 @@ def get(self): | |
|
||
return | ||
|
||
def send_notifications(self, config, elink_api, elink_char, elink_eve, | ||
notify_descriptions): | ||
|
||
headers = elink_char.notifications() | ||
message_ids = set(headers[h]['id'] for h in headers | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why build this set if you're doing to build a dict right afterwards with the same kinds of keys |
||
if headers[h]['type_id'] in config.notify_types ) | ||
|
||
headers = dict((headers[h]['id'], headers[h]) for h in headers) | ||
|
||
message_ids_to_relay = set() | ||
sender_ids = set() | ||
|
||
for m_id in message_ids: | ||
seen = (memcache.get('nseen-%s' % m_id) or | ||
SeenNotification.gql("WHERE notify_id = :1", m_id).get()) | ||
if not seen: | ||
message_ids_to_relay.add(m_id) | ||
sender_ids.add(headers[m_id]['sender_id']) | ||
else: | ||
memcache.set('nseen-%s' % m_id, True) | ||
|
||
if not message_ids_to_relay: | ||
self.response.out.write("No pending notifications.<br/>") | ||
return | ||
|
||
bodies = elink_char.notification_texts(message_ids_to_relay) | ||
senders = elink_eve.character_names_from_ids(sender_ids) | ||
|
||
e = EmailMessage() | ||
e.to = config.dest_email | ||
e.sender = '[email protected]' | ||
for m_id in message_ids_to_relay: | ||
sender = senders[headers[m_id]['sender_id']] | ||
timestamp = headers[m_id]['timestamp'] | ||
e.subject = ('[EVE Notify] %s' % | ||
notify_descriptions.filter( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This pretty much defeats the point of having these in memcache (not to mention you never set the memcache value, so memcache will never hit) |
||
"type_id = ", headers[m_id]['type_id']).get().description) | ||
e.html = self.format_notification(bodies[m_id], timestamp, sender, | ||
elink_eve) | ||
e.send() | ||
SeenNotification(notification_id=m_id).put() | ||
memcache.set('nseen-%s' % m_id, True) | ||
self.response.out.write("Processed notification ID %s.<br/>\n" % m_id) | ||
|
||
return | ||
|
||
def format_message(self, body, timestamp, sender): | ||
mtime = datetime.datetime.fromtimestamp(timestamp) | ||
body = re.sub(r'</?font.*?>', r'', body) | ||
body = "<p>Sent by %s at %s EVE Time</p>%s" % (sender, mtime, body) | ||
return body | ||
|
||
def format_notification(self, data, timestamp, sender, elink_eve): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You never use elink_eve here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant to use elink_eve here to resolve numeric alliance/corp/person names to actual names and tickers. I'll fix that up. Unfortunately, resolving moon ids and system id's requires a static database dump :/ |
||
mtime = datetime.datetime.fromtimestamp(timestamp) | ||
body = ("Attack by %s <%s> [%s] against %s located at " + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this point you'd be better of using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes. Good call. |
||
"moon %s in system %s. Health is %f/%f/%f.") % ( | ||
data['aggressorID'], | ||
data['aggressorCorpID'], | ||
data['aggressorAllianceID'], | ||
data['typeID'], | ||
data['moonID'], | ||
data['solarSystemID'], | ||
data['shieldValue'], | ||
data['armorValue'], | ||
data['hullValue'], | ||
) | ||
body = "<p>Sent by %s at %s EVE Time</p>%s" % (sender, mtime, body) | ||
return body | ||
|
||
class NullHandler(webapp2.RequestHandler): | ||
def get(self): | ||
pass | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,14 @@ class Configuration(db.Model): | |
rcpt_org = db.IntegerProperty(required=True) | ||
rcpt_org2 = db.IntegerProperty() | ||
dest_email = db.EmailProperty(required=True) | ||
notify_types = db.ListProperty(int, required=True) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "The value of a ListProperty cannot be None. It can, however, be an empty list. When None is specified for the default argument (or when the default argument is not specified), the default value of the property is the empty list." Nevertheless, I can drop 'required' |
||
|
||
class NotificationTypes(db.Model): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What populates this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll need some kind of upgrade script to populate this. For testing purposes only, I manually added the '75' structure under attack message by hand to the database. |
||
type_id = db.IntegerProperty(required=True) | ||
description = db.StringProperty(required=True) | ||
|
||
class SeenMail(db.Model): | ||
mail_id = db.IntegerProperty(required=True) | ||
|
||
class SeenNotification(db.Model): | ||
notification_id = db.IntegerProperty(required=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mutliple-select boxes are terrible UX imo - a set of checkboxes would be preferrable. having to use modifier keys to fill out a form is ugh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, will fix.