Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix:issue with meeting dates extending beyond the ending date. #6028

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion app/assets/javascripts/components/timeline/week.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Block from './block.jsx';
import DateCalculator from '../../utils/date_calculator.js';
import SpringBlock from './SpringBlock';
import BlockList from './BlockList';
import ExtractDate from '../../utils/date_extractor.js';

const Week = createReactClass({
displayName: 'Week',
Expand Down Expand Up @@ -76,7 +77,12 @@ const Week = createReactClass({
const dateCalc = new DateCalculator(this.props.timeline_start, this.props.timeline_end, this.props.index, { zeroIndexed: false });
let weekDatesContent;
let meetDates;
if (this.props.meetings && this.props.meetings.length > 0) {
let meetingDate;
if(this.props.meetings){
const dateExtractor = new ExtractDate(this.props.meetings[0]);
meetingDate = dateExtractor.getDate();
}
if (this.props.meetings && this.props.meetings.length > 0 && meetingDate < dateCalc.end()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it makes more sense to do this somewhere else, such as the weekMeetings function in course_date_utils.js?

I don't like the idea of extracting the date from a string, when we've already generated this data structure by starting from dates and converting them into strings.

Copy link
Contributor Author

@shishiro26 shishiro26 Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ragesoss Actually, the issue is that the function responsible for converting the date to a string doesn't currently take the weeks (which contain the start and end dates for each week) as props. Because of this, I need to perform this check outside the date-to-string conversion function. I'll try to implement this check in the weekMeetings function within course_date_utils.js instead of writing it seperately .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you still working on this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yuppp..

meetDates = `Meetings: ${this.props.meetings.join(', ')}`;
}
if (this.props.meetings) {
Expand Down
41 changes: 41 additions & 0 deletions app/assets/javascripts/utils/date_extractor.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
class ExtractDate {
/**
* Class to extract a meeting date from a specified date format.
*
* Date format expected: "Day (MM/DD)" where MM is the month and DD is the day.
*
* @param {string} date - The input date string from which to extract the date.
*/
constructor(date) {
this.date = date;
}

/**
* Extracts the date in MM/DD format from the stored date string.
*
* The method uses a regular expression to find the date within parentheses.
* The expected format is (MM/DD), with possible whitespace around the date.
*
* @returns {string|null} - Returns the extracted date in MM/DD format (without parentheses) if found,
* or null if no valid date is present.
*/
getDate() {
// Use a regular expression to match the date format (MM/DD).
// The regex breakdown:
// - `\(`: Matches the literal opening parenthesis '('.
// - `\s*`: Matches any whitespace characters (spaces or tabs) zero or more times.
// - `(\d{1,2}\/\d{1,2})`: Capturing group that matches:
// - `\d{1,2}`: Exactly one or two digits (for MM).
// - `\/`: Matches the literal '/' character.
// - `\d{1,2}`: Exactly one or two digits (for DD).
// - `\s*`: Matches any whitespace characters zero or more times.
// - `\)`: Matches the literal closing parenthesis ')'.
const match = this.date.trim().match(/\(\s*(\d{1,2}\/\d{1,2})\s*\)/);

// Check if a match was found and return the first capturing group (MM/DD).
// If no match is found, return null.
return match && match[1] ? match[1] : null;
}
}

export default ExtractDate;
Loading