Textfilter reload on invalid#1381
Conversation
|
Use Run test server using develop.opencast.org as backend: Specify a different backend like stable.opencast.org: It may take a few seconds for the interface to spin up. |
priya-tik
force-pushed
the
textfilter-reload-on-invalid
branch
from
1059465 to
6d1e763
Compare
|
I have my reservations against this. First of all, if we want to ensure that textFilter can only ever be an array, a selector is not the place for that. The reducers should take care of that. Secondly, I don't want to add a catch block that deletes local storage everytime we change the type of a variable in local storage to every selector (and potentially other places) that use that variable. |
|
Thanks. You're right that selectors should stay pure and not cause side effects like clearing storage or triggering reloads. I added the check to prevent the crash users are seeing, but I agree it's better to fix this earlier, either in the reducer when loading state, or with app versioning logic. I'll move the validation out of the selector and update the PR. Thanks for the feedback |
|
I'd like to apply the “Be pragmatic” rule to #1395 or this pull request. I understand that it is annoying to deal with changed data structures in the browser's local storage, but the reality is that we have to if we don't want users to end up with a completely broken interface. I got contacted a bunch of times about develop.opencast.org being broken. Usually by technical people. They couldn't fix that locally (they didn't realize that they had to drop their in-browser storage) and just assumed Opencast was down. I'm also fine with working out a more generic solution for this, like dropping the local storage automatically if users run into an exception. But we can't just reject valid solutions for the problem at hand just because there might be a better one. That's something for later. Opencast is kind of broken for users now. Can we merge one patch and bring it in today's release? |
|
I agreed and started testing this patch. Now I recommend we merge the other patch after all. This patch causes the UI to end up in an endless reload loop: admin-interface-endless-loop.mp4 |
Investigating people complaining develop.opencast.org being broken, it
seems like there was a change to the filters which cannot handle the
previous data which might still be stored in browsers. Like here means,
it completely crashes the interface and users just see a blank page.
The error they get is:
```
Uncaught TypeError: t.find is not a function
fk tableFilterSelectors.ts:23
D reselect.mjs:647
i reselect.mjs:584
P reselect.mjs:659
i reselect.mjs:584
u TableFilters.tsx:55
<anonymous> Redux
L use-sync-external-store-with-selector.production.js:40
h use-sync-external-store-with-selector.production.js:63
React 2
useSyncExternalStoreWithSelector use-sync-external-store-with-selector.production.js:74
n Redux
qc TableFilters.tsx:55
React 14
K scheduler.production.js:152
tableFilterSelectors.ts:23:32
```
This patch introduces a general validation for filters, resetting them
if they are corrupted in the sense that the data structure is not
correct.
This closes opencast#1381
This closes opencast#1395
Investigating people complaining develop.opencast.org being broken, it
seems like there was a change to the filters which cannot handle the
previous data which might still be stored in browsers. Like here means,
it completely crashes the interface and users just see a blank page.
The error they get is:
```
Uncaught TypeError: t.find is not a function
fk tableFilterSelectors.ts:23
D reselect.mjs:647
i reselect.mjs:584
P reselect.mjs:659
i reselect.mjs:584
u TableFilters.tsx:55
<anonymous> Redux
L use-sync-external-store-with-selector.production.js:40
h use-sync-external-store-with-selector.production.js:63
React 2
useSyncExternalStoreWithSelector use-sync-external-store-with-selector.production.js:74
n Redux
qc TableFilters.tsx:55
React 14
K scheduler.production.js:152
tableFilterSelectors.ts:23:32
```
This patch introduces a general validation for filters, resetting them
if they are corrupted in the sense that the data structure is not
correct.
This closes opencast#1381
This closes opencast#1395
|
We went with yet another alternative in #1418 |
5 participants
This PR adds a guard in the
getTextFilterselector to safely handle cases wheretextFilteris not an array. This prevents the following runtime error from crashing the app