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

✏️ Rewrite KVStore, ClientStats, and CommHelper #1040

Merged

Commits on Sep 27, 2023

  1. add storage.ts, to replace KVStore

    storage.ts is replacing storage.js which had the KVStore service inside it.
    storage.ts will provide a set of functions performing the same duties as KVStorage's functions did.
    
    - `get` -> `storageGet`
    - `set` -> `storageSet`
    - `remove` -> `storageRemove`
    - `getDirect` -> `storageGetDirect`
    - `syncAllWebAndNativeValues` -> `storageSyncLocalAndNative`
    
    storage.ts will still use the "BEMUserCache" Cordova plugin in exactly the same way that KVStore did.
    However, instead of using the `angular-local-storage` package as a wrapper around localStorage, we will use it directly.
    A couple functions were added ('localStorageSet` and `localStorageGet`) to facilitate using localStorage directly - localStorage requires us to stringify objects before storing, and parse them on retrieval.
    
    Other than these substitutions, and being rewritten in modern JS with some typings, the logic is exactly the same as it was in KVStore.
    
    --To facilitate this change, storage.js is temporarily renamed to ngStorage.js so that it doesn't conflict with the storage.ts filename. ngStorage.js will be removed soon after it is not used anymore.
    JGreenlee committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    73a8259 View commit details
    Browse the repository at this point in the history
  2. use storage.ts everywhere instead of KVStore

    storage.ts replaces KVStore, so we can replace all uses of KVStore throughout the codebase, substituting the new functions from storage.ts as follows:
    
    - `get` -> `storageGet`
    - `set` -> `storageSet`
    - `remove` -> `storageRemove`
    - `getDirect` -> `storageGetDirect`
    - `syncAllWebAndNativeValues` -> `storageSyncLocalAndNative`
    JGreenlee committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    b99e001 View commit details
    Browse the repository at this point in the history
  3. remove KVStore / ngStorage.js

    Rewritten into storage.ts, this Angular service is not needed anymore. We can remove the file, remove it from index.js, and remove it as a dependency from all of the Angular modules that previously used it.
    JGreenlee committed Sep 27, 2023
    Configuration menu
    Copy the full SHA
    6ffbf18 View commit details
    Browse the repository at this point in the history

Commits on Sep 28, 2023

  1. use JSDOM as jest env so localStorage works

    Followed the advice of https://stackoverflow.com/a/74309873 so that we can have a mocked out localStorage to use in tests
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    cdabde4 View commit details
    Browse the repository at this point in the history
  2. add tests for storage.ts, plus the needed mocks

    To test storage.ts we need to be able to mock the usercache cordova plugin, as well as fill in the missing window.Logger.
    I created mocks for both of these which are kept in the __mocks__ folder and simply called at the start of storage.test.ts.
    
    Because storage.ts calls getAngularService, angular-react-helper.tsx was included in the process. That file imported PaperProvider, which did not work with jest.
    Because we are not angularizing components anymore, we do not actually need much of the code in angular-react-helper anyway. Let's just take it out so we can safely implicate getAngularService.
    
    As for the storage tests themselves, they check whether the storage functions are able to store and retrieve data, and crucially they ensure that local and native storage can compensate for each other if one of them gets cleared.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    fd4f9d5 View commit details
    Browse the repository at this point in the history
  3. add clientStats.js, to replace ClientStats service

    js/plugin/clientStats.ts will replace js/stats/clientstats.js
    - camelcase filename convention
    - we don't need a dedicated folder for stats because clientstats.js was the only thing in it
    
    Nothing significant has changed here. I just refactored into modern JS and added type definitions for each function's parameters.
    
    mappings of old/new exported methods:
    - getStatKeys -> statKeys (now a variable, not a function)
    - getAppVersion -> getAppVersion
    - getStatsEvent -> getStatsEvent
    - addReading -> addStatReading
    - addEvent -> addStatEvent
    - addError -> addStatError
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    8ada4f8 View commit details
    Browse the repository at this point in the history
  4. use clientStats.ts everywhere, not old ClientStats

    ClientStats was rewritten into clientStats.ts.
    This commit simply substitutes in the new methods for the old ones, everywhere that client stats are recorded, as follows:
    
    - getStatKeys -> statKeys (now a variable, not a function)
    - getAppVersion -> getAppVersion
    - getStatsEvent -> getStatsEvent
    - addReading -> addStatReading
    - addEvent -> addStatEvent
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    6e59af5 View commit details
    Browse the repository at this point in the history
  5. remove the old ClientStats / clientstats.js

    Rewritten into a new file clientStats.ts, this old file is not needed anymore. We can remove the file, remove it from index.js, and remove it as a dependency from all of the Angular modules that previously used it.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    25c7077 View commit details
    Browse the repository at this point in the history
  6. fix getAppVersion in clientstats

    This is used in ProfileSettings is for the "App Version" row at the very bottom of the Profile page.
    getAppVersion() was not working correctly - the correct function to call is `getVersionNumber` which returns a promise. In a .then block, we can memoize this value in the local let appVersion.
    
    Since it is memoized, we don't need a dedicated state value in ProfileSettings for appVersion. We can just call getAppVersion() directly where it's used in the SettingRow.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    1a47634 View commit details
    Browse the repository at this point in the history
  7. add some cordova mocks: cordova,device,appversion

    There are a few other 'window' variables that are provided in a Cordova/Ionic app and are expected in various few places throughout the codebase. To test, we need to be able to mock these too.
    
    mockCordova and mockDevice just provide fake platform/version info.
    mockGetAppVersion mocks the cordova-plugin-app-version, which is a third-party plugin we use to get the version of the app. This is used in clientStats.ts, so we will need to use this mock when we test that. The mock returns info for a "Mock App", version 1.2.3.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    598d1fb View commit details
    Browse the repository at this point in the history
  8. make getAppVersion work async

    getAppVersion() calls cordova-plugin-app-version's getVersionNumber, which is asynchronous. So getAppVersion should be asynchronous too. I changed it to return a promise.
    We handle this in clientStats by adopting async/await syntax.
    We handle this in ProfileSettings by storing it as a ref once the promise is resolved. Then we just access it as appVersion.current.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    47aceaf View commit details
    Browse the repository at this point in the history
  9. add tests for clientStats.ts

    Tests the methods exported from clientStats.ts, including getAppVersion, addStatReading, addStatEvent, and addStatError.
    
    The tests record these stats and then query the usercache for the messages, expecting to see a new entry filled in with the fake data.
    
    putMessage and getAllMessages needed to be added to the mockBEMUserCache, since the client stats are recorded as 'messages' rather than 'key-value' pairs
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    c7b06b7 View commit details
    Browse the repository at this point in the history
  10. rewrite CommHelper service into commHelper.ts

    CommHelper from js/services.js is being rewritten into commHelper.ts.
    This commit moves over the functions from CommHelper that we need. So now the following functions are exported:
    - fetchUrlCached (was already in commHelper.ts)
    - getRawEntries
    - getPipelineRangeTs
    - getPipelineCompleteTs
    - getMetrics
    - getAggregateData
    - registerUser
    - updateUser
    - getUser
    
    The following functions were not moved over to the new commHelper.ts because they are not currently used in the codebase and I don't anticipate them being used in the foreseeable future:
    - putOne
    - getTimelineForDay
    - habiticaRegister
    - habiticaProxy
    - moment2Localdate
    - moment2Timestamp
    - getRawEntriesForLocalDate
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    c16bde5 View commit details
    Browse the repository at this point in the history
  11. use commHelper.ts everywhere, not old CommHelper

    CommHelper from js/services.js was rewritten into js/commHelper.ts. This commit switches all functions calls to the new commHelper.ts.
    All of the functions are named the same and perform the same logic.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    1dc7451 View commit details
    Browse the repository at this point in the history
  12. remove the old CommHelper service

    Rewritten into commHelper.ts, this Angular service is not needed anymore. We can remove the 'factory' declaration from js/services.js and remove it as a dependency from all of the Angular modules that previously used it.
    JGreenlee committed Sep 28, 2023
    Configuration menu
    Copy the full SHA
    f78dab1 View commit details
    Browse the repository at this point in the history

Commits on Sep 29, 2023

  1. Configuration menu
    Copy the full SHA
    5826213 View commit details
    Browse the repository at this point in the history
  2. add commHelper.test.ts

    -Added one test for `fetchUrlCached` - uses a mocked 'fetch' to simulate retreieving data from a URL. It ensures that the second time this URL is queried, the data comes back faster (because it gets cached in the localStorage after the first call)
    
    Note at the bottom explains why other functions were not tested.
    JGreenlee committed Sep 29, 2023
    Configuration menu
    Copy the full SHA
    77fe30a View commit details
    Browse the repository at this point in the history

Commits on Oct 11, 2023

  1. remove backwards-compat munge in storage.ts

    per https://github.com/e-mission/e-mission-phone/pull/1040/files#r1351511049, removes the backwards compat to munge when filling in native values from local storage.
    
    Also removes the backwards-compat comment, replacing it with other comments describing how we fill in missing local or native values
    JGreenlee committed Oct 11, 2023
    Configuration menu
    Copy the full SHA
    a5fcbfe View commit details
    Browse the repository at this point in the history
  2. don't displayErrorMsg in storage.ts

    A user-facing popup here is likely to just annoy or confuse users.
    Let's set these log statements at the "WARN" level so it is more visible than other log statements if we do need to debug it, but not intrusive or detrimental to UX
    JGreenlee committed Oct 11, 2023
    Configuration menu
    Copy the full SHA
    40ce229 View commit details
    Browse the repository at this point in the history
  3. remove unneeded comment

    It is fairly self-explanatory that {key: value} was the chosen approach
    JGreenlee committed Oct 11, 2023
    Configuration menu
    Copy the full SHA
    37ac065 View commit details
    Browse the repository at this point in the history
  4. show error if 'db' not defined in clientStats.ts

    Per https://github.com/e-mission/e-mission-phone/pull/1040/files#r1351477569, we'll show an error message here if the BEMUserCache 'db' is not defined.
    We'll also ensure that if the Logger plugin is undefined, we do not try to call it as this would cause an error.
    JGreenlee committed Oct 11, 2023
    Configuration menu
    Copy the full SHA
    cfd7829 View commit details
    Browse the repository at this point in the history

Commits on Oct 12, 2023

  1. re-implement functions of commHelper

    A couple of the functions that were excluded from the rewrite were requested to be added back: e-mission#1040 (comment)
    
    The logic is the same, but with modernized syntax, and using Luxon instead of Moment to deal with timestamps
    JGreenlee committed Oct 12, 2023
    Configuration menu
    Copy the full SHA
    ea2b8c5 View commit details
    Browse the repository at this point in the history
  2. don't "processErrorMessages" in commHelper

    Appending "user-friendy" descriptions to error popups is already handled in logger.ts (see `displayErrorMsg`), so it is unnecessary in this file.
    JGreenlee committed Oct 12, 2023
    Configuration menu
    Copy the full SHA
    641e8aa View commit details
    Browse the repository at this point in the history
  3. update comment in commHelper.test.ts

    - Add the 2 functions to the list that were re-implemented after the initial rewrite
    - More accurately describe how we would ideally test server comm
    JGreenlee committed Oct 12, 2023
    Configuration menu
    Copy the full SHA
    03dc94e View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    acb36aa View commit details
    Browse the repository at this point in the history
  5. Configuration menu
    Copy the full SHA
    e546d73 View commit details
    Browse the repository at this point in the history