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

ResultGrid: Don't hard-fail if some cell value is missing in filter checkboxes #2083

Conversation

richardebeling
Copy link
Member

@richardebeling richardebeling commented Dec 3, 2023

Related to #1739.

Currently, if some of the rows contain filter values that do not correspond to any values in the grid header, the javascript will error out (because we're accessing .values of undefined). Conceptually, the datagrid assumes that the header checkboxes it is passed always perfectly match the data given. Due to outdated caches, this might not be the case.

To test this, simply merge two course types and then open the results page.

I'm open for other suggestions of how to handle this in javascript. Having undefined as a stored filter value seems semantically useful to me, but we can also coalesce to false. I'd also be open to issue a warning to the console if this happens, to make potential future debugging easier.

It is still non-perfect if we run into the outdated caches situation because now there will be rows that can not be filtered for, but at least the rest of the datagrid will function as expected. Users would simply be missing the right checkbox. I've been thinking about making the ResultGrid build the header checkboxes, but currently that really isn't its responsibility, so that would be a bigger change.

IMO, this change doesn't make it "good", but it makes it better, which is somewhat overdue.

Copy link
Member

@niklasmohrin niklasmohrin left a comment

Choose a reason for hiding this comment

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

Yeah, not crashing is definitely an improvement :D

I think we probably want to generate the checkboxes from the actual data at some point. The point of having the server send them would just be that a) all actual kinds appear in the list and b) the boxes are in a desired order.

I think that a) is not really important because they will appear anyways. For b) we embed some ordering keys into the html server side and use those (with a fallback)? Or should we rather try to focus on always generating accurate checkboxes in the backend?

@richardebeling richardebeling merged commit 48c58e8 into e-valuation:main Dec 11, 2023
11 checks passed
@richardebeling richardebeling deleted the resultgrid-header-data-mismatch branch December 11, 2023 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants