Implement handleNotification to ensure compatibility with iOS 10 and earlier #31

Andrew-Tan · 2018-08-25T07:01:39Z

This pull request resolves the issue specified in here: #30

…S 10 and earlier

Merge latest changes

shankari · 2018-08-25T15:58:31Z

@Andrew-Tan can you update the PR with the testing that you did?

Andrew-Tan · 2018-08-26T01:50:39Z

Tested on an iPhone emulator with iOS 9.2
Formerly the browser got stuck after clicking login. After the fix, the app successfully returns back to the main screen with the proper user information.

shankari · 2018-08-26T01:55:15Z

Cool! I have created a build with the fix and am uploading to TestFlight.
@uwtcat if you can test it before I submit for review, that would ensure that the bug is really fixed.
Can you send me your appleID via email so that I can invite you to TestFlight and then send you a link to test the new native version...

uwtcat · 2018-08-27T02:02:29Z

@shankari, thank you! I'll send you my developer username in private to avoid spam.

shankari

Error handling sometimes feels tedious, but it is actually really important...

shankari · 2018-08-27T15:00:04Z

src/ios/OpenIDAuth.m

+    // For compatibility with iOS 10 and earlier
+    NSURL* url = [notification object];
+    if ([url.scheme isEqualToString:@"emission.auth"]) {
+        [self.currentAuthorizationFlow resumeAuthorizationFlowWithURL:url];


don't you have to set currentAuthorizationFlow to nil if successful?
https://github.com/openid/AppAuth-iOS/blob/619bb7c7d5f83cc2ed19380d425ca8afa279644c/README.md#authorizing-ios

shankari · 2018-08-30T23:24:13Z

While testing this, found that the update to the ionic deploy plugin (that supports downloading a custom UI), required to fix https://github.com/e-mission/e-mission-phone/issues/443, broke on iOS 10. It still works on iOS11. The observed broken behavior is that ionic detects that there is no update on iOS 10, but that there is an update on iOS 11. So clicking on the update link on iOS 10 doesn't popup the prompt to download the new UI-only update.

Filed an issue against ionic - hopefully they will respond soon.

shankari · 2018-08-30T23:28:09Z

Just confirmed that old version of the plugin works fine on iOS10.

shankari · 2018-08-30T23:30:26Z

@uwtcat if ionic doesn't respond soon, or doesn't have a great answer as part of their response, I am tempted to roll back the update to the ionic deploy plugin, since it seems flaky, and focus on only the other fixes for the immediate release. I'll let you know by sometime tomorrow.

Andrew-Tan · 2018-09-01T17:49:57Z

Added completion handling logic

shankari · 2018-09-02T04:49:36Z

@uwtcat @Andrew-Tan has just added one more code fix. I have just uploaded version 1.0.6 that includes that fix. I tried it out in the emulator, and it worked - can you please verify that this version also works on a physical device before I submit it?

Also, @Andrew-Tan, after logging in, I clicked on "Open AccessMap", which tried to redirect to https://emission.accessmap.io/emission?uuid=34a4de78-87e2-4c29-ba3c-2b5a6413420d, but the resulting page is blank.

Given that it is the main button right after logging in, we should probably fix that too before deploying. If you can verify that this is not a native issue, we can submit now and fix the javascript later...

shankari · 2018-09-02T04:54:36Z

@uwtcat @Andrew-Tan I tested in the emulator, and the accessmap page loads on iOS11 but not on iOS10. Anat, have you tried clicking the button to load the AccessMap page?

shankari · 2018-09-02T05:03:53Z

Ok, so the website load issue has nothing to do with e-mission. If I launch safari on the 10.2 simulator and try to navigate to https://emission.accessmap.io, I still get a blank screen. On 11.4, the same URL loads the page. @uwtcat you can verify this too.

The error that I see from safari is

[Error] TypeError: Object.values is not a function. (In 'Object.values(n.i(o.a)(r.a))', 'Object.values' is undefined)
	./reducers/defaults.js (map.bundle.js:73:20060)
	t (vendor.bundle.js:1:106)
	./reducers/activities.js (map.bundle.js:73:16401)
	t (vendor.bundle.js:1:106)
	./reducers/index.js (map.bundle.js:73:21014)
	t (vendor.bundle.js:1:106)
	./store/create-store.js (map.bundle.js:74:9026)
	t (vendor.bundle.js:1:106)
	./index.js (map.bundle.js:73:14227)
	t (vendor.bundle.js:1:106)
	webpackJsonp (vendor.bundle.js:1:420)
	Global Code (map.bundle.js:1)

This looks like something that needs to be fixed in the code for accessmap; we don't need to discuss it here. @Andrew-Tan let me know if you want to you want me to open another issue if you want to continue discussing the WSOD error.

shankari · 2018-09-02T05:07:06Z

Action items:

@uwtcat verify that emission 1.0.6 still looks good
@Andrew-Tan figure out root cause of the WSOD, and confirm that it doesn't require native changes
@shankari after 1) and 2) are verified, submit to app store review

Andrew-Tan · 2018-09-02T05:32:33Z

The Emission AccessMap site is developed by @nbolten, so he should be able to look into this issue.
Nick, can you look at the problem @shankari described in this thread and try fixing it?

The app itself has nothing to do with the website (they are decoupled completely at current stage), so after @uwtcat had verified the change, please feel free to submit to app store review

Thanks you :)

uwtcat · 2018-09-02T05:43:40Z

@shankari @Andrew-Tan @nbolten I believe this is a known issue. @nbolten corrected this on our staging server, but likely did not update this version (designed specifically for the emissions deployment). @nbolten can you support the emissions end-point for access map as it seems the same iOS issue persists here. Thanks!

…

On Sat, Sep 1, 2018 at 9:54 PM shankari ***@***.***> wrote: @uwtcat <https://github.com/uwtcat> @Andrew-Tan <https://github.com/Andrew-Tan> I tested in the emulator, and the accessmap page loads on iOS11 but not on iOS10. Anat, have you tried clicking the button to load the AccessMap page? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ2RhCRsPYcOlZW_UydSlDNXt9-M0Cqmks5uW2SMgaJpZM4WMRve> .

-- Anat Caspi, PhD Director, Taskar Center for Accessible Technology Computer Science & Engineering, University of Washington caspian@cs.washington.edu

shankari · 2018-09-02T14:42:39Z

@uwtcat ok, so once you verify that 1.0.6 still works properly, I will submit to the app store.

nbolten · 2018-09-02T20:28:20Z

Hey all! I've pushed the polyfills that I fixed in the dev branch, should work fine on slightly older iOS now and be testable.

If this impacts the general release, give me a bit of a heads up, as this endpoint was meant for testing and we need to verify that analytics are working properly.

uwtcat · 2018-09-03T16:12:26Z

Hi Nick- We absolutely need to make sure analytics are working properly on this endpoint. This is the endpoint for our data collection and validation. Thanks! Anat

…

On Sun, Sep 2, 2018 at 1:28 PM Nick Bolten ***@***.***> wrote: Hey all! I've pushed the polyfills that I fixed in the dev branch, should work fine on slightly older iOS now and be testable. If this impacts the general release, give me a bit of a heads up, as this endpoint was meant for testing and we need to verify that analytics are working properly. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ2RhJp7Oy2VwN3KTlVP67HE_0TT0j_Rks5uXD9kgaJpZM4WMRve> .

-- Anat Caspi, PhD Director, Taskar Center for Accessible Technology Computer Science & Engineering, University of Washington caspian@cs.washington.edu

shankari · 2018-09-03T16:19:36Z

@uwtcat if you can verify that 1.0.6 works well on TestFlight, I can submit to App store today morning

uwtcat · 2018-09-03T16:31:30Z

Unfortunately, it still has the same phenotype- doesn't redirect properly after logging into OpenToAll.

…

On Mon, Sep 3, 2018 at 9:19 AM shankari ***@***.***> wrote: @uwtcat <https://github.com/uwtcat> if you can verify that 1.0.6 works well on TestFlight, I can submit to App store today morning — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ2RhK2-xVLI9IV2jRpmw7XUx4ilYMv_ks5uXVaYgaJpZM4WMRve> .

-- Anat Caspi, PhD Director, Taskar Center for Accessible Technology Computer Science & Engineering, University of Washington caspian@cs.washington.edu

shankari · 2018-09-03T17:57:33Z

@uwtcat can you uninstall the old app and try with reinstalling 1.0.6? I just tried on my test phone (9.2) and it worked fine. "Open AccessMap" worked too.

uwtcat · 2018-09-03T23:59:13Z

@shankari- I apologize, it turned out I had two versions of the app on my phone. One really old one that I hadn't realized was there. Once removed, everything is working well including the emissions.accessmap endpoint. Please go ahead and submit if you are able. Many thanks!!! Anat

…

On Mon, Sep 3, 2018 at 10:57 AM shankari ***@***.***> wrote: @uwtcat <https://github.com/uwtcat> can you uninstall the old app and reinstall 1.0.6? I just tried on my test phone (9.2) and it worked fine. "Open AccessMap" worked too. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJ2RhOhWt1BDcRVwrBQ7cmY3U9NCvRU2ks5uXW2NgaJpZM4WMRve> .

-- Anat Caspi, PhD Director, Taskar Center for Accessible Technology Computer Science & Engineering, University of Washington caspian@cs.washington.edu

shankari · 2018-09-04T01:58:37Z

Just submitted. Current review times are 5 days.
http://appreviewtimes.com/

In the meanwhile, android users, and iOS 11+ users can continue testing...

Andrew-Tan and others added 3 commits August 24, 2018 23:56

implement handleNotification callback to ensure compatibility with iO…

672c9bb

…S 10 and earlier

Merge pull request #3 from e-mission/master

fe0cfd2

Merge latest changes

bump up version number

42e3d5f

shankari requested changes Aug 27, 2018

View reviewed changes

better completion handling

e03bc4d

shankari approved these changes Sep 2, 2018

View reviewed changes

shankari merged commit 27f24b4 into e-mission:master Sep 4, 2018

shankari mentioned this pull request Sep 4, 2018

AppAuth not working on certain versions of iOS #30

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement handleNotification to ensure compatibility with iOS 10 and earlier #31

Implement handleNotification to ensure compatibility with iOS 10 and earlier #31

Andrew-Tan commented Aug 25, 2018 •

edited

Loading

shankari commented Aug 25, 2018

Andrew-Tan commented Aug 26, 2018

shankari commented Aug 26, 2018

uwtcat commented Aug 27, 2018

shankari left a comment

shankari Aug 27, 2018

shankari commented Aug 30, 2018 •

edited

Loading

shankari commented Aug 30, 2018

shankari commented Aug 30, 2018

Andrew-Tan commented Sep 1, 2018 •

edited

Loading

shankari commented Sep 2, 2018

shankari commented Sep 2, 2018

shankari commented Sep 2, 2018

shankari commented Sep 2, 2018

Andrew-Tan commented Sep 2, 2018

uwtcat commented Sep 2, 2018 via email •

edited

Loading

shankari commented Sep 2, 2018

nbolten commented Sep 2, 2018

uwtcat commented Sep 3, 2018 via email

shankari commented Sep 3, 2018

uwtcat commented Sep 3, 2018 via email

shankari commented Sep 3, 2018 •

edited

Loading

uwtcat commented Sep 3, 2018 via email

shankari commented Sep 4, 2018 •

edited

Loading

Implement handleNotification to ensure compatibility with iOS 10 and earlier #31

Implement handleNotification to ensure compatibility with iOS 10 and earlier #31

Conversation

Andrew-Tan commented Aug 25, 2018 • edited Loading

shankari commented Aug 25, 2018

Andrew-Tan commented Aug 26, 2018

shankari commented Aug 26, 2018

uwtcat commented Aug 27, 2018

shankari left a comment

Choose a reason for hiding this comment

shankari Aug 27, 2018

Choose a reason for hiding this comment

shankari commented Aug 30, 2018 • edited Loading

shankari commented Aug 30, 2018

shankari commented Aug 30, 2018

Andrew-Tan commented Sep 1, 2018 • edited Loading

shankari commented Sep 2, 2018

shankari commented Sep 2, 2018

shankari commented Sep 2, 2018

shankari commented Sep 2, 2018

Andrew-Tan commented Sep 2, 2018

uwtcat commented Sep 2, 2018 via email • edited Loading

shankari commented Sep 2, 2018

nbolten commented Sep 2, 2018

uwtcat commented Sep 3, 2018 via email

shankari commented Sep 3, 2018

uwtcat commented Sep 3, 2018 via email

shankari commented Sep 3, 2018 • edited Loading

uwtcat commented Sep 3, 2018 via email

shankari commented Sep 4, 2018 • edited Loading

Andrew-Tan commented Aug 25, 2018 •

edited

Loading

shankari commented Aug 30, 2018 •

edited

Loading

Andrew-Tan commented Sep 1, 2018 •

edited

Loading

uwtcat commented Sep 2, 2018 via email •

edited

Loading

shankari commented Sep 3, 2018 •

edited

Loading

shankari commented Sep 4, 2018 •

edited

Loading