Merge lp://qastaging/~charlesk/indicator-datetime/lp-1622682-use-valarm-description-property-in-popups into lp://qastaging/indicator-datetime
| Status: | Needs review |
|---|---|
| Proposed branch: | lp://qastaging/~charlesk/indicator-datetime/lp-1622682-use-valarm-description-property-in-popups |
| Merge into: | lp://qastaging/indicator-datetime |
| Diff against target: |
43 lines (+4/-4) 2 files modified
src/snap.cpp (+2/-2) tests/test-notification.cpp (+2/-2) |
| To merge this branch: | bzr merge lp://qastaging/~charlesk/indicator-datetime/lp-1622682-use-valarm-description-property-in-popups |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| unity-api-1-bot | continuous-integration | Approve | |
| dobey (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Use the valarm 'description' property correctly when displaying alarm popups
Description of the change
Use the valarm 'description' property correctly when displaying alarm popups.
The code is currently the vevent's summary property in the popup notification, but should be displaying the valarm's description property instead. (RFC 2445: 'When the action is "DISPLAY", the [v]alarm MUST also include a "DESCRIPTION" property, which contains the text to be displayed when the alarm is triggered.')
After looking through the code history, I think this bug comes from the time when we used to only handle one valarm per vevent. This code didn't get fixed when event-to-alarm became a one-to-many relationship.
| unity-api-1-bot (unity-api-1-bot) wrote | # |
Unmerged revisions
- 465. By Charles Kerr
-
sync tests to r463 to look for Alarm.description as the popup notification's text
- 464. By Charles Kerr
-
fix test-notification bug that invoked Snap() with the wrong Alarm argument
- 463. By Charles Kerr
-
use Alarm.text, rather than Appointment.
summary, when showing popup notifications for an alarm.
Looks ok to me.