openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Review: Needs Fixing

Traceback (most recent call last):
  File "/home/tim/Projects/OpenLP/openlp/click-slide-to-go-live-from-blank/openlp/core/ui/slidecontroller.py", line 1401, in on_preview_double_click
    elif not Registry().get_flag('has doubleclick added item to service') is True:
  File "/home/tim/Projects/OpenLP/openlp/click-slide-to-go-live-from-blank/openlp/core/common/registry.py", line 160, in get_flag
    raise KeyError('Working Flag {key} not found in list'.format(key=key))
KeyError: 'Working Flag has doubleclick added item to service not found in list'

Blank live and double click on a preview slide.

Diff comments:

> 
> === modified file 'openlp/core/lib/mediamanageritem.py'
> --- openlp/core/lib/mediamanageritem.py	2016-05-17 13:15:53 +0000
> +++ openlp/core/lib/mediamanageritem.py	2016-07-16 15:46:57 +0000
> @@ -488,6 +488,8 @@
>                                                          'You must select one or more items to preview.'))
>          else:
>              log.debug('%s Preview requested' % self.plugin.name)
> +            # Reset the flag for: "has doubleclick added item to service" to False.
> +            Registry().set_flag('has doubleclick added item to service', False)

comment superfluous (not needed)

>              service_item = self.build_service_item()
>              if service_item:
>                  service_item.from_plugin = True
> 
> === modified file 'openlp/core/ui/slidecontroller.py'
> --- openlp/core/ui/slidecontroller.py	2016-05-20 16:22:06 +0000
> +++ openlp/core/ui/slidecontroller.py	2016-07-16 15:46:57 +0000
> @@ -797,12 +798,18 @@
>  
>      def replace_service_manager_item(self, item):
>          """
> -        Replacement item following a remote edit
> +        Replacement item following a remote edit.
> +        This action  also takes place when a song that is sent to live from Service Manager is edited.
> +        If display is blanked, it will get unblanked if automatic unblanking is enabled. We prevent this from happening

Comments are to help but you do not need to write pseudo code next to the real code.

> +        by setting a flag to "True" and then to "False" after the processing is done.
> +        The flag is also set to "False" on startup so display may be unblanked properly.
>  
>          :param item: The current service item
>          """
>          if item == self.service_item:
> +            Registry().set_flag('replace service manager item', True)
>              self._process_item(item, self.preview_widget.current_slide_number())
> +            Registry().set_flag('replace service manager item', False)
>  
>      def add_service_manager_item(self, item, slide_no):
>          """
> @@ -971,9 +978,12 @@
>  
>      def on_slide_unblank(self):
>          """
> -        Handle the slidecontroller unblank event
> +        Handle the slidecontroller unblank event.
> +        If we are re-processing service item, don't unblank the display
> +        (Found in def replace_service_manager_item)

Not needed

>          """
> -        self.on_blank_display(False)
> +        if not Registry().get_flag('replace service manager item') is True:
> +            self.on_blank_display(False)
>  
>      def on_blank_display(self, checked=None):
>          """
> @@ -1104,6 +1114,14 @@
>                  self.log_debug('Could not get lock in slide_selected after waiting %f, skip to avoid deadlock.'
>                                 % timeout)
>              return
> +        # If "click live slide to unblank" is enabled, unblank the display. And start = Item is sent to Live.
> +        # Note: If this if statement is placed at the bottom of this function instead of top slide transitions are lost.
> +        if self.is_live and Settings().value('core/click live slide to unblank'):
> +            # With this display stays blanked when "auto unblank" setting is not enabled and new item is sent to Live.
> +            if not Settings().value('core/auto unblank') and start:
> +                ()

??????

> +            if not start:
> +                Registry().execute('slidecontroller_live_unblank')
>          row = self.preview_widget.current_slide_number()
>          old_selected_row = self.selected_row
>          self.selected_row = 0
> 
> === modified file 'tests/functional/openlp_core_ui/test_slidecontroller.py'
> --- tests/functional/openlp_core_ui/test_slidecontroller.py	2016-06-01 21:42:54 +0000
> +++ tests/functional/openlp_core_ui/test_slidecontroller.py	2016-07-16 15:46:57 +0000
> @@ -713,6 +713,54 @@
>              slide_controller.theme_screen, slide_controller.blank_screen
>          ])
>  
> +    @patch('openlp.core.ui.slidecontroller.Settings')
> +    def on_preview_double_click_unblank_display_test(self, MockedSettings):
> +        # GIVEN: A slide controller, actions needed, settins set to True.
> +        slide_controller = SlideController(None)
> +        mocked_settings = MagicMock()
> +        mocked_settings.return_value = True
> +        MockedSettings.return_value = mocked_settings
> +        slide_controller.service_item = MagicMock()
> +        slide_controller.service_item.is_media = MagicMock()
> +        slide_controller.on_media_close = MagicMock()
> +        slide_controller.on_go_live = MagicMock()
> +        slide_controller.on_preview_add_to_service = MagicMock()
> +        slide_controller.media_reset = MagicMock()
> +        Registry.create()
> +        reg_value = True

Why do we need reg_value.  just set to true in the call

> +        Registry().set_flag('has doubleclick added item to service', reg_value)
> +
> +        # WHEN: on_preview_double_click is called
> +        slide_controller.on_preview_double_click()
> +
> +        # THEN: The call to addActions should be correct
> +        self.assertEqual(1, slide_controller.on_go_live.call_count, 'on_go_live should have been called once.')
> +        self.assertEqual(0, slide_controller.on_preview_add_to_service.call_count, 'Should have not been called.')
> +
> +    @patch('openlp.core.ui.slidecontroller.Settings')
> +    def on_preview_double_click_add_to_service_test(self, MockedSettings):
> +        # GIVEN: A slide controller, actions needed, settins set to False.
> +        slide_controller = SlideController(None)
> +        mocked_settings = MagicMock()
> +        mocked_settings.value.return_value = False
> +        MockedSettings.return_value = mocked_settings
> +        slide_controller.service_item = MagicMock()
> +        slide_controller.service_item.is_media = MagicMock()
> +        slide_controller.on_media_close = MagicMock()
> +        slide_controller.on_go_live = MagicMock()
> +        slide_controller.on_preview_add_to_service = MagicMock()
> +        slide_controller.media_reset = MagicMock()
> +        Registry.create()
> +        reg_value = False
> +        Registry().set_flag('has doubleclick added item to service', reg_value)
> +
> +        # WHEN: on_preview_double_click is called
> +        slide_controller.on_preview_double_click()
> +
> +        # THEN: The call to addActions should be correct
> +        self.assertEqual(0, slide_controller.on_go_live.call_count, 'on_go_live Should have not been called.')
> +        self.assertEqual(1, slide_controller.on_preview_add_to_service.call_count, 'Should have been called once.')
> +
>      @patch(u'openlp.core.ui.slidecontroller.SlideController.image_manager')
>      @patch(u'PyQt5.QtCore.QTimer.singleShot')
>      def test_update_preview_live(self, mocked_singleShot, mocked_image_manager):


-- 
https://code.launchpad.net/~suutari-olli/openlp/click-slide-to-go-live-from-blank/+merge/300257
Your team OpenLP Core is subscribed to branch lp:openlp.