[Merge] lp:~artmello/gallery-app/gallery-app-remove_photo_from_album into lp:gallery-app
Chris Gagnon
chris.gagnon at canonical.com
Thu Jun 12 15:40:14 UTC 2014
Review: Needs Fixing
There are some mixed tab/spaces which cause errors and other pep8 issues
1 album_view.py|10 col 1| F401 'os' imported but unused
2 album_view.py|141 col 80| E501 line too long (83 > 79 characters)
3 album_view.py|149 col 80| E501 line too long (84 > 79 characters)
4 album_view.py|155 col 80| E501 line too long (84 > 79 characters)
5 album_view.py|157 col 80| E501 line too long (82 > 79 characters)
6 album_view.py|180 col 5| E101 indentation contains mixed spaces and tabs
7 album_view.py|180 col 5| W191 indentation contains tabs
8 album_view.py|185 col 5| E101 indentation contains mixed spaces and tabs
9 album_view.py|185 col 5| W191 indentation contains tabs
10 album_view.py|190 col 5| E101 indentation contains mixed spaces and tabs
11 album_view.py|190 col 5| W191 indentation contains tabs
12 album_view.py|195 col 5| E101 indentation contains mixed spaces and tabs
13 album_view.py|195 col 5| W191 indentation contains tabs
14 album_view.py|200 col 5| E101 indentation contains mixed spaces and tabs
15 album_view.py|200 col 5| W191 indentation contains tabs
16 album_view.py|205 col 5| E101 indentation contains mixed spaces and tabs
17 album_view.py|205 col 5| W191 indentation contains tabs
18 album_view.py|215 col 80| E501 line too long (87 > 79 characters)
tests/test_album_view.py:11:1: F401 'Is' imported but unused
tests/test_album_view.py:129:13: E128 continuation line under-indented for visual indent
tests/test_album_view.py:149:13: E128 continuation line under-indented for visual indent
tests/test_album_view.py:171:13: E128 continuation line under-indented for visual indent
I've also left an inline code comment for an additional assert
Diff comments:
> === modified file 'rc/qml/MediaViewer/MediaViewer.qml'
> --- rc/qml/MediaViewer/MediaViewer.qml 2014-05-20 15:32:39 +0000
> +++ rc/qml/MediaViewer/MediaViewer.qml 2014-06-12 14:28:19 +0000
> @@ -65,8 +65,8 @@
>
> // tooolbar actions for the full view
> property Item tools: media ? (media.type === MediaSource.Photo ?
> - d.photoToolbar : d.videoToolbar)
> - : null
> + d.photoToolbar : d.videoToolbar)
> + : null
>
> /*!
> */
> @@ -257,7 +257,7 @@
> sharePanel.userAccountId = facebook.id;
> sharePanel.visible = true;
> viewerWrapper.tools.opened = false;
> - _lastPopover = null;
> + _lastPopover = null;
> }
> }
> }
> @@ -311,6 +311,50 @@
> }
> }
>
> + Component {
> + id: removeFromAlbumDialog
> + Dialog {
> + id: dialogue
> + objectName: "removePhotoFromAlbumDialog"
> + title: i18n.tr("Remove a photo from album")
> +
> + function finishRemove() {
> + if (model.count === 0)
> + galleryPhotoViewer.closeRequested();
> + }
> +
> + Button {
> + objectName: "removeFromAlbumButton"
> + text: i18n.tr("Remove from Album")
> + color: Gallery.HIGHLIGHT_BUTTON_COLOR
> + onClicked: {
> + PopupUtils.close(dialogue)
> + viewerWrapper.model.removeMediaFromAlbum(album, galleryPhotoViewer.media);
> + galleryPhotoViewer.currentIndexChanged();
> + dialogue.finishRemove();
> + }
> + }
> +
> + Button {
> + objectName: "removeFromAlbumAndDeleteButton"
> + text: i18n.tr("Remove from Album and Delete")
> + onClicked: {
> + PopupUtils.close(dialogue)
> + viewerWrapper.model.destroyMedia(galleryPhotoViewer.media, true);
> + galleryPhotoViewer.currentIndexChanged();
> + dialogue.finishRemove();
> + }
> + }
> +
> + Button {
> + objectName: "removeFromAlbumCancelButton"
> + text: i18n.tr("Cancel")
> + onClicked: PopupUtils.close(dialogue)
> + }
> + }
> +
> + }
> +
> onCloseRequested: viewerWrapper.closeRequested()
> onEditRequested: viewerWrapper.editRequested(media)
> }
> @@ -488,7 +532,10 @@
> text: i18n.tr("Delete")
> iconSource: "../../img/delete.png"
> onTriggered: {
> - PopupUtils.open(deleteDialog, null);
> + if (album)
> + PopupUtils.open(removeFromAlbumDialog, null);
> + else
> + PopupUtils.open(deleteDialog, null);
> }
> }
> text: i18n.tr("Delete")
> @@ -592,5 +639,4 @@
> }
> }
> }
> -
> }
>
> === modified file 'src/qml/qml-media-collection-model.cpp'
> --- src/qml/qml-media-collection-model.cpp 2014-03-21 18:13:28 +0000
> +++ src/qml/qml-media-collection-model.cpp 2014-06-12 14:28:19 +0000
> @@ -112,6 +112,24 @@
> }
>
> /*!
> + * \brief QmlMediaCollectionModel::removeMediaFromAlbum
> + * \param valbum
> + * \param vmedia
> + */
> +void QmlMediaCollectionModel::removeMediaFromAlbum(QVariant valbum, QVariant vmedia)
> +{
> + Album* album = VariantToObject<Album*>(valbum);
> + if (album == NULL)
> + return;
> +
> + MediaSource* media = VariantToObject<MediaSource*>(vmedia);
> + if (media == NULL)
> + return;
> +
> + album->detach(media);
> +}
> +
> +/*!
> * \brief QmlMediaCollectionModel::monitored
> * \return
> */
>
> === modified file 'src/qml/qml-media-collection-model.h'
> --- src/qml/qml-media-collection-model.h 2014-03-21 18:13:28 +0000
> +++ src/qml/qml-media-collection-model.h 2014-06-12 14:28:19 +0000
> @@ -48,6 +48,7 @@
> Q_INVOKABLE QVariant createAlbumFromSelected();
> Q_INVOKABLE void destroySelectedMedia();
> Q_INVOKABLE void destroyMedia(QVariant vmedia, bool destroy_backing);
> + Q_INVOKABLE void removeMediaFromAlbum(QVariant valbum, QVariant vmedia);
>
> bool monitored() const;
> void setMonitored(bool monitor);
>
> === modified file 'tests/autopilot/gallery_app/emulators/album_view.py'
> --- tests/autopilot/gallery_app/emulators/album_view.py 2014-06-10 19:00:23 +0000
> +++ tests/autopilot/gallery_app/emulators/album_view.py 2014-06-12 14:28:19 +0000
> @@ -5,13 +5,22 @@
> # under the terms of the GNU General Public License version 3, as published
> # by the Free Software Foundation.
>
> -from testtools.matchers import GreaterThan, LessThan
> +import logging
> +import re
> +import os
> +
> +from testtools.matchers import GreaterThan, LessThan, Equals, Is
> +from autopilot.matchers import Eventually
> +
> +from autopilot import logging as autopilot_logging
>
> from gallery_app.emulators.gallery_utils import(
> GalleryUtils,
> GalleryAppException,
> )
>
> +logger = logging.getLogger(__name__)
> +
>
> class AlbumView(GalleryUtils):
> """An emulator class that makes it easy to interact with the gallery app"""
> @@ -41,8 +50,7 @@
>
> def get_album_photo_view(self):
> """Returns the photo view of the album viewer"""
> - view = self.get_album_view()
> - return view.select_single("PopupPhotoViewer")
> + return self.app.select_single("PopupPhotoViewer")
>
> def get_spread_view(self):
> """Returns the inner spread view to access the pages"""
> @@ -126,3 +134,84 @@
> :param page_number: The starting page number you are swiping from
> '''
> self._swipe_setup(page_number, 'right')
> +
> + def _get_remove_from_album_dialog(self):
> + """Returns the photo viewer remove from album dialog."""
> + return self.app.wait_select_single("Dialog",
> + objectName="removePhotoFromAlbumDialog")
> +
> + def _remove_from_album_dialog_shown(self):
> + dialog = self.app.select_many("Dialog",
> + objectName="removePhotoFromAlbumDialog")
> + return len(dialog) >= 1
> +
> + def _get_remove_from_album_popover_remove_item(self):
> + """Returns the remove from album button of the remove from album popover."""
> + return self.app.select_single("Button",
> + objectName="removeFromAlbumButton",
> + visible=True)
> +
> + def _get_remove_from_album_popover_delete_item(self):
> + """Returns the remove and delete button of the remove from album popover."""
> + return self.app.select_single("Button",
> + objectName="removeFromAlbumAndDeleteButton",
> + visible=True)
> +
> + def _get_remove_from_album_popover_cancel_item(self):
> + """Returns the cancel button of the remove from album popover."""
> + return self.app.select_single("Button",
> + objectName="removeFromAlbumCancelButton",
> + visible=True)
> +
> + def _ensure_remove_from_album_dialog_is_open(self):
> + """Ensure that the remove from album dialog is fully opened."""
> + remove_dialog = self._get_remove_from_album_dialog()
> + self.assertThat(remove_dialog.visible, Eventually(Equals(True)))
> + self.assertThat(remove_dialog.opacity, Eventually(Equals(1)))
> +
> + def _ensure_remove_from_album_dialog_is_close(self):
> + """Ensure that the remove from album dialog is fully closed."""
> + self.assertThat(self._remove_from_album_dialog_shown,
> + Eventually(Is(False)))
> +
> + @autopilot_logging.log_action(logger.info)
> + def click_remove_from_album_remove_button(self):
> + """Click on the remove from album button of the remove dialog."""
> + self._ensure_remove_from_album_dialog_is_open()
> +
> + remove_item = self._get_remove_from_album_popover_remove_item()
> + self.pointing_device.click_object(remove_item)
> +
> + self._ensure_remove_from_album_dialog_is_close()
> +
> + @autopilot_logging.log_action(logger.info)
> + def click_remove_from_album_delete_button(self):
> + """Click on the remove and delete of the remove dialog."""
> + self._ensure_remove_from_album_dialog_is_open()
> +
> + delete_item = self._get_remove_from_album_popover_delete_item()
> + self.pointing_device.click_object(delete_item)
> +
> + self._ensure_remove_from_album_dialog_is_close()
> +
> + @autopilot_logging.log_action(logger.info)
> + def click_remove_from_album_cancel_button(self):
> + """Click on the cancel of the remove dialog."""
> + self._ensure_remove_from_album_dialog_is_open()
> +
> + cancel_item = self._get_remove_from_album_popover_cancel_item()
> + self.pointing_device.click_object(cancel_item)
> +
> + self._ensure_remove_from_album_dialog_is_close()
> +
> + @autopilot_logging.log_action(logger.info)
> + def click_first_photo(self):
> + """Click on the first photo of the album and return the path of it."""
> + photo = self.get_first_photo()
> + images = photo.select_many('QQuickImage')
> + path = ''
> + for i in images:
> + if str(i.source).startswith('image://gallery-standard/'):
> + path = re.sub('^image://gallery-standard/', '', i.source).split('?')[0]
> + self.pointing_device.click_object(photo)
> + return path
>
> === modified file 'tests/autopilot/gallery_app/emulators/gallery_utils.py'
> --- tests/autopilot/gallery_app/emulators/gallery_utils.py 2014-05-01 15:27:13 +0000
> +++ tests/autopilot/gallery_app/emulators/gallery_utils.py 2014-06-12 14:28:19 +0000
> @@ -6,14 +6,15 @@
> # by the Free Software Foundation.
> import ubuntuuitoolkit.emulators
>
> +from autopilot.testcase import AutopilotTestCase
> +
> from time import sleep
>
> -
> class GalleryAppException(Exception):
> pass
>
>
> -class GalleryUtils(object):
> +class GalleryUtils(AutopilotTestCase):
> """An emulator class that makes it easy to interact with
> general components of the gallery app."""
>
>
> === modified file 'tests/autopilot/gallery_app/emulators/photo_viewer.py'
> --- tests/autopilot/gallery_app/emulators/photo_viewer.py 2014-05-01 15:27:13 +0000
> +++ tests/autopilot/gallery_app/emulators/photo_viewer.py 2014-06-12 14:28:19 +0000
> @@ -86,7 +86,6 @@
> return self.app.select_single("Button",
> objectName="deletePhotoDialogNo",
> visible=True)
> -
> def get_opened_photo(self):
> """Returns the first opened photo."""
> return self.app.wait_select_single("ZoomablePhotoComponent",
>
> === modified file 'tests/autopilot/gallery_app/tests/test_album_view.py'
> --- tests/autopilot/gallery_app/tests/test_album_view.py 2014-05-22 17:06:01 +0000
> +++ tests/autopilot/gallery_app/tests/test_album_view.py 2014-06-12 14:28:19 +0000
> @@ -8,15 +8,17 @@
>
> """Tests the album view of the gallery app"""
>
> -from testtools.matchers import Equals, GreaterThan, LessThan
> +from testtools.matchers import Equals, GreaterThan, LessThan, Is
> from autopilot.matchers import Eventually
>
> from gallery_app.emulators.album_view import AlbumView
> from gallery_app.emulators.albums_view import AlbumsView
> from gallery_app.emulators.media_selector import MediaSelector
> +from gallery_app.emulators.photo_viewer import PhotoViewer
> from gallery_app.emulators import album_editor
> from gallery_app.tests import GalleryTestCase
>
> +import os
> from time import sleep
> from unittest import skip
>
> @@ -36,6 +38,10 @@
> def media_selector(self):
> return MediaSelector(self.app)
>
> + @property
> + def photo_viewer(self):
> + return PhotoViewer(self.app)
> +
> def setUp(self):
> self.ARGS = []
> super(TestAlbumView, self).setUp()
> @@ -105,6 +111,68 @@
> lambda: self.album_view.number_of_photos(),
> Eventually(Equals(num_photos_start + 1)))
>
> + def test_remove_photo_from_album(self):
> + self.main_view.close_toolbar()
> + self.open_first_album()
> + num_photos_start = self.album_view.number_of_photos()
> + self.assertThat(num_photos_start, Equals(1))
> +
> + path = self.album_view.click_first_photo()
> +
> + photo_view = self.album_view.get_album_photo_view()
> + self.assertThat(photo_view.visible, Eventually(Equals(True)))
> +
> + self.main_view.open_toolbar().click_button("deleteButton")
> + self.album_view.click_remove_from_album_remove_button()
> +
> + self.assertThat(lambda: self.album_view.number_of_photos(),
> + Eventually(Equals(num_photos_start - 1)))
> +
> + self.assertThat(lambda: os.path.exists(path),
> + Eventually(Equals(True)))
> +
> + def test_remove_photo_from_album_and_delete(self):
> + self.main_view.close_toolbar()
> + self.open_first_album()
> + num_photos_start = self.album_view.number_of_photos()
> + self.assertThat(num_photos_start, Equals(1))
> +
> + path = self.album_view.click_first_photo()
> +
we should assert that the path exists here
> + photo_view = self.album_view.get_album_photo_view()
> + self.assertThat(photo_view.visible, Eventually(Equals(True)))
> +
> + self.main_view.open_toolbar().click_button("deleteButton")
> + self.album_view.click_remove_from_album_delete_button()
> +
> + self.assertThat(lambda: self.album_view.number_of_photos(),
> + Eventually(Equals(num_photos_start - 1)))
> +
> + self.assertThat(lambda: os.path.exists(path),
> + Eventually(Equals(False)))
> +
> + def test_cancel_remove_photo_from_album(self):
> + self.main_view.close_toolbar()
> + self.open_first_album()
> + num_photos_start = self.album_view.number_of_photos()
> + self.assertThat(num_photos_start, Equals(1))
> +
> + path = self.album_view.click_first_photo()
> +
> + photo_view = self.album_view.get_album_photo_view()
> + self.assertThat(photo_view.visible, Eventually(Equals(True)))
> +
> + self.main_view.open_toolbar().click_button("deleteButton")
> + self.album_view.click_remove_from_album_cancel_button()
> +
> + self.main_view.open_toolbar().click_button("backButton")
> +
> + self.assertThat(lambda: self.album_view.number_of_photos(),
> + Eventually(Equals(num_photos_start)))
> +
> + self.assertThat(lambda: os.path.exists(path),
> + Eventually(Equals(True)))
> +
> def test_add_photo_to_new_album(self):
> self.main_view.open_toolbar().click_button("addButton")
> self.ui_update()
>
--
https://code.launchpad.net/~artmello/gallery-app/gallery-app-remove_photo_from_album/+merge/222739
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~artmello/gallery-app/gallery-app-remove_photo_from_album into lp:gallery-app.
More information about the Ubuntu-reviews
mailing list