Refactor: Revert filter workarounds

openedx / tutor-contrib-aspects

The Open Analytics Reference System - Tutor plugin

Apache License 2.0

10 stars 15 forks source link

Refactor: Revert filter workarounds #437

Closed bmtcril closed 10 months ago

bmtcril commented 1 year ago

Due to bugs in Superset 2.1 we had to change our whole filtering strategy to prevent courses and blocks from courses that instructors didn't have access to being displayed as filter choices. This should be fixed in Superset 3.0 so we should revert those changes:

Course name should be required any place where charts won't show without it
Problem name / Video name should be required any place where charts won't show without them
Any places where we short circuit virtual datasets based on not having a filter chosen should be removed, it this means the virtual dataset can be moved back to dbt we should do that

https://github.com/openedx/tutor-contrib-aspects/pull/377 most changes came from this PR, so it should be a good guide for what needs to be undone.

SoryRawyer commented 1 year ago

Maybe this is a caching issue, but I'm still seeing incorrect values for filters when enabling "select first value by default" (e.g. seeing demo course problem names despite having the UMONTREAL org selected). Could someone else confirm if they're seeing this behavior as well?

bmtcril commented 1 year ago

@SoryRawyer I'm not seeing it on the beta site, either as instructor or admin. I've changed the filters in the -en instructor dashboard to make course name required and "select first filter value by default". Could be a data ordering issue (which course was loaded first?), but can you want to try it out on the beta site and let me know what you think?

bmtcril commented 1 year ago

You may also want to make sure you've got the right version of the Superset image as there were some bumps for me upgrading locally to Superset 3. The image should be around edunext/aspects-superset:0.52.2 or so.

SoryRawyer commented 1 year ago

Interesting. I'm seeing the correct behavior on the beta server, however I also briefly see "2 options" under the course run filter, followed quickly by a change to "1 option". When I make course run a required filter locally, I get a non-UMONTREAL course run as the default value and a non-UMONTREAL problem/video name as the default value. It seems like even with course run as an optional filter, I see a non-UMONTREAL problem/video name. If you make problem/video names required on the beta server, does that behavior come up?

bmtcril commented 1 year ago

I've configured the "-en" dashboard on the beta server to make the problem and video names required and still can't replicate the behavior, which is maddening. I'm not too worried about the quick-change while filters load, but am curious what you see now on the beta.

SoryRawyer commented 1 year ago

Huh, this is what I see when I look at the problem performance tab: Screenshot from 2023-10-12 11-05-56

Similarly, I see "A shared culture" as the default video when I look at the video performance tab.

bmtcril commented 1 year ago

Looks like this isn't fixed after all, we should punt on it for now. I'll mark this as stale so we don't lose it.

bmtcril commented 10 months ago

This is going to be obviated by changes for the v1 dashboards based on the new PRD.