Merge lp:~ubuntu-calendar-dev/ubuntu-calendar-app/revised-fix-for-bug1334883-dev into lp:ubuntu-calendar-app

[Jason]

A revised fix for Launchpad Bug 1334883 in the Ubuntu Calendar App.

A revised fix for Launchpad Bug 1334883 in the Ubuntu Calendar App. In this bug, the 'delete' button appeared to have no effect as events appeared to remain on the DayView after being deleted.

This was happening because the destroyAllChildren() function, which is responsible for cleaning up existing events on the DayView was not getting triggered when the model changed (due to an event being removed).

To reproduce this bug (without this branch):
1) Navigate to the DayView
2) Create an event.
3) Click on the Event to see its EventDetails page
4) Hit the 'Trash Can'/Delete icon.
5) You should be taken back to the DayView, but the event is still there.

With this branch, the behavior is changed to
5') You are taken back to the DayView, and the event is deleted.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:360
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~ubuntu-calendar-dev/ubuntu-calendar-app/revised-fix-for-bug1334883-dev/+merge/228960/+edit-commit-message

http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/653/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1215/console
    FAILURE: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/211/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/653/rebuild

review: Needs Fixing (continuous-integration)

[Nicholas Skaggs (nskaggs)]

I think this is ready to roll. The bug we discovered is noted in the tests, and we can write a new test for it when we merge the fix.

https://bugs.launchpad.net/ubuntu-calendar-app/+bug/1350605

review: Approve

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:362
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/654/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1216
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1216/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/212

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/654/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/ubuntu-calendar-app-autolanding/249/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1217
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1217/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-autolanding/69

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:363
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/655/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1218
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1218/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/213

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/655/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:363
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/659/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1223
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1223/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/217

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/659/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:364
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/660/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1224
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1224/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/218

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/660/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/ubuntu-calendar-app-autolanding/250/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1226
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1226/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-autolanding/70

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

review: Approve (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Autolanding.
Unapproved changes made after approval.
http://91.189.93.70:8080/job/ubuntu-calendar-app-autolanding/251/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1228
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1228/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-autolanding/71

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:365
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/662/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1230
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1230/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/220

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/662/rebuild

review: Approve (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

review: Approve (continuous-integration)

=== modified file 'TimeLineBaseComponent.qml'
--- TimeLineBaseComponent.qml	2014-07-12 02:08:13 +0000
+++ TimeLineBaseComponent.qml	2014-07-31 17:11:55 +0000
@@ -136,9 +136,11 @@
 
                         model: mainModel
                         Component.onCompleted: {
+                            model.addModelChangeListener(destroyAllChildren);
                             model.addModelChangeListener(createEvents);
                         }
                         Component.onDestruction: {
+                            model.removeModelChangeListener(destroyAllChildren);
                             model.removeModelChangeListener(createEvents);
                         }
                     }

=== modified file 'tests/autopilot/calendar_app/emulators.py'
--- tests/autopilot/calendar_app/emulators.py	2014-07-23 18:17:21 +0000
+++ tests/autopilot/calendar_app/emulators.py	2014-07-31 17:11:55 +0000
@@ -240,14 +240,16 @@
 
     """Autopilot helper for the Day View page."""
 
-    def get_events(self, filter_duplicates=False):
+    @autopilot.logging.log_action(logger.info)
+    def get_events(self, visible=True):
         """Return the events for this day.
 
+        :param visible: toggles filtering for only visible events
         :return: A list with the events. Each event is a tuple with name, start
            time and end time.
 
         """
-        event_bubbles = self._get_selected_day_event_bubbles(filter_duplicates)
+        event_bubbles = self._get_selected_day_event_bubbles()
 
         # sort by y, x
         event_bubbles = sorted(
@@ -256,10 +258,40 @@
 
         events = []
         for event in event_bubbles:
-            events.append(event.get_information())
+            # Event-bubbles objects are recycled, only show visible ones.
+            if visible:
+                if event.visible:
+                    events.append(event.get_information())
+            else:
+                events.append(event.get_information())
 
         return events
 
+    @autopilot.logging.log_action(logger.info)
+    def get_event(self, event_name, visible=True):
+        """Return a specific event from current day.
+
+        :param visible: toggles filtering for only visible events
+        :param event_name: the name of the event.
+            If more than one name matches, return the first matching event
+        :return: The event object
+        """
+        event_bubbles = self._get_selected_day_event_bubbles()
+
+        # sort by y, x
+        event_bubbles = sorted(
+            event_bubbles,
+            key=lambda bubble: (bubble.globalRect.y, bubble.globalRect.x))
+
+        for event in event_bubbles:
+            # Event-bubbles objects are recycled, only show visible ones.
+            if event.get_name() == event_name:
+                if (visible and event.visible) or not visible:
+                    matched_event = event
+                    return matched_event
+
+        raise CalendarException('No event found for %s' % event_name)
+
     def _get_current_day_component(self):
         components = self.select_many('TimeLineBaseComponent')
         for component in components:
@@ -270,19 +302,12 @@
             raise CalendarException(
                 'Could not find the current day component.')
 
-    def _get_selected_day_event_bubbles(self, filter_duplicates):
+    def _get_selected_day_event_bubbles(self):
         selected_day = self._get_current_day_component()
-        return self._get_event_bubbles(selected_day, filter_duplicates)
+        return self._get_event_bubbles(selected_day)
 
-    def _get_event_bubbles(self, selected_day, filter_duplicates):
+    def _get_event_bubbles(self, selected_day):
         event_bubbles = selected_day.select_many(EventBubble)
-        if filter_duplicates:
-            # XXX remove this once bug http://pad.lv/1334833 is fixed.
-            # --elopio - 2014-06-26
-            separator_id = selected_day.select_single(
-                'QQuickRectangle', objectName='separator').id
-            event_bubbles = self._remove_duplicate_events(
-                separator_id, event_bubbles)
         return event_bubbles
 
     def _remove_duplicate_events(self, separator_id, event_bubbles):
@@ -294,14 +319,14 @@
         return events
 
     @autopilot.logging.log_action(logger.info)
-    def open_event(self, name, filter_duplicates=False):
+    def open_event(self, name):
         """Open an event.
 
         :param name: The name of the event to open.
         :return: The Event Details page.
 
         """
-        event_bubbles = self._get_selected_day_event_bubbles(filter_duplicates)
+        event_bubbles = self._get_selected_day_event_bubbles()
         for bubble in event_bubbles:
             if bubble.get_name() == name:
                 return bubble.open_event()
@@ -310,14 +335,14 @@
                 'Could not find event with name {}.'.format(name))
 
     @autopilot.logging.log_action(logger.info)
-    def delete_event(self, name, filter_duplicates=False):
+    def delete_event(self, name):
         """Delete an event.
 
         :param name: The name of the event to delete.
         :return: The Day View page.
 
         """
-        event_details_page = self.open_event(name, filter_duplicates)
+        event_details_page = self.open_event(name)
         return event_details_page.delete()
 
     @autopilot.logging.log_action(logger.info)

=== modified file 'tests/autopilot/calendar_app/tests/test_new_event.py'
--- tests/autopilot/calendar_app/tests/test_new_event.py	2014-07-23 18:19:42 +0000
+++ tests/autopilot/calendar_app/tests/test_new_event.py	2014-07-31 17:11:55 +0000
@@ -21,7 +21,7 @@
 import logging
 
 from autopilot.matchers import Eventually
-from testtools.matchers import HasLength
+from testtools.matchers import Equals, NotEquals
 
 from calendar_app import data
 from calendar_app.tests import CalendarTestCase
@@ -39,13 +39,44 @@
     # http://pad.lv/1328600 on Autopilot.
     # --elopio - 2014-06-26
 
-    def try_delete_event(self, event_name, filter_duplicates):
+    def _try_delete_event(self, event_name):
         try:
             day_view = self.main_view.go_to_day_view()
-            day_view.delete_event(event_name, filter_duplicates)
+            day_view.delete_event(event_name)
         except Exception as exception:
             logger.warn(str(exception))
 
+    def _add_event(self):
+        test_event = data.Event.make_unique()
+        day_view = self.main_view.go_to_day_view()
+
+        new_event_page = self.main_view.go_to_new_event()
+        new_event_page.add_event(test_event)
+
+        # workaround bug 1350605
+        day_view = self._workaround_bug_1350605()
+
+        return day_view, test_event
+
+    def _event_exists(self, event_name):
+        try:
+            day_view = self.main_view.go_to_day_view()
+            day_view.get_event(event_name, False)
+        except Exception:
+            return False
+        return True
+
+    def _workaround_bug_1350605(self):
+        # due to bug 1350605, let's force load another view
+        # before returning to dayview to prevent refresh issues
+        self.main_view.go_to_month_view()
+        day_view = self.main_view.go_to_day_view()
+        return day_view
+
+    # TODO, add test to check events are displayed properly
+    # after multiple operations
+    # https://bugs.launchpad.net/ubuntu-calendar-app/+bug/1350605
+
     def test_add_new_event_with_default_values(self):
         """Test adding a new event with the default values.
 
@@ -53,40 +84,25 @@
         with an end time, without recurrence and without reminders.
 
         """
-        test_event = data.Event.make_unique()
-
-        day_view = self.main_view.go_to_day_view()
-        original_events = day_view.get_events()
-
-        new_event_page = self.main_view.go_to_new_event()
-        # XXX remove this once bug http://pad.lv/1334833 is fixed.
-        # --elopio - 2014-06-26
-        filter_duplicates = len(original_events) > 0
-        self.addCleanup(
-            self.try_delete_event, test_event.name, filter_duplicates)
-        day_view = new_event_page.add_event(test_event)
-
-        def get_new_events():
-            return day_view.get_events(filter_duplicates)
-
-        self.assertThat(
-            get_new_events, Eventually(HasLength(len(original_events) + 1)))
+        day_view, test_event = self._add_event()
+
+        self.addCleanup(self._try_delete_event, test_event.name)
+
+        event_bubble = lambda: day_view.get_event(test_event.name)
+        self.assertThat(event_bubble, Eventually(NotEquals(None)))
+
         event_details_page = day_view.open_event(test_event.name)
-        self.assertEqual(
-            test_event, event_details_page.get_event_information())
+
+        self.assertEqual(test_event,
+                         event_details_page.get_event_information())
 
     def test_delete_event_must_remove_it_from_day_view(self):
         """Test deleting an event must no longer show it on the day view."""
-        # TODO remove the skip once the bug is fixed. --elopio - 2014-06-26
-        self.skipTest('This test fails because of bug http://pad.lv/1334883')
-        event = data.Event.make_unique()
-
-        day_view = self.main_view.go_to_day_view()
-        original_events = day_view.get_events()
-
-        new_event_page = self.main_view.go_to_new_event()
-        day_view = new_event_page.add_event(event)
-        day_view = day_view.delete_event(event.name, len(original_events) > 0)
-
-        events_after_delete = day_view.get_events()
-        self.assertEqual(original_events, events_after_delete)
+        day_view, test_event = self._add_event()
+
+        day_view.delete_event(test_event.name)
+
+        self._workaround_bug_1350605()
+
+        self.assertThat(lambda: self._event_exists(test_event.name),
+                        Eventually(Equals(False)))