Skip to content

adding time in calculations of yesterday and today #66

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

adding time in calculations of yesterday and today #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
35dbdae
adding time in calculations of yesterday and today
vedansh-5 Mar 27, 2025
1f7981b
correcting date calulation
vedansh-5 Mar 28, 2025
59c8c4a
potential fix
vedansh-5 Mar 31, 2025
8cddb5e
fix 2
vedansh-5 Mar 31, 2025
a328c33
fix 3
vedansh-5 Mar 31, 2025
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
32 changes: 18 additions & 14 deletions src/scripts/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,11 @@ function getLastWeek() {
var today = new Date();
var noDays_to_goback = gsoc == 0 ? 7 : 1;
var lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - noDays_to_goback);
var lastWeekMonth = lastWeek.getMonth() + 1;
var lastWeekDay = lastWeek.getDate();
var lastWeekYear = lastWeek.getFullYear();
lastWeek.setHours(0, 0, 0 , 0);
var extendedLastWeek = new Date(lastWeek.getTime() + (14 *60 *60 * 1000));
var lastWeekMonth = extendedLastWeek.getMonth() + 1;
var lastWeekDay = extendedLastWeek.getDate();
var lastWeekYear = extendedLastWeek.getFullYear();
var lastWeekDisplayPadded =
('0000' + lastWeekYear.toString()).slice(-4) +
'-' +
Expand All @@ -112,17 +114,19 @@ function getLastWeek() {
}
function getToday() {
var today = new Date();
var Week = new Date(today.getFullYear(), today.getMonth(), today.getDate());
var WeekMonth = Week.getMonth() + 1;
var WeekDay = Week.getDate();
var WeekYear = Week.getFullYear();
var WeekDisplayPadded =
('0000' + WeekYear.toString()).slice(-4) +
'-' +
('00' + WeekMonth.toString()).slice(-2) +
'-' +
('00' + WeekDay.toString()).slice(-2);
return WeekDisplayPadded;
var today = new Date(today.getFullYear(), today.getMonth(), today.getDate());
Copy link

Choose a reason for hiding this comment

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

suggestion: Reconsider duplicated date formatting logic in getToday().

getToday() in main.js manually formats the date after adjusting with the offset, while scrumHelper.js uses toISOString slicing. Aligning these approaches (potentially via a common utility function) could improve consistency and reduce potential formatting discrepancies.

today.setHours(23, 59, 59, 999);
var extendedToday = new Date(today.getTime() + (14 * 60 * 60 * 1000));
var todayMonth = extendedToday.getMonth() + 1;
var todayDay = extendedToday.getDate();
var todayYear = extendedToday.getFullYear();
var todayDisplayPadded =
('0000' + todayYear.toString()).slice(-4) +
'-' +
('00' + todayMonth.toString()).slice(-2) +
'-' +
('00' + todayDay.toString()).slice(-2);
return todayDisplayPadded;
}

function handleGithubUsernameChange() {
Expand Down
59 changes: 37 additions & 22 deletions src/scripts/scrumHelper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,35 +101,26 @@ function allIncluded() {
endingDate = getToday();
startingDate = getLastWeek();
}

function getLastWeek() {
var today = new Date();
var noDays_to_goback = gsoc == 0 ? 7 : 1;
var lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - noDays_to_goback);
var lastWeekMonth = lastWeek.getMonth() + 1;
var lastWeekDay = lastWeek.getDate();
var lastWeekYear = lastWeek.getFullYear();
var lastWeekDisplayPadded =
('0000' + lastWeekYear.toString()).slice(-4) +
'-' +
('00' + lastWeekMonth.toString()).slice(-2) +
'-' +
('00' + lastWeekDay.toString()).slice(-2);
return lastWeekDisplayPadded;
lastWeek.setHours(0, 0, 0, 0);

const utc = new Date(lastWeek.getTime() + (14 * 60* 60* 1000));

return utc.toISOString().slice(0,19) + "Z";
Copy link

Choose a reason for hiding this comment

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

issue (complexity): Consider extracting the date transformation logic into a shared helper function to avoid repetition and improve code maintainability

Extract a shared helper to handle the date transformation so you don’t repeat the setHours and offset logic. For example:

function formatDateWithOffset(date, isStart) {
  if (isStart) {
    date.setHours(0, 0, 0, 0);
  } else {
    date.setHours(23, 59, 59, 999);
  }
  const offsetHours = 14;
  const utc = new Date(date.getTime() + (offsetHours * 60 * 60 * 1000));
  return utc.toISOString().slice(0, 19) + "Z";
}

Then update your functions as follows:

function getLastWeek() {
  const today = new Date();
  const daysBack = gsoc === 0 ? 7 : 1;
  const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - daysBack);
  return formatDateWithOffset(lastWeek, true);
}

