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

🐛 Re-enable "Download JSON dump" #1174

Merged
merged 3 commits into from
Sep 7, 2024

Conversation

shankari
Copy link
Contributor

@shankari shankari commented Sep 6, 2024

After the giant refactor, both "email log" and "download JSON dump" were broken. We fixed "email log" in
#1160 but didn't fix "Download JSON dump then, because of lack of time.

This fixes the "Download JSON dump" as well by making it similar to implementation in "email log".

It was already fairly similar, but for some reason, was reading the data before sharing the file

             const reader = new FileReader();

             reader.onloadend = () => {
             const readResult = this.result as string;
             logDebug(`Successfull file read with ${readResult.length} characters`);

and "this" doesn't exist, resulting in an undefined error. Since the shareObj takes in a file name anyway, I just made this similar to the emailLog changes by passing in the filename directly.

Also, similar ot the "Email Log", added a .txt extension so that the file can be sent on iOS.

Testing done:

  • Before this change: clicking on "Download JSON dump" did not do anything
  • After this change: clicking on "Download JSON dump" launched the share menu

I haven't actually tried sharing yet because gmail is not configured on the emulator.

@shankari
Copy link
Contributor Author

shankari commented Sep 6, 2024

@JGreenlee after this change, the email log and download JSON dump code is almost the same. Would be good to cleanup the next time we touch this code.

Copy link

codecov bot commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 30.13%. Comparing base (2561050) to head (3b6ecd5).
Report is 11 commits behind head on master.

Files with missing lines Patch % Lines
www/js/services/controlHelper.ts 0.00% 16 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1174      +/-   ##
==========================================
+ Coverage   30.02%   30.13%   +0.10%     
==========================================
  Files         118      118              
  Lines        5182     5164      -18     
  Branches     1163     1108      -55     
==========================================
  Hits         1556     1556              
+ Misses       3624     3606      -18     
  Partials        2        2              
Flag Coverage Δ
unit 30.13% <0.00%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
www/js/services/controlHelper.ts 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

@shankari
Copy link
Contributor Author

shankari commented Sep 6, 2024

Retrieving file Share menu
Screenshot 2024-09-06 at 3 30 25 PM Screenshot 2024-09-06 at 3 31 19 PM

After the giant refactor, both "email log" and "download JSON dump" were
broken. We fixed "email log" in
e-mission#1160 but didn't fix "Download
JSON dump then, because of lack of time.

This fixes the "Download JSON dump" as well by making it similar to
implementation in "email log".

It was already fairly similar, but for some reason, was reading the data before
sharing the file

```
             const reader = new FileReader();

             reader.onloadend = () => {
             const readResult = this.result as string;
             logDebug(`Successfull file read with ${readResult.length} characters`);
```

and "this" doesn't exist, resulting in an undefined error.
Since the shareObj takes in a file name anyway, I just made this similar to the
emailLog changes by passing in the filename directly.

Also, similar ot the "Email Log", added a `.txt` extension so that the file can
be sent on iOS.

Testing done:
- Before this change: clicking on "Download JSON dump" did not do anything
- After this change: clicking on "Download JSON dump" launched the share menu

I haven't actually tried sharing yet because gmail is not configured on the emulator.
@shankari shankari force-pushed the fix_json_dump_download branch from b527f37 to b81804a Compare September 7, 2024 00:09
Before this fix, we checked for fleet status only in the EXITED_GEOFENCE case.
This meant that the VISIT transitions started trip tracking, which led to
spurious trips. Fixed by adding a fleet check to this case as well.

Related PR: e-mission/e-mission-data-collection#234
Related release: https://github.com/e-mission/e-mission-data-collection/releases/tag/v1.9.0
This reverts commit 399e9e7.

I _thought_ this wasn't needed because cordova configures `build.gradle` so
that the correct version of gradle is downloaded and installed on the first
run.

However, when I tried to rebuild on a fresh installation, I got the following error

```
Using Android SDK: /Users/kshankar/Library/Android/sdk
Could not find an installed version of Gradle either in Android Studio,
or on your system to install the gradle wrapper. Please include gradle
in your path, or install Android Studio
```

It looks like although cordova installs gradle, the system needs to have a
version of gradle installed as well to bootstrap the gradle wrapper.

This error doesn't show up in the CI/CD pipeline because the CI environment has
gradle installed.

@catarial did you encounter this during your setup as well?! Or maybe it has
not been an issue so far because you are focused on the iOS build.
@shankari shankari merged commit 18047b0 into e-mission:master Sep 7, 2024
7 of 8 checks passed
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.

1 participant