From 181b4fdcc9673bb626ad36422dba93397841c4ee Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Fri, 15 May 2020 22:28:21 -0700 Subject: [PATCH] Fix various issues and return current theme in API: - New API method to return current theme - Use single method to set service items via both position and uuid - Update tests to match above new features - Fix issue where chords were not being returned to the API - Fix the clear/backspace icon (the old one doesn't exist in Debian) --- .pre-commit-config.yaml | 5 +++ openlp/core/api/__init__.py | 8 ++++ openlp/core/api/versions/v2/controller.py | 18 ++++++++- openlp/core/api/versions/v2/service.py | 38 +++++++------------ openlp/core/api/websockets.py | 20 +++++++--- openlp/core/app.py | 2 +- openlp/core/common/utils.py | 9 +++++ openlp/core/lib/serviceitem.py | 3 +- openlp/core/ui/icons.py | 7 ++-- .../openlp_core/api/v2/test_controller.py | 7 +++- .../openlp_core/common/test_httputils.py | 2 +- .../openlp_core/lib/test_pluginmanager.py | 2 +- tests/openlp_core/common/test_utils.py | 17 ++++++++- 13 files changed, 96 insertions(+), 42 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..e3c468e6f --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,5 @@ +repos: + - repo: https://gitlab.com/pycqa/flake8 + rev: 3.8.1 + hooks: + - id: flake8 diff --git a/openlp/core/api/__init__.py b/openlp/core/api/__init__.py index 0b8b3c015..42a6782e5 100644 --- a/openlp/core/api/__init__.py +++ b/openlp/core/api/__init__.py @@ -18,6 +18,7 @@ # You should have received a copy of the GNU General Public License # # along with this program. If not, see . # ########################################################################## +import logging from flask import Flask from flask_cors import CORS @@ -26,6 +27,7 @@ from openlp.core.api.versions import v1 from openlp.core.api.versions import v2 from openlp.core.api.main import main_views +log = logging.getLogger(__name__) app = Flask(__name__) CORS(app) @@ -36,3 +38,9 @@ v2.register_blueprints(app) def register_blueprint(blueprint, url_prefix=None): app.register_blueprint(blueprint, url_prefix) + + +@app.errorhandler(500) +def internal_server_error(error): + log.error('Unhandled HTTP error: {error}'.format(error=error)) + return 'Internal server error', 500 diff --git a/openlp/core/api/versions/v2/controller.py b/openlp/core/api/versions/v2/controller.py index e94a55d84..4d70a6589 100644 --- a/openlp/core/api/versions/v2/controller.py +++ b/openlp/core/api/versions/v2/controller.py @@ -50,9 +50,9 @@ def controller_text_api(): if current_item.is_text(): if frame['verse']: item['tag'] = str(frame['verse']) - item['chords_text'] = str(frame.get('chords_text', '')) item['text'] = frame['text'] - item['html'] = current_item.get_rendered_frame(index) + item['html'] = current_item.rendered_slides[index]['text'] + item['chords'] = current_item.rendered_slides[index]['chords'] elif current_item.is_image() and not frame.get('image', '') and \ Registry().get('settings_thread').value('api/thumbnails'): thumbnail_path = os.path.join('images', 'thumbnails', frame['title']) @@ -176,6 +176,20 @@ def get_themes(): return jsonify(theme_list) +@controller_views.route('/theme', methods=['GET']) +@login_required +def get_theme(): + """ + Get the current theme + """ + theme_level = Registry().get('settings').value('themes/theme level') + if theme_level == ThemeLevel.Service: + theme = Registry().get('settings').value('servicemanager/service theme') + else: + theme = Registry().get('settings').value('themes/global theme') + return jsonify(theme) + + @controller_views.route('/theme', methods=['POST']) @login_required def set_theme(): diff --git a/openlp/core/api/versions/v2/service.py b/openlp/core/api/versions/v2/service.py index 057984e29..cb4cb0752 100644 --- a/openlp/core/api/versions/v2/service.py +++ b/openlp/core/api/versions/v2/service.py @@ -19,11 +19,12 @@ # along with this program. If not, see . # ########################################################################## import logging -from openlp.core.api.lib import login_required from flask import jsonify, request, abort, Blueprint +from openlp.core.api.lib import login_required from openlp.core.common.registry import Registry +from openlp.core.common.utils import is_uuid service_views = Blueprint('service', __name__) @@ -57,35 +58,24 @@ def service_items(): @service_views.route('/show', methods=['POST']) +@service_views.route('/show/', methods=['POST']) @login_required -def service_set(): +def service_set(item_id=None): data = request.json - if not data: - log.error('Missing request data') + item_id = item_id or data.get('id') or data.get('uid') + if not item_id: + log.error('Missing item id') abort(400) + if is_uuid(item_id): + Registry().get('service_manager').servicemanager_set_item_by_uuid.emit(item_id) + return '', 204 try: - id = int(data.get('id', -1)) + id = int(item_id) + Registry().get('service_manager').servicemanager_set_item.emit(id) + return '', 204 except ValueError: - log.error('Invalid data passed ' + data) + log.error('Invalid item id: ' + item_id) abort(400) - Registry().get('service_manager').servicemanager_set_item.emit(id) - return '', 204 - - -@service_views.route('/show_id', methods=['POST']) -@login_required -def service_set_by_uid(): - data = request.json - if not data: - log.error('Missing request data') - abort(400) - try: - id = str(data.get('uid', '')) - except ValueError: - log.error('Invalid data passed ' + data) - abort(400) - Registry().get('service_manager').servicemanager_set_item_by_uuid.emit(id) - return '', 204 @service_views.route('/progress', methods=['POST']) diff --git a/openlp/core/api/websockets.py b/openlp/core/api/websockets.py index 0b271fdf5..f053cfc1c 100644 --- a/openlp/core/api/websockets.py +++ b/openlp/core/api/websockets.py @@ -104,12 +104,20 @@ class WebSocketWorker(ThreadWorker, RegistryProperties, LogMixin): """ Stop the websocket server """ - if hasattr(self.server, 'ws_server'): - self.server.ws_server.close() - elif hasattr(self.server, 'server'): - self.server.server.close() - self.event_loop.stop() - self.event_loop.close() + try: + if hasattr(self.server, 'ws_server'): + self.server.ws_server.close() + elif hasattr(self.server, 'server'): + self.server.server.close() + except RuntimeError: + # Sometimes it is already closed + pass + try: + self.event_loop.stop() + self.event_loop.close() + except RuntimeError: + # Sometimes it is already closed + pass class WebSocketServer(RegistryProperties, LogMixin): diff --git a/openlp/core/app.py b/openlp/core/app.py index 61b54707b..cddc888da 100644 --- a/openlp/core/app.py +++ b/openlp/core/app.py @@ -430,7 +430,7 @@ def main(): translators = LanguageManager.get_translators(language) for translator in translators: if not translator.isEmpty(): - app.installTranslator(translator) + application.installTranslator(translator) if not translators: log.debug('Could not find translators.') if args and not args.no_error_form: diff --git a/openlp/core/common/utils.py b/openlp/core/common/utils.py index bcc68cb3d..20b3ba6b8 100644 --- a/openlp/core/common/utils.py +++ b/openlp/core/common/utils.py @@ -22,6 +22,7 @@ The :mod:`~openlp.core.display.window` module contains the display window """ import logging +import re import time from openlp.core.common.registry import Registry @@ -47,3 +48,11 @@ def wait_for(check_func, error_message='Timed out waiting for completion', timeo time.sleep(0.001) app.process_events() return success + + +def is_uuid(uuid): + if not isinstance(uuid, (str, bytes)): + return False + return ( + re.match(r'^[0-9A-F]{8}-[0-9A-F]{4}-[14][0-9A-F]{3}-[0-9A-F]{4}-[0-9A-F]{12}$', uuid, re.IGNORECASE) is not None + ) diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index b995ae500..2765a452f 100644 --- a/openlp/core/lib/serviceitem.py +++ b/openlp/core/lib/serviceitem.py @@ -221,8 +221,9 @@ class ServiceItem(RegistryProperties): rendered_slide = { 'title': raw_slide['title'], 'text': render_tags(page), + 'chords': render_tags(page, can_render_chords=True), 'verse': index, - 'footer': self.footer_html, + 'footer': self.footer_html } self._rendered_slides.append(rendered_slide) display_slide = { diff --git a/openlp/core/ui/icons.py b/openlp/core/ui/icons.py index 49ee464be..e99511bf3 100644 --- a/openlp/core/ui/icons.py +++ b/openlp/core/ui/icons.py @@ -64,7 +64,8 @@ class UiIcons(metaclass=Singleton): 'authentication': {'icon': 'fa.exclamation-triangle', 'attr': 'red'}, 'address': {'icon': 'fa.book'}, 'back': {'icon': 'fa.step-backward'}, - 'backspace': {'icon': 'fa-times-circle'}, + 'backspace': {'icon': 'fa.times'}, + # 'backspace': {'icon': 'fa.caret-square-o-left'}, 'bible': {'icon': 'fa.book'}, 'blank': {'icon': 'fa.times-circle'}, 'blank_theme': {'icon': 'fa.file-image-o'}, @@ -181,10 +182,10 @@ class UiIcons(metaclass=Singleton): else: setattr(self, key, qta.icon(icon)) except Exception: - import sys - log.error('Unexpected error: %s' % sys.exc_info()) + log.exception('Unexpected error for icon: {icon}'.format(icon=icon)) setattr(self, key, qta.icon('fa.exclamation-circle', color='red')) except Exception: + log.exception('Unexpected error for icon: {icon}'.format(icon=icon)) setattr(self, key, qta.icon('fa.exclamation-circle', color='red')) @staticmethod diff --git a/tests/functional/openlp_core/api/v2/test_controller.py b/tests/functional/openlp_core/api/v2/test_controller.py index 8221a60c2..9fb84515d 100644 --- a/tests/functional/openlp_core/api/v2/test_controller.py +++ b/tests/functional/openlp_core/api/v2/test_controller.py @@ -104,9 +104,12 @@ def test_controller_get_themes_retrieves_themes_list(flask_client, settings): assert type(res) is list -def test_controller_set_theme_does_not_accept_get(flask_client): +def test_controller_get_theme_returns_current_theme(flask_client, settings): + Registry().get('settings').setValue('themes/theme level', 1) + Registry().get('settings').setValue('themes/global theme', 'Default') res = flask_client.get('/api/v2/controller/theme') - assert res.status_code == 405 + assert res.status_code == 200 + assert res.get_json() == 'Default' def test_controller_set_theme_aborts_if_no_theme(flask_client, settings): diff --git a/tests/functional/openlp_core/common/test_httputils.py b/tests/functional/openlp_core/common/test_httputils.py index 49ea37964..422b30373 100644 --- a/tests/functional/openlp_core/common/test_httputils.py +++ b/tests/functional/openlp_core/common/test_httputils.py @@ -101,7 +101,7 @@ def test_get_random_user_agent_default(): user_agent = get_random_user_agent() # THEN: The user agent is a Linux (or ChromeOS) user agent - assert 'NetBSD'in user_agent, 'The user agent should be the default user agent' + assert 'NetBSD' in user_agent, 'The user agent should be the default user agent' def test_get_web_page_no_url(): diff --git a/tests/interfaces/openlp_core/lib/test_pluginmanager.py b/tests/interfaces/openlp_core/lib/test_pluginmanager.py index fa9bc71c2..06517dbee 100644 --- a/tests/interfaces/openlp_core/lib/test_pluginmanager.py +++ b/tests/interfaces/openlp_core/lib/test_pluginmanager.py @@ -95,5 +95,5 @@ class TestPluginManager(TestCase, TestMixin): assert 'images' in plugin_names, 'There should be a "images" plugin' assert 'media' in plugin_names, 'There should be a "media" plugin' assert 'custom' in plugin_names, 'There should be a "custom" plugin' - assert 'songusage'in plugin_names, 'There should be a "songusage" plugin' + assert 'songusage' in plugin_names, 'There should be a "songusage" plugin' assert 'alerts' in plugin_names, 'There should be a "alerts" plugin' diff --git a/tests/openlp_core/common/test_utils.py b/tests/openlp_core/common/test_utils.py index 77b0d81a7..825e0a123 100644 --- a/tests/openlp_core/common/test_utils.py +++ b/tests/openlp_core/common/test_utils.py @@ -24,7 +24,7 @@ Interface tests to test the themeManager class and related methods. from unittest.mock import MagicMock from openlp.core.common.registry import Registry -from openlp.core.common.utils import wait_for +from openlp.core.common.utils import wait_for, is_uuid def test_wait_for(registry): @@ -42,3 +42,18 @@ def test_wait_for(registry): # THEN: the functions got called assert mock_func.call_count == 2 + + +def test_uuid_not_str(): + """Test that a non-string UUID returns False""" + assert not is_uuid(10), 'is_uuid() should return False when given something that is not a string' + + +def test_uuid_not_valid(): + """Test that an invalid string UUID returns False""" + assert not is_uuid('this is a string'), 'is_uuid() should return False when given an invalid string' + + +def test_uuid_valid(): + """Test that an valid string UUID returns True""" + assert is_uuid('2596ac84-9735-11ea-a665-8fa61362d04a'), 'is_uuid() should return True when given a valid UUID'