Skip to content
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

Finish the upgrade to the most recent versions #704

Merged
merged 32 commits into from
Aug 1, 2020

Conversation

shankari
Copy link
Contributor

@shankari shankari commented Aug 1, 2020

shankari added 30 commits July 15, 2020 08:23
So that we can stuck mucking around with our plugins properly
Similar to 9c705ab
but for the other package.json

Note that this includes phonegap and the resulting upgrade
apache/cordova-fetch#87
- Any plugins defined in config.xml are now ignored
    - TESTING DONE: Ran `npx cordova prepare` multiple times and no plugins were added
    - it looks like `package.json` is the only source of truth
    - so removed all plugins from `config.cordovabuild.xml`.

`config.cordovabuild.xml` and `config.serve.xml` are now almost identical except for the `<edit_config>` bits and the native hooks

- Remove preferences related to obsolete features
    - splashscreen
    - crosswalk

- Upgrade the `minSdkVersion` to 22 to be consistent with cordova-android@9.0.0
All plugin ids are now `cordova-plugin-em-*`
Paths are still relative file paths since we haven't pushed and released the plugin code yet
+ downgrade socialsharing to a version that does not include androidX
…in-ionic-keyboard`

Per
ionic-team/ionic-plugin-keyboard#305

Also remove related javascript code because of the two options:
- one is already the default
- it is unclear what the second does and whether it is needed
+ enable androidX (e-mission/e-mission-docs#554 (comment))
+ remaining instances of `android.support
    e-mission/e-mission-docs#554 (comment)
+ add the androidx adapter plugin to fix them

Everything compiles now
Fix action format - `identifier` -> `id`
Fix the listeners which now listen for the action id instead of `action`

Now, using "CHOOSE" also pops up the trip end notification and "SNOOZE" and "MUTE" also work as usual
…s well

+ handled this similar to the server code with multiple promises in parallel
e-mission#703

+ handle stupid bug leading to "invalid date". We were populating the day using
"day", but in moment, that is the day of the week. We need to use "date" instead.
Since dates in moment are 1 indexed, this manifests as a invalid date if tested
on Sunday

Bonus of working on the weekend
0.8 -> 0.9 introduced some breaking changes.
This adapts to those changes. The biggest change is that we need to listen to
the action ID instead of to 'action' and then check the ID.

After these changes, and the hack to fix the timestamp on the emulator
generated locations, e-mission/e-mission-transition-notify#25 (comment)
and the minor fix for the start transition, everything works Just Fine

This (partially) fixes
e-mission/e-mission-transition-notify#25
Since the current version has the data pre-parsed before it is received
e-mission/e-mission-transition-notify#25 (comment)
Note that this is consistent with the sample app, but is NOT consistent with
the master branch. There is a regression in the master branch - inline actions
don't work although they are in the README. Specifying the action group ahead
of time does work in isolation, but seems to require configuring the action
group just before the schedule.
e-mission/e-mission-transition-notify#25 (comment)

So we go back to `0.9.0-beta.3` and the working spec
This was used to determine the regression
e-mission/e-mission-transition-notify#25 (comment)

Currently commented out until we have a chance to test the most recent version again
Similar to the way that `package.json` adds them. This will avoid unnecessary
diffs as we add and remove platforms and plugins.
Since it looks like they are not needed
Icons created using

```
npx ionic cordova resources
```

Nothing more to add. Should probably stop checking these in going forward.
ngCordova is now deprecated
https://github.com/ionic-team/ng-cordova#readme

We could move to IonicNative, but not sure how much longer we are going to
stick with Ionic.

Going back to regular cordova plugin calls, which we use in other places as well.
The only places where we were using ngCordova wrappers was for the in-app
browser and the email composer

+ changed the options for the in-app browser to match the new spec in which the
options are in a single string instead of a JSON dict. Without this change, the
browser is not launched properly
Instead of the local file system
wrt
e-mission/e-mission-docs#554 (comment)

the native instructions suggest adding it to build.gradle
So I looked at `build.gradle` and found

```
if (cdvHelpers.getConfigPreference('GradlePluginGoogleServicesEnabled', 'false').toBoolean()) {
    apply plugin: 'com.google.gms.google-services'
}
```

That just clarified that we don't have to wait for the plugins to edit the config file,
we can just edit the config file directly.

After adding the preferences, I don't get the push notification error on startup, AND

```
platforms/android/app//build/generated/res/google-services/debug/values/values.xml
```

has the appropriate value.

This should be the last pending issue
The CI build was not failing although there were errors.
That is because we switched from `source` to `bash`, so the job doesn't crash any more.
Instead it exits.

Moving the checks to the next action so that the setup is the last action and
exiting with a failure code causes the action to fail.

+ try to add some other commands to the cocoapods uninstall
Although it was claimed to be supported
https://guides.rubygems.org/command-reference/

RubyGems will ask for confirmation if you are attempting to uninstall a gem that is a dependency of an existing gem. You can use the –ignore-dependencies option to skip this check.
@shankari shankari merged commit be9ab2f into e-mission:master Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

⬆️ Track upgrade to cordova android@9.0.0 and ios@6.0.1
1 participant