Standardize plugin search options

openlp/core/api/versions/v2/plugins.py

@@ -54,6 +54,22 @@ def add(plugin_name, id):
         getattr(plugin.media_item, '{action}_add_to_service'.format(action=plugin_name)).emit([item_id, True])
 
 
+def get_options(plugin_name):
+    plugin = Registry().get('plugin_manager').get_plugin_by_name(plugin_name)
+    if plugin.status == PluginStatus.Active and plugin.media_item:
+        options = plugin.media_item.search_options()
+        return options
+    return []
+
+
+def set_option(plugin_name, search_option, value):
+    plugin = Registry().get('plugin_manager').get_plugin_by_name(plugin_name)
+    success = False
+    if plugin.status == PluginStatus.Active and plugin.media_item:
+        success = plugin.media_item.set_search_option(search_option, value)
+    return success
+
+
 @plugins.route('/<plugin>/search')
 @login_required
 def search_view(plugin):
@@ -93,13 +109,7 @@ def search_options(plugin):
     Get the plugin's search options
     """
     log.debug(f'{plugin}/search-options called')
-    if plugin == 'bibles':
+    return jsonify(get_options(plugin))
-        bible_plugin = Registry().get('bible_plugin')
-        bibles = list(bible_plugin.manager.get_bibles().keys())
-        primary = Registry().get('settings').value('bibles/primary bible')
-        return jsonify(primary=primary, bibles=bibles)
-    else:
-        return '', 501
 
 
 @plugins.route('/<plugin>/search-options', methods=['POST'])
@@ -110,22 +120,17 @@ def set_search_option(plugin):
     """
     log.debug(f'{plugin}/search-options-set called')
     data = request.json
-    option = ''
     if not data:
         log.error('Missing request data')
         abort(400)
-    elif type(data.get('option')) is not (str or int):
+    option = data.get('option', None)
-        abort(400)
+    value = data.get('value', None)
-    try:
+    if value is None:
-        option = data.get('option')
+        log.error('Invalid data passed in value')
-    except ValueError:
-        log.error('Invalid data passed: ' + option)
         abort(400)
 
-    if plugin == 'bibles':
+    if set_option(plugin, option, value):
-        Registry().get('settings').setValue('bibles/primary bible', option)
-        Registry().execute('populate_bible_combo_boxes')
         return '', 204
     else:
-        log.error('Unimplemented method')
+        log.error('Invalid option or value')
-        return '', 501
+        return '', 400

openlp/core/lib/mediamanageritem.py

@@ -700,6 +700,27 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties):
             if item:
                 self.auto_select_id = item.data(QtCore.Qt.UserRole)
 
+    def search_options(self, option=None):
+        """
+        Returns a list of search options and values for bibles
+        Must return in this format:
+        [{name:'<option_name>',list:[<possible values...>],selected:<selected_value>}]
+
+        :param option: Can be set to an option to only return that option
+        """
+        # By default plugins have no search options
+        return []
+
+    def set_search_option(self, search_option, value):
+        """
+        Sets a search option
+
+        :param search_option: The option to be set
+        :param value: The new value for the search option
+        :return: True if the search_option was successfully set
+        """
+        return False
+
     def search(self, string, show_error=True):
         """
         Performs a plugin specific search for items containing ``string``

openlp/plugins/bibles/lib/mediaitem.py

@@ -1002,6 +1002,39 @@ class BibleMediaItem(MediaManagerItem):
         }[self.settings_tab.display_style]
         return '{{su}}{bracket[0]}{verse_text}{bracket[1]}{{/su}} '.format(verse_text=verse_text, bracket=bracket)
 
+    def search_options(self, option=None):
+        """
+        Returns a list of search options and values for bibles
+
+        :param option: Can be set to an option to only return that option
+        """
+        if (option is not None and option != 'primary bible'):
+            return []
+        bibles = list(self.plugin.manager.get_bibles().keys())
+        primary = Registry().get('settings').value('bibles/primary bible')
+        return [
+            {
+                'name': 'primary bible',
+                'list': bibles,
+                'selected': primary
+            }
+        ]
+
+    def set_search_option(self, search_option, value):
+        """
+        Sets a search option
+
+        :param search_option: The option to be set
+        :param value: The new value for the search option
+        :return: True if the search_option was successfully set
+        """
+        if search_option == 'primary bible' and value in self.search_options('primary bible')[0]['list']:
+            Registry().get('settings').setValue('bibles/primary bible', value)
+            Registry().execute('populate_bible_combo_boxes')
+            return True
+        else:
+            return False
+
     def search(self, string, show_error=True):
         """
         Search for some Bible verses (by reference).

tests/functional/openlp_core/api/v2/test_plugins.py

@@ -21,40 +21,70 @@
 from unittest.mock import MagicMock
 
 from openlp.core.common.registry import Registry
+from openlp.core.common.enum import PluginStatus
 
 
 # Search options tests
-def test_bibles_search_options_returns_bibles_list(flask_client, settings):
+def test_bibles_search_options_returns_list(flask_client, settings):
-    Registry().register('bible_plugin', MagicMock())
+    """
+    Test a list is returned when a plugin's search options are requested.
+
+    Returns an empty list here because the plugin is mocked so there are no options.
+    """
+    # GIVEN: A mocked plugin
+    mock_plugin_manager = MagicMock()
+    mock_plugin_manager.get_plugin_by_name.return_value = MagicMock()
+    Registry().register('plugin_manager', mock_plugin_manager)
+
+    # WHEN: Search options are requested
     res = flask_client.get('/api/v2/plugins/bibles/search-options').get_json()
-    assert res.get('primary') == ""
+
-    assert type(res.get('bibles')) is list
+    # THEN: A empty list should be returned
+    assert res == []
 
 
 def test_bibles_set_search_options_sets_bible_version(flask_client, settings):
-    Registry().register('bible_plugin', MagicMock())
+    """
-    res = flask_client.post('/api/v2/plugins/bibles/search-options', json=dict(option='foo'))
+    Test that a search option post request sends the correct values to the plugin,
+    and returns the correct status code.
+    """
+    # GIVEN: A mocked bible plugin
+    mock_plugin_manager = MagicMock()
+    mock_bible_plugin = MagicMock()
+    mock_bible_plugin.status = PluginStatus.Active
+    mock_bible_plugin.media_item = MagicMock(set_search_option=MagicMock(return_value=True))
+    mock_plugin_manager.get_plugin_by_name.return_value = mock_bible_plugin
+    Registry().register('plugin_manager', mock_plugin_manager)
+
+    # WHEN: The primary bible is requested to change via the api
+    res = flask_client.post('/api/v2/plugins/bibles/search-options', json=dict(option='primary bible', value='foo'))
+
+    # THEN: Returns correct status code and sends correct values to plugin.
     assert res.status_code == 204
-    assert Registry().get('settings').value('bibles/primary bible') == 'foo'
+    mock_bible_plugin.media_item.set_search_option.assert_called_once_with('primary bible', 'foo')
 
 
 def test_plugin_set_search_option_aborts_if_no_option(flask_client, settings):
+    Registry().register('plugin_manager', MagicMock())
     res = flask_client.post('/api/v2/plugins/songs/search-options')
     assert res.status_code == 400
 
 
 def test_plugin_set_search_option_aborts_if_invalid_option(flask_client, settings):
+    Registry().register('plugin_manager', MagicMock())
     res1 = flask_client.post('/api/v2/plugins/songs/search-options', json=dict(option=['']))
     res2 = flask_client.post('/api/v2/plugins/songs/search-options', json=dict(option={1: '', 2: ''}))
     assert res1.status_code == 400
     assert res2.status_code == 400
 
 
-def test_plugin_search_options_returns_plugin_exception(flask_client, settings):
+def test_plugin_search_options_returns_no_search_options(flask_client, settings):
+    Registry().register('plugin_manager', MagicMock())
     res = flask_client.get('/api/v2/plugins/songs/search-options')
-    assert res.status_code == 501
+    assert res.status_code == 200
 
 
 def test_plugin_set_search_option_returns_plugin_exception(flask_client, settings):
+    Registry().register('plugin_manager', MagicMock())
     res = flask_client.post('/api/v2/plugins/songs/search-options', json=dict(option=''))
-    assert res.status_code == 501
+    assert res.status_code == 400

tests/functional/openlp_core/lib/test_mediamanageritem.py

@@ -118,3 +118,31 @@ def test_on_double_clicked_single_click_preview(mocked_on_preview_click, mocked_
     # THEN: on_live_click() should have been called
     assert 0 == mocked_on_live_click.call_count, 'on_live_click() should not have been called'
     assert 0 == mocked_on_preview_click.call_count, 'on_preview_click() should not have been called'
+
+
+def test_search_options(media_env):
+    """
+    Test that the default search options return a empty list
+    """
+    # GIVEN: A media manager item
+    mmi = MediaManagerItem(None)
+
+    # WHEN: The search options are requested
+    result = mmi.search_options()
+
+    # THEN: should return a empty list
+    assert result == []
+
+
+def test_set_search_option(media_env):
+    """
+    Test that the default set search option return false because it's setting a non existant setting
+    """
+    # GIVEN: A media manager item
+    mmi = MediaManagerItem(None)
+
+    # WHEN: The a search option is set
+    result = mmi.set_search_option('my option', 'my value')
+
+    # THEN: should return false
+    assert result is False

tests/functional/openlp_plugins/bibles/test_mediaitem.py

@@ -1508,3 +1508,75 @@ def test_display_results_results(media_item):
         # THEN: addItem should have been with the display items
         mocked_add_built_results_to_list_widget.assert_called_once_with(
             [{'item_title': 'Title 1'}, {'item_title': 'Title 2'}])
+
+
+def test_search_options(media_item):
+    """
+    Test that the bible search options are returned
+    """
+    # GIVEN: An instance of BibleMediaItem, and mocked primary bible setting
+    media_item.plugin.manager.get_bibles.return_value = MagicMock(keys=MagicMock(return_value=['a', 'b', 'c']))
+    media_item.settings.value = lambda key: {'bibles/primary bible': 'b'}[key]
+
+    # WHEN: calling search_options
+    result = media_item.search_options()
+
+    # THEN: result should be correct
+    assert result == [{'name': 'primary bible', 'list': ['a', 'b', 'c'], 'selected': 'b'}]
+
+
+def test_set_search_option(media_item):
+    """
+    Test that the bible set search option function works
+    """
+    # GIVEN: An instance of BibleMediaItem, with a mocked get_bibles and a mocked populate_bible_combo_boxes function
+    media_item.plugin.manager.get_bibles.return_value = MagicMock(keys=MagicMock(return_value=['a', 'b', 'c']))
+    populate_bible_combo_boxes = MagicMock()
+    Registry().remove_function('populate_bible_combo_boxes', media_item.populate_bible_combo_boxes)
+    Registry().register_function('populate_bible_combo_boxes', populate_bible_combo_boxes)
+
+    # WHEN: calling set_search_option
+    result = media_item.set_search_option('primary bible', 'c')
+
+    # THEN: result should be correct and the new bible set, and combo boxes updated
+    assert result is True
+    media_item.settings.setValue.assert_called_once_with('bibles/primary bible', 'c')
+    populate_bible_combo_boxes.assert_called_once()
+
+
+def test_set_search_option_invalid_option(media_item):
+    """
+    Test that the bible set search option function rejects when using non existing option
+    """
+    # GIVEN: An instance of BibleMediaItem, with a mocked get_bibles and a mocked populate_bible_combo_boxes function
+    media_item.plugin.manager.get_bibles.return_value = MagicMock(keys=MagicMock(return_value=['a', 'b', 'c']))
+    populate_bible_combo_boxes = MagicMock()
+    Registry().remove_function('populate_bible_combo_boxes', media_item.populate_bible_combo_boxes)
+    Registry().register_function('populate_bible_combo_boxes', populate_bible_combo_boxes)
+
+    # WHEN: calling set_search_option with invalid option
+    result = media_item.set_search_option('not an option', 'a')
+
+    # THEN: result should be False and nothing set, and combo boxes not updated
+    assert result is False
+    media_item.settings.setValue.assert_not_called()
+    populate_bible_combo_boxes.assert_not_called()
+
+
+def test_set_search_option_invalid_value(media_item):
+    """
+    Test that the bible set search option function rejects when using non existing value
+    """
+    # GIVEN: An instance of BibleMediaItem, with a mocked get_bibles and a mocked populate_bible_combo_boxes function
+    media_item.plugin.manager.get_bibles.return_value = MagicMock(keys=MagicMock(return_value=['a', 'b', 'c']))
+    populate_bible_combo_boxes = MagicMock()
+    Registry().remove_function('populate_bible_combo_boxes', media_item.populate_bible_combo_boxes)
+    Registry().register_function('populate_bible_combo_boxes', populate_bible_combo_boxes)
+
+    # WHEN: calling set_search_option with invalid value
+    result = media_item.set_search_option('primary bible', 'not a value')
+
+    # THEN: result should be False and nothing set, and combo boxes not updated
+    assert result is False
+    media_item.settings.setValue.assert_not_called()
+    populate_bible_combo_boxes.assert_not_called()