function getToday() {
  const today = new Date();
  return formatDateWithOffset(today, false);
}

This refactoring reduces duplication while keeping the functionality identical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is used in only two functions, also its a single line, I don't think a function is necessary.

Copy link

Choose a reason for hiding this comment

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

I'll respond to the comment by the developer:

While I understand your perspective that the code is used in only two functions and appears to be a single line, creating a helper function offers several benefits:

  1. Code Readability: The extracted function makes the date transformation logic more explicit and easier to understand at a glance.

  2. Maintainability: If the date transformation logic needs to change in the future, you'll only need to modify it in one place.

  3. Reusability: Even if it's currently used in just two functions, having a dedicated helper makes it easier to reuse the logic elsewhere if needed.

  4. Consistency: The helper ensures that date transformations are handled consistently across different parts of the code.

The proposed refactoring is lightweight and doesn't add unnecessary complexity. It actually simplifies the existing code by:

  • Removing manual string formatting
  • Centralizing the time zone and offset logic
  • Making the intent of the code clearer

I recommend considering the proposed refactoring, but if you feel strongly against it, that's okay too. The most important thing is maintaining clean, understandable code.

Would you like to discuss this further or share your thoughts on the suggested approach?

}
function getToday() {
var today = new Date();
var Week = new Date(today.getFullYear(), today.getMonth(), today.getDate());
var WeekMonth = Week.getMonth() + 1;
var WeekDay = Week.getDate();
var WeekYear = Week.getFullYear();
var WeekDisplayPadded =
('0000' + WeekYear.toString()).slice(-4) +
'-' +
('00' + WeekMonth.toString()).slice(-2) +
'-' +
('00' + WeekDay.toString()).slice(-2);
return WeekDisplayPadded;
}
today.setHours(23, 59, 59, 999);

const utc = new Date(today.getTime() + (14 * 60 * 60 * 1000));
Comment on lines +117 to +119
Copy link

Choose a reason for hiding this comment

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

suggestion: Review the repetitive pattern of applying a timezone offset.

Both getLastWeek() and getToday() apply the same 14-hour offset adjustment. It might be worthwhile to refactor this repeated logic into a shared helper function for clarity and maintainability.

Suggested implementation:

// Helper function to apply a 14-hour timezone offset and format the date as ISO string
function applyTimezoneOffset(date) {
    const offsetInMilliseconds = 14 * 60 * 60 * 1000;
    const utcDate = new Date(date.getTime() + offsetInMilliseconds);
    return utcDate.toISOString().slice(0, 19) + "Z";
}
		today.setHours(23, 59, 59, 999);
		return applyTimezoneOffset(today);
	}

You should also update the getLastWeek() function (if it uses the same pattern) in a similar fashion by replacing its 14-hour offset logic with a call to applyTimezoneOffset().


return utc.toISOString().slice(0, 19) + "Z";
}

// fetch github data
function fetchGithubData() {
var issueUrl =
Expand Down Expand Up @@ -185,6 +176,30 @@ function allIncluded() {
githubUserData = data;
},
});
filterAndStoreData();
}

function filterDataByDate(data, startingDate, endingDate){

const localStart = new Date(startingDate + 'T00:00:00Z');
const localEnd = new Date(endingDate + 'T23:59:59Z');

return data.items.filter(item => {
const updatedAt = new Date(item.updated_at);
return updatedAt >= localStart && updatedAt <= localEnd;
})
}

function filterAndStoreData() {
if(githubIssuesData) {
githubIssuesData.items = filterDataByDate(githubIssuesData, startingDate, endingDate);
}
if(githubPrsReviewData) {
githubPrsReviewData.items = filterDataByDate(githubPrsReviewData, startingDate, endingDate);
}
if(githubUserData) {
githubUserData.items = filterDataByDate(githubUserData, startingDate, endingDate);
}
}

function formatDate(dateString) {
Expand Down