-
-
Notifications
You must be signed in to change notification settings - Fork 196
feat: add tracking in analytics for company using NativeScript CLI #4984
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
Merged
Conversation
This file contains hidden or 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
lib/common/test/unit-tests/services/json-file-settings-service.ts
Outdated
Show resolved
Hide resolved
lib/common/test/unit-tests/services/json-file-settings-service.ts
Outdated
Show resolved
Hide resolved
lib/common/test/unit-tests/services/json-file-settings-service.ts
Outdated
Show resolved
Hide resolved
77c4bec
to
d92227d
Compare
DimitarTachev
approved these changes
Aug 28, 2019
test cli-device cli-smoke cli-run cli-debug |
test cli-device cli-run cli-debug |
test cli-device cli-debug |
d92227d
to
a6861ea
Compare
test cli-run cli-device |
Fatme
approved these changes
Aug 29, 2019
test cli-device cli-debug |
Add tracking with information about the company from which CLI is being used. Use the insights endpoint to get information about the company and track it as custom dimensions on each hit. >NOTE: Information will be tracked only when CLI's usage-reporting is enabled. In case you do not want to have this information tracked, execute `tns usage-reporting disable`.
Add a new ipService to get the current ip address. As this information is critical for us, use three different ways to determine the ip address, so even if one of them fails, use the result from the next one. Use the `play.nativescript.org/api/whoami` endpoint to get data for the public ip address.
Currently `$injector` can have non-shared instances by setting the third argument of the `register` method to false, i.e.: ``` $injector.register("myClass", MyClass, false); ``` However, in this case, the injector caches only the last created instance and this way it will not call the dispose method of all created instances. Fix this by caching all instances in an array and iterrate over them when disposing them. This way we can have correct resolving of instances by name when constructor has non-injectable dependencies: ``` class A { constructor(private stringArg: string) { } } $injector.register("a", A, false); // in another file: $injector.resolve("a", { stringArg: "stringValue" }); //returns new instance; $injector.resolve("a", { stringArg: "different stringValue" }); //returns new instance; ```
Intorduce a new service - `jsonFileSettingsService` to work with JSON files in which you need to persist specific settings. The read/write operation of the file are locked, which makes the service usable in multiple cases. Having a separate service allows us to use multiple files to store data instead of writing everything in the user-settings.json file. To create the service reuse the logic from the old UserSettingsServiceBase and make it available through `$injector`. Add logic in the service to allow writing/reading data within specific cache timeout. In case you want to save data with cache information, just pass this setting to saveSetting method and the service will automatically add required information in the file. When you read information from the file, the service will check if you have passed cacheTimeout and will respect it when checking the value. In case you've passed cacheTimeout, but the value in the file does not have cache information, the service will return null as it is unable to determine if the cache is valid. In case there's cache information, the service will return the value in case the cache had not expired and null otherwise. In case the value in the file has cache information, but the readSetting is called without cacheTimeout, the value is returned immediately without checking the cache. The service is marked as non-shared, so for each `$injector.resolve("jsonFileSettingsService", { jsonFileSettingsPath: "myPath.json" })`, CLI will create a new instance of the service. As all actions are locked via lockfile, this shouldn't be a problem.
Add local caching in a file when getting the company information. This allows us to minimize the number of calls to external services. As the companyInsightsService now uses multiple other services and is also exposed publically, rename it to companyInsightsController. Set the expiration of the cache to 2 days.
a6861ea
to
54a579f
Compare
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.
feat: add tracking for companies
Add tracking with information about the company from which CLI is being used. Use the insights endpoint to get information about the company and track it as custom dimensions on each hit.
feat: add ipService to get current ip address
Add a new ipService to get the current ip address. As this information is critical for us, use three different ways to determine the ip address, so even if one of them fails, use the result from the next one.
Use the
play.nativescript.org/api/whoami
endpoint to get data for the public ip address.fix: handle correctly non-shared instances in injector
Currently
$injector
can have non-shared instances by setting the third argument of theregister
method to false, i.e.:However, in this case, the injector caches only the last created instance and this way it will not call the dispose method of all created instances. Fix this by caching all instances in an array and iterrate over them when disposing them.
This way we can have correct resolving of instances by name when constructor has non-injectable dependencies:
feat: introduce jsonFileSettingsService to work with JSON files
Intorduce a new service -
jsonFileSettingsService
to work with JSON files in which you need to persist specific settings. The read/write operation of the file are locked, which makes the service usable in multiple cases.Having a separate service allows us to use multiple files to store data instead of writing everything in the user-settings.json file. To create the service reuse the logic from the old UserSettingsServiceBase and make it available through
$injector
.Add logic in the service to allow writing/reading data within specific cache timeout. In case you want to save data with cache information, just pass this setting to saveSetting method and the service will automatically add required information in the file.
When you read information from the file, the service will check if you have passed cacheTimeout and will respect it when checking the value. In case you've passed cacheTimeout, but the value in the file does not have cache information, the service will return null as it is unable to determine if the cache is valid.
In case there's cache information, the service will return the value in case the cache had not expired and null otherwise. In case the value in the file has cache information, but the readSetting is called without cacheTimeout, the value is returned immediately without checking the cache.
The service is marked as non-shared, so for each
$injector.resolve("jsonFileSettingsService", { jsonFileSettingsPath: "myPath.json" })
, CLI will create a new instance of the service. As all actions are locked via lockfile, this shouldn't be a problem.feat: add caching for getting company information
Add local caching in a file when getting the company information. This allows us to minimize the number of calls to external services. As the companyInsightsService now uses multiple other services and is also exposed publically, rename it to companyInsightsController.
Set the expiration of the cache to 2 days
PR Checklist
What is the current behavior?
CLI does not track company related information in Google Analytics.
What is the new behavior?
In case analytics are enabled, CLI tracks information for the company from which it is being executed. This can be checked by passing
--analyticsLogFile <path to some file>
to any CLI command.Fixes/Implements/Closes #[Issue Number].