-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⬆️ Upgrade to latest everything 🎉 #680
Comments
Switched to cordova [email protected], which is the most recent version (https://cordova.apache.org/announcements/2021/08/16/cordova-android-10.1.0.html). It generated a warning about the targetSDK version being 30+, which is also required for any updates to the play store. Looking at the behavior changes between android 10 (API 29) and android 11 (API 30), we have:
|
ok, so the hibernation changes are pretty evil and are going to get more evil in android 12 (API 31). In API 31, if the app is not launched (in the foreground) "for a few months", the app hibernates. In API 30, the app runtime permissions (including location) are reset.
This is pretty darn terrible. The location permission will be reset, and we won't have the ability to notify the user that anything is wrong through notifications. Apparently, if your app is primarily targets with background operation, you can ask users to exempt the app from hibernation. We are going to have to go down this route; fortunately, there is a programmatic way to do this. Maybe we should just upgrade to API 31 right now and be a bit more future-proof. Let's see what other changes API 31 needs. |
Note also that we can't put this off forever because, in Dec 2021, this reset permission is going to come to devices that: "Runs Android 6.0 (API level 23) to Android 10 (API level 29), inclusive, and is powered by Google Play services" as long as the targetSDK is 11. And we need targetSDK 11 for any native code updates. So we may be able to limp along for a bit but will have to deal with it fully sometime. |
|
The foreground service launch restrictions are the worst. Basically, we can't restart the foreground service except for exemptions. Most of those don't apply.
|
Given the number of changes in android 12, including the foreground service changes, and the ominous "motion sensor rate limiting", I am going to stick with android 11 for now. I will add the hibernation check for now, to get that out of the way, although it is less important with target API 30, where background operations and notifications are not restricted. |
First, upgrading android...
|
Whitelist is deprecated in cordova android 10.x and greater (https://github.com/apache/cordova-plugin-whitelist). However, removing it requires updating the ionic plugin.
|
We are currently at Builds now!!
|
Note other android 10 updates (https://cordova.apache.org/announcements/2021/07/20/cordova-android-10.0.0.html), notably:
|
wrt file:// plugins, we use them only in the context of the post-trip prompt.
so we need to see how the local notifications work with it. Fortunately, it is pretty clear that since we were using the webview plugin all along, it won't really affect us in many other ways. I'm going to downgrade the priority of this. |
Android Studio cannot open the project, though. Trying to figure out how to upgrade the android support plugin. Might just upgrade to a newer version of Android studio if it is easier. |
Now, let's upgrade the external plugins.
The two complicated upgrades are:
|
Getting some errors with building pods
and
Before resetting everything, I got a similar error with the new I am still getting the same error for that plugin.
|
updated the pod repo
per dpa99c/cordova-plugin-firebasex#43 (comment)
and then upgrading the version to the selected 1.11.2 works! but with this error
|
We are now down to 25 plugins, presumably
Presumably this is because
and those plugins have indeed been removed. |
android build succeeds. iOS build fails with error
Builds fine using xcode and launches in a 14.4 emulator. |
At this point, I am going to test a bit and then push out the first version of the beta releases for testing. At least this will get the beta testing pipeline working. Then, as part of upgrading the |
Upgraded gradle plugin, but then build on android studio failed with error
Tried:
It seems to work now, need to see which one fixed it.
With that fix, we were able to recompile after enabling cleartext traffic during debugging ONLY. I will plan on merging these changes to the CEO branch and starting beta testing tomorrow.
|
Includes all changes until: e-mission/e-mission-docs#680 (comment) These are primarily: - bump up the version number - bump up the SDK to API 30 - bump up android to 10.1.0 - bump up ios to 6.2.0 - bump up versions of all non-OpenPATH plugins required, including switching to new repo hosts if needed. + add barcode scanner + add badge plugin manully (to get the most recent version and support the new android gradle plugin) - bump up versions of gradle and cocoapods - remove temporary workaround to uninstall tools 31 since we have upgraded to android 10
@PatGendre @asiripanich @kafitz @jf87 anybody want to help test as I upgrade everything again? |
ios build failed CI (expected) Cleared everything and reinstalled from scratch.
|
happy to help. ;) |
Found the same issue in the push plugin repo, but the working version from the user is on cordova-android9 The most obvious error is while trying to import Trying to use the same FCM version as |
to be consistent with havesource/cordova-plugin-push#83 (comment) This seems to fix the error in e-mission/e-mission-docs#680 (comment)
Archived and uploaded 1.2.0 (20) to itunes connect last night after some wrestling with the signing permissions. Unfortunately, found a silly error while testing in the emulator - had to change the survey launch to match Fixed and uploaded 1.2.1 (21) to itunes. Sent invite to beta users. |
Android is a bit trickier because we need to change our upload process to support the aab bundles. It looks like apk is also available for older apps, so we could use that workaround for now, but what better time to make the inevitable change than during a formal upgrade process. |
Testing android on the emulator first (after being bitten by a weird bug on iOS). The survey stuff works fine, but we appear to have a persistent error with reading the location.
|
High level changes: - create a new method in the foreground delegate - checks and returns appropriate plugin results - moved from `TripDiarySensorControlChecks` - create a new interface for the async operation to read the activity results, which is the only way to generate the prompt - moved this from the `cordova-server-sync` repo e-mission/cordova-server-sync#50 - stored callback functions and invoked them asynchronously, similar to the location changes in ca979bd - ensured that the simulator always returns TRUE to allow easy testing of other features - changed the error messages to improve user instructions, including: - if motion activity is not supported on the device, uninstall - if the permission is off, fix in app settings - if the setting is off, fix and restart the app Related testing: e-mission/e-mission-docs#680 (comment) e-mission/e-mission-docs#680 (comment)
After enabling notification permissions, we are almost done. Except for one really bizarre issue. On iOS13, when we open the app settings, the location permission is not visible. After a few hours spent debugging this, the situation is:
So my current thoughts are:
Root cause: We need to actually start trying to read locations. If I comment out this one line Solution: The solution is to start tracking before opening the app for the location case, and stopping tracking afterwards. Testing done: After commenting out the line |
One more super-duper final fix - when we add the plugin, the newly added files don't appear to be added to the project. We had to add them manually. Let's debug that briefly and then finish this merge. ROOT CAUSE: we were labeling the source files as |
… the app page It turns out that we need to actually start tracking the location before the location permission would be visible, similar to the motion activity permission (e-mission@f3c673a). In the previous version, we would call `markConsented` and then popup permissions as required. So the `startUpdatingLocation` call would happen in parallel with opening up the status screen. However, now we get all permissions upfront and then call `markConsented`. So we need to include a dummy `startUpdatingLocation` call to see the location. We can stop updating the location once the user has given the permission. We didn't see this earlier (in ca979bd) because we tested on a physical phone which was running 12.x and can prompt for permissions inline. For excrutiating detail, see e-mission/e-mission-docs#680 (comment)
- Bump up overall app version - Also bump up versions for the following plugins: - data collection: bulk of the changes, bump up a full minor version - server sync: move the fitness permission init code into the new permissions module - unified logger: add a new method to generate plugin compatible notifications so we can tell the user when their status is bad and automatically open the page to fix it. - usercache: includes code that calls the local notifications, so needed to change method signature to match the notification state. This pretty much concludes e-mission/e-mission-docs#680
@PatGendre @ericafenyo @lgharib I have just finished an initial version of the status screen. I will likely tweak it going forward, but wanted to get it out of the door for feedback. I would encourage you to test it out, provide feedback and then update your versions of the apps. It is likely to make it easier to install correctly and to keep the app running correctly - e.g. solve the "iOS reset permissions" issue. @ericafenyo this involves a significant expansion of the interface for the data collection library/plugin and a new UI screen. The new interface is fairly simple, mainly consisting of |
@asiripanich I just pushed the new iOS version to TestFlight in case you want to take a look. Open to feedback on other status metrics to include. |
@asiripanich The status screen opens from the profile. You should also experiment with turning various permissions off and seeing what happens 😄 |
Ok I will test and let you know when I have questions. Thanks. |
I will do that and report back today. :) |
@shankari I like the new permission status screen. However, the scrolling issue is still there (we discussed it somewhere but I can't find the related issue). |
@asiripanich can you elaborate more on the scrolling issue? Some context may help me find the related issue. |
@asiripanich note also that the permission status screen is not only for onboarding. We also use it to track the status on an ongoing basis. If the user responds to the "XXX has been using your location in the background, do you want to continue?" with "no", then we will prompt the user that the app settings are incorrect, and open the app settings screen when they click on the notification so they can fix the issue easily. I encourage you to experiment with turning location settings/permissions on/off, motion activity settings off, etc to see this behavior and whether it makes sense. |
Changes are primarily related to the status screen changes: e-mission/e-mission-docs#680 related UI string changes are at: 77df14c
@shankari great ! Unfortunately we have updated our app 2 weeks ago and do not plan the next update until a few weeks ahead. But we will test the status screen again as soon as we can. |
I remember now that it was something I discussed with @atton16. Expand to see a video |
@asiripanich Is this on android or iOS? |
This was on my iPhone 13 Pro with iOS 14.3. |
Alas, I don't have access to an iOS 14.3. The oldest simulator in my xcode is 14.5, and my test phones are all 12.x I'm actually not sure what the video is showing. Is it that you cannot access the "I accept" or that it takes a few tries to get there?
|
I haven't run it in the simulator yet. I think it was the latter. However, I'm quite familiar with the app so I know that you need to scroll up and down a few times before it let you scroll to the end. Not sure if an average user would know how to resolve this. |
We will remove it completely when we remove the openid auth functionality from the phone e-mission/e-mission-docs#680 (comment)
At long last, after clearing the decks with
e-mission/e-mission-phone#799
e-mission/e-mission-phone#800
e-mission/e-mission-phone#801
e-mission/e-mission-phone#802
I'm ready to start the annual ritual of upgrading to the most recent version of everything.
I will also plan to address #678
and add silent push notifications to android as well to address #677
The text was updated successfully, but these errors were encountered: