-
Notifications
You must be signed in to change notification settings - Fork 114
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
Pull from master to get the new onboarding android flow #1080
Closed
Conversation
This file contains 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
dt might be undefined here (like in the case of the most recent / current place in the timeline) In that case we should return nothing instead of letting Luxon create a new date (it would fallback to the current date&time which we don't want)
this is used in a few places - it's tidier to have a common type definition
i) instead of providing timelineLabelMap and timelineNotesMap to the entire component tree under LabelTab, we can pass accessor functions (userInputFor, labelFor, notesFor) to get user inputs in a cleaner way ii) Unify the structure of how multilabel vs enketo user inputs are kept on the phone. Now both will be the 'raw' format (an object with 'data', 'metadata' etc). Note that there is still a discrepancy on the server - for multilabel inputs we get a plain string and for enketo inputs we get the whole userinput entry. The plan is to unify this on the server at some point, so I am doing it here on the phone in advance. iii) instead of using a 'justRepopulated' flag to signify that an entry should not be filtered within 30 seconds of having a user input recorded on it, we can just look at the write_ts in the metadata of that entry's inputs and see if it was within 30 seconds ago. If so, it's immune to filtering. After a label is recorded and added to the timelineLabelMap, we'll trigger a refilter after 30 seconds by calling setLastFilteredTs. This approach negates the need for an extra property on the timelineEntry object - which is good because we want it to match the server type.
This useEffect was watching timelineEntry for updates, but we changed the mechanism so timelineMap is no longer updated -- only the timelineLabelMap gets updated when a new response is recorded. By watching the result of userInputFor(...), this component will recieve the update.
This has always been a property on the server for trips as well as places. For some reason I must have missed it on the first pass of making the ConfirmedPlace type definition.
It keeps the codebase more logical (and more modular) to keep Enketo survey-related type definitions encapsulated away in enketoHelper.
When we have Typescript compiler in 'watch' mode (useful for linting/ finding type errors in VSCode), it outputs to the root level directory 'dist'. Adding this directory to gitignore so we don't end up needlessly committing these output files.
Each tab gets wrapped by an error boundary. We were doing this in the body of the App component which means every time the App component was updated/rerendered, the tabs were recomputed too. Doing the error boundary wrapping outside of the App component ensures that it only happens once and we use the same tab components each time
-Fix size of Leaflet markers -- this changed when we removed all the extra CSS bloat and easy one line restores the previous size -Unify app bar styles -- removed elevation: 3 to soften the shadow (default is 1 and matches the rest of the app better - we were only using elevation 3 to match the style of the Ionic header) -- use 'colors.surface' instead of hardcoded 'white' - unify 'notesButton' style of diary cards - add paddingHorizontal to ModesIndicator -- this way, if there are a lot of modes, they have room to breathe
As in fa1dfbf, TypeScript puts output files here. We don't need git NOR Jest to pick up on these 'dist' files.
With invalid inputs, this function should return early rather than continuing and resulting in a zero-length duration. Fixes the diaryHelper test that was failing.
We need a safe way to mark that validation fails in this function, without actually throwing an error. Let's use a third parameter for this purpose which is a callback function 'onFail' to handle the invalid case. - Updated tests accordingly - now passing
Typescript wants us to be more explicit about what could be null/undefined and what couldn't be. transitionTrip2TripObj should return a Promise that could resolve as an UnprocessedTrip or undefined. (If undefined, it gets filtered later in readUnprocessedTrips) We also need to address that Luxon's DateTime.toISO has string or null as the return type – null is for if the input was invalid. It never should be, but if it was for some reason we can have an error dialog and assert type with an empty string.
mockUnprocessedTrip was unnecessary and not used anywhere mockFilterLocation didn't need all these fake properties and instead could just include the few that are needed and then declare it 'as FilteredLocation'
enketoHelper.test.ts had a lot of the wrong types in the fake responses. They were all strings but I've since updated them so there some strings, some numbers, objects. EnketoResponseData in enketoHelper is correct. - some other type casting (with 'any') in both of these tests to ensure fake inputs satisfy parameters types
this test still had LabelOptions type for the userInput values instead of the UserInputEntry type. It didn't cause the tests to fail because inferFinalLabels and verifiabilityForTrip only check that the values exist - they don't care what properties they have. Anyhow, the types line up now.
-- Part of the fix for e-mission/e-mission-docs#1034 When a user input is recorded, we put it in storage. For snappy performance we also cache it in the timelineLabelMap / timelineNotesMap state. However, if we switch filters or load more trips, this state gets overridden. So, we need to call updateLocalUnprocessedInputs at some point. It is an async function but we can call it without 'await' and just allow it to execute in the background.
-- Part of the fix for e-mission/e-mission-docs#1034 When the yellow checkmark is used to verify a pair of inferred mode+purpose labels, both labels should be verified by a single click. The labels get stored fine; however, the way we update state after doing so prevents both labels from being updated simultaneously and only one of them gets filled in. The resulting UX is that it takes 2 clicks and this is not desired. We could: update state in succession, one after the other (await label A, then proceed label B), OR we could rework these functions to support adding labels for a particular trip as a batch (so the same function call could store mode+purpose at the same time). This was decided as a better and more performant option. It uses Promise.all to store all labels from the batch, then it updates the state with all changes from that batch. When a single label is recorded, it is a batch of 1 - when verifying inferences, it may be a batch of multiple labels.
When selecting date, the following behavior occurs: - If 2+ days, behave as normal (Range of Day1 -> Day2) - If "open Ended" (i.e., only start is picked), fetch a range of days between that day and present (Day1 -> Today) - If 1 day, fetch range for 48 hours from that day TODO: The 1 day range going to 48 hours is meant as a patch, so that this WSOD fix can go to staging. To make a single day only fetch 24 hrs, some changes to the server may be needed.
Instead of mutating an extra property 'displayDt' onto the entries, let's use a get function that returns the displayDt. This should fix issue (4) in e-mission/e-mission-docs#1034. And let's also memoize that function so performance is not impacted
If something was thrown, it could display 'undefined' if 'err' was an error message and not the error object. Using the displayError function is a more proper way to guarantee we see any error messages
- 'key' could be kept in 'data' or 'metadata' depending on if it is a processed or unprocessed entry (this is an inconsistency we may want to revisit later) - handle possible null/ undefined parameters in functions: check for truthy inputs before executing
…-mission-phone into migration-wrapup
We recently added the ability to refresh the config from inside the app. But whenever the config is downloaded for the first time, we cache the resources referenced in it by URL. Subsequent config downloads would still use the resources from the first time. fetchUrlCached now accepts options to pass through to the fetch API; if cache is 'reload', we will skip checking our localStorage cache for a previously stored value. This option will also cause the fetch API will also skip its own internal cache The result of this is that when we refresh the config, URL-referenced resources inside it will also be refreshed.
Fermata fixes apr17
Add 'collectCoverageForm' configuration to track all files for Jest coverage
e-mission/e-mission-data-collection#226 is now merged and a new release https://github.com/e-mission/e-mission-data-collection/releases/tag/v1.8.5 has been created So we can now use the version number instead of a branch while adding the plugin With this commit, the initial implementation in #1144 is done, and we can merge it
* lock React version to ~18.2.0 React 19 and React 18.3 are out (https://react.dev/blog/2024/04/25/react-19-upgrade-guide#react-18-3), but the React Native ecosystem is still on React 18.2. We need to lock the versions down until the React Native packages update. * 💩 Temporarily pin the version to 14.1 so that the CI is successful Consisent with #1148 (comment) * 💩 Revert back even further to xcode 14 Consistent with #1148 (comment) * 📌 Ensure that we are using the correct version of java Before this, we set only the `JAVA_HOME` to pin the java version This was working until a couple of days ago, when, even after setting the version, the java version was not changed. This may be due to the location for the java executable changing, so let's try to set the PATH as well Also print out several environment variables to help debug further Related info: #1148 (comment) #1148 (comment) * 🔧 Change the environment variable to the one used in the new github actions runner Related context: #1148 (comment) * 📌 Pin the mac version to 13 To be consistent with #1148 (comment) Before we fix e-mission/e-mission-docs#1060 * 📌 Pin the mac version to 13 Consistent with #1148 (comment) and b5072f3 (for iOS) * ⏪️ Revert previous attempts to fix the issue since pinning to OSX-13 fixed it #1148 (comment) * 📌 Pin the xcode version again Because apparently the default version, even on the OSX 13 runner, is now xcode 15 #1148 (comment) * Ensure that the switch command is run as root --------- Co-authored-by: K. Shankari <[email protected]>
So we don't get false negatives based on currently active pull requests
Expand the profile screen to simulate bluetooth actions separately and handle them properly in the native code
…est` matches At some point, we changed the mode and purpose user inputs as objects that were stored to be full objects, with start and end timestamps, instead of just the labels. We changed all uses of the MODE and PURPOSE to match it, but apparently forgot to change this location, where the replaced mode button is conditionally displayed. This is a quick change to make the usage here consistent with the rest of the code so that we can push this out ASAP. @JGreenlee, please let me know if there is a more principled fix, it is late and I don't want to experiment any further. Testing done: Please see PR
Since the fleet deployments are installed on work phones, and the background restriction is apparently grayed out on android work phones. e-mission/e-mission-docs#1070 This is a hack (hence the 💩 emoji) but it will allow us to make progress through the onboarding, get the logs and then resolve the issue the right way. Or if we can't figure out how to access it the right way, this can become a config option for deployments that plan to use work phones. This should unblock e-mission/e-mission-docs#1070 (comment)
The previous unit tests for the input matching assumed that the user input would be of the form of the label options, so reused the label options as mock options. However, they actually are of the form of user input objects with data and metadata and the input as a label. We create new mock objects with the correct format, and use them in the tests. This is the reason why #1150 was not caught for so long. And when we fixed the code, the test broke. #1150 (comment) #1150 (comment) Testing done: ``` npx jest www/__tests__/confirmHelper.test.ts Test Suites: 1 passed, 1 total Tests: 11 passed, 11 total ```
…ed_app_check_for_android_fermata
🚑️ 🐛 Ensure that we do show the replaced mode when the `mode_of_inter…
In the previous commit, we had disabled all android background checks. This means that the optimization check would also be disabled, and we would not be able to start the foreground service from the background. Let's change this to only disable to unused apps check
- We want to upgrade the pinned version of the OS to the next one - We don't want to run on `latest` because then changes to the underlying runner will break all tests, including for pull requests, and block development. Instead, we should schedule a periodic (~ once a week) check against `latest` so we know when the latest has changed, and can fix it before again bumping up the pinned version @louisg1337 Also, there was a typo in `latest` :smile
After the react rewrite, we require a min version of 13 e-mission/e-mission-docs#932 (comment) to e-mission/e-mission-docs#932 (comment) to e-mission/e-mission-docs#932 (comment) to e-mission/e-mission-docs#932 (comment)
…ck_for_android_fermata 💩 Turn off the background checks for fleet deployments
…into ios_build_fix
iOS build fix for Xcode 15
New onboarding flow has been done for a while. |
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.
No description provided.