Merge lp:~mihirsoni/ubuntu-calendar-app/editEvent into lp:ubuntu-calendar-app

Proposed by Mihir Soni
Status: Merged
Approved by: Kunal Parmar
Approved revision: 161
Merged at revision: 177
Proposed branch: lp:~mihirsoni/ubuntu-calendar-app/editEvent
Merge into: lp:ubuntu-calendar-app
Diff against target: 169 lines (+58/-19)
2 files modified
EventDetails.qml (+11/-4)
NewEvent.qml (+47/-15)
To merge this branch: bzr merge lp:~mihirsoni/ubuntu-calendar-app/editEvent
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Kunal Parmar Approve
David Planella Approve
Olivier Tilloy (community) Needs Fixing
Review via email: mp+190106@code.launchpad.net

Commit message

Added Edit event support

Description of the change

Added edit event support using new UDS presentation.

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Olivier Tilloy (osomon) wrote :

53 + title: isEdit===false? i18n.tr("New Event"):i18n.tr("Edit Event")

this would be clearer if written:

    title: isEdit ? i18n.tr("Edit Event") : i18n.tr("New Event")

Revision history for this message
Olivier Tilloy (osomon) wrote :

38 + console.log("I am adding");
43 + console.log("I am editing");

Please remove those logs if the functionality is working fine.

Revision history for this message
Olivier Tilloy (osomon) wrote :

I got stuck on the edit screen by doing the following:

 - create a new event today
 - in the day view click on the event to see its details
 - click on the edit button in the toolbar
 - click on the cancel button to go back to the event details
 - click on the back button to go back to the day view -> what happens is I’m taken back to the "edit event" page instead

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

61 - pageStack.pop();
62 + if(isEdit)
63 + pageStack.push(Qt.resolvedUrl("EventDetails.qml"),{"event":event});
64 + else
65 + pageStack.pop();

You dont need to push EventDetails page again.

62 + if(isEdit)
63 + pageStack.push(Qt.resolvedUrl("EventDetails.qml"),{"event":event});

In any case we need to pop current page to go back to original page. so this modification is not necessary,

review: Needs Fixing
Revision history for this message
David Planella (dpm) wrote :

Mihir, now the EDS branch has landed and we can land the new views too. Have you had the chance to look into the fixes Kunal was mentioning in his last comment?

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
David Planella (dpm) wrote :

176 +}

Just a small nitpick: could you revert the change to the manifest file? It's not related to this branch.

Other than that, it looks good to me, thanks!

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

Its almost ready, but still clean up is required

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

147 + function clearFocus() {
148 + Qt.inputMethod.hide()
149 + titleEdit.focus = false
150 + locationEdit.focus = false
151 + personEdit.focus = false
152 + startTime.focus = false
153 + endTime.focus = false
154 + messageEdit.focus = false
155 + }

