Ubuntu Calendar App

Merge lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber into lp:ubuntu-calendar-app

  1. WeekNumber
  2. Merge into trunk
Proposed by Kunal Parmar on 2015-04-02
Status: Merged
Approved by: Kunal Parmar on 2015-05-09
Approved revision: 615
Merged at revision: 642
Proposed branch: lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber
Merge into: lp:ubuntu-calendar-app
Diff against target: 98 lines (+18/-14)
4 files modified
TimeLineBaseComponent.qml (+1/-1)
TimeLineHeader.qml (+4/-2)
dateExt.js (+4/-10)
tests/autopilot/calendar_app/tests/test_dayview.py (+9/-1)
To merge this branch: bzr merge lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber
Reviewer Review Type Date Requested Status
Carla Sella 2015-04-24 Approve on 2015-05-07
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-04-29
Nicholas Skaggs Needs Fixing on 2015-04-23
Alan Pope 🍺🐧🐱 🦄 2015-04-02 Approve on 2015-04-12
Review via email: mp+255078@code.launchpad.net

Commit Message

Resolves Bug #1435542
Calendar app displays wrong week number

Description of the Change

Resolves Bug #1435542
Calendar app displays wrong week number

To post a comment you must log in.
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Alan Pope 🍺🐧🐱 🦄 (popey) wrote :

Looks good!

review: Approve
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber updated on 2015-04-19
612. By Kunal Parmar on 2015-04-19

conflict resolved

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber updated on 2015-04-19
613. By Kunal Parmar on 2015-04-19

logs added

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber updated on 2015-04-22
614. By Kunal Parmar on 2015-04-22

logging added

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Nicholas Skaggs (nskaggs) wrote :

If you watch the video, it shows going to dayview selects the 19th of April, which imho, would be the start of week 17. But it still displays week 16.

http://91.189.93.70:8080/job/generic-mediumtests-utopic/2626/artifact/calendar_app.tests.test_dayview.TestDayView.test_default_view.ogv

I also wonder why selecting dayview takes you to the following week, instead of the current week.

review: Needs Fixing
Nicholas Skaggs (nskaggs) wrote :

If you need help, I'll ask Carla to take a look here. She's familar with the codebase and the locale issues here ;-)

Carla Sella (carla-sella) wrote :

@Nicholas: I have tested this branch and it is ok. When I put en_US locale weeks start on Sundays and end on Saturdays and Week numbers are right (w17 starts Sun 19 Apr ends Sat 25 Apr - w18 starts Sun 26 Apr ends Sat 2 May). When I put it_IT locale weeks start on Mondays and end on Sundays (w17 starts Mon 20 Apr and ends Sun 26 Apr - w18 starts Mon 27 Apr and ends Sun 3 May).

I wanted to fix the test: TestDayView.test_default_view, but I haven't found yet how to get what is the first day of the week for each country.

This works:

        locale = self.app.main_view.get_default_locale()
        if locale == ('it_IT', 'UTF-8'):
            week = int(now.strftime("%W"))+1
        elif locale == ('en_US', 'UTF-8'):
            week = int(now.strftime("%U"))+1

But I do not think it's the best way to do it as I will have to put all the countries in.

I am still tyring to find a way to get the first day of the week using something like calendar.firstweekday() but this gives me always 0 as first day of the week both in it_IT and en_US so it's the right to use.

Have you guys got an idea how to do this ?

Carla

lp:~pkunal-parmar/ubuntu-calendar-app/WeekNumber updated on 2015-04-29
615. By Kunal Parmar on 2015-04-29

First day of week exposed to python

Kunal Parmar (pkunal-parmar) wrote :

Hi Carla,

I exposed firstDayOfWeek property to python by introducing below in TimeLineHeader.qml

property int firstDayOfWeek: Qt.locale().firstDayOfWeek

Let me know if that helps.

Thanks,
Kunal

Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Carla Sella (carla-sella) wrote :

@Kunal: Great! Thanks that's very useful. I have only one doubt though: I have set locale en_US on my desktop but the firstDayOfWeek property is equal to 0 I would expect it to be 6. If I set locale to it_IT firstDayOfWeek property equals 1, I would expect it to be 0.

I can fix the code to work woth 0 and 1, but just to be sure this is what you wanted to achieve and that this will work with other locals.

Kunal Parmar (pkunal-parmar) wrote :

> @Kunal: Great! Thanks that's very useful. I have only one doubt though: I have
> set locale en_US on my desktop but the firstDayOfWeek property is equal to 0 I
> would expect it to be 6. If I set locale to it_IT firstDayOfWeek property
> equals 1, I would expect it to be 0.
>
> I can fix the code to work woth 0 and 1, but just to be sure this is what you
> wanted to achieve and that this will work with other locals.

Hi Carla,

I am quite not sure what you mean.
Qt locale will return 0 for Sunday as first day of week and will return 1 for monday.

Let me know if you need more information.

Carla Sella (carla-sella) wrote :

Hi Kunal, I ment that with calendar related functions (like calendar.firstweekday()) you get 0 for Monday and 6 for Sunday. So with en_US set as locale I was expecting in return an with it_IT set as local as was expeting 0.
But I suppose what you did could still be ok, I will just have to put a note in the code otherwise after some time I will not remember that 0 is for Sunday as first day of week and 1 is for monday.

Carla Sella (carla-sella) wrote :

I will put a note in my tests. So this is ok for me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'TimeLineBaseComponent.qml'
2--- TimeLineBaseComponent.qml 2015-02-19 13:50:29 +0000
3+++ TimeLineBaseComponent.qml 2015-04-29 12:30:37 +0000
4@@ -31,7 +31,7 @@
5 property var keyboardEventProvider;
6
7 property date startDay: DateExt.today();
8- property int weekNumber: startDay.weekNumber();
9+ property int weekNumber: startDay.weekNumber(Qt.locale().firstDayOfWeek);
10 property bool isActive: false
11 property alias contentY: timeLineView.contentY
12 property alias contentInteractive: timeLineView.interactive
13
14=== modified file 'TimeLineHeader.qml'
15--- TimeLineHeader.qml 2015-04-17 05:34:37 +0000
16+++ TimeLineHeader.qml 2015-04-29 12:30:37 +0000
17@@ -28,6 +28,7 @@
18 property int type: ViewType.ViewTypeWeek
19 property date startDay;
20 property double contentX;
21+ property int firstDayOfWeek: Qt.locale().firstDayOfWeek
22
23 signal dateSelected(var date);
24
25@@ -45,8 +46,9 @@
26 Label{
27 id: weekNumLabel
28 objectName: "weeknumber"
29- // TRANSLATORS: W refers to Week, followed by the actual week number (%1)
30- text: i18n.tr("W%1").arg(root.weekNumber)
31+
32+ // TRANSLATORS: W refers to Week, followed by the actual week number (%1)
33+ text: i18n.tr("W%1").arg(startDay.weekNumber(Qt.locale().firstDayOfWeek))
34 fontSize: "small"
35 height: units.gu(5)
36 width: parent.width
37
38=== modified file 'dateExt.js'
39--- dateExt.js 2014-08-18 16:48:54 +0000
40+++ dateExt.js 2015-04-29 12:30:37 +0000
41@@ -85,16 +85,10 @@
42 return this.midnight().addDays(1 - this.getDate())
43 }
44
45-Date.prototype.weekNumber = function() {
46- var date = this.weekStart(1).addDays(3) // Thursday midnight
47- var newYear = new Date(date.getFullYear(), 0 /*Jan*/, 1 /*the 1st*/)
48- var n = 0
49- var tx = date.getTime(), tn = newYear.getTime()
50- while (tn < tx) {
51- tx = tx - Date.msPerWeek
52- n = n + 1
53- }
54- return n
55+Date.prototype.weekNumber = function(weekStartDay) {
56+ var date = this.weekStart(weekStartDay).addDays(3) // Thursday midnight
57+ var onejan = new Date(this.getFullYear(), 0, 3);
58+ return Math.ceil((((date - onejan) / 86400000) + onejan.getDay()+1)/7);
59 }
60
61 Date.prototype.weeksInMonth = function(weekday) {
62
63=== modified file 'tests/autopilot/calendar_app/tests/test_dayview.py'
64--- tests/autopilot/calendar_app/tests/test_dayview.py 2015-04-10 18:50:59 +0000
65+++ tests/autopilot/calendar_app/tests/test_dayview.py 2015-04-29 12:30:37 +0000
66@@ -20,6 +20,7 @@
67
68 import datetime
69 import calendar
70+import logging
71
72 # from __future__ import range
73 # (python3's range, is same as python2's xrange)
74@@ -33,6 +34,8 @@
75 from calendar_app.tests import CalendarAppTestCase
76 from datetime import date
77
78+logger = logging.getLogger(__name__)
79+
80
81 class TestDayView(CalendarAppTestCase):
82
83@@ -72,9 +75,14 @@
84 self.assertEquals(
85 self.day_view.get_datelabel(today).text, str(now.day))
86
87+ week = int(now.strftime("%U"))+1
88+
89+ logger.warn(now)
90+ logger.warn(str(week))
91+ logger.warn(self.day_view.get_weeknumer(today).text)
92 # Checking week number is correct
93 self.assertEquals(
94- self.day_view.get_weeknumer(today).text, 'W' + now.strftime("%W"))
95+ self.day_view.get_weeknumer(today).text, 'W' + str(week))
96
97 # Check day is scrolled to the current time
98 self.assertEquals(self.day_view.get_scrollHour(), now.hour)

Subscribers

People subscribed via source and target branches

to status/vote changes: