-
Notifications
You must be signed in to change notification settings - Fork 115
Fix client reset on start #470
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
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
I guess this has been working on production due to timing issues but it is not a long-term fix. All native code should really be inside an ionicPlatform.ready block and not in random places during file load. This includes all callbacks
Back in the old days, we tried to move all storage to local storage so that we could read it without waiting for ionicPlatform.ready and startup quickly. Well, it turned out that sometimes local storage was deleted on iOS, so we ended up with the case where local intro_done was deleted, so we would reset the UI. Which looked stupid and like a bug (https://github.com/e-mission/e-mission-phone/issues/443). Re-writing to pull out the KV storage into a service, write primarily to native with local as backup, and read from both and unify. Everything was already promisified because we were reading consent from native code, so I am not sure that this actually affects performance. And it is a heck of a lot easier to understand. Testing done: - started app -> went to intro - restarted app after restarting emulator -> went to current screen. For fixes to the current screen, see 0ecfdae - cleared out intro_done and consent from local storage and restarted app -> went to current screen - cleared all native storage, generated popups for intro_done and consent, then crashed due to e-mission/e-mission-transition-notify#18 On relaunch, launched properly without any popups. I declare that this is done. ``` [phonegap] [console.log] DEBUG:uc_stored_val = null ls_stored_val = {"intro_done":"2018-09-25T13:14:51-07:00"} ```
To indicate that you need to enter a prompt, not a token
so that we know when it happened and it is not just in a popup that people might have ignored.
@krisma really really wanted it, but there is no evidence that anybody else cares. And there's a bunch of random code sitting around in here making it complex. Removing it since we are cleaning up the base code anyway.
Dark theme is now gone for good
|
We want to move away from angularLocalStorage, since local storage can be deleted by the OS, at least on iOS, when the device is low on disk space. This means that we can't store persistent data in local storage, only session data. In a09385b, fixed this for the startup code. This is the first of a series of changes that will refactor other code code to remove all instances. In this change, we remove the module dependency if it is no longer used. Simple changes!
In particular: - metrics stores the user display preference for calories - metrics stores the user BMI data to correct for calorie burn - updatecheck stores the channel Also change the control panel to read the data using promises instead of directly from local storage. This also involved minor changes (function -> field) that also required changes to the HTML. Also, add new clear methods to the storage function and call them from the control panel. Testing done: - Navigated between dashboard and profile multiple times with no errors - Set the user information in the dashboard - verified that it was visible in the profile
error is JSON, plugin expects string, so we can't deserialize properly
This is intended to return data that: - we want to return inline (for easy porting) - is not catastrophic if it is cleared out (e.g. walkthrough code), OR - is used primarily for session storage so will not be cleared out (e.g. referral code) We can replace this with promisified native storage in a future PR if needed The code does copy the native value to local storage in the background, so even if this is stripped out, it will work on retry.
goals code in general switches to native storage, but the referral code alone, since it is not intended for long-term storage, uses the new local-only storage. Note that we can change the referral code to store a single object that includes all referral objects, e.g. { REFERRED_KEY: REFERRED_GROUP_ID: REFERRED_USER_ID: REFERRAL_NAVIGATION_KEY: } ... I had initially implemented this change, but the navigation key is modified separately from the others, so we would need to deal with it correctly, and I don't really want to re-test this code when nobody is actively using it. Bonus fix: clean up the survey code that we had marked with a TODO
This was a bit tricky because of the way that the slider is hooked into the variable. If you read the data immediately without promisifying AND the load happens before the store, then the slider is set to the loaded value and the store retains the correct value. HOWEVER, if you read the data through a promise, then you get the store first, and without additional checks, the value is reset to the default. The solution is to add additional checks to only store valid values.
The changes are fairly straightforward.
Other modules also stored data in local storage. Moved almost all of them to native storage.
I also tried to move out the walkthrough into its own module for clarity but that seems to subtly interfere with the timing, so moved it back. |
Future improvements may include:
|
Merged
Split out the munging into its own function. And then use the new functin to temporarily munge values before syncing local -> remote
There are multiple reasons for this: - If the user has not logged in, this generates an error - The most common use case is for the user to set the channel and then log in In that case, this fails anyway. - The channels typically set their own values in their code anyway ``` $scope.login = function() { window.cordova.plugins.BEMJWTAuth.signIn().then(function(userEmail) { // ionicToast.show(message, position, stick, time); // $scope.next(); ionicToast.show(userEmail, 'middle', false, 2500); CommHelper.registerUser(function(successResult) { $scope.finish(); }, function(errorResult) { $scope.alertError('User registration error', errorResult); $scope.finish(); }); }, function(error) { $scope.alertError('Sign in error', error); $scope.finish(); }); }; ```
Since we no longer update the channel on the server when we set the client (2737600)
If you are returning `store_val`, make sure to define `store_val` and not `storage_val`. Without this fix, we get the error ``` variable store_val not found ```
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.
Main change is to switch to native storage, consistent with https://github.com/e-mission/e-mission-phone/issues/443#issuecomment-423066729
Bonus fixes: