gallery-app

Merge lp:~artmello/gallery-app/gallery-app-cancel_album into lp:gallery-app

  1. gallery-app-cancel_album
  2. Merge into trunk
Proposed by Arthur Mello
Status: Merged
Approved by: Gustavo Pichorim Boiko
Approved revision: 956
Merged at revision: 956
Proposed branch: lp:~artmello/gallery-app/gallery-app-cancel_album
Merge into: lp:gallery-app
Diff against target: 169 lines (+49/-0)
5 files modified
rc/qml/AlbumEditor/AlbumEditor.qml (+6/-0)
src/album/album.cpp (+26/-0)
src/album/album.h (+5/-0)
src/qml/qml-album-collection-model.cpp (+1/-0)
tests/autopilot/gallery_app/tests/test_albums_view.py (+11/-0)
To merge this branch: bzr merge lp:~artmello/gallery-app/gallery-app-cancel_album
Reviewer Review Type Date Requested Status
Gustavo Pichorim Boiko (community) Approve
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+215471@code.launchpad.net

Commit message

Make gallery does not create a new album if process is canceled

Description of the change

Make gallery does not create a new album if process is canceled

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:953
http://jenkins.qa.ubuntu.com/job/gallery-app-ci/792/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-amd64-ci/217
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-armhf-ci/218
        deb: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-armhf-ci/218/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-i386-ci/217
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-trusty-touch/59
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4802
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/84
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-trusty-armhf/70
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5973
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4148
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4934
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4934/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gallery-app-ci/792/rebuild

review: Needs Fixing (continuous-integration)
lp:~artmello/gallery-app/gallery-app-cancel_album updated
954. By Arthur Mello

Add autopilot test to check if cancelling an album just after adding it does not add the album

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:954
http://jenkins.qa.ubuntu.com/job/gallery-app-ci/794/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-amd64-ci/219
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-armhf-ci/220
        deb: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-armhf-ci/220/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-i386-ci/219
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-trusty-touch/61
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4804
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/86
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-trusty-armhf/72
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5977
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4150
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4936
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4936/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gallery-app-ci/794/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Arthur Mello (artmello) wrote :

Are there any related MPs required for this MP to build/function as expected? Please list:
No

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes):
Yes

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?:
Yes

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/gallery-app) on device or emulator?
Yes

If you changed the UI, was the change specified/approved by design?
No UI changes.

If you changed the packaging (debian), did you subscribe a core-dev to this MP?
No packaging changes.

Revision history for this message
Gustavo Pichorim Boiko (boiko) wrote :

51 + * \brief Album::isClosed
52 + * \return
53 + */
54 +bool Album::isNewAlbum() const

Can you just fix the comment, and maybe write a real brief description of what the method means?

review: Needs Fixing
Revision history for this message
Gustavo Pichorim Boiko (boiko) wrote :

67 + * \brief Album::setNewAlbum
68 + * \param closed
69 + */

And also here.

review: Needs Fixing
lp:~artmello/gallery-app/gallery-app-cancel_album updated
955. By Arthur Mello

Fix comment errors
Initialize m_newAlbum correctly

956. By Arthur Mello

Add comments to the changes

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:956
http://jenkins.qa.ubuntu.com/job/gallery-app-ci/795/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-amd64-ci/220
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-armhf-ci/221
        deb: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-armhf-ci/221/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/gallery-app-trusty-i386-ci/220
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-trusty-touch/63
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4842
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-click-autopilot-runner-mako/88
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-click-builder-trusty-armhf/74
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6041
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/4185
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4979
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4979/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gallery-app-ci/795/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Gustavo Pichorim Boiko (boiko) wrote :

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?
Yes

Did CI run pass? If not, please explain why.
No, but not related to the changes on this MR.

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?
Yes

Code looks good and works as expected.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'rc/qml/AlbumEditor/AlbumEditor.qml'
2--- rc/qml/AlbumEditor/AlbumEditor.qml 2014-04-02 22:29:32 +0000
3+++ rc/qml/AlbumEditor/AlbumEditor.qml 2014-04-15 13:28:07 +0000
4@@ -40,6 +40,8 @@
5 text: i18n.tr("Cancel")
6 width: units.gu(10)
7 onClicked: {
8+ if(album.newAlbum)
9+ albumModel.destroyAlbum(album);
10 albumEditor.closeRequested(albumEditor.album, false);
11 }
12 }
13@@ -204,9 +206,13 @@
14 onAddClicked: {
15 var album = albumEditor.album;
16 var firstPhoto = album.addSelectedMediaSources(selection.model.selectedMedias);
17+ if(album.newAlbum && selection.model.selectedCount > 0)
18+ album.newAlbum = false;
19 }
20
21 onHidden: {
22+ if(album.newAlbum)
23+ albumModel.destroyAlbum(album);
24 albumEditor.closeRequested(albumEditor.album, true);
25 }
26 }
27
28=== modified file 'src/album/album.cpp'
29--- src/album/album.cpp 2013-07-03 11:53:48 +0000
30+++ src/album/album.cpp 2014-04-15 13:28:07 +0000
31@@ -51,6 +51,7 @@
32 m_albumTemplate(0),
33 m_title(DEFAULT_TITLE),
34 m_subtitle(DEFAULT_SUBTITLE),
35+ m_newAlbum(false),
36 m_albumTable(0)
37 {
38 initInstance();
39@@ -75,6 +76,7 @@
40 m_albumTemplate(0),
41 m_title(title),
42 m_subtitle(subtitle),
43+ m_newAlbum(false),
44 m_albumTable(0)
45 {
46 initInstance();
47@@ -356,6 +358,16 @@
48 }
49
50 /*!
51+ * \brief Album::isNewAlbum Check if the album is beeing created, that changes
52+ * the behavior of some options on the gallery interface
53+ * \return
54+ */
55+bool Album::isNewAlbum() const
56+{
57+ return m_newAlbum;
58+}
59+
60+/*!
61 * \brief Album::totalPageCount
62 * \return
63 */
64@@ -465,6 +477,20 @@
65 }
66
67 /*!
68+ * \brief Album::setNewAlbum Set if the album is beeing created, that changes
69+ * the behavior of some options on the gallery interface
70+ * \param newAlbum
71+ */
72+void Album::setNewAlbum(bool newAlbum)
73+{
74+ if (m_newAlbum == newAlbum)
75+ return;
76+
77+ m_newAlbum = newAlbum;
78+ emit newAlbumChanged();
79+}
80+
81+/*!
82 * \brief Album::setId
83 * \param id
84 */
85
86=== modified file 'src/album/album.h'
87--- src/album/album.h 2013-12-17 11:27:36 +0000
88+++ src/album/album.h 2014-04-15 13:28:07 +0000
89@@ -75,6 +75,7 @@
90 Q_PROPERTY(int currentPage READ currentPage WRITE setCurrentPage
91 NOTIFY currentPageChanged)
92 Q_PROPERTY(bool closed READ isClosed WRITE setClosed NOTIFY closedChanged)
93+ Q_PROPERTY(bool newAlbum READ isNewAlbum WRITE setNewAlbum NOTIFY newAlbumChanged)
94 Q_PROPERTY(QString coverNickname READ coverNickname WRITE setCoverNickname
95 NOTIFY coverNicknameChanged)
96
97@@ -86,6 +87,7 @@
98 void titleChanged();
99 void subtitleChanged();
100 void closedChanged();
101+ void newAlbumChanged();
102 void contentPagesChanged();
103 void pageCountChanged();
104 void coverNicknameChanged();
105@@ -122,6 +124,7 @@
106 AlbumTemplate* albumTemplate() const;
107 void setAlbumTemplate(AlbumTemplate *albumTemplate);
108 bool isClosed() const;
109+ bool isNewAlbum() const;
110 int totalPageCount() const;
111 int contentPageCount() const;
112 int populatedContentPageCount() const;
113@@ -133,6 +136,7 @@
114 int currentPage() const;
115 void setCurrentPage(int page);
116 void setClosed(bool closed);
117+ void setNewAlbum(bool newAlbum);
118 void setId(qint64 id);
119 qint64 id() const;
120 QString coverNickname() const;
121@@ -177,6 +181,7 @@
122 QDateTime m_creationDateDime;
123 int m_currentPage; // Page number of the left page of the current spread.
124 bool m_closed;
125+ bool m_newAlbum;
126 int m_populatedPagesCount;
127 SourceCollection* m_contentPages;
128 QList<MediaSource*> m_allMediaSources;
129
130=== modified file 'src/qml/qml-album-collection-model.cpp'
131--- src/qml/qml-album-collection-model.cpp 2013-07-03 11:53:48 +0000
132+++ src/qml/qml-album-collection-model.cpp 2014-04-15 13:28:07 +0000
133@@ -82,6 +82,7 @@
134 QVariant QmlAlbumCollectionModel::createOrphan()
135 {
136 Album *album = new Album();
137+ album->setNewAlbum(true);
138 album->setAlbumTable(GalleryManager::instance()->database()->getAlbumTable());
139 album->setAlbumTemplate(GalleryManager::instance()->albumDefaultTemplate());
140 return QVariant::fromValue(album);
141
142=== modified file 'tests/autopilot/gallery_app/tests/test_albums_view.py'
143--- tests/autopilot/gallery_app/tests/test_albums_view.py 2014-03-24 11:01:01 +0000
144+++ tests/autopilot/gallery_app/tests/test_albums_view.py 2014-04-15 13:28:07 +0000
145@@ -16,6 +16,7 @@
146
147 from gallery_app.tests import GalleryTestCase
148 from gallery_app.emulators.albums_view import AlbumsView
149+from gallery_app.emulators import album_editor
150
151 from time import sleep
152 from os import environ as env
153@@ -55,6 +56,16 @@
154 self.assertThat(lambda: self.albums_view.number_of_albums_in_albums_view(),
155 Eventually(Equals(albums+1)))
156
157+ def test_add_album_and_cancel(self):
158+ """Add one album, cancel it and checks if the number of albums does not change"""
159+ albums = self.albums_view.number_of_albums_in_albums_view()
160+ self.main_view.open_toolbar().click_button("addButton")
161+ editor = self.app.select_single(album_editor.AlbumEditorAnimated)
162+ editor.ensure_fully_open()
163+ self.main_view.get_toolbar().click_custom_button("cancelButton")
164+ self.assertThat(lambda: self.albums_view.number_of_albums_in_albums_view(),
165+ Eventually(Equals(albums)))
166+
167 # Check if Camera Button is not visible at Desktop mode
168 def test_camera_button_visible(self):
169 self.main_view.open_toolbar()

Subscribers

People subscribed via source and target branches