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

fix: fix issue with category icons not loading within reasonable time after importing a config #748

Merged
merged 1 commit into from
Sep 23, 2024

Conversation

achou11
Copy link
Member

@achou11 achou11 commented Sep 23, 2024

Fixes #611

Doing this as a patch for the backend for now. Will port this to core shortly so that it's available in the next release.

The primary issue was that stopping the HTTP server was taking a long time due to Node HTTP's default behavior when calling its close() method. In this case, it attempts to gracefully close longstanding connections (such as keep-alive connections created by a fetch client). In our case, what was happening when importing a config was:

  1. Leave app to go to file picker. This triggered the 'pause' event in the backend, in which we close the HTTP server:

    rnBridge.app.on('pause', async (pauseLock) => {
    log('App went into background')
    await fastifyController.stop()
    pauseLock.release()
    })

    Due to the behavior described earlier, the initial stopping of the server here could take many seconds (I saw 40 seconds on my device). This affects the resume behavior as described in the next step.

  2. Return to the app after choosing a file. This triggered the 'resume' event in the backend, in which we start the HTTP server again:

    rnBridge.app.on('resume', () => {
    log('App went into foreground')
    fastifyController.start()
    })

    Because we wait for the server to stop before starting again, what was happening here is that this step was waiting for the many seconds that step (1) took. During this time, the app is interactive again but does not have access to the server that provides the HTTP-based assets (such as category icons), so things fail to load and you see lots of loading UI as a result.

Not sure if this entirely fixes all issues related to category icons not loading properly, but it fixes a common user path which is more related to the changing config and how we need to refetch data after that.


611-icons-loading-after-config-import.mp4

Copy link
Contributor

@cimigree cimigree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unclear if this will fix the issue at other times than when importing a new config, but at least it helps in that situation. I tested it in as many ways as I could think of and I never saw any loading issues with the icons at all even in other situations.

@achou11
Copy link
Member Author

achou11 commented Sep 23, 2024

I am unclear if this will fix the issue at other times than when importing a new config, but at least it helps in that situation. I tested it in as many ways as I could think of and I never saw any loading issues with the icons at all even in other situations.

Yeah likewise - I think some of the other issues that are logged with this one in Notion are not fully-related to this and probably caused by other things in the code. Will close the issue in question for now by merging this though.

@achou11 achou11 merged commit 13bfaef into develop Sep 23, 2024
3 checks passed
@achou11 achou11 deleted the 611/icons-stuck-loading branch September 23, 2024 20:43
achou11 added a commit that referenced this pull request Sep 24, 2024
achou11 added a commit that referenced this pull request Sep 24, 2024
ErikSin added a commit that referenced this pull request Oct 14, 2024
* chore: Github action for creating release candidate (#618)

* chore: update version

* chore: create flow and configure release types

* chore: edit title names

* chore: documentation

* chore: spelling

* chore: remove 'the'

* chore: 'there are features'

* chore: 'repo uses github actions'

* chore: 'develop'

* chore: add 'RC' and SHA to version

* chore: update name

* chore: typos

* chore: take out unnecessary file

* chore: update release candidate action to properly name pr (#639)

* fix: allow metrics env variables to be inlined (#643)

To quote [Expo's docs][0]:

> **Every environment variable must be statically referenced as a
> property of `process.env` using JavaScript's dot notation for it to be
> inlined.** For example, the expression `process.env.EXPO_PUBLIC_KEY`
> is valid and will be inlined.

Our metrics environment variables weren't doing this, so they couldn't
be inlined.

Closes [#640].

[0]: https://docs.expo.dev/guides/environment-variables/#how-to-read-from-environment-variables
[#640]: #640

* fix: use proper timestamp for location metadata (#645)

We should be using an ISO timestamp instead of a string number here.

Because `position` can be undefined and we were checking that earlier, I
moved things around a tiny bit to only check that once.

Closes [#644].

[#644]: #644

* feature: edit a track (#633)

* Adds edit icon to track. 
* Navigates to edit track screen if it is user's track. 
* Edit track screen added using the editor component
* Hook added for editing a track
* Doesn't allow preset to change if it is a track

* feat: show config details and upload new config (#608)

* chore: scaffolding for screens

* chore: translations

* chore: added import button

* chore: translations

* chore: create select file function

* chore: create select file and import config hook

* chore: use hook

* chore: use reusable file picker

* chore: update file extension to be 'comapeocat'

* Update `@mapeo/core` to 9.0.0-alpha.23 (#630)

* WIP: update `@mapeo/core` to 9.0.0-alpha.17

This isn't finished but does most of the upgrading needed.

* WIP: update to `@mapeo/[email protected]`

* chore: update to latest core, ipc, and schema

* chore: update to latest core, ipc, and schema in backend

* chore: update type

* chore: remove attachments from tracks

* Update `@mapeo/core` to latest (again)

---------

Co-authored-by: ErikSin <[email protected]>

* chore: EXPO_PUBLIC_FEATURE_TEST_DATA_UI set to true in RC profile (#652)

* chore: test data generator

* chore: take out npx executable

* Changes to using the new hook for matching a device to itself. (#653) (#667)

Co-authored-by: cimigree <[email protected]>

* Fixes ternary on editing an observation for changing preset (#668)

* fix: leave current project when joining another project (#646) (#670)

* fix: project name appears after successfully joining via invite (#672) (#690)

* chore: update copy to warn user about potential loss of data (#691) (#693)

* chore: copy

* chore: translations

* chore: copy

* chore: copy

* chore:translations

* chore: add period

* chore: styling

* feat: show date user added to project and screen instructing user on how to leave project (#674) (#692)

* chore: update device card and device name

* chore: translations

* chore: update device card

* chore: mapeo => CoMapeo

* chore: unnecessary array

* chore: update deviceCard

* chore: title sizing

* chore: bold

* chore: add question icon

* chore: remove unusedimport

* chore: remove unneccesary justify content

* fix: manage "obscured" config variables via babel env variable transform (#656) (#695)

Co-authored-by: Gregor MacLennan <[email protected]>

* fix: prevent text overflow in invitee role selection screen (#682) (#698)

* fix: update copy to replace Mapeo with CoMapeo (#684) (#699)

* fix: presets forever loading (#642) (#701)

Co-authored-by: cimigree <[email protected]>

* fix: handle category-related changes after importing a config more elegantly (#687) (#700)

* fix: do not set observation metadata when using manual gps coordinate (#705) (#706)

Co-authored-by: Evan Hahn <[email protected]>

* chore: tidy type for location callback (#710)

Cherry-pick of 306aa51.

* fix: simplify "accurate location" hook (#712)

Cherry picked from commit a8c7390.

* fix: create test data with proper metadata (#713)

Cherry picked from commit 81514f9.

* fix: always specify a device type (#711)

Cherry picked from commit cf09155.

* chore: add env file template (#697) (#714)

* fix: populate subject field when sharing observation via email (#680) (#715)

* fix: manual gps prompt when no location service or innacurate location (#702) (#719)

* fix: prompts user to manually add gps when no or inaccurate location

* chore: translations

* chore: small fixes

* chore: pr fixes

* chore: remove project config icon (#717) (#721)

* chore: New Crowdin updates (#694) (#722)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (German)

* New translations en.json (Japanese)

* New translations en.json (Mongolian)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Indonesian)

* New translations en.json (Khmer)

* New translations en.json (Tamil)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Sinhala)

* New translations en.json (Swahili)

* New translations en.json (Nepali)

* New translations en.json (Ogiek)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (German)

* New translations en.json (Japanese)

* New translations en.json (Mongolian)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Indonesian)

* New translations en.json (Khmer)

* New translations en.json (Tamil)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Sinhala)

* New translations en.json (Swahili)

* New translations en.json (Nepali)

* New translations en.json (Ogiek)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

---------

Co-authored-by: Digidem Bot <[email protected]>

* chore: update dependencies (#716) (#723)

* chore: update dependencies

This updates all of our (Co)Mapeo dependencies to their latest versions.
This should be the last time we do this before the 1.0 releases of those
modules.

* Default config's extension is `.comapeocat`

* chore: update mapeo-schema for ESM fix

* chore: update backend deps

* chore: update @electron/asar patch

* fix: temp fix

---------

Co-authored-by: Evan Hahn <[email protected]>
Co-authored-by: Gregor MacLennan <[email protected]>

* chore: update EAS config (#726) (#727)

* chore: update EAS config node version & VCS

* remove versionCode from app.json

* chore: change how versions are named (#725) (#728)

* chore: change how versions are named

* set node version used for builds

* chore: update EAS config node version & VCS

* remove versionCode from app.json

* nit: capitalize RC

* Changes isEnabled for metrics permission to be enabled by default. (#734)

* fix: update message content when sharing observation (#676) (#736)

* chore: New Crowdin updates (#733) (#737)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (German)

* New translations en.json (Japanese)

* New translations en.json (Mongolian)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Indonesian)

* New translations en.json (Khmer)

* New translations en.json (Tamil)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Sinhala)

* New translations en.json (Swahili)

* New translations en.json (Nepali)

* New translations en.json (Ogiek)

Co-authored-by: Digidem Bot <[email protected]>

* chore: update CoMapeo dependencies to final release versions (#732) (#738)

This updates CoMapeo dependencies to their final release versions.
Notably, several names changed. For example, `@mapeo/core` is now
`@comapeo/core`.

There are no other changes other than renames and version bumps.

Co-authored-by: Evan Hahn <[email protected]>

* fix: mapeo/schema to comapeo/schema (#739) (#741)

* chore: remove test data generator (#735)

* fix: update to `@comapeo/[email protected]` (#746)

Cherry pick of 9b121f0.

* fix: fix issue with category icons not loading within reasonable time after importing a config (#748) (#750)

* chore: move query invalidation of config-related queries to useImportProjectConfig() (#749) (#751)

* chore: uninstall unused mock data package (#752) (#753)

This change should have no user impact.

This uninstalls `@mapeo/mock-data`, as it is unused.

Co-authored-by: Evan Hahn <[email protected]>

* chore: run CI workflow on pushes to develop and release-related branches (#740) (#754)

* fix: Only allow coordinator to update config (#756) (#759)

* chore: check if user is coordinator

* chore: create useGetOwnRole hook

* chore: useGetOwnRole hook in config screen

* chore: change role key

* chore: update `@types/react-native-zeroconf` to latest version (#763) (#773)

I think this is a useful change on its own but should also make [an
upcoming change][0] easier.

[0]: #747

Co-authored-by: Evan Hahn <[email protected]>

* fix: unpublish dns-sd service (#747) (#774)

Co-authored-by: Gregor MacLennan <[email protected]>
Co-authored-by: Evan Hahn <[email protected]>
Co-authored-by: ErikSin <[email protected]>

* chore: configure Sentry environment (#730) (#775)

Co-authored-by: Gregor MacLennan <[email protected]>

* chore: update copy (#767) (#776)

* chore: updated copy

* chore: take out unused variables

* New Crowdin updates (#743) (#777)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (German)

* New translations en.json (Japanese)

* New translations en.json (Mongolian)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Indonesian)

* New translations en.json (Khmer)

* New translations en.json (Tamil)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Sinhala)

* New translations en.json (Swahili)

* New translations en.json (Nepali)

* New translations en.json (Ogiek)

* New translations en.json (Spanish)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (German)

* New translations en.json (Japanese)

* New translations en.json (Mongolian)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Indonesian)

* New translations en.json (Khmer)

* New translations en.json (Tamil)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Sinhala)

* New translations en.json (Swahili)

* New translations en.json (Nepali)

* New translations en.json (Ogiek)

* New translations en.json (Spanish)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Spanish)

* New translations en.json (French)

* New translations en.json (Spanish)

* New translations en.json (German)

* New translations en.json (Japanese)

* New translations en.json (Mongolian)

* New translations en.json (Dutch)

* New translations en.json (Vietnamese)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Indonesian)

* New translations en.json (Khmer)

* New translations en.json (Tamil)

* New translations en.json (Thai)

* New translations en.json (Burmese)

* New translations en.json (Sinhala)

* New translations en.json (Swahili)

* New translations en.json (Nepali)

* New translations en.json (Ogiek)

---------

Co-authored-by: Digidem Bot <[email protected]>

* feat: set preset language to match app language (#783) (#784)

* feat: set preset language to match app language

* feat: translate fields

* test: load env variables so `npm test` works locally (#742) (#790)

We [set environment variables in CI][0] so that `npm test` works, but it
doesn't work locally because Jest isn't loading them.

This loads them when running Jest.

[0]: https://github.com/digidem/comapeo-mobile/blob/9b6cf8f71a0d960174c7e8cd5ce1b6bb8f1dd5a7/.github/workflows/ci.yml#L72-L75

Co-authored-by: Evan Hahn <[email protected]>
Co-authored-by: Gregor MacLennan <[email protected]>

* test: update Jest to latest version, 29.7.0 (#781) (#791)

This updates `jest` and `@types/jest` to their latest versions.

Co-authored-by: Evan Hahn <[email protected]>

* fix: Fix changing preset & store `presetRef` on observation (#785) (#792)

* fix: Fix changing preset & store preset ref on observation

* fix: revert removed ts-expect-error

(my vscode was using a newer version of typescript to check this, leading me to think this was not needed)

* chore: improve code legibility of usePreset

* chore: add unit test for updatePreset

* fix: missing presetRef when first creating observation without photos

* fix: editing an observation was not correctly removing tags

Co-authored-by: Gregor MacLennan <[email protected]>

* New Crowdin updates (#780) (#793)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Portuguese, Brazilian)

* New translations en.json (Spanish)

Co-authored-by: Digidem Bot <[email protected]>

* chore: take our background location permission

---------

Co-authored-by: ErikSin <[email protected]>
Co-authored-by: Evan Hahn <[email protected]>
Co-authored-by: cimigree <[email protected]>
Co-authored-by: Gregor MacLennan <[email protected]>
Co-authored-by: Andrew Chou <[email protected]>
Co-authored-by: Digidem Bot <[email protected]>
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.

Preset icons are sometimes stuck loading
2 participants