Merge lp:~trb143/openlp/allow_local_stage_views into lp:openlp

[Tim Bentley]

Allow Custom stage views.

Under the config directory create a stages directory.
Within that create user directories i.e trb143 then stage/trb143 will server the stage.html file from within that directory.
If you wish to change the css file amend the files/stage.css in the html file to stages/trb143/yourcss.css.
Create a yourcss.css file and it will get loaded.

lp:~trb143/openlp/allow_local_stage_views (revision 2575)
[SUCCESS] https//ci.openlp.io/job/Branch-01-Pull/1187/
[SUCCESS] https//ci.openlp.io/job/Branch-02-Functional-Tests/1110/
[SUCCESS] https//ci.openlp.io/job/Branch-03-Interface-Tests/1051/
[SUCCESS] https//ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/898/
[SUCCESS] https//ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/494/
[SUCCESS] https//ci.openlp.io/job/Branch-05a-Code_Analysis/610/
[SUCCESS] https//ci.openlp.io/job/Branch-05b-Test_Coverage/481/

[Raoul Snyman (raoul-snyman)]

Why not just have a single file called "remote/stage/custom.css" or "remote/stage.css" ?

review: Needs Information

[Tim Bentley (trb143)]

Do you mean we have

remotes
---Certificate files
---trb143
------stage,html
------stage
-----------stage.css
---superfly
------stage,html
------stage
-----------stage.css

[Raoul Snyman (raoul-snyman)]

I don't understand why we need the username files. This is already in the users home directory. Why not just use stage.css?

[Tim Bentley (trb143)]

This is to allow custom stage views per user. All the current files in inside core and we have requests to allow them to be changeable.

This would allow the html and the css to be customised for a user and have any number of customisations.

The current files are served from within the plugin so cannot be changed (without a hack).

[Tomas Groth (tomasgroth)]

I'm convinced :)

review: Approve

[Charles Crossan (crossan007)]

The sound of distant cheering is heard by the whole group.
On Nov 29, 2015 8:21 AM, "Tomas Groth" <email address hidden> wrote:

> Review: Approve
>
> I'm convinced :)
> --
>
> https://code.launchpad.net/~trb143/openlp/allow_local_stage_views/+merge/276940
> You are subscribed to branch lp:openlp.
>

[Raoul Snyman (raoul-snyman)]

But there's only ever one user, so it makes no sense.

[Raoul Snyman (raoul-snyman)]

I understand custom stylesheets, and I'm totally for it, but there's only ever one user, so why the username?

[Tim Bentley (trb143)]

It's an example it could be

remotes
---Certificate files
---drums
------stage,html
------stage
-----------stage.css
---bass
------stage,html
------stage
-----------stage.css
---worshipleader
------stage,html
------stage
-----------stage.css

[Raoul Snyman (raoul-snyman)]

So when would each custom style be used? We only support one at a time at the moment.

[Tim Bentley (trb143)]

We support two in core.
/Stage is a stage view
/Main is a picture view.

These can be used by any number of people.
This change allows customisations of view also we have people who want to change the look and feel and this is not possible with the current set up.

[Raoul Snyman (raoul-snyman)]

OK, I kinda get it. I still don't really see the point of multiple directories. I don't think the majority of people will ever use more than just one.

You have a method called "temp_path" (ugh, bad name) in your method signature, but you never actually use it in your method?

    def stages(self, temp_path, file_name)

Also, no print statements!

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

review: Approve

=== modified file 'openlp/plugins/remotes/lib/httprouter.py'
--- openlp/plugins/remotes/lib/httprouter.py	2015-10-22 16:23:40 +0000
+++ openlp/plugins/remotes/lib/httprouter.py	2015-12-05 12:49:02 +0000
@@ -150,6 +150,7 @@
         self.routes = [
             ('^/$', {'function': self.serve_file, 'secure': False}),
             ('^/(stage)$', {'function': self.serve_file, 'secure': False}),
+            ('^/(stage)/(.*)$', {'function': self.stages, 'secure': False}),
             ('^/(main)$', {'function': self.serve_file, 'secure': False}),
             (r'^/files/(.*)$', {'function': self.serve_file, 'secure': False}),
             (r'^/(\w+)/thumbnails([^/]+)?/(.*)$', {'function': self.serve_thumbnail, 'secure': False}),
@@ -170,6 +171,7 @@
         self.settings_section = 'remotes'
         self.translate()
         self.html_dir = os.path.join(AppLocation.get_directory(AppLocation.PluginsDir), 'remotes', 'html')
+        self.config_dir = os.path.join(AppLocation.get_data_path(), 'stages')
 
     def do_post_processor(self):
         """
@@ -340,24 +342,32 @@
             'settings': translate('RemotePlugin.Mobile', 'Settings'),
         }
 
-    def serve_file(self, file_name=None):
+    def stages(self, url_path, file_name):
         """
-        Send a file to the socket. For now, just a subset of file types and must be top level inside the html folder.
-        If subfolders requested return 404, easier for security for the present.
+        Allow Stage view to be delivered with custom views.
 
-        Ultimately for i18n, this could first look for xx/file.html before falling back to file.html.
-        where xx is the language, e.g. 'en'
+        :param url_path: base path of the URL. Not used but passed by caller
+        :param file_name: file name with path
+        :return:
         """
         log.debug('serve file request %s' % file_name)
-        if not file_name:
-            file_name = 'index.html'
-        elif file_name == 'stage':
-            file_name = 'stage.html'
-        elif file_name == 'main':
-            file_name = 'main.html'
-        path = os.path.normpath(os.path.join(self.html_dir, file_name))
-        if not path.startswith(self.html_dir):
+        parts = file_name.split('/')
+        if len(parts) == 1:
+            file_name = os.path.join(parts[0], 'stage.html')
+        elif len(parts) == 3:
+            file_name = os.path.join(parts[1], parts[2])
+        path = os.path.normpath(os.path.join(self.config_dir, file_name))
+        if not path.startswith(self.config_dir):
             return self.do_not_found()
+        return self._process_file(path)
+
+    def _process_file(self, path):
+        """
+        Common file processing code
+
+        :param path: path to file to be loaded
+        :return: web resource to be loaded
+        """
         content = None
         ext, content_type = self.get_content_type(path)
         file_handle = None
@@ -380,10 +390,32 @@
         self.end_headers()
         return content
 
+    def serve_file(self, file_name=None):
+        """
+        Send a file to the socket. For now, just a subset of file types and must be top level inside the html folder.
+        If subfolders requested return 404, easier for security for the present.
+
+        Ultimately for i18n, this could first look for xx/file.html before falling back to file.html.
+        where xx is the language, e.g. 'en'
+        """
+        log.debug('serve file request %s' % file_name)
+        if not file_name:
+            file_name = 'index.html'
+        elif file_name == 'stage':
+            file_name = 'stage.html'
+        elif file_name == 'main':
+            file_name = 'main.html'
+        path = os.path.normpath(os.path.join(self.html_dir, file_name))
+        if not path.startswith(self.html_dir):
+            return self.do_not_found()
+        return self._process_file(path)
+
     def get_content_type(self, file_name):
         """
         Examines the extension of the file and determines what the content_type should be, defaults to text/plain
         Returns the extension and the content_type
+
+        :param file_name: name of file
         """
         ext = os.path.splitext(file_name)[1]
         content_type = FILE_TYPES.get(ext, 'text/plain')
@@ -392,6 +424,10 @@
     def serve_thumbnail(self, controller_name=None, dimensions=None, file_name=None):
         """
         Serve an image file. If not found return 404.
+
+        :param file_name: file name to be served
+        :param dimensions: image size
+        :param controller_name: controller to be called
         """
         log.debug('serve thumbnail %s/thumbnails%s/%s' % (controller_name, dimensions, file_name))
         supported_controllers = ['presentations', 'images']
@@ -496,6 +532,8 @@
     def controller_text(self, var):
         """
         Perform an action on the slide controller.
+
+        :param var: variable - not used
         """
         log.debug("controller_text var = %s" % var)
         current_item = self.live_controller.service_item
@@ -629,6 +667,8 @@
     def go_live(self, plugin_name):
         """
         Go live on an item of type ``plugin``.
+
+        :param plugin_name: name of plugin
         """
         try:
             request_id = json.loads(self.request_data)['request']['id']
@@ -642,6 +682,8 @@
     def add_to_service(self, plugin_name):
         """
         Add item of type ``plugin_name`` to the end of the service.
+
+        :param plugin_name: name of plugin to be called
         """
         try:
             request_id = json.loads(self.request_data)['request']['id']

=== modified file 'openlp/plugins/remotes/lib/httpserver.py'
--- openlp/plugins/remotes/lib/httpserver.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/remotes/lib/httpserver.py	2015-12-05 12:49:02 +0000
@@ -167,7 +167,7 @@
         local_data = AppLocation.get_directory(AppLocation.DataDir)
         self.socket = ssl.SSLSocket(
             sock=socket.socket(self.address_family, self.socket_type),
-            ssl_version=ssl.PROTOCOL_TLSv1,
+            ssl_version=ssl.PROTOCOL_TLSv1_2,
             certfile=os.path.join(local_data, 'remotes', 'openlp.crt'),
             keyfile=os.path.join(local_data, 'remotes', 'openlp.key'),
             server_side=True)

=== modified file 'tests/functional/openlp_plugins/remotes/test_router.py'
--- tests/functional/openlp_plugins/remotes/test_router.py	2015-11-25 21:47:56 +0000
+++ tests/functional/openlp_plugins/remotes/test_router.py	2015-12-05 12:49:02 +0000
@@ -342,3 +342,39 @@
 
             # THEN: service_manager.next_item() should have been called
             self.assertTrue(mocked_previous_item.called, 'previous_item() should have been called in service_manager')
+
+    def remote_stage_personal_html_test(self):
+        """
+        Test the stage url with a parameter after loaded a url/stage.html file
+        """
+        # GIVEN: initial route
+        self.router.config_dir = ''
+        self.router.send_response = MagicMock()
+        self.router.send_header = MagicMock()
+        self.router.end_headers = MagicMock()
+        self.router.wfile = MagicMock()
+        self.router._process_file = MagicMock()
+
+        # WHEN: I call stage with a suffix
+        self.router.stages('stages', 'trb')
+
+        # THEN: we should use the specific stage file instance
+        self.router._process_file.assert_called_with(os.path.join('trb', 'stage.html'))
+
+    def remote_stage_personal_css_test(self):
+        """
+        Test the html with reference stages/trb/trb.css then loaded a stages/trb/trb.css file
+        """
+        # GIVEN: initial route
+        self.router.config_dir = ''
+        self.router.send_response = MagicMock()
+        self.router.send_header = MagicMock()
+        self.router.end_headers = MagicMock()
+        self.router.wfile = MagicMock()
+        self.router._process_file = MagicMock()
+
+        # WHEN: I call stage with a suffix
+        self.router.stages('stages', 'stages/trb/trb.css')
+
+        # THEN: we should use the specific stage file instance
+        self.router._process_file.assert_called_with(os.path.join('trb', 'trb.css'))