Fix State Mutation in Timeline Reducer and Refactor TimelineHandler for Immutable Week Title Updates

[Abishekcs]

Note

Previously, I considered opening this as two separate PRs (#6138 and #6143). However, later i realized both require a common component change, specifically the TimelineHandler component.

For testing please set enableMutationChecks to true in create_store.js

What this PR does

• Fixes state immutability issues in the weeks property of the timeline slice done by getWeeksArray selector.
• Fixes mutation in timeline reducer(s) which are handling the action RESET_TITLES & UPDATE_TITLE

Cause of Bug (For getWeeksArray selector)

• The bug lies in the direct modification of the newWeek object, which is a reference to the weeks property of the
  Redux state (Timeline Slice).

WikiEduDashboard/app/assets/javascripts/selectors/index.js

Line 257 in 88a89ec

const newWeek = weeks[weekId];

Fix (For getWeeksArray selector)

• Resolved by using spread operator since we are modifying only the top level properties of the object.

Cause of Bug (For RESET_TITLES & UPDATE_TITLE)

For this finding the cause of bug was easy as the error message exaclty indicated where mutation was as shown below:

For RESET_TITLES:

For UPDATE_TITLE

The spread operator doesn’t work as expected with deeply nested objects because it copies the values of the top-level properties of weeks to new addresses/references, but the inner objects retain the same addresses/references which are the one's updated by mutation.

Fix (For RESET_TITLES & UPDATE_TITLE)

Resolved by using Immer Produce.

Why the changes in TimelineHandler component?

Previously, the getWeeksArray selector was mutating the weeks property of the timeline slice directly. This meant that dispatching SAVED_TIMELINE from the saveTimeline function immediately after dispatching RESET_TITLES from the _resetTitles function worked without issues because both were referencing the same mutated weeks array from getWeeksArray selector.

So, after updating immutable way immediately dispatching won't work as the existing weeks in the TimlineHandler from selector doesn't have the same reference.

Little bit detail about redux for future reference:

• In Redux:
  • Mutation: Changes are immediate since we were previously directly modifying the same object reference -
    any code holding that reference sees the changes instantly.
  • Immutable: Creates a new object reference, requiring React/Redux to process and batch the state update,
    making the changes not immediately available. Also, after resetting the timeline week titles, the saveTimeline
    was called immediately, leading to an API call right away. During this time, Redux had not yet received the
    new updates.
  • Source: Immutability in React and Redux: The Complete Guide

Screenshots (For getWeeksArray selector)

Before:

Before.mp4

After:

After.mp4

Screenshots (For RESET_TITLES & UPDATE_TITLE)

Before:

Before.mp4

After:

After.mp4

[Abishekcs]

@ragesoss would you be open to using Immer for immutability updates? Redux Toolkit has Immer built-in, but we'd need to refactor our reducer file to use it.

[ragesoss]

@Abishekcs sure, seems reasonable.

[ragesoss]

This seems fine to me, but since it's introducing a new tool, the PR should probably include some explanation of it in frontend.md.

Also, as part of the PR description, please note how adding this package affect the bundle size.

[ragesoss]

Since it looks like we're already using a different version of immer, is it possible to just use the newer version without the older one (possibly by upgrading whatever dependency was already requiring it)?

[Abishekcs]

@ragesoss i think both version ranges (^10.0.3 and ^10.1.1) are being satisfied by a single installation of version 10.1.1. as shown in yarn .

WikiEduDashboard/yarn.lock

Line 6871 in 8e6a3a2

version: 10.1.1

So there's only one version installed, and it satisfies both requirements.
And, while installing Redux Toolkit (RTK) in PR #5867 , it brought in immer as a dependency since RTK uses immer internally for its immutable state updates.

[ragesoss]

Great. So it shouldn't affect the bundle size either.

[Abishekcs]

Great. So it shouldn't affect the bundle size either.

Yes.

[Abishekcs]

This seems fine to me, but since it's introducing a new tool, the PR should probably include some explanation of it in frontend.md.

Also, as part of the PR description, please note how adding this package affect the bundle size.

Okay

[Abishekcs]

@ragesoss i have updated the frontend.md.

a) There, is one link i.e Thinking in React which leads to page not found.
b) Also, the doc needs to be updated to reflect the recent changes in react and redux. I will make those changes in seperate PR related only to the docs.

[ragesoss]

Thanks! This looks good to me; I'll merge once the tests pass.