Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Reporting does not work when specifying deploymentKey programmatically #117

Closed
Justin-Credible opened this issue May 7, 2016 · 4 comments

Comments

@Justin-Credible
Copy link

Steps to reproduce:

  1. Ensure deployment key is NOT present in config.xml
  2. Add a new deployment via the CLI
  3. In the app, call codePush.sync(...) with a SyncOptions object populated with the deploymentKey
  4. After update check install metrics from the command line using: code-push history MyApp MyDeployment

Expected results: Install metrics should have updated.

Actual results: Install metrics have NOT updated.

Prior to #108, specifying the deployment key programmatically would cause a crash on iOS. #110 fixed this crash, but the way the fix was implemented will prevent install metrics from being reported. See here and here where the reporting code is short circuited with an early return if the deployment key is not in config.xml.

@geof90
Copy link
Contributor

geof90 commented May 8, 2016

@Justin-Credible Thanks for reporting. From what I understand looking at the code, after an app is installed, it should pull the deploymentKey from the downloaded package metadata instead of the config.xml https://github.com/Microsoft/cordova-plugin-code-push/blob/master/src/android/CodePush.java#L122 so the deployment key should not be null. Therefore the #110 fix should not impact this. However, I will test this some more later in the day and see if I can repro this.

@geof90
Copy link
Contributor

geof90 commented May 8, 2016

@Justin-Credible Just an update that I was not able to repro this issue, and I did see install metrics being reported correctly in my CLI, with an update that I got by specifying the deployment key in the call to sync() but NOT the config.xml.

Just for more information for us, is your app an Android or iOS app?
What version of the plugin are you using?
When does your call to sync() happen in your app code? Is it the deviceReady handler?

@Justin-Credible
Copy link
Author

@geof90 After re-testing it looks like the metadata is being reported properly. My mistake! I was hung up on the fact that the method was being short circuited.

@geof90
Copy link
Contributor

geof90 commented May 9, 2016

Cool! Thanks for confirming.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants