-
Notifications
You must be signed in to change notification settings - Fork 72k
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
Workaround for re-loading bug in profile editor: don't show profile form during save #4074
Conversation
So hey - I've never seen this behavior, so I'm wondering what the root cause for the reload is? Or at least - in our instance, I can edit the profiles and the page doesn't reload and I've never lost edits. Which browser are you testing on? |
Hey @sulkaharo, I've seen this on Windows 10 Chrome and Edge, and iPhone XS Max safari. When it happens there are regular "Client connected to server" logs in the console. I think it's more likely to happen when there's a lot of data being transferred, maybe a timeout is causing socket IO to reconnect? Basically the 'connect' event is fired on the initial connection and any subsequent re-connection, so there's always a chance the callback could be called more than once. It's easy enough to reproduce: just switch your network connection off then on again. |
@DigitalDan1 thanks for this PR. Since this issue can cause problems for users I applied this, so that we can include it in Nightscout 0.11. I tested this PR and it looks good to me. My profile is only hidden very briefly and I noticed the status |
@PieterGit, sounds good, thanks. Just want to capture a few comments here for anyone (including me) that may come back to work on a more elegant fix. I agree the route cause should be fixed, but also think every page should gracefully handle a socket IO reconnect (I’m pretty sure the admin page has the same issue). Hiding screen elements addresses a different problem; if the save takes a long time the user can carry on making changes that are lost when the save actually finishes. I’d usually show a dialog (like a Bootstrap Modal) whilst waiting for the save to return. Hiding the form is crude, but does the job for now. |
Draft release notes for upcoming 0.11 release (currently release candidate phase). # Changes Over 360 commits, 89 files changed, +8,428 / −6,569 lines of changes (full list of changes here: https://github.com/nightscout/cgm-remote-monitor/pull/4022/commits ) ## New features - Fully secure by default out of the box. Unsecure access via http is not allowed anymore by default. This might force you to re-authenticate with your `API_SECRET` or token if you were using unsecure access. (@PieterGit ) - No outdated packages with vulnerabilities are being used anymore (@PieterGit ) - Add Week to Week report (@jpcunningh, #4123 ) - Add Loopalyzer report to analyse looping. Visualize your loop (@lixgbg, #3629 #4235 ) - Add predictions support to Day to Day report (@lixgbg, #3179 ) - Add cgm sensor stop to Careportal (@jpcunningh, #4060) ## Removed features - remove `mqtt` module, because it had a security issue and was not used - remove `sgvdata` module, because it had a security issue, added a lot of complexity and wasn't needed (@PieterGit ). Replacement implementation for CSV and TSV export (@sulkaharo ). ## Improvements - Fix MongoDB database insert handling. Log error on inserts and don't crash in case the MongoDB disk is full or MongoDB quota is reached (@sulkaharo and @jpcunningh) - Upgrade packages to recent version, fixing all known security issues with dependencies (@PieterGit) - Redirect redirect HTTP to HTTPS and implement HSTS (@jweismann, @PieterGit, #4044 and #4010 and #4253 ) - Technical improvement: Migrate from `uglify-js` to `terser-webpack-plugin` (@PieterGit) - Streamlined Heroku deployment template with more descriptive text and more appropriate defaults for new users (@unsoluble, #4116 ) ## Bug fixes - Fix CGM voltageb battery warning level to match xDrip+ (@jpcunningh, #3954 ) - Fix daylight saving and reloading bug in profile editor, (@DigitalDan1, @Kywalh #4029 and #4074 ) - Reduce the amount of Profile Switch treatments being loaded to fix UI slowdown and Nightscout home screen losing AAPS data from >3 hours ago, (@sulkaharo, @vickster1, #4055 ) - Upgrade to [share2nightscout 0.2.0](https://github.com/nightscout/share2nightscout-bridge/releases/tag/0.2.0). Prevent Nightscout server crashes in case Dexcom server does not respond (@PieterGit, @veryfancy) - Fix UI so pills are updated immediately after new data is loaded (@sulkaharo) - Fixes to If-Modified-Since HTTP header handling for BG data (@sulkaharo) ## Documentation and language updates - Language updates for Danish, Dutch, German, Hebrew, Norwegian, Russian - New languages: Japanese, Turkish - Update Alexa documentation. Note that some Alexa improvements are postponed to Nightscout 0.12 because the Alexa plugin needs refactoring, see #4168 (comment) - Update IFTTT maker-setup.md docs (@Dave9111, @unsoluble, #4206 ) - Updated various docs, including [CONTRIBUTING](https://github.com/nightscout/cgm-remote-monitor/blob/dev/CONTRIBUTING.md) documentation # Upgrade notes - We only allow Nightscout to start with a secure Node JS. - Latest Node 8 LTS (8.15.0 or later) and Latest Node 10 LTS (10.15.1 or later) are recommended and supported. - Latest Node version on Azure (currently 10.14.0) is tolerated, but not recommended - Other versions will not start - The [rawbg](https://github.com/nightscout/cgm-remote-monitor#rawbg-raw-bg) settings are converted to a single setting tri-state variable. - We improved security and added several new environment variables such as [INSECURE_USE_HTTP and SECURE_HSTS_HEADER](https://github.com/nightscout/cgm-remote-monitor/#predefined-values-for-your-server-settings-optional) - Your site redirects to https by default. If you don't want that or use a Nginx or Apache proxy, set `INSECURE_USE_HTTP` to `true`. - We enabled [HTTP Strict Transport Security (HSTS)](https://en.wikipedia.org/wiki/HTTP_Strict_Transport_Security) headers by default, settings `SECURE_HSTS_HEADER` and `SECURE_HSTS_HEADER_*` ## Upgrade notes for Azure users We recommend Azure users consider migrating their hosting to Heroku, as we've observed Heroku users have significantly less issues with having their sites work reliably. If you want to continue using Azure, change the following configuration variables in Azure before updating to the latest Nightscout version: ``` WEBSITE_NODE_DEFAULT_VERSION=10.14.1 SCM_COMMAND_IDLE_TIMEOUT=300 ``` # Install instructions Install instructions can be found: https://github.com/nightscout/cgm-remote-monitor/blob/master/README.md#install # Contributors to this release The release coordination for this release was done by @PieterGit We would like to thank the following people for their contribution (in alphabetical order): @anderser, @apanasef, @balshor, @bewest, @blocklist_twitter, @CaroGo, @cascer1, @cluckj, @danamlewis, @Dave9111, @diabetlum, @herzogmedia, @janrpn, @jasoncalabrese, @jpcunningh, @jweismann, @kenstack, @Kywalh, @lixgbg, @LuminaryXion, @MilosKozak, @mitrei, @PaperT1D, @PieterGit, @unsoluble, @rarneson, @renegadeandy, @scottleibrand, @sulkaharo, @T-o-b-i-a-s, @tynbendad, @unsoluble, @veryfancy, @viq, @wootmasterslick (if I forgot somebody, please respond) # TODO TODO: Translations, Languages with less than 80% will be removed in a future Nightscout version. Currently the following languages are at risk: 中文(繁體) (zh_tw), Hrvatski (hr), Ελληνικά (el), 한국어 (ko) See https://gitter.im/nightscout/public?at=5bef2f34de42d46bba766f66 TODO: Fix Codacy errors, https://app.codacy.com/app/Nightscout/cgm-remote-monitor/issues?bid=2452379&filters=W3siaWQiOiJDYXRlZ29yeSIsInZhbHVlcyI6WyJFcnJvciBQcm9uZSJdfV0= TODO: test dev after all new features are merged for at least two weeks
This fixes a problem I noticed in the profile editor where it re-loads and re-draws every time there's an update from the server (due to socket.io emit calling the callback).
This was causing issues where the user could be making changes to their profile, then the data would re-load and re-draw, so they'd lose their changes and have to start again.
It was also causing the save button's event to be hooked up multiple times, which meant saving was taking longer and longer, and made it more likely that any further changes made by the user were lost. Users would have seen the user interface update whilst editing, replacing their values with a previous version (from whichever save came back last). This change (crudely) hides the form whilst the save is happening to make sure the user doesn't carry on making changes that will be lost when the re-draw happens.
This should fix #4056.