diff --git a/README.rst b/README.rst index dca13dd..4b43d17 100644 --- a/README.rst +++ b/README.rst @@ -186,11 +186,11 @@ If you don't specify this setting, the default will be number-sequence. Send error mails when harvesting fails (optional) ================================================= -If you want to send and email when a Harvest Job fails, you can set the following configuration option in the ini file: +If you want to send an email when a Harvest Job fails, you can set the following configuration option in the ini file: ckan.harvest.status_mail.errored = True -That way, all CKAN Users who are declared as Sysadmins will receive the Error emails at their configured email address. +That way, all CKAN Users who are declared as Sysadmins will receive the Error emails at their configured email address. If the Harvest-Source of the failing Harvest-Job belongs to an organization, the error-mail will also be sent to the organization-members who have the admin-role if their E-Mail is configured. If you don't specify this setting, the default will be False. diff --git a/ckanext/harvest/helpers.py b/ckanext/harvest/helpers.py index fab9147..c43ecfd 100644 --- a/ckanext/harvest/helpers.py +++ b/ckanext/harvest/helpers.py @@ -18,7 +18,7 @@ def package_list_for_source(source_id): ''' limit = 20 page = int(request.params.get('page', 1)) - fq = 'harvest_source_id:"{0}"'.format(source_id) + fq = '+harvest_source_id:"{0}"'.format(source_id) search_dict = { 'fq' : fq, 'rows': limit, @@ -66,7 +66,7 @@ def package_count_for_source(source_id): Returns the current package count for datasets associated with the given source id ''' - fq = 'harvest_source_id:"{0}"'.format(source_id) + fq = '+harvest_source_id:"{0}"'.format(source_id) search_dict = {'fq': fq} context = {'model': model, 'session': model.Session} result = logic.get_action('package_search')(context, search_dict) diff --git a/ckanext/harvest/logic/action/update.py b/ckanext/harvest/logic/action/update.py index f19b020..e56648a 100644 --- a/ckanext/harvest/logic/action/update.py +++ b/ckanext/harvest/logic/action/update.py @@ -621,14 +621,35 @@ def send_error_mail(context, source_id, status): msg += '\n--\n' msg += toolkit._('You are receiving this email because you are currently set-up as Administrator for {0}. Please do not reply to this email as it was sent from a non-monitored address.').format(config.get('ckan.site_title')) + recipients = [] + + # gather sysadmins model = context['model'] - sysadmins = model.Session.query(model.User).filter(model.User.sysadmin == True).all() + for sysadmin in sysadmins: + recipients.append({ + 'name': sysadmin.name, + 'email': sysadmin.email + }) - # for recipient in email_recipients: - for recipient in sysadmins: - email = {'recipient_name': recipient, - 'recipient_email': recipient, + # gather organization-admins + if source.get('organization'): + members = get_action('member_list')(context, { + 'id': source['organization']['id'], + 'object_type': 'user', + 'capacity': 'admin' + }) + for member in members: + member_details = get_action('user_show')(context, {'id': member[0]}) + if member_details['email']: + recipients.append({ + 'name': member_details['name'], + 'email': member_details['email'] + }) + + for recipient in recipients: + email = {'recipient_name': recipient['name'], + 'recipient_email': recipient['email'], 'subject': config.get('ckan.site_title') + ' - Harvesting Job - Error Notification', 'body': msg} diff --git a/ckanext/harvest/queue.py b/ckanext/harvest/queue.py index 482305e..d211e88 100644 --- a/ckanext/harvest/queue.py +++ b/ckanext/harvest/queue.py @@ -64,6 +64,7 @@ def get_connection_redis(): import redis return redis.StrictRedis(host=config.get('ckan.harvest.mq.hostname', HOSTNAME), port=int(config.get('ckan.harvest.mq.port', REDIS_PORT)), + password=config.get('ckan.harvest.mq.password', None), db=int(config.get('ckan.harvest.mq.redis_db', REDIS_DB))) diff --git a/ckanext/harvest/tests/test_action.py b/ckanext/harvest/tests/test_action.py index c5f8d3f..549867a 100644 --- a/ckanext/harvest/tests/test_action.py +++ b/ckanext/harvest/tests/test_action.py @@ -617,6 +617,83 @@ class TestHarvestErrorMail(FunctionalTestBase): toolkit.get_action('harvest_source_reindex')(context, {'id': harvest_source['id']}) return context, harvest_source, job + def _create_harvest_source_with_owner_org_and_job_if_not_existing(self): + site_user = toolkit.get_action('get_site_user')( + {'model': model, 'ignore_auth': True}, {})['name'] + + context = { + 'user': site_user, + 'model': model, + 'session': model.Session, + 'ignore_auth': True, + } + + test_org = ckan_factories.Organization() + test_other_org = ckan_factories.Organization() + org_admin_user = ckan_factories.User() + org_member_user = ckan_factories.User() + other_org_admin_user = ckan_factories.User() + + toolkit.get_action('organization_member_create')( + context.copy(), + { + 'id': test_org['id'], + 'username': org_admin_user['name'], + 'role': 'admin' + } + ) + + toolkit.get_action('organization_member_create')( + context.copy(), + { + 'id': test_org['id'], + 'username': org_member_user['name'], + 'role': 'member' + } + ) + + toolkit.get_action('organization_member_create')( + context.copy(), + { + 'id': test_other_org['id'], + 'username': other_org_admin_user['name'], + 'role': 'admin' + } + ) + + source_dict = { + 'title': 'Test Source', + 'name': 'test-source', + 'url': 'basic_test', + 'source_type': 'test', + 'owner_org': test_org['id'], + 'run': True + } + + try: + harvest_source = toolkit.get_action('harvest_source_create')( + context.copy(), + source_dict + ) + except toolkit.ValidationError: + harvest_source = toolkit.get_action('harvest_source_show')( + context.copy(), + {'id': source_dict['name']} + ) + pass + + try: + job = toolkit.get_action('harvest_job_create')(context.copy(), { + 'source_id': harvest_source['id'], 'run': True}) + except HarvestJobExists: + job = toolkit.get_action('harvest_job_show')(context.copy(), { + 'id': harvest_source['status']['last_job']['id']}) + pass + + toolkit.get_action('harvest_jobs_run')(context.copy(), {}) + toolkit.get_action('harvest_source_reindex')(context.copy(), {'id': harvest_source['id']}) + return context, harvest_source, job + @patch('ckan.lib.mailer.mail_recipient') def test_error_mail_not_sent(self, mock_mailer_mail_recipient): context, harvest_source, job = self._create_harvest_source_and_job_if_not_existing() @@ -652,6 +729,28 @@ class TestHarvestErrorMail(FunctionalTestBase): assert_equal(1, status['last_job']['stats']['errored']) assert mock_mailer_mail_recipient.called + @patch('ckan.lib.mailer.mail_recipient') + def test_error_mail_sent_with_org(self, mock_mailer_mail_recipient): + context, harvest_source, job = self._create_harvest_source_with_owner_org_and_job_if_not_existing() + + # create a HarvestGatherError + job_model = HarvestJob.get(job['id']) + msg = 'System error - No harvester could be found for source type %s' % job_model.source.type + err = HarvestGatherError(message=msg, job=job_model) + err.save() + + status = toolkit.get_action('harvest_source_show_status')(context, {'id': harvest_source['id']}) + + send_error_mail( + context, + harvest_source['id'], + status + ) + + assert_equal(1, status['last_job']['stats']['errored']) + assert mock_mailer_mail_recipient.called + assert_equal(2, mock_mailer_mail_recipient.call_count) + class TestHarvestDBLog(unittest.TestCase): @classmethod