Repository navigation
Conversation
There was a bug where missing values for sort parameters of new tables where overriding the new default values. This fix is to change merging order, i.e. to make this work the other way around. This will only affect missing values, so actual stored values will still take precedence over the defaults.
|
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. |
|
This pull request is deployed at test.admin-interface.opencast.org/1689/2026-10-15_08-13-21/ . |
Arnei
left a comment
There was a problem hiding this comment.
Looks good and works. The stuff below is less "request changes" and more "comprehension questions".
| // by setting `undefined:undefined` | ||
| const sortBy = getTableSortingForResource(state, resource); | ||
| const direction = getTableDirectionForResource(state, resource); | ||
| if (sortBy && direction && direction !== "NONE") { |
There was a problem hiding this comment.
So this is like a second safe guard beyond the stateReconciler, and not strictly necessary? Should there be some kind of warning logging statement, so that at least developers can be made aware that something is wrong?
Does the backend not take NONE as a value? (Sure, if we want to sort by none we might as well not sent any sort at all, this is really a technicality).
There was a problem hiding this comment.
Correct, it's not strictly necessary and I debated whether to include this. I suppose we could add a logging statement, or just remove this again.
I am actually not sure if the backend takes "NONE", I just kept that condition from before.
There was a problem hiding this comment.
I'll leave it to you whether you want to add a logging statement or remove this, I'm fine with either.
There was a problem hiding this comment.
Yeah ok lets just leave it as is.
|
This should probably go into |
ae5ad54 to
5faf60b
Compare
There was a bug where missing values for sort parameters of new tables were overriding the new default values.
This fix is to change merging order, i.e. to make this work the other way around. This will only affect missing values, so actual stored values will still take precedence over the defaults.
Fixes #1688