980723 - No warnings or prompts before deleting a saved event

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia::Calendar, defect)

Description

[Peter deHaan [:pdehaan]]

Found using Jason's B2G 1.4 LG device

Steps to reproduce:
1. Launch the Calendar app
2. Create an event.
3. Click the event, and hit Edit.
4. In the event details, click the "Delete event" button.


Actual results:
The event is deleted. BOOM! No warning, no 'Are you sure', no confirmation whatsoever.


Expected results:
Courtesy warning of a destructive, non-undoable action.

Comment 2

[Jason Smith [:jsmith]]

Not a blocker - not a regression & a nice to have at best.

Comment 3

[Joshua Smith [:joshua-s]]

Attachment: PR on GitHub

I am a bit unsure about the localization integration, so it would be great if someone could double-check that part specifically.

Comment 4

[Kevin Grandon :kgrandon]

Comment on attachment 8421281 [details] [review]
PR on GitHub

Interesting, I filed bug 814337 for this exact issue, but was told that we should use an undo pattern. I'm not against landing this though as we have a confirmation pattern elsewhere in the system (contacts app for example).

Comment 5

[Kevin Grandon :kgrandon]

Comment on attachment 8421281 [details] [review]
PR on GitHub

Hey Joshua - thanks for the patch! Unfortunately I can't R+ yet due to some linter issues on travis, but if you can fix those I will take another look.

Also - do you think you could add a unit test for this case which ensures that confirm is called? If you don't think you can add the test, or need guidance on creating unit tests, let me know and I can help out. It should be able to stub the confirm function with something like: https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/test/unit/activity_handler_test.js#L675

Comment 6

[Evan Tseng [:evanxd]]

Nice!

Comment 7

[Joshua Smith [:joshua-s]]

I am still working on the Unit Tests for this.  Please stay tuned =)

Comment 9

[Joshua Smith [:joshua-s]]

All: my apologies for the delay on fixing this bug. If someone could assist me with creating a unit test, that would be great!

Comment 10

[Miller Medeiros [:millermedeiros]]

let's confirm with UX first what is the expected behavior and what message should we display.

Comment 11

[Harly Hsu[:harly]]

Attachment: confirm dialog.png

I think a confirmation dialog with a message "Delete event?" will be sufficient for this case. Attaching a screenshot for reference.

Comment 12

[Joshua Smith [:joshua-s]]

Now that we have UX guidance, we can proceed with the unit tests. Miller, can you assist me?

Comment 13

[Joshua Smith [:joshua-s]]

I have rebased my pull request: https://github.com/mozilla-b2g/gaia/pull/19173

Comment 14

[Miller Medeiros [:millermedeiros]]

Joshua, it would be great if you implemented marionette tests instead of a unit test for this feature (https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_integration_tests) you can see the other tests inside "apps/calendar/test/marionette/" for reference.

I would expect 2 tests, basically `app.createEvent() + app.monthDay.events[0].click() + app.readEvent.waitForDisplay() + app.readEvent.edit() + app.editEvent.waitForDisplay() + app.editEvent.delete()` and check if event was deleted after you confirm/cancel.

Comment 15

[Gareth Aye [:gaye] (back from PTO)]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/27391

Sorry didn't mean to snipe work (I started working on this without looking at bug history). I hope I didn't disrupt anyone's work. I did go ahead and add the two marionette tests that Miller requested though.

Comment 16

[Joshua Smith [:joshua-s]]

No problem! I have been swamped with other work, and this needs to be fixed soon.

Comment 17

[Joshua Mitchell (Inactive)]

Hey Gareth - 

I came across this issue today and found this bug in my bugzilla search - it looks like your patch 'stalled out' in the review process?

Can you help get this moving again?

Comment 18

[GitHub Autolander Bot]

Attachment: [gaia] ruvsmirnov:bug-980723-notifications-deleting-event > mozilla-b2g:master

Comment 19

[vladimir.smirnov2]

In spite this bug already have 2 open PRs submitted a year ago we have been asked by Morpheus to check it. So, this is the third attempt. 

Actually I just took Joshua's source code and rebased it on the latest master branch. Hope that it doesn't hurt anyone.

Comment 20

[Kevin Grandon :kgrandon]

Comment on attachment 8692922 [details] [review]
[gaia] ruvsmirnov:bug-980723-notifications-deleting-event > mozilla-b2g:master

I think that this is incorrect and that we want to use an undo pattern instead, but Gareth should take a look here.

Comment 21

[BMO Automation]

Firefox OS is not being worked on