internal object already has defined clearFocus() method, please remove redundant code
    QtObject {
        id: internal

Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

70 + internal.eventModel = GlobalModel.gloablModel(event);

what does this do? I do not see gloablModel() method which takes event as parameter? did you forgot to merge?

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

61 + if( pageStack.header )
62 + pageStack.header.visible = true;
63 + if(event === null){
64 + internal.eventModel = GlobalModel.gloablModel();
65 + isEdit =false;
66 + addEvent();
67 + }
68 + else{
69 + pageStack.header.visible = true;

pageStack.header.visible = true;
is called twice

I think first call should be able to handle all case, you are facing some issue ?

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Autolanding.
Approved revid is not set in launchpad. This is most likely a launchpad issue and re-approve should fix it. There is also a chance (although a very small one) this is a permission problem of the ps-jenkins bot.
http://91.189.93.70:8080/job/ubuntu-calendar-app-autolanding/51/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests/1108
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-precise-amd64-autolanding/51
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-quantal-amd64-autolanding/53
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-raring-amd64-autolanding/51
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-amd64-autolanding/51

review: Needs Fixing (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

Trying edit event is showing error

ile:///home/kunal/Dropbox/projects/qt/qml/ubuntu/calendar/review/editEvent/NewEvent.qml:61: Error: Cannot assign QQmlListReference to QString
virtual void QOrganizerEDSEngine::requestDestroyed(QtOrganizer::QOrganizerAbstractRequest*)

100 + if(e.attendees){
101 + personEdit.text = e.attendees;
102 + }

Due to above line, please see event details how to get attendee's name from attendees array.

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

59 - internal.eventModel = GlobalModel.gloablModel();
60 -
61 + if( pageStack.header )

May be you need to restore "internal.eventModel = GlobalModel.gloablModel();" call and remove code you added in condition.

review: Needs Fixing
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

115 + if(!isEdit) // if it is new event create new object
116 + event = Qt.createQmlObject("import QtOrganizer 5.0; Event { }", Qt.application,"NewEvent.qml");

I think its better to move event creation to addEvent method now.

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

            if( personEdit.text != "") {
                var attendee = Qt.createQmlObject("import QtOrganizer 5.0; EventAttendee{}", Qt.application, "NewEvent.qml");
                attendee.name = personEdit.text;
                attendee.emailAddress = "<email address hidden>";
                event.setDetail(attendee);
            }

In case of edit, I think we will need to delete all existing attendees first, I am not sure thought, we need to try it out this and see how it works.

bool OrganizerItem::removeDetail ( detail )

This method should be useful.

Revision history for this message
Mihir Soni (mihirsoni) wrote :

> if( personEdit.text != "") {
> var attendee = Qt.createQmlObject("import QtOrganizer 5.0;
> EventAttendee{}", Qt.application, "NewEvent.qml");
> attendee.name = personEdit.text;
> attendee.emailAddress = "<email address hidden>";
> event.setDetail(attendee);
> }
>
> In case of edit, I think we will need to delete all existing attendees first,
> I am not sure thought, we need to try it out this and see how it works.
>
> bool OrganizerItem::removeDetail ( detail )
>
> This method should be useful.

I have used this instead of remove details :
// attendees clear
        event.attendees = [];

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

This is looking nice now,
Can you remove this line, I added this for some testing and forgot to remove it.

136 attendee.emailAddress = "<email address hidden>";

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

There is typo here,
108 + if(j!== e.attendess.length-1)

it needs to be

e.attendees.length

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Kunal Parmar (pkunal-parmar) wrote :

this is looking good now, thanks :)

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
161. By Mihir Soni

Removed lines

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'EventDetails.qml'
--- EventDetails.qml 2013-10-15 23:52:44 +0000
+++ EventDetails.qml 2013-12-20 19:18:14 +0000
@@ -21,7 +21,15 @@
21 if( pageStack.header )21 if( pageStack.header )
22 pageStack.header.visible = true;22 pageStack.header.visible = true;
23 }23 }
2424 Connections{
25 target: pageStack
26 onCurrentPageChanged:{
27 if( pageStack.currentPage === root) {
28 pageStack.header.visible = false;
29 showEvent(event);
30 }
31 }
32 }
25 function showEvent(e) {33 function showEvent(e) {
26 var location = "";34 var location = "";
2735
@@ -72,17 +80,16 @@
72 }80 }
73 }81 }
74 */82 */
75 /*83
76 ToolbarButton {84 ToolbarButton {
77 action:Action {85 action:Action {
78 text: i18n.tr("Edit");86 text: i18n.tr("Edit");
79 iconSource: Qt.resolvedUrl("edit.svg");87 iconSource: Qt.resolvedUrl("edit.svg");
80 onTriggered: {88 onTriggered: {
81 print(text + " not implemented");89 pageStack.push(Qt.resolvedUrl("NewEvent.qml"),{"event":event});
82 }90 }
83 }91 }
84 }92 }
85 */
86 }93 }
87 Rectangle {94 Rectangle {
88 id:eventDetilsView95 id:eventDetilsView
8996
=== modified file 'NewEvent.qml'
--- NewEvent.qml 2013-10-31 15:09:14 +0000
+++ NewEvent.qml 2013-12-20 19:18:14 +0000
@@ -8,15 +8,21 @@
88
9Page {9Page {
10 id: root10 id: root
11
12 property var date: new Date();11 property var date: new Date();
12
13 property var event:null;
14
13 property var startDate;15 property var startDate;
14 property var endDate;16 property var endDate;
15 property int optionSelectorWidth: frequencyLabel.width > remindLabel.width ? frequencyLabel.width : remindLabel.width17 property int optionSelectorWidth: frequencyLabel.width > remindLabel.width ? frequencyLabel.width : remindLabel.width
1618
17 property alias scrollY: flickable.contentY19 property alias scrollY: flickable.contentY
20 property bool isEdit: false
1821
19 Component.onCompleted: {22 Component.onCompleted: {
23
24 pageStack.header.visible = true;
25
20 // If startDate is setted by argument we have to not change it26 // If startDate is setted by argument we have to not change it
21 if (typeof(startDate) === 'undefined')27 if (typeof(startDate) === 'undefined')
22 startDate = new Date(date)28 startDate = new Date(date)
@@ -26,37 +32,65 @@
26 endDate = new Date(date)32 endDate = new Date(date)
27 endDate.setMinutes( endDate.getMinutes() + 10)33 endDate.setMinutes( endDate.getMinutes() + 10)
28 }34 }
29
30 internal.eventModel = GlobalModel.gloablModel();35 internal.eventModel = GlobalModel.gloablModel();
3136
37 if(event === null){
38 isEdit =false;
39 addEvent();
40 }
41 else{
42 isEdit = true;
43 editEvent(event);
44 }
45 }
46 //Data for Add events
47 function addEvent() {
48 event = Qt.createQmlObject("import QtOrganizer 5.0; Event { }", Qt.application,"NewEvent.qml");
49 startDate = new Date(date)
50 endDate = new Date(date)
51 endDate.setMinutes( endDate.getMinutes() + 10)
52
32 startTime.text = Qt.formatDateTime(startDate, "dd MMM yyyy hh:mm");53 startTime.text = Qt.formatDateTime(startDate, "dd MMM yyyy hh:mm");
33 endTime.text = Qt.formatDateTime(endDate, "dd MMM yyyy hh:mm");54 endTime.text = Qt.formatDateTime(endDate, "dd MMM yyyy hh:mm");
34 }55 }
3556 //Editing Event
57 function editEvent(e) {
58 startDate =new Date(e.startDateTime);
59 endDate = new Date(e.endDateTime);
60 startTime.text = Qt.formatDateTime(e.startDateTime, "dd MMM yyyy hh:mm");
61 endTime.text = Qt.formatDateTime(e.endDateTime, "dd MMM yyyy hh:mm");
62 if(e.displayLabel)
63 titleEdit.text = e.displayLabel;
64 if(e.location)
65 locationEdit.text = e.location;
66 if( e.description ) {
67 messageEdit.text = e.description;
68 }
69 if(e.attendees){
70 for( var j = 0 ; j < e.attendees.length ; ++j ) {
71 personEdit.text += e.attendees[j].name;
72 if(j!== e.attendees.length-1)
73 personEdit.text += ",";
74 }
75 }
76 }
77 //Save the new or Existing event
36 function saveToQtPim() {78 function saveToQtPim() {
37
38 internal.clearFocus()79 internal.clearFocus()
39
40 if ( startDate >= endDate ) {80 if ( startDate >= endDate ) {
41 PopupUtils.open(errorDlgComponent,root,{"text":i18n.tr("End time can't be before start time")});81 PopupUtils.open(errorDlgComponent,root,{"text":i18n.tr("End time can't be before start time")});
42 } else {82 } else {
43
44 var event = Qt.createQmlObject("import QtOrganizer 5.0; Event { }", Qt.application,"NewEvent.qml");
45
46 event.startDateTime = startDate;83 event.startDateTime = startDate;
47 event.endDateTime = endDate;84 event.endDateTime = endDate;
48 event.displayLabel = titleEdit.text;85 event.displayLabel = titleEdit.text;
49 event.description = messageEdit.text;86 event.description = messageEdit.text;
50
51 event.location = locationEdit.text87 event.location = locationEdit.text
5288 event.attendees = []; // if Edit remove all attendes & add them again if any
53 if( personEdit.text != "") {89 if( personEdit.text != "") {
54 var attendee = Qt.createQmlObject("import QtOrganizer 5.0; EventAttendee{}", Qt.application, "NewEvent.qml");90 var attendee = Qt.createQmlObject("import QtOrganizer 5.0; EventAttendee{}", Qt.application, "NewEvent.qml");
55 attendee.name = personEdit.text;91 attendee.name = personEdit.text;
56 attendee.emailAddress = "none@nowhere.com";
57 event.setDetail(attendee);92 event.setDetail(attendee);
58 }93 }
59
60 internal.eventModel.saveItem(event);94 internal.eventModel.saveItem(event);
61 pageStack.pop();95 pageStack.pop();
62 }96 }
@@ -65,7 +99,7 @@
65 width: parent.width99 width: parent.width
66 height: parent.height100 height: parent.height
67101
68 title: i18n.tr("New Event")102 title: isEdit ? i18n.tr("Edit Event"):i18n.tr("New Event")
69103
70 tools: ToolbarItems {104 tools: ToolbarItems {
71 //keeping toolbar always open105 //keeping toolbar always open
@@ -107,7 +141,6 @@
107 }141 }
108 }142 }
109 }143 }
110
111 Component {144 Component {
112 id: timePicker145 id: timePicker
113 TimePicker {146 TimePicker {
@@ -194,7 +227,6 @@
194 }227 }
195 }228 }
196 }229 }
197
198 NewEventEntryField{230 NewEventEntryField{
199 id: titleEdit231 id: titleEdit
200 width: parent.width232 width: parent.width

Subscribers

People subscribed via source and target branches

to status/vote changes: