Upgrade to latest cordova #700
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
which fixes all current android build issues
So they are consistent with config.xml
To use the correct config file. This fixes e-mission/e-mission-docs#518
Upgrade the config.xml and plugins
by reusing the setup script for the CI
Essentially commit periodically until everything works -------------------------- * Fix the sign_and_align_keys script - Run cordova the new way - label the resulting apk correctly * Vastly simplify the instructions for installation by reusing the setup script for the CI * First version of the ios build * Let's see whether the ios build works * Actually switch to osx * Fixed name + case on macos runner * Switch back to ubuntu To see if this is launched now * Some more tries to get the ios build to work * Add another test To see if we can even launch two tests * Rename the working CI and see if it still works * Now change it to run on osx * Now change the line from an echo to sth meaningful * Now finally run sth related to xcode * Finally run the setup script + remove the malformed workflows * showbuildsettings only works in the ios directory which we don't have yet * Fix the xcodeselect + actually check in setup files * Fix the setup script + actually add ios and android builds * Fix infinite loop + remove android build * Fix plugin check + cordova launch * Specify a login shell * Refactor the setup scripts to be common v/s native * Upgrade cocoapods This no longer requires `pod setup` so is much faster and takes much less room * Let's also tear things down * Add in the teardown scripts as well * Don't have to remove cocoapods + minor fix to formatting * Lets try the gem uninstall without the piped y * Handle telemetry warnings better * Fixing this forever?! * Set the CI to true woo! woo! * Test out an initial android build Including some sense of the build environments * Run only on mac to avoid compile error + update README with android studio dependency * Try this on ubuntu * Try the android build * Add the actual build steps to the android workflow * Fix the ANDROID_HOME to use existing value * Remove the sdkmanager configurations since it looks like we can't modify them ``` setup/setup_android_native.sh: line 40: /usr/local/lib/android/sdk/tools/bin/sdkmanager: Permission denied ``` * More fixes * Remove the sdkmanager call since it doesn't work * Some more sdkman fixes * Make the setup changes "stick" * Last try to figure out how to use the proper gradle * Get cordova to actually fail on error + minor refactoring of common code * Upgrade node and npm versions This allows us to use the --unhandled-rejections CLI flag * Fix the sed script to work on the real file Instead of the symlinked location * Fail only for android for now * Let's put it back for iOS as well The previous failure seemed to be transient * More debugging for the gradle issue on android * Run android on os only as well Since: - the ubuntu build appears to be flaky - most app developers will build on mac in order to support ios as well
+ refactor the common elements out
To actually run the shared native code
+ ignore `.app.zip` files since that is how we generate iOS deliverables
Import the setup scripts and the CI workflows for Github Actions
Cherry-picked part of https://github.com/fabmob/e-mission-phone-fabmob/commit/df421d9d7524c692963933be20f4fa62fd445baf.patch since I don't think that the OpenID change is correct. Or at least, it requires a lot more discussion. This should also really go into the plugin, but no time for niceties now.
Testing done: - Built android - Saw the list of files being modified - Checked `AndroidManifest.xml` and ensured that the provider was indeed renamed
Verified via `vimdiff` from the covid19 app. Alas, we can't use cherry-pick because of the unrelated changes (e.g. app name, app version) e-mission also uses plugins that are not in the covid19 app, so we upgrade those to the most recent versions as well. A particularly important update is the webview update to 3.2.0 to handle the WKWebView issue. We also update the push plugin to the most recent version. This has been flaky in the past, but we were already at v2, so hopefully this is not a big deal. The push plugin also requires `google-services.json` and `GoogleService-Info.plist` to be downloaded from the Firebase console. These are obviously different for each different native build, AND we are obviously not going to check in the versions for the base app. So we check in fake versions of the files which have the same format as the original files but with faked out ids and project names and modify the setup scripts to copy them over. This will work for the build CI for now - for actual testing CI, we will need to set up encrypted storage as part of the workflow. Which will only run on pushes to the main repo not on the forks, so it will be complicated to test.
Since both android and iOS are in the configuration and plugins for both platforms are installed during every `prepare`
There are 3 additional plugins pulled in as dependencies: - `cordova-support-google-services 1.3.2` (from `phonegap-plugin-push`) - `phonegap-plugin-multidex 1.0.0` (from `phonegap-plugin-push`) - `es6-promise-plugin 4.2.2` (from ???)
To keep the ionic plugins happy
No more crosswalk
- Run cordova the new way - label the resulting apk correctly
+ bump up the cordova android version + use the matching kotlin SDK library + adapt the sign and release script to the new apk locations in cordova-android 7+
+ add links to the actual workflows
Attempt to fix the native build
Since not everybody wants to commit back, and it causes the script to not be idempotent
+ modify workflow file to use `ionic --version` which is the new valid argument for the version
Since it moved to "Contributing" instead
Ci for serve install
In order to be compatible with restrictions in Android Pie. https://android-developers.googleblog.com/2018/04/protecting-users-with-tls-by-default-in.html Both of them support https already so there is no reason to not switch
- which explains which permissions we need - why we need them, and - which prompt to select
Since after upgrading to cordova-ios@5.1.1, the build Just Works even with the new build system
To clarify which button people should press, specific to the version. This fixes e-mission/e-mission-docs#483
The switch to WKWebView forced the use of CORS. There is no workaround. https://ionicframework.com/docs/v3/wkwebview/#cors We didn't encounter this in covid-19 repo since we made all calls through the built-in native plugin that automatically adds the user token and sends out the call. In emission, though, we need make aggregate calls that should not have a user token. We had been using XHR for this (through the angular `$http` server) and all those calls broke. While we could make the appropriate change on the server side, that would not be consistent with our long term goal (e-mission/e-mission-docs#408, e-mission/e-mission-docs#288) So for now, I use the alternative documented here https://ionicframework.com/docs/v3/wkwebview/#i-cant-implement-cors Concretely: - add the native plugin - add a new method `CommHelper.getAggregateData` that wraps it in a promise - change all instances of `$http.post` -> `CommHelper.getAggregateData` - use the configured connectUrl instead of hardcoding - remove the hardcoded URL from index.html Bonus fix: - Dealt with error in the heatmap if there was no data and thus, no bounds by checking to see if the bounds were valid Testing done: Navigated through the app screens until all the aggregate calls were invoked (see below). No errors in the console. ``` [Log] getting aggregate data without user authentication from http://localhost:8080/result/metrics/timestamp with arguments {"freq":"D","start_time":1586131200,"end_time":1587254400,"metric_list":["duration","median_speed","count","distance"],"is_return_aggregate":true} (cordova.js, line 1540) [Log] getting aggregate data without user authentication from http://localhost:8080/result/heatmap/incidents/timestamp with arguments {"start_time":1587187062,"end_time":1587359861,"sel_region":null} (cordova.js, line 1540) [Log] getting aggregate data without user authentication from http://localhost:8080/result/heatmap/pop.route/local_date with arguments {"modes":null,"from_local_date":{"year":2020,"month":4,"day":17,"hour":22},"to_local_date":{"year":2020,"month":4,"day":18,"hour":22},"sel_region":null} (cordova.js, line 1540) [Log] getting aggregate data without user authentication from http://localhost:8080/result/heatmap/incidents/local_date with arguments {"modes":null,"from_local_date":{"year":2020,"month":4,"day":17,"hour":22},"to_local_date":{"year":2020,"month":4,"day":18,"hour":22},"sel_region":null} (cordova.js, line 1540) ```
This involved two fixes:
- recreating the config on resume so that the end date is reset (`list.js`), and
- setting "today" to the correct date when the datepicker is opened
(`ionic-datepicker.bundle.min.js`)
I have also added build instructions to my fork of the ionic-datepicker repo,
but in case I delete the fork later, the instructions are:
Install node 10.19.0 ([gulp-sass will fail for more recent version of node](nodejs/node-gyp#1763))
```
node -v
v10.19.0
```
Install and check gulp version
```
npm install
npm list | grep gulp
├─┬ gulp@3.9.0
```
Build
```
npx gulp build
```
Testing done:
Followed the steps in e-mission/e-mission-docs#531 (comment)
Both end date and today are updated correctly
Fix 'today' in the datepicker
…into port_ui_fixes
…n-phone into port_ui_fixes
Major changes:
- the interface is already promisified, so we don't need to wrap the calls in promises
- some of the functions return slightly different values, supporting them now
- the updatecheck returns a build number that we display for greater debuggability
- Need to add the ionic hosts back to the CSP
Testing done:
- Ran the app with these changes
- Clicked on the link at
https://e-mission.eecs.berkeley.edu/#/client_setup?new_client=urap2017emotion
- UI was updated to the emotion UI
After the update, we had the following errors:
```
[Error] Failed to load resource: the server responded with a status of 404 () (others.js, line 0)
[Error] Ionic Deploy: Unable to check for updates – TypeError: deploy.init is not a function. (In 'deploy.init(config, resolve, reject)', 'deploy.init' is undefined) — updatecheck.js:38
TypeError: deploy.init is not a function. (In 'deploy.init(config, resolve, reject)', 'deploy.init' is undefined) — updatecheck.js:38(anonymous function) — updatecheck.js:38Promise(anonymous function) — updatecheck.js:33(anonymous function) — updatecheck.js:163(anonymous function) — updatecheck.js:204processQueue — ionic.bundle.js:29127(anonymous function) — ionic.bundle.js:29143$digest — ionic.bundle.js:30211(anonymous function) — ionic.bundle.js:30434completeOutstandingRequest — ionic.bundle.js:19194(anonymous function) — ionic.bundle.js:19470
(anonymous function) (cordova.js:1540)
(anonymous function) (updatecheck.js:199)
promiseReactionJob
[Error] Refused to load unsafe:ionic://localhost/img/neutralbear.gif because it does not appear in the img-src directive of the Content Security Policy.
```
These are because the channel has not yet been updated to use the new UI
changes with the new updatecheck code and the `ionic:` whitelist. Some of these
(like `others.js`) may also just be obsolete.
Port over the UI fixes related to the plugin interface changes
Added to workaround CORS issues with making http calls from javascript directly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Two pending issues:
This fixes #475 and #480. I will close #519 once we have confirmation of OpenIDConnect working