Create filters component for dashboard organization section

acelaya commented 2 months ago

This PR introduces a new component that wraps the filtering dropdowns for the dashboard homepage. For now, dropdowns will show all courses, all assignments and all students, but this may change later as we clarify expected behaviors.

Filtering components for other sections will be implemented separately. You can find why I choose this approach here: https://github.com/hypothesis/lms/pull/6416#issuecomment-2236120392

The only purpose of this PR is to introduce the visual components, and handle the logic to display selected filters and forward them to the metrics endpoint and also filter among themselves. There are a few missing things that will be implemented afterwards:

Load paginated information in filter dropdowns. Only first page is currently loaded.
Persist selected filters in the page query string, so that it's possible to bookmark, reload or navigate through different selections.
As mentioned above, implement the filters for the other dashboard sections. Once this is tackled, some logic introduced here will likely be extracted for reusing.

https://github.com/user-attachments/assets/b2e83b0b-b492-4b77-8289-5e802b5a119e

TODO

[x] Write tests.
[x] Evaluate type definitions: Optional props vs different types. See https://github.com/hypothesis/lms/pull/6453/files#r1684282200
[x] ~Make selection in filters affect each other. See https://github.com/hypothesis/lms/pull/6453#issuecomment-2242438396~ Finally not doing this due to some needed clarifications. We can merge this and change it later if needed.

robertknight commented 2 months ago

The only purpose of this PR is to introduce the visual components, and handle the logic to display selected filters and forward them to the metrics endpoint. There are a dew missing things that will be implemented afterwards:

What is the plan for filtering the contents of the drop-down based on the selections in other drop-downs?

acelaya commented 2 months ago

The only purpose of this PR is to introduce the visual components, and handle the logic to display selected filters and forward them to the metrics endpoint. There are a dew missing things that will be implemented afterwards:

What is the plan for filtering the contents of the drop-down based on the selections in other drop-downs?

This is the main reason I decided to go with different filters components per section, as it will make this much easier.

In the case of the home page (this PR), no filtering is needed.
For the course page, courses won't be filtered, and assignments and students will be filtered by the active course, which ID we'll get from the URL.
For the assignment page, courses and assignments won't be filtered, and students will be filtered by the active assignment, which ID we'll get from the URL.

If we end up with a lot of duplication and once done I see an easy way to merge logic and make it dynamic, I'll do it, but my first attempt on doing that ended up with a too complex props API and a too conditional implementation.

robertknight commented 2 months ago

In the case of the home page (this PR), no filtering is needed.

My expectation was that making a selection in one of the filters would affect the contents of the other filters. For example, selecting a student would then filter the courses drop-down to only show courses that student had taken part in. Am I mistaken about how this is going to work?

acelaya commented 2 months ago

In the case of the home page (this PR), no filtering is needed.

My expectation was that making a selection in one of the filters would affect the contents of the other filters. For example, selecting a student would then filter the courses drop-down to only show courses that student had taken part in. Am I mistaken about how this is going to work?

Hmmm, now that you mention that, it does sound like a reasonable expectation. Let me check the document where the expected behavior was documented.

acelaya commented 2 months ago

In the case of the home page (this PR), no filtering is needed.

My expectation was that making a selection in one of the filters would affect the contents of the other filters. For example, selecting a student would then filter the courses drop-down to only show courses that student had taken part in. Am I mistaken about how this is going to work?

Hmmm, now that you mention that, it does sound like a reasonable expectation. Let me check the document where the expected behavior was documented.

From this document: https://docs.google.com/document/d/16fcSys6mT-avsMX8mpNkZguHGQAVAoPA8Ve-_xDYuzw

The filters react to one another, i.e.

All courses (available to the user) are always visible

Assignments are visible based on the course selection (all assignments are visible when “All courses” is selected)

Students are visible based on the Course + Assignment selection (all students are visible when “All courses” and “All assignments” are selected).

So yes, you were right. I will adapt the implementation here.

acelaya commented 2 months ago

In the case of the home page (this PR), no filtering is needed.

My expectation was that making a selection in one of the filters would affect the contents of the other filters. For example, selecting a student would then filter the courses drop-down to only show courses that student had taken part in. Am I mistaken about how this is going to work?

Hmmm, now that you mention that, it does sound like a reasonable expectation. Let me check the document where the expected behavior was documented.

From this document: https://docs.google.com/document/d/16fcSys6mT-avsMX8mpNkZguHGQAVAoPA8Ve-_xDYuzw

The filters react to one another, i.e.

All courses (available to the user) are always visible

Assignments are visible based on the course selection (all assignments are visible when “All courses” is selected)

Students are visible based on the Course + Assignment selection (all students are visible when “All courses” and “All assignments” are selected).

So yes, you were right. I will adapt the implementation here.

I think we need to clarify this with the product team.

I just noticed the BE does not expect an array of assignments and/or courses in the filters endpoints, but just one item (see assignments and users). That means the assumption is to filter those only based on the active section, not in the selection in other dropdowns.

At the same time, if I re-read the snippet I pasted above, I'm now no longer sure if it refers to the other dropdowns or the active section.

So I'm going to leave that out of this PR's scope just to keep it simpler, and once we have confirmed this point I'll bring back the proper changes.

hypothesis / lms

Create filters component for dashboard organization section #6453

TODO