openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Review: Needs Fixing

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!

Diff comments:

> === 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-11-08 21:30:50 +0000
> @@ -340,24 +342,33 @@
>              'settings': translate('RemotePlugin.Mobile', 'Settings'),
>          }
>  
> -    def serve_file(self, file_name=None):
> +    def stages(self, temp_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 temp_path: base path of the URL
> +        :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:
> +            print(parts)

No print statements!

> +            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


-- 
https://code.launchpad.net/~trb143/openlp/allow_local_stage_views/+merge/276940
Your team OpenLP Core is subscribed to branch lp:openlp.