feat(milestones):add dates to milestones section

[shishiro26]

What this PR does

This PR addresses the issue of missing date information in the Milestones section on the Home tab. Previously, while the Milestones section displayed the week numbers derived from the Timeline data, it did not include the corresponding start and end dates for each milestone block.

Screenshots

Before:

After:

279

[ragesoss]

Can you add some screenshots that show that the dates are the same in the Milestones vs the Timeline and This Week?

The tricky part of this particular issue is to guarantee that we're using the same algorithm to calculate the dates everywhere they are shown — ideally, only calculating them once, or if done multiple times, then using the same function with the same inputs.

[shishiro26]

Can you add some screenshots that show that the dates are the same in the Milestones vs the Timeline and This Week?

Timeline: Milestone:

The tricky part of this particular issue is to guarantee that we're using the same algorithm to calculate the dates everywhere they are shown — ideally, only calculating them once, or if done multiple times, then using the same function with the same inputs.

I am using the same DateCalculator that is used in the Timeline section.

[ragesoss]

You've shown the first week, but it's important to check for how it handles empty weeks as well.

[ragesoss]

Empty weeks can come at the beginning of a course, by including a gap between course start and timeline_start, and also in the middle of the course by using the calendar (via Edit Course Dates on the Timeline tab) to create weeks with no meeting days.

[shishiro26]

You've shown the first week, but it's important to check for how it handles empty weeks as well.

In the Milestones section, empty weeks are not displayed

Milestones:

Weeks 159 and 164 are empty, and they are not displayed in the Milestones section.

Timeline:

I also tried using another course for this: University of Pennsylvania Medical Missionaries to Community Partners (Fall 2023).

[ragesoss]

cool. this looks like it's working properly. do you see a good way to avoid repeating the date calculations so that both the timeline and the milestones component can use the same value without a risk that they will diverge with later changes to the timeline component? i think it's dangerous to have these hard-to-understand parallel implementations.

[shishiro26]

cool. this looks like it's working properly. do you see a good way to avoid repeating the date calculations so that both the timeline and the milestones component can use the same value without a risk that they will diverge with later changes to the timeline component? i think it's dangerous to have these hard-to-understand parallel implementations.

@ragesoss Yes, but it’s only two lines of code. I hope there won’t be any divergence in the future, as I believe it’s a straightforward implementation. If we implement a new way to eliminate redundancy in the code, we would still need to pass timelineStart, timelineEnd, and navIndex, which would make the code redundant again, right?

const dateCalc = new DateCalculator(this.props.timelineStart, this.props.timelineEnd, navIndex, {
    zeroIndexed: true
});

[ragesoss]

@shishiro26 I think the ideal strategy would be to do whatever data manipulation and calculation is necessary in a selector that operates on data from the redux store, so that the same selector (which can also memo-ize the result) is used in both cases. However, that likely requires a significant refactor of the Timeline component. I think I'm okay with this current implementation that you have done, but it should include some explanatory comments to note how this implementation is coupled to the code in the Timeline component and that we need to make sure both have the same results and account for empty weeks the same way.

[shishiro26]

@ragesoss I have reviewed the codebase and confirmed that the date-related data is derived from the course or stored in the Redux store. In line with your previous suggestion, I plan to create a selector that extracts the course timelines and returns the start and end dates. This approach will help prevent discrepancies in date calculations and ensure that components such as Milestones and Timeline display consistent data. Centralizing date calculations within the selector will enhance uniformity across the application. Would you suggest any additional refactoring beyond this plan?

[ragesoss]

@shishiro26 I think it's wise to do the minimum amount of refactoring necessary to unify this particular calculation, and if you notice anything along the way that you think ought to be refactored, make a note of it.

[shishiro26]

@ragesoss, I will ensure that I do the minimum necessary refactoring and update the commit accordingly. I will also make sure to note any important refactoring opportunities if they arise.

[shishiro26]

@ragesoss I have updated the code. Could you please check if this aligns with what we discussed?

[ragesoss]

Build is currently failing because of JS linting errors.

[shishiro26]

I would like to do this since I have been using Docker. Could you provide me with the steps to run lint in Docker?

[ragesoss]

You're kind of on your own with that, as we don't have up-to-date docs for docker, and I don't use it.

[shishiro26]

I have resolved all the linting issues, @ragesoss.

[shishiro26]

@ragesoss I came across an issue while fixing the lint errors. I saw the line lint-non-build": "eslint 'test/*/.{jsx,js}' '*.js'" and I’m not sure if this is the only line responsible for linting, as I couldn't find any other run functions in the codebase. Last time, when I encountered lint issues, I fixed everything explicitly. Could you guide me on what I need to do for this?

[ragesoss]

The failing tests seem to be related to this change.

[ragesoss]

Looks like the one failing test is related.