WikiEducationFoundation / WikiEduDashboard

Wiki Education Foundation's Wikipedia course dashboard system
https://dashboard.wikiedu.org
MIT License
392 stars 630 forks source link

Blocks in 'This Week' may appear in a different order than on the Timeline #5696

Closed ragesoss closed 8 months ago

ragesoss commented 8 months ago

A user reported that the Home tab was showing timeline Blocks in a different order in the 'This Week' component than they are supposed to appear based on the Timeline tab ordering.

Here's the course where it was reported: https://dashboard.wikiedu.org/courses/Victor_Valley_Community_College/CIS50_136_-_Biographies_(Spring_2024)/home

I can replicate the problem by first loading the Home tab, and seeing the blocks for the current week in reverse order, and then navigating to the Timeline tab, then back to the home and now they are in the correct order. So this is probably a bug with how the redux store data is getting transformed for rendering by 'This Week'.

Example 1

Home tab (This Week component)

wikiedu_homepage_1

Timeline tab

wikiedu_timeline_1

Example 2

Home tab (This Week component)

wikiedu_homepage_2

Timeline tab

wikiedu_timeline_2
ragesoss commented 8 months ago

I believe what is happening is that the rendering of Timeline involves looping through the weeks, and sorting the blocks in each week by their order property, and this is actually mutating the redux state:

    this.props.weeks.forEach(w =>
      w.blocks.sort((a, b) => a.order - b.order)
    );

So when 'This Week' is rendered before the Timeline is rendered, the sorting has not been done and so it is just showing blocks in the order that they are provided from the server (which is actually database order, but also happens to coincide with order properties for most timelines that have not been customized).

An efficient minimal fix would probably be to push the block sorting into the Week component (which is shared by ThisWeek and Timeline.

A better fix would be to refactor the handling of weeks and blocks data so that sorting by order is done in the reducer or via a selector, and get rid of the state mutation. I would prefer to implement a minimal fix now, and file a new issue about the state mutation and refactor. The Timeline component is too complex, so I'm wary of messing with it much without first simplifying it and breaking it up into smaller components.