beeminder / BeeSwift

Official Beeminder for iOS app
Other
30 stars 6 forks source link

App Crash in Edit Notifications #172

Open krugerk opened 4 years ago

krugerk commented 4 years ago
### Desiderata
- [ ] Confirm replicata in latest version
- [ ] TBD

Replicata

  1. From the Gallery navigate to Settings -> emergency notifications
  2. Tap one of the goals (or default)
  3. Tap 'start notification at', which turns bold
  4. Tap 'goal deadline:', where the deadline is 10pm

Expectata

App doesn't crash.

Resultata

App crashes.

Nota Nebulosa

Sentry:BEESWIFT-K5 tested tag 5.6, appears there as well

Cognata

Verbata: crashes, changing deadline,

krugerk commented 4 years ago

In the case I could reproduce:

In EditNotificationsViewController.setTimePickerComponents hour is calculated to be -2 (from offsetFromMidnight of -7200). Device is not using 24h mode Hour is not greater than 12 Thus https://github.com/beeminder/BeeSwift/blob/f3d421de1f057dce5e2d648f23f44791735f58ce/BeeSwift/EditNotificationsViewController.swift#L177 attempts setting row to minus two, which is out of bounds, causing the crash.

In this particular case we would expect component 2 to be set to 1 (indicating PM) and the row corresponding to 10 to be set (12 + (-2))

krugerk commented 4 years ago

While looking into this, it seems also that the view controller lets us select a bunch of different times (hours + minutes) but many of them do not work - server returns 422.

According to the API

deadline (number): Seconds by which your deadline differs from midnight. Negative is before midnight, positive is after midnight. Allowed range is -17 3600 to 6 3600 (7am to 6am). leadtime (number): Days before derailing we start sending you reminders. Zero means we start sending them on the beemergency day, when you will derail later that day. alertstart (number): Seconds after midnight that we start sending you reminders (on the day that you're scheduled to start getting them, see leadtime above).

we should expect the 422 for setting a number of seconds outside of the allowed range.

Thus if one picks 11:00 AM, rather than sending 39600 (39600 / 60 / 60 = 11 hours past midnight), the 11 should instead be interpretted as the 11 after earliest 7 but before latest 6: -46800 (aka 39600 - 24*3600).

sentry-io[bot] commented 4 years ago

Sentry issue: BEESWIFT-K5

krugerk commented 3 years ago

Still broken. In 24h mode, it seems setting a (default) goal deadline to any time after 0600 results in the request being ignored by the server. That is, the deadline is not changed.

krugerk commented 2 months ago

With 6.7 (47) on iOS 18 I could not reproduce the crash. Instead the time picker's initial/selected values are not the ones displayed in start notification or goal deadline.

image

It is also not clear when values selected are being saved/sent to the server nor is it clear at this screen that that was successful.