Merge pull request #95 from tableau/development

Merge changes for v2.0.1

config/vizalerts.yaml

@@ -51,3 +51,6 @@ threads: 2                                      # Number of threads VizAlerts wi
                                                      # Higher = More alerts process at once, increased server load
                                                      # Lower = Fewer alerts processed at once, decreased server load
                                                      # Content references within a single alert are processed serially
+
+data.coldelimiter: ','                          # Character used to separate field values in CSV files exported from Tableau Server
+                                                     # Some regions use semicolons for this, in which case switch it to ';'

version_history.txt

@@ -2,6 +2,21 @@
 VERSION HISTORY
 ===============
 
+Version 2.0.1
+=================================
+-Fixed issue where unlicensed users subscribed to Simple Alerts generate failure emails (Issue #88)
+-Fixed issue where VIZ_LINK content reference doesn't work if placed after other content references (Issue #83)
+-Improved error handling when subscriber to an Advanced Alert is not the owner (Issue #82)
+-Fixed issue where trailing commas in recipients lists cause a "missing field" error (Issue #61)
+-Fixed issue where locales defaulting to semicolon delimiters cannot use Advanced Alerts (Issue #17)
+-Added a few small notes to the VizAlertsConfig workbook (Issues #87,  #94)
+-Install guide clarifications and corrections (Issues #86, #85, #84)
+-User Guide clarifications and corrections (Issue #77)
+-Allow use of additional characters to break up list of recipient email addresses (Issue #92)
+-Added error handling for invalid regex expressions in configuration viz (Issue #90)
+-Fixed issue where Case should be ignored in email address regex pattern comparisons (Issue #93)
+=================================
+
 Version 2.0.0
 =================================
 -Added support for SMS messages through Twilio (Issue #57)

vizalert/config.py

@@ -13,7 +13,10 @@
 configs = []
 
 # Global Variable Definitions
-valid_conf_keys = \
+
+
+# yaml configuration values that we require
+required_conf_keys = \
     ['log.dir',
     'log.dir.file_retention_seconds',
     'log.level',
@@ -45,6 +48,14 @@
     'vizalerts.source.viz',
     'vizalerts.source.site']
 
+# yaml configuration values that we accept, but are not required
+optional_conf_keys = \
+    ['data.coldelimiter']
+
+# default delimiter for CSV exports
+DEFAULT_COL_DELIMITER = u','
+
+
 def validate_conf(configfile):
     """Import config values and do some basic validations"""
     try:
@@ -58,14 +69,22 @@ def validate_conf(configfile):
         log.logger.error(errormessage)
         sys.exit(1)
 
-    # test for missing config values
-    missingkeys = set(valid_conf_keys).difference(localconfigs.keys())
+    # test for missing required config values
+    missingkeys = set(required_conf_keys) - set(localconfigs.keys())
     if len(missingkeys) != 0:
         errormessage = u'Missing config values {}'.format(missingkeys)
         print errormessage
         log.logger.error(errormessage)
         sys.exit(1)
 
+    # test for unrecognized config values
+    extrakeys = set(localconfigs.keys()) - (set(required_conf_keys) | set(optional_conf_keys))
+    if len(extrakeys) != 0:
+        errormessage = u'Extraneous config values found. Please examine for typos: {}'.format(extrakeys)
+        print errormessage
+        log.logger.error(errormessage)
+        sys.exit(1)
+
     # test specific conf values and prep if possible
     for dir in [localconfigs['schedule.state.dir'], localconfigs['log.dir'], localconfigs['temp.dir']]:
         if not os.path.exists(os.path.dirname(dir)):
@@ -135,6 +154,16 @@ def validate_conf(configfile):
             log.logger.error(errormessage)
             sys.exit(1)
 
+    # validate data.coldelimiter
+    if 'data.coldelimiter' in localconfigs.keys():
+        if len(localconfigs['data.coldelimiter']) > 1:
+            errormessage = u'Configuration value data.coldelimiter cannot be more than one character.'
+            print errormessage
+            log.logger.error(errormessage)
+            sys.exit(1)
+    else:
+        localconfigs['data.coldelimiter'] = DEFAULT_COL_DELIMITER
+
     config.configs = localconfigs
 
 

vizalert/emailaction.py

@@ -29,7 +29,7 @@
 import vizalert
 
 # regular expression used to split recipient address strings into separate email addresses
-EMAIL_RECIP_SPLIT_REGEX = u'[; ,]*'
+EMAIL_RECIP_SPLIT_REGEX = u'[\t\n; ,]*'
 
 
 def send_email(fromaddr, toaddrs, subject, content, ccaddrs=None, bccaddrs=None, inlineattachments=None,
@@ -65,18 +65,18 @@ def send_email(fromaddr, toaddrs, subject, content, ccaddrs=None, bccaddrs=None,
         msg['Subject'] = Header(subject.encode('utf-8'), 'UTF-8').encode()
 
         # Process direct recipients
-        toaddrs = re.split(EMAIL_RECIP_SPLIT_REGEX, toaddrs.strip())
+        toaddrs = re.split(EMAIL_RECIP_SPLIT_REGEX, toaddrs.strip(EMAIL_RECIP_SPLIT_REGEX))
         msg['To'] = Header(', '.join(toaddrs))
         allrecips = toaddrs
 
         # Process indirect recipients
         if ccaddrs:
-            ccaddrs = re.split(EMAIL_RECIP_SPLIT_REGEX, ccaddrs.strip())
+            ccaddrs = re.split(EMAIL_RECIP_SPLIT_REGEX, ccaddrs.strip(EMAIL_RECIP_SPLIT_REGEX))
             msg['CC'] = Header(', '.join(ccaddrs))
             allrecips.extend(ccaddrs)
 
         if bccaddrs:
-            bccaddrs = re.split(EMAIL_RECIP_SPLIT_REGEX, bccaddrs.strip())
+            bccaddrs = re.split(EMAIL_RECIP_SPLIT_REGEX, bccaddrs.strip(EMAIL_RECIP_SPLIT_REGEX))
             # don't add to header, they are blind carbon-copied
             allrecips.extend(bccaddrs)
 
@@ -140,12 +140,12 @@ def send_email(fromaddr, toaddrs, subject, content, ccaddrs=None, bccaddrs=None,
     except Exception as e:
         log.logger.error(u'Email failed to send: {}'.format(e))
         raise e
-        
+
 
 def addresses_are_invalid(emailaddresses, emptystringok, regex_eval=None):
     """Validates all email addresses found in a given string, optionally that conform to the regex_eval"""
     log.logger.debug(u'Validating email field value: {}'.format(emailaddresses))
-    address_list = re.split(EMAIL_RECIP_SPLIT_REGEX, emailaddresses.strip())
+    address_list = re.split(EMAIL_RECIP_SPLIT_REGEX, emailaddresses.strip(EMAIL_RECIP_SPLIT_REGEX)) # trim separator chars from ends
     for address in address_list:
         log.logger.debug(u'Validating presumed email address: {}'.format(address))
         if emptystringok and (address == '' or address is None):
@@ -173,7 +173,7 @@ def address_is_invalid(address, regex_eval=None):
     # Validate address according to admin regex
     if regex_eval:
         log.logger.debug("testing address {} against regex {}".format(address, regex_eval))
-        if not re.match(regex_eval, address):
+        if not re.match(regex_eval, address, re.IGNORECASE):
             errormessage = u'Address must match regex pattern set by the administrator: {}'.format(regex_eval)
             log.logger.error(errormessage)
             return errormessage
@@ -294,27 +294,32 @@ def validate_addresses(vizdata,
     rownum = 2  # account for field header in CSV
 
     for row in vizdata:
-        result = addresses_are_invalid(row[email_to_field], False,
-                                       allowed_recipient_addresses)  # empty string not acceptable as a To address
-        if result:
-            errorlist.append(
-                {'Row': rownum, 'Field': email_to_field, 'Value': result['address'], 'Error': result['errormessage']})
-        if email_from_field:
-            result = addresses_are_invalid(row[email_from_field], False,
-                                           allowed_from_address)  # empty string not acceptable as a From address
-            if result:
-                errorlist.append({'Row': rownum, 'Field': email_from_field, 'Value': result['address'],
-                                  'Error': result['errormessage']})
-        if email_cc_field:
-            result = addresses_are_invalid(row[email_cc_field], True, allowed_recipient_addresses)
-            if result:
-                errorlist.append({'Row': rownum, 'Field': email_cc_field, 'Value': result['address'],
-                                  'Error': result['errormessage']})
-        if email_bcc_field:
-            result = addresses_are_invalid(row[email_bcc_field], True, allowed_recipient_addresses)
+        if len(row) > 0:
+            log.logger.debug(u'Validating "To" addresses: {}'.format(row[email_to_field]))
+            result = addresses_are_invalid(row[email_to_field], False,
+                                           allowed_recipient_addresses)  # empty string not acceptable as a To address
             if result:
-                errorlist.append({'Row': rownum, 'Field': email_bcc_field, 'Value': result['address'],
-                                  'Error': result['errormessage']})
+                errorlist.append(
+                    {'Row': rownum, 'Field': email_to_field, 'Value': result['address'], 'Error': result['errormessage']})
+            if email_from_field:
+                log.logger.debug(u'Validating "From" addresses')
+                result = addresses_are_invalid(row[email_from_field], False,
+                                               allowed_from_address)  # empty string not acceptable as a From address
+                if result:
+                    errorlist.append({'Row': rownum, 'Field': email_from_field, 'Value': result['address'],
+                                      'Error': result['errormessage']})
+            if email_cc_field:
+                log.logger.debug(u'Validating "CC" addresses')
+                result = addresses_are_invalid(row[email_cc_field], True, allowed_recipient_addresses)
+                if result:
+                    errorlist.append({'Row': rownum, 'Field': email_cc_field, 'Value': result['address'],
+                                      'Error': result['errormessage']})
+            if email_bcc_field:
+                log.logger.debug(u'Validating "BCC" addresses')
+                result = addresses_are_invalid(row[email_bcc_field], True, allowed_recipient_addresses)
+                if result:
+                    errorlist.append({'Row': rownum, 'Field': email_bcc_field, 'Value': result['address'],
+                                      'Error': result['errormessage']})
         rownum += 1
 
     return errorlist

vizalert/vizalert.py

@@ -88,9 +88,9 @@
 class UnicodeCsvReader(object):
     """Code from http://stackoverflow.com/questions/1846135/general-unicode-utf-8-support-for-csv-files-in-python-2-6"""
     def __init__(self, f, encoding="utf-8", **kwargs):
-        self.csv_reader = csv.reader(f, **kwargs)
+        self.csv_reader = csv.reader(f, delimiter=str(config.configs['data.coldelimiter']), **kwargs)
         self.encoding = encoding
-
+        
     def __iter__(self):
         return self
 
@@ -407,14 +407,14 @@ def parse_action_fields(self):
                 # ensure the subscriber is the owner of the viz
                 #  we need to do this check straight away so we don't send any more info to the subscriber
                 if self.subscriber_sysname != self.owner_sysname:
-                    errormessage = u'You must be the owner of the workbook in order to use Advanced Alerts<br><br>.' \
+                    errormessage = u'You must be the owner of the workbook in order to use Advanced Alerts.<br><br>' \
                                    u'Subscriber {} to advanced alert subscription_id {} is not the owner, {}'.format(
                                     self.subscriber_sysname,
                                     self.subscription_id,
                                     self.owner_sysname)
                     log.logger.error(errormessage)
                     self.error_list.append(errormessage)
-                    return None  # provide no more info, and do no more work
+                    return []  # provide no more info, and do no more work
 
                 # check for issues in each of the fields
                 for action_field in self.action_field_dict:
@@ -437,7 +437,7 @@ def parse_action_fields(self):
                             if not config.configs['smsaction.enable']:
                                 self.action_field_dict[action_field].error_list.append(
                                     u'SMS actions are not enabled, per administrative settings')
-                            if not self.action_enabled_sms:
+                            elif not self.action_enabled_sms:
                                 self.action_field_dict[action_field].error_list.append(
                                     u'SMS actions are not allowed for this alert, per administrative settings')
                             elif not smsaction.smsclient:
@@ -614,7 +614,7 @@ def execute_alert(self):
             else:
                 # they're not the owner, so this is a simple alert. just ignore them and log that we did.
                 errormessage = u'Ignoring subscription_id {}: User {} is unlicensed.'.format(
-                    self.subscription_id)
+                    self.subscription_id, self.subscriber_sysname)
                 log.logger.error(errormessage)
                 self.error_list.append(errormessage)
                 return
@@ -1449,10 +1449,10 @@ def append_body_and_inlineattachments(self, body, inlineattachments, row, vizcom
                             replacestring = u'<a href="' + self.get_view_url(vizcompleterefs[vizref]['view_url_suffix']) + u'">' + \
                                             vizcompleterefs[vizref]['view_url_suffix'] + u'</a>'
 
-            replaceresult = replace_in_list(body, vizref, replacestring)
+                    replaceresult = replace_in_list(body, vizref, replacestring)
 
-            if replaceresult['foundstring']:
-                body = replaceresult['outlist']\
+                    if replaceresult['foundstring']:
+                        body = replaceresult['outlist']\
 
         return body, inlineattachments
 

vizalerts.py

@@ -4,7 +4,7 @@
 
 __author__ = 'Matt Coles'
 __credits__ = 'Jonathan Drummey'
-__version__ = '2.0.0'
+__version__ = '2.0.1'
 
 # generic modules
 import logging
@@ -15,6 +15,7 @@
 import time
 import fileinput
 import codecs
+import re
 from Queue import Queue
 import threading
 from operator import attrgetter
@@ -215,9 +216,29 @@ def get_alerts():
             raise UserWarning(''.join(source_viz.error_list))
 
     except Exception as e:
-        quit_script('Could not download or read source viz data for the following reasons:<br/><br/>{}'.format(
+        quit_script('Could not process source viz data from {} for the following reasons:<br/><br/>{}'.format(
+			config.configs['vizalerts.source.viz'],
             e.message))
 
+    # test for regex invalidity
+    try:
+        fieldlist = ('allowed_from_address','allowed_recipient_addresses','allowed_recipient_numbers')
+        currentfield = ''
+        currentfieldvalue = ''
+
+        for line in results:
+            for field in fieldlist:
+                currentfield = field
+                currentfieldvalue = line[field]
+                re.compile('{}'.format(currentfieldvalue))
+    except Exception as e:
+        quit_script('Could not process source viz data from {} for the following reason:<br/><br/>' \
+		    'Invalid regular expression found. Could not evaluate expression \'{}\' in the field {}. Raw error:<br/><br/>{}'.format(
+                config.configs['vizalerts.source.viz'],
+                currentfieldvalue,
+                currentfield,
+                e.message))
+
     # retrieve schedule data from the last run and compare to current
     statefile = config.configs['schedule.state.dir'] + SCHEDULE_STATE_FILENAME
 
@@ -240,6 +261,7 @@ def get_alerts():
 
     # Create VizAlert instances for all the alerts we've retrieved
     try:
+        results = source_viz.read_trigger_data() # get the results again to start at the beginning
         for line in results:
             # build an alert instance for each line            
             alert = vizalert.VizAlert(line['view_url_suffix'],