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 site crash when scenario was deleted from DB #329

Merged
merged 6 commits into from
Feb 23, 2024

Conversation

JonasGilg
Copy link
Collaborator

Description

Fixes an issue with the persistent store when a scenario is deleted from the database (scenario ID returns 500 on API fetch).
This issue ocurrs when the redux store still has the scenario id cached in the activeScenarios and scenarioList and the Simulation Chart or Data Cards try to fetch data with this scenario id from the API.
To fix this, IDs in the activeScenarios are ignored if the ID is not in the sceanrioList.
Additionally the scenarioList is not persisted and fetched newly on every load.

Checklist

I, the author of this PR checked the following requirements for good software quality:

  • The code is properly formatted (I ran the formatter)
  • The code is written with our software quality standards (I ran the linter)
  • The code is written using our code style
  • Extensive in source documentation has been added
  • Unit and/or integration tests have been added
  • All texts have been internationalized with at least the following languages:
    • English
    • German
  • I tried addressing all new accessibility problems displayed in the console and documented if they can't be fixed
  • I attached performance measurements to prevent performance degradation
  • I added the changes to the next release section of the changelog

I, the reviewer checked the following things:

  • I ran the software once and tried all new and related functionality to this PR
  • I looked at all new and changed lines of code and commented on possible problems
  • I read the added documentation and checked if it is understandable and clear
  • I checked the added tests for completeness
  • I checked the internationalized strings for spelling errors
  • I checked the performance metrics for problems or unexplained degradation
  • I checked that the changes are noted in the changelog

@JonasGilg JonasGilg added class:bug Something isn't working type:web-frontend labels Jan 30, 2024
@JonasGilg JonasGilg requested a review from NXXR January 30, 2024 13:26
@JonasGilg JonasGilg self-assigned this Jan 30, 2024
Copy link

github-actions bot commented Jan 30, 2024

Unit Test Results

  1 files  +  1  14 suites  +14   51s ⏱️ +51s
32 tests +32  32 ✔️ +32  0 💤 ±0  0 ±0 
33 runs  +33  33 ✔️ +33  0 💤 ±0  0 ±0 

Results for commit 6f8eac9. ± Comparison against base commit b1befd5.

♻️ This comment has been updated with latest results.

@NXXR
Copy link
Collaborator

NXXR commented Jan 30, 2024

@annawendler can you checkout this branch and see if it fixes your issue with the updating scenario?

@annawendler
Copy link
Contributor

I still have the issue, when I update the scenario I get the error message "Uncaught TypeError: theme.custom.scenarios[props.scenario.id] is undefined".

@annawendler
Copy link
Contributor

Now the display works after updating the scenario without clearing the cache. However, when I click on the scenario to view the uncertainties the site goes blank. If I remember correctly this worked before but I'm not sure or if this is a different issue.
image

@JonasGilg
Copy link
Collaborator Author

Now the display works after updating the scenario without clearing the cache. However, when I click on the scenario to view the uncertainties the site goes blank. If I remember correctly this worked before but I'm not sure or if this is a different issue. image

Ok, I think I fixed this too. Please try again.

@annawendler
Copy link
Contributor

It's working now! The only thing is that now the colors of the uncertainties aren't matching with the line of the scenario anymore.
image

@JonasGilg
Copy link
Collaborator Author

It's working now! The only thing is that now the colors of the uncertainties aren't matching with the line of the scenario anymore. image

This should be fixed as well. We will rework how scenarios are colored in the future, since we currently assume that the scenarios have rising ids starting from 1. See #330.

@annawendler
Copy link
Contributor

It is fixed now.

NXXR
NXXR previously approved these changes Feb 23, 2024
Copy link

Test Results

34 tests  ±0   34 ✅ ±0   5s ⏱️ ±0s
15 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 700c81f. ± Comparison against base commit 59ea85d.

@annawendler annawendler merged commit a30024d into develop Feb 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class:bug Something isn't working type:web-frontend
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants