openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

Needs a test just a minor fix is not enough 
Some inline comments.

Diff comments:

> 
> === modified file 'openlp/plugins/bibles/forms/editbibleform.py'
> --- openlp/plugins/bibles/forms/editbibleform.py	2016-05-21 08:31:24 +0000
> +++ openlp/plugins/bibles/forms/editbibleform.py	2016-09-18 20:29:10 +0000
> @@ -61,10 +61,24 @@
>          """
>          log.debug('Load Bible')
>          self.bible = bible
> -        self.version_name_edit.setText(self.manager.get_meta_data(self.bible, 'name').value)
> -        self.copyright_edit.setText(self.manager.get_meta_data(self.bible, 'copyright').value)
> -        self.permissions_edit.setText(self.manager.get_meta_data(self.bible, 'permissions').value)
> -        book_name_language = self.manager.get_meta_data(self.bible, 'book_name_language')
> +        """

This is an all or nothing approach.
Each field should be in it's own try block so if it can be loaded it will.
book_name_language should not be in the try block

> +        Try loading the metadata, if the field does not exist in the metadata, continue executing the code,
> +        missing fields will be created on "self.accept" (save). Also set "book_name_language",
> +        there would otherwise be a traceback for reference before assignment.
> +        """
> +        try:
> +            self.version_name_edit.setText(self.manager.get_meta_data(self.bible, 'name').value)
> +            self.version_name_edit.setPlaceholderText(UiStrings().RequiredShowInFooter)
> +            self.copyright_edit.setText(self.manager.get_meta_data(self.bible, 'copyright').value)
> +            self.copyright_edit.setPlaceholderText(UiStrings().RequiredShowInFooter)
> +            self.permissions_edit.setText(self.manager.get_meta_data(self.bible, 'permissions').value)
> +            self.permissions_edit.setPlaceholderText(UiStrings().OptionalShowInFooter)
> +            self.full_license_edit.setPlainText(self.manager.get_meta_data(self.bible, 'full_license').value)
> +            self.full_license_edit.setPlaceholderText(UiStrings().OptionalHideInFooter)
> +            book_name_language = self.manager.get_meta_data(self.bible, 'book_name_language')
> +        except AttributeError:
> +            book_name_language = self.manager.get_meta_data(self.bible, 'book_name_language')
> +            pass
>          if book_name_language and book_name_language.value != 'None':
>              self.language_selection_combo_box.setCurrentIndex(int(book_name_language.value) + 1)
>          self.books = {}
> 
> === modified file 'openlp/plugins/bibles/lib/manager.py'
> --- openlp/plugins/bibles/lib/manager.py	2016-09-04 16:15:57 +0000
> +++ openlp/plugins/bibles/lib/manager.py	2016-09-18 20:29:10 +0000
> @@ -376,17 +376,17 @@
>          else:
>              return None
>  
> -    def save_meta_data(self, bible, version, copyright, permissions, book_name_language=None):
> +    def save_meta_data(self, bible, version, copyright, permissions, full_license, book_name_language=None):
>          """
>          Saves the bibles meta data.
>          """
> -        log.debug('save_meta data {bible}, {version}, {copyright}, {perms}'.format(bible=bible,
> -                                                                                   version=version,
> -                                                                                   copyright=copyright,
> -                                                                                   perms=permissions))
> +        log.debug('save_meta data {bible}, {version}, {copyright},'

why change the formatting

> +                  ' {perms}, {full_license}'.format(bible=bible, version=version, copyright=copyright,
> +                                                    perms=permissions, full_license=full_license))
>          self.db_cache[bible].save_meta('name', version)
>          self.db_cache[bible].save_meta('copyright', copyright)
>          self.db_cache[bible].save_meta('permissions', permissions)
> +        self.db_cache[bible].save_meta('full_license', full_license)
>          self.db_cache[bible].save_meta('book_name_language', book_name_language)
>  
>      def get_meta_data(self, bible, key):


-- 
https://code.launchpad.net/~suutari-olli/openlp/add-bible-license-field/+merge/306048
Your team OpenLP Core is subscribed to branch lp:openlp.