From ed2e79c30296d62fef5b5f169ea218d55fdfad81 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 14 2020 11:12:53 +0000 Subject: [PATCH 1/5] Port the multi-email validator to email_validator Signed-off-by: Pierre-Yves Chibon --- diff --git a/fedocal/forms.py b/fedocal/forms.py index 631ec66..2725cec 100644 --- a/fedocal/forms.py +++ b/fedocal/forms.py @@ -28,6 +28,7 @@ from datetime import time from fedocal.fedocal_babel import lazy_gettext as _ from fedocal import i18nforms +import email_validator from pytz import common_timezones import wtforms @@ -64,16 +65,16 @@ def validate_multi_email(form, field): """ Raises an exception if the content of the field does not contain one or more email. """ - pattern = re.compile(r'^.+@([^.@][^@]+)$', re.IGNORECASE) data = field.data.replace(' ', ',') for entry in data.split(','): entry = entry.strip() if not entry: continue - match = pattern.match(field.data or '') - if not match: - message = field.gettext(_('Invalid input.')) - raise wtforms.ValidationError(message) + try: + email_validator.validate_email(entry) + except email_validator.EmailNotValidError as e: + # email is not valid, exception message is human-readable + raise wtforms.ValidationError(str(e)) class AddCalendarForm(i18nforms.Form): From 555bde7669277420a91aaeffacd9e73e8003c83d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 14 2020 11:14:55 +0000 Subject: [PATCH 2/5] Drop the flask10_only decorator and fix/adjust the tests Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/__init__.py b/tests/__init__.py index 19955b2..295caec 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -71,19 +71,6 @@ ICS_FILE_NOTOK = os.path.join(os.path.dirname( os.path.abspath(__file__)), 'ical_wrong.txt') -def flask10_only(function): - """ Decorator to skip tests if the flask version is lower than 0.10 """ - @wraps(function) - def decorated_function(*args, **kwargs): - """ Decorated function, actually does the work. """ - import flask - ver = flask.__version__.split('.') - if int(ver[0]) >= 0 and int(ver[1]) >= 10: - return function(*args, **kwargs) - return 'Skipped' - return decorated_function - - @contextmanager def user_set(APP, user): """ Set the provided user as fas_user in the provided application.""" diff --git a/tests/test_flask.py b/tests/test_flask.py index b9abc4d..b0947b1 100644 --- a/tests/test_flask.py +++ b/tests/test_flask.py @@ -48,7 +48,7 @@ sys.path.insert(0, os.path.join(os.path.dirname( import fedocal import fedocal.fedocallib as fedocallib import fedocal.fedocallib.model as model -from tests import (Modeltests, FakeUser, flask10_only, user_set, TODAY, +from tests import (Modeltests, FakeUser, user_set, TODAY, ICS_FILE, ICS_FILE_NOTOK) @@ -841,16 +841,13 @@ class Flasktests(Modeltests): self.assertFalse( fedocal.is_safe_url('https://fedoraproject.org/')) - @flask10_only def test_auth_login(self): """ Test the auth_login function. """ self.__setup_db() user = FakeUser([], username='pingou') with user_set(fedocal.APP, user): - output = self.app.get('/login/', follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn('Home - Fedocal', output_text) + output = self.app.get('/login/') + self.assertEqual(output.status_code, 302) def test_locations(self): """ Test the locations function. """ @@ -905,7 +902,6 @@ class Flasktests(Modeltests): 'class="errors">No location named foobar could be found', output_text) - @flask10_only def test_add_calendar(self): """ Test the add_calendar function. """ user = None with user_set(fedocal.APP, user): - output = self.app.get('/calendar/add/', follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - # discoveryfailure happens if there is no network - self.assertTrue( - 'OpenID transaction in progress' - in output_text or 'discoveryfailure' in output_text) + output = self.app.get('/calendar/add/') + self.assertEqual(output.status_code, 302) user = FakeUser(['test']) with user_set(fedocal.APP, user): @@ -1057,7 +1047,6 @@ class Flasktests(Modeltests): '="errors">Could not add this calendar to the databasetest_calendar', output_text) - @flask10_only def test_clear_calendar(self): """ Test the clear_calendar function. """ self.__setup_db() @@ -1241,7 +1229,6 @@ class Flasktests(Modeltests): '
  • Calendar cleared
    • ', output_text) - @flask10_only def test_edit_calendar(self): """ Test the edit_calendar function. """ self.__setup_db() @@ -1307,7 +1294,7 @@ class Flasktests(Modeltests): self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertEqual( - output_text.count('This field is required.'), 2) + output_text.count('This field is required.'), 3) self.assertIn( '

    Edit calendar "test_calendar"

    ', output_text) @@ -1339,7 +1326,6 @@ class Flasktests(Modeltests): 'election1', output_text) - @flask10_only def test_auth_logout(self): """ Test the auth_logout function. """ user = FakeUser(fedocal.APP.config['ADMIN_GROUP']) @@ -1362,7 +1348,6 @@ class Flasktests(Modeltests): '
  • You have been logged out
    • ', output_text) - @flask10_only def test_my_meetings(self): """ Test the my_meetings function. """ self.__setup_db() @@ -1397,7 +1382,6 @@ class Flasktests(Modeltests): self.assertIn( ' Test meeting with reminder ', output_text) - @flask10_only def test_add_meeting(self): """ Test the add_meeting function. """ self.__setup_db() @@ -1615,8 +1599,8 @@ class Flasktests(Modeltests): 'meeting_time_stop': time(14, 0), 'meeting_timezone': 'Europe/Paris', 'frequency': '', - 'reminder_from': 'pingou@fp.o', - 'reminder_who': 'pingou@fp.org', + 'reminder_from': 'pingou@fp', + 'remind_who': 'pingou@fp.org', 'remind_when': 'H-12', 'csrf_token': csrf_token, } @@ -1640,7 +1624,7 @@ class Flasktests(Modeltests): 'meeting_timezone': 'Europe/Paris', 'frequency': '', 'reminder_from': 'pingou@fp.o', - 'reminder_who': 'pingou@fp.org, pingou@fp.o', + 'remind_who': 'pingou@fp.org, pingou@fp', 'remind_when': 'H-12', 'csrf_token': csrf_token, } @@ -1652,7 +1636,8 @@ class Flasktests(Modeltests): self.assertIn( '

    New meeting

    ', output_text) self.assertIn( - 'Invalid email address.', output_text) + 'The domain name fp is not valid. It should have a period.', + output_text) # Works - with one email as recipient of the reminder data = { @@ -1663,7 +1648,7 @@ class Flasktests(Modeltests): 'meeting_timezone': 'Europe/Paris', 'frequency': '', 'reminder_from': 'pingou@fp.org', - 'reminder_who': 'pingou@fp.org', + 'remind_who': 'pingou@fp.org', 'remind_when': 'H-12', 'csrf_token': csrf_token, } @@ -1692,7 +1677,7 @@ class Flasktests(Modeltests): 'meeting_timezone': 'Europe/Paris', 'frequency': '', 'reminder_from': 'pingou@fp.org', - 'reminder_who': 'pingou@fp.org,pingou@p.fr', + 'remind_who': 'pingou@fp.org,pingou@p.fr', 'remind_when': 'H-12', 'csrf_token': csrf_token, } @@ -1712,7 +1697,6 @@ class Flasktests(Modeltests): self.assertNotIn( 'href="/meeting/20/?from_date=', output_text) - @flask10_only def test_edit_meeting(self): """ Test the edit_meeting function. """ self.__setup_db() @@ -1801,7 +1785,7 @@ class Flasktests(Modeltests): self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( - 'Not a valid choice', output_text) + 'This field is required.', output_text) self.assertIn( 'Edit meeting - Fedocal', output_text) @@ -1883,6 +1867,8 @@ class Flasktests(Modeltests): meeting_information='Full day meeting 2', calendar_name='test_calendar_disabled', full_day=True) + self.session.add(obj) + self.session.flush() obj.add_manager(self.session, ['toshio']) obj.save(self.session) self.session.commit() @@ -1950,7 +1936,6 @@ class Flasktests(Modeltests): self.assertEqual( output_text.count('*'), 6) - @flask10_only def test_delete_meeting(self): """ Test the delete_meeting function. """ self.__setup_db() @@ -2082,6 +2067,8 @@ class Flasktests(Modeltests): meeting_information='Full day meeting 2', calendar_name='test_calendar_disabled', full_day=True) + self.session.add(obj) + self.session.flush() obj.add_manager(self.session, ['toshio']) obj.save(self.session) self.session.commit() @@ -2289,7 +2276,6 @@ class Flasktests(Modeltests): self.assertIn( '

    This is a test calendar

    ', output_text) - @flask10_only def test_upload_calendar(self): """ Test the upload_calendar function. """ self.__setup_db() @@ -2371,7 +2357,6 @@ class Flasktests(Modeltests): 'extension "txt" which is not an allowed ' 'format', output_text) - @flask10_only def test_markdown_preview(self): """ Test the markdown_preview function. """ user = FakeUser(['gitr2spec'], username='kevin') diff --git a/tests/test_flask_extras.py b/tests/test_flask_extras.py index 0ed8aad..d9f47c4 100644 --- a/tests/test_flask_extras.py +++ b/tests/test_flask_extras.py @@ -45,7 +45,7 @@ sys.path.insert(0, os.path.join(os.path.dirname( import fedocal import fedocal.fedocallib as fedocallib import fedocal.fedocallib.model as model -from tests import (Modeltests, FakeUser, flask10_only, user_set, TODAY) +from tests import (Modeltests, FakeUser, user_set, TODAY) # pylint: disable=E1103 @@ -75,7 +75,6 @@ class ExtrasFlasktests(Modeltests): fedocal.SESSION = self.session self.app = fedocal.APP.test_client() - @flask10_only def test_start_date_edit_meeting_form(self): """ Test the content of the start_date in the edit meeting form. """ @@ -93,6 +92,8 @@ class ExtrasFlasktests(Modeltests): calendar_name='test_calendar', recursion_frequency=14, recursion_ends=TODAY + timedelta(days=90)) + self.session.add(obj) + self.session.flush() obj.add_manager(self.session, 'pingou,') obj.save(self.session) self.session.commit() @@ -110,7 +111,7 @@ class ExtrasFlasktests(Modeltests): # If no date is specified, it returns the next occurence self.assertIn( - '' % (next_date), output_text ) @@ -123,9 +124,8 @@ class ExtrasFlasktests(Modeltests): output2_text = output2.get_data(as_text=True) self.assertIn( - '' % (TODAY + timedelta(days=28)), - output2_text + '' % (TODAY + timedelta(days=28)), output2_text ) # If an exact date in the future is specified, return that date @@ -136,7 +136,7 @@ class ExtrasFlasktests(Modeltests): output2_text = output2.get_data(as_text=True) self.assertIn( - '' % (TODAY + timedelta(days=14)), output2_text ) @@ -146,11 +146,10 @@ class ExtrasFlasktests(Modeltests): output2_text = output2.get_data(as_text=True) self.assertIn( - '' % (TODAY), output2_text ) - @flask10_only def test_start_date_delete_meeting_form(self): """ Test the content of the start_date in the delete meeting form. """ @@ -168,6 +167,8 @@ class ExtrasFlasktests(Modeltests): calendar_name='test_calendar', recursion_frequency=14, recursion_ends=TODAY + timedelta(days=90)) + self.session.add(obj) + self.session.flush() obj.add_manager(self.session, 'pingou,') obj.save(self.session) self.session.commit() @@ -206,7 +207,7 @@ class ExtrasFlasktests(Modeltests): self.assertEqual(output2.status_code, 200) output_text = output2.get_data(as_text=True) - self.assertIn('
  • Date: %s
    • ' % next_date, output_text) + self.assertIn('
  • Date: %s
    • ' % (TODAY + timedelta(days=14)), output_text) # If an old date in the future is specified, return the first date output2 = self.app.get('/meeting/delete/1/?from_date=2000-01-01') From d5e2bf49763ad24d32e5f5b7773b4420c01850a3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Oct 14 2020 11:18:40 +0000 Subject: [PATCH 3/5] Make fedocal rely on fedocal-messages to check the structure of the messages sent Signed-off-by: Pierre-Yves Chibon --- diff --git a/fedocal/fedocallib/fedmsgshim.py b/fedocal/fedocallib/fedmsgshim.py index 1937c3b..65ab3e8 100644 --- a/fedocal/fedocallib/fedmsgshim.py +++ b/fedocal/fedocallib/fedmsgshim.py @@ -11,6 +11,8 @@ import logging import fedora_messaging.api from fedora_messaging.exceptions import PublishReturned, ConnectionException +import fedocal_messages.messages as schema + _log = logging.getLogger(__name__) @@ -18,10 +20,59 @@ _log = logging.getLogger(__name__) def publish(topic, msg): # pragma: no cover _log.debug('Publishing a message for %s: %s', topic, msg) try: - message = fedora_messaging.api.Message( - topic='fedocal.%s' % topic, - body=msg - ) + if topic == "reminder": + message = schema.ReminderV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "calendar.new": + message = schema.CalendarNewV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "calendar.update": + message = schema.CalendarUpdateV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "calendar.upload": + message = schema.CalendarUploadV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "calendar.delete": + message = schema.CalendarDeleteV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "calendar.clear": + message = schema.CalendarClearV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "meeting.new": + message = schema.MeetingNewV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "meeting.update": + message = schema.MeetingUpdateV1( + topic='fedocal.%s' % topic, + body=msg + ) + elif topic == "meeting.delete": + message = schema.MeetingDeleteV1( + topic='fedocal.%s' % topic, + body=msg + ) + else: + _log.warning( + "fedocal is about to send a message that has no schemas" + ) + message = fedora_messaging.api.Message( + topic='fedocal.%s' % topic, + body=msg + ) fedora_messaging.api.publish(message) _log.debug("Sent to fedora_messaging") except PublishReturned as e: diff --git a/requirements.txt b/requirements.txt index 4b90907..48ba10c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,3 +23,4 @@ flask_multistatic flask_oidc fedora-messaging email_validator +fedocal-messages>=1.1.0 diff --git a/tests/test_cron.py b/tests/test_cron.py index 2973a5e..23a5b46 100644 --- a/tests/test_cron.py +++ b/tests/test_cron.py @@ -36,7 +36,8 @@ import os from datetime import timedelta, datetime -from fedora_messaging import api, testing +import fedocal_messages.messages as schema +from fedora_messaging import testing from mock import ANY, patch sys.path.insert(0, os.path.join(os.path.dirname( @@ -180,7 +181,7 @@ class Crontests(Modeltests): self.session.commit() self.assertNotEqual(obj, None) - with testing.mock_sends(api.Message( + with testing.mock_sends(schema.ReminderV1( topic="fedocal.reminder", body={ 'meeting': { @@ -244,7 +245,7 @@ class Crontests(Modeltests): self.session.commit() self.assertNotEqual(obj, None) - with testing.mock_sends(api.Message( + with testing.mock_sends(schema.ReminderV1( topic="fedocal.reminder", body={ 'meeting': { diff --git a/tests/test_flask.py b/tests/test_flask.py index b0947b1..b4c56a2 100644 --- a/tests/test_flask.py +++ b/tests/test_flask.py @@ -42,6 +42,10 @@ from datetime import timedelta import flask import six +import fedocal_messages.messages as schema +from fedora_messaging import testing +from mock import ANY, patch + sys.path.insert(0, os.path.join(os.path.dirname( os.path.abspath(__file__)), '..')) @@ -1024,12 +1028,26 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/calendar/add/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - '
  • Calendar added
    • ', output_text) + with testing.mock_sends(schema.CalendarNewV1( + topic="fedocal.calendar.new", + body={ + 'agent': 'username', + 'calendar': { + 'calendar_name': 'election1', + 'calendar_contact': 'election1', + 'calendar_description': '', + 'calendar_editor_group': '', + 'calendar_admin_group': '', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/calendar/add/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
  • Calendar added
    • ', output_text) # This calendar already exists data = { @@ -1124,18 +1142,32 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/calendar/delete/test_calendar/', - data=data, follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - 'Home - Fedocal', output_text) - self.assertIn( - '
  • Calendar deleted
    • ', - output_text) - self.assertNotIn( - 'test_calendar', - output_text) + with testing.mock_sends(schema.CalendarDeleteV1( + topic="fedocal.calendar.delete", + body={ + 'agent': 'kevin', + 'calendar': { + 'calendar_name': 'test_calendar', + 'calendar_contact': 'test@example.com', + 'calendar_description': 'This is a test calendar', + 'calendar_editor_group': 'fi-apprentice', + 'calendar_admin_group': 'infrastructure-main2', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/calendar/delete/test_calendar/', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Home - Fedocal', output_text) + self.assertIn( + '
  • Calendar deleted
    • ', + output_text) + self.assertNotIn( + 'test_calendar', + output_text) def test_clear_calendar(self): """ Test the clear_calendar function. """ @@ -1217,17 +1249,31 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/calendar/clear/test_calendar/', - data=data, follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - 'test_calendar - Fedocal', output_text) - self.assertIn( - 'test_calendar - Fedocal', output_text) - self.assertIn( - '
  • Calendar cleared
    • ', - output_text) + with testing.mock_sends(schema.CalendarClearV1( + topic="fedocal.calendar.clear", + body={ + 'agent': 'kevin', + 'calendar': { + 'calendar_name': 'test_calendar', + 'calendar_contact': 'test@example.com', + 'calendar_description': 'This is a test calendar', + 'calendar_editor_group': 'fi-apprentice', + 'calendar_admin_group': 'infrastructure-main2', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/calendar/clear/test_calendar/', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'test_calendar - Fedocal', output_text) + self.assertIn( + 'test_calendar - Fedocal', output_text) + self.assertIn( + '
  • Calendar cleared
    • ', + output_text) def test_edit_calendar(self): """ Test the edit_calendar function. """ @@ -1310,21 +1356,35 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/calendar/edit/test_calendar/', - data=data, follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - 'Election1 - Fedocal', output_text) - self.assertIn( - '
  • Calendar updated
    • ', - output_text) - self.assertNotIn( - 'test_calendar', - output_text) - self.assertNotIn( - 'election1', - output_text) + with testing.mock_sends(schema.CalendarUpdateV1( + topic="fedocal.calendar.update", + body={ + 'agent': 'kevin', + 'calendar': { + 'calendar_name': 'Election1', + 'calendar_contact': 'election1', + 'calendar_description': '', + 'calendar_editor_group': '', + 'calendar_admin_group': '', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/calendar/edit/test_calendar/', + data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Election1 - Fedocal', output_text) + self.assertIn( + '
  • Calendar updated
    • ', + output_text) + self.assertNotIn( + 'test_calendar', + output_text) + self.assertNotIn( + 'election1', + output_text) def test_auth_logout(self): """ Test the auth_logout function. """ @@ -1534,16 +1594,43 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/test_calendar/add/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - '
  • Meeting added
    • ', output_text) - self.assertIn( - 'href="/meeting/16/?from_date=', output_text) - self.assertNotIn( - 'href="/meeting/17/?from_date=', output_text) + with testing.mock_sends(schema.MeetingNewV1( + topic="fedocal.meeting.new", + body={ + 'agent': 'pingou', + 'meeting': { + 'meeting_id': 16, + 'meeting_name': 'guess what?', + 'meeting_manager': ['pingou'], + 'meeting_date': TODAY.strftime('%Y-%m-%d'), + 'meeting_date_end': TODAY.strftime('%Y-%m-%d'), + 'meeting_time_start': '13:00:00', + 'meeting_time_stop': '14:00:00', + 'meeting_timezone': 'Europe/Paris', + 'meeting_information': '', + 'meeting_location': '', + 'calendar_name': 'test_calendar' + }, + 'calendar': { + 'calendar_name': 'test_calendar', + 'calendar_contact': 'test@example.com', + 'calendar_description': 'This is a test calendar', + 'calendar_editor_group': 'fi-apprentice', + 'calendar_admin_group': 'infrastructure-main2', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/test_calendar/add/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
  • Meeting added
    • ', output_text) + self.assertIn( + 'href="/meeting/16/?from_date=', output_text) + self.assertNotIn( + 'href="/meeting/17/?from_date=', output_text) # Works - with a wiki_link data = { @@ -1824,14 +1911,41 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/meeting/edit/1/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - '
  • Meeting updated
    • ', output_text) - self.assertIn( - 'Meeting "guess what?" - Fedocal', output_text) + with testing.mock_sends(schema.MeetingUpdateV1( + topic="fedocal.meeting.update", + body={ + 'agent': 'pingou', + 'meeting': { + 'meeting_id': 1, + 'meeting_name': 'guess what?', + 'meeting_manager': ['pingou'], + 'meeting_date': TODAY.strftime('%Y-%m-%d'), + 'meeting_date_end': TODAY.strftime('%Y-%m-%d'), + 'meeting_time_start': '13:00:00', + 'meeting_time_stop': '14:00:00', + 'meeting_timezone': 'Europe/Paris', + 'meeting_information': '', + 'meeting_location': None, + 'calendar_name': 'test_calendar' + }, + 'calendar': { + 'calendar_name': 'test_calendar', + 'calendar_contact': 'test@example.com', + 'calendar_description': 'This is a test calendar', + 'calendar_editor_group': 'fi-apprentice', + 'calendar_admin_group': 'infrastructure-main2', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/meeting/edit/1/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
  • Meeting updated
    • ', output_text) + self.assertIn( + 'Meeting "guess what?" - Fedocal', output_text) # Calendar disabled data = { @@ -2032,14 +2146,41 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/meeting/delete/1/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - 'test_calendar - Fedocal', output_text) - self.assertIn( - '
  • Meeting deleted
    • ', output_text) + with testing.mock_sends(schema.MeetingDeleteV1( + topic="fedocal.meeting.delete", + body={ + 'agent': 'pingou', + 'meeting': { + 'meeting_id': 1, + 'meeting_name': 'Fedora-fr-test-meeting', + 'meeting_manager': [], + 'meeting_date': TODAY.strftime('%Y-%m-%d'), + 'meeting_date_end': TODAY.strftime('%Y-%m-%d'), + 'meeting_time_start': '19:50:00', + 'meeting_time_stop': '20:50:00', + 'meeting_timezone': 'UTC', + 'meeting_information': 'This is a test meeting', + 'meeting_location': None, + 'calendar_name': 'test_calendar' + }, + 'calendar': { + 'calendar_name': 'test_calendar', + 'calendar_contact': 'test@example.com', + 'calendar_description': 'This is a test calendar', + 'calendar_editor_group': 'fi-apprentice', + 'calendar_admin_group': 'infrastructure-main2', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/meeting/delete/1/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'test_calendar - Fedocal', output_text) + self.assertIn( + '
  • Meeting deleted
    • ', output_text) # Delete all data = { @@ -2320,24 +2461,38 @@ class Flasktests(Modeltests): 'enctype': 'multipart/form-data', 'csrf_token': csrf_token, } - output = self.app.post('/calendar/upload/test_calendar/', - follow_redirects=True, data=data) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - if '
  • ' not in output_text: - self.assertIn( - 'test_calendar - Fedocal', - output_text) - self.assertIn( - '

    This is a test calendar

    ', output_text) - self.assertIn( - 'li class="message">Calendar uploaded
    • ', - output_text) - else: - self.assertIn( - '
  • The submitted candidate has the ' - 'MIME type "application/octet-stream" which ' - 'is not an allowed MIME type
    • ', output_text) + with testing.mock_sends(schema.CalendarUploadV1( + topic="fedocal.calendar.upload", + body={ + 'agent': 'kevin', + 'calendar': { + 'calendar_name': 'test_calendar', + 'calendar_contact': 'test@example.com', + 'calendar_description': 'This is a test calendar', + 'calendar_editor_group': 'fi-apprentice', + 'calendar_admin_group': 'infrastructure-main2', + 'calendar_status': 'Enabled' + } + } + )): + output = self.app.post('/calendar/upload/test_calendar/', + follow_redirects=True, data=data) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + if '
  • ' not in output_text: + self.assertIn( + 'test_calendar - Fedocal', + output_text) + self.assertIn( + '

    This is a test calendar

    ', output_text) + self.assertIn( + 'li class="message">Calendar uploaded
    • ', + output_text) + else: + self.assertIn( + '
  • The submitted candidate has the ' + 'MIME type "application/octet-stream" which ' + 'is not an allowed MIME type
    • ', output_text) with open(ICS_FILE_NOTOK, 'rb') as stream: data = { From 04da204e3ac3a567da5fc6289bee1236deadd5cc Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 27 2021 12:49:55 +0000 Subject: [PATCH 4/5] Simplify how dicts are turned into fedora-messaging messages This relies on an utility method added to fedocal-messages in 1.2.0 but we've found a bug in that library which has been fixed in 1.4.0 thus the dependency on 1.4.0+ Signed-off-by: Pierre-Yves Chibon --- diff --git a/fedocal/fedocallib/fedmsgshim.py b/fedocal/fedocallib/fedmsgshim.py index 65ab3e8..a010d68 100644 --- a/fedocal/fedocallib/fedmsgshim.py +++ b/fedocal/fedocallib/fedmsgshim.py @@ -11,6 +11,7 @@ import logging import fedora_messaging.api from fedora_messaging.exceptions import PublishReturned, ConnectionException +import fedocal_messages import fedocal_messages.messages as schema @@ -20,59 +21,18 @@ _log = logging.getLogger(__name__) def publish(topic, msg): # pragma: no cover _log.debug('Publishing a message for %s: %s', topic, msg) try: - if topic == "reminder": - message = schema.ReminderV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "calendar.new": - message = schema.CalendarNewV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "calendar.update": - message = schema.CalendarUpdateV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "calendar.upload": - message = schema.CalendarUploadV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "calendar.delete": - message = schema.CalendarDeleteV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "calendar.clear": - message = schema.CalendarClearV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "meeting.new": - message = schema.MeetingNewV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "meeting.update": - message = schema.MeetingUpdateV1( - topic='fedocal.%s' % topic, - body=msg - ) - elif topic == "meeting.delete": - message = schema.MeetingDeleteV1( - topic='fedocal.%s' % topic, - body=msg - ) - else: + + msg_cls = fedocal_messages.get_message_object_from_topic( + 'fedocal.%s' % topic + ) + + if not hasattr(msg_cls, "app_name") is False: _log.warning( - "fedocal is about to send a message that has no schemas" - ) - message = fedora_messaging.api.Message( - topic='fedocal.%s' % topic, - body=msg + "fedocal is about to send a message that has no schemas: %s", + topic ) + + message = msg_cls(body=msg) fedora_messaging.api.publish(message) _log.debug("Sent to fedora_messaging") except PublishReturned as e: diff --git a/requirements.txt b/requirements.txt index 48ba10c..7312293 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,4 +23,4 @@ flask_multistatic flask_oidc fedora-messaging email_validator -fedocal-messages>=1.1.0 +fedocal-messages>=1.4.0 diff --git a/tests/test_flask.py b/tests/test_flask.py index b4c56a2..02105a7 100644 --- a/tests/test_flask.py +++ b/tests/test_flask.py @@ -1240,11 +1240,13 @@ class Flasktests(Modeltests): output_text = output.get_data(as_text=True) self.assertIn( 'test_calendar - Fedocal', output_text) - self.assertIn( - 'test_calendar - Fedocal', output_text) + self.assertNotIn( + '
  • Calendar cleared
    • ', output_text) # Delete data = { + # html booleans are: arg missing = False, arg present = True + # regardless of the value of the arg passed 'confirm_delete': False, 'csrf_token': csrf_token, } @@ -1270,8 +1272,6 @@ class Flasktests(Modeltests): self.assertIn( 'test_calendar - Fedocal', output_text) self.assertIn( - 'test_calendar - Fedocal', output_text) - self.assertIn( '
  • Calendar cleared
    • ', output_text) @@ -1644,16 +1644,17 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/test_calendar/add/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - '
  • Meeting added
    • ', output_text) - self.assertIn( - 'href="/meeting/17/?from_date=', output_text) - self.assertNotIn( - 'href="/meeting/18/?from_date=', output_text) + with testing.mock_sends(schema.MeetingNewV1): + output = self.app.post('/test_calendar/add/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
  • Meeting added
    • ', output_text) + self.assertIn( + 'href="/meeting/17/?from_date=', output_text) + self.assertNotIn( + 'href="/meeting/18/?from_date=', output_text) # Calendar disabled data = { @@ -1740,20 +1741,21 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/test_calendar/add/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - '
  • Meeting added
    • ', output_text) - self.assertIn( - 'href="/meeting/17/?from_date=', output_text) - self.assertIn( - 'href="/meeting/18/?from_date=', output_text) - self.assertIn( - 'Reminder', output_text) - self.assertNotIn( - 'href="/meeting/19/?from_date=', output_text) + with testing.mock_sends(schema.MeetingNewV1): + output = self.app.post('/test_calendar/add/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
  • Meeting added
    • ', output_text) + self.assertIn( + 'href="/meeting/17/?from_date=', output_text) + self.assertIn( + 'href="/meeting/18/?from_date=', output_text) + self.assertIn( + 'Reminder', output_text) + self.assertNotIn( + 'href="/meeting/19/?from_date=', output_text) # Works - with two emails as recipient of the reminder data = { @@ -1769,20 +1771,21 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/test_calendar/add/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - '
  • Meeting added
    • ', output_text) - self.assertIn( - 'href="/meeting/18/?from_date=', output_text) - self.assertIn( - 'href="/meeting/19/?from_date=', output_text) - self.assertIn( - 'Reminder2', output_text) - self.assertNotIn( - 'href="/meeting/20/?from_date=', output_text) + with testing.mock_sends(schema.MeetingNewV1): + output = self.app.post('/test_calendar/add/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
  • Meeting added
    • ', output_text) + self.assertIn( + 'href="/meeting/18/?from_date=', output_text) + self.assertIn( + 'href="/meeting/19/?from_date=', output_text) + self.assertIn( + 'Reminder2', output_text) + self.assertNotIn( + 'href="/meeting/20/?from_date=', output_text) def test_edit_meeting(self): """ Test the edit_meeting function. """ @@ -2189,14 +2192,15 @@ class Flasktests(Modeltests): 'csrf_token': csrf_token, } - output = self.app.post('/meeting/delete/8/', data=data, - follow_redirects=True) - self.assertEqual(output.status_code, 200) - output_text = output.get_data(as_text=True) - self.assertIn( - 'test_calendar - Fedocal', output_text) - self.assertIn( - '
  • Meeting deleted
    • ', output_text) + with testing.mock_sends(schema.MeetingDeleteV1): + output = self.app.post('/meeting/delete/8/', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'test_calendar - Fedocal', output_text) + self.assertIn( + '
  • Meeting deleted
    • ', output_text) # Add a meeting to the test_calendar_disabled calendar obj = model.Meeting( # id:16 From e959fcdd07e46ceee73d873282df412ff77e256e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Jan 27 2021 12:50:26 +0000 Subject: [PATCH 5/5] Only send a notification on the bus when a calendar has been cleared Before this commit we were sending a notification on the bus regardless of whether the user had confirmed their intent to clear the calendar. With this commit, we're only going to send a notification if we went through with clearing up the calendar. Signed-off-by: Pierre-Yves Chibon --- diff --git a/fedocal/__init__.py b/fedocal/__init__.py index bb3fa94..365e956 100644 --- a/fedocal/__init__.py +++ b/fedocal/__init__.py @@ -1307,10 +1307,10 @@ def clear_calendar(calendar_name): LOG.exception(err) flask.flash(gettext( 'Could not clear this calendar.'), 'errors') - fedmsg.publish(topic="calendar.clear", msg=dict( - agent=flask.g.fas_user.username, - calendar=calendarobj.to_json(), - )) + fedmsg.publish(topic="calendar.clear", msg=dict( + agent=flask.g.fas_user.username, + calendar=calendarobj.to_json(), + )) return flask.redirect(flask.url_for( 'calendar', calendar_name=calendar_name)) return flask.render_template(