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

Closed
wants to merge 6 commits into from
Closed

adding time in calculations of yesterday and today #66

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 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
82f3473
using getTimezoneOffset()
vedansh-5 May 15, 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));
Copy link
Member

Choose a reason for hiding this comment

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

You need to add some comments to explain your algorithm, why adding 14 hours.

Copy link
Contributor Author

@vedansh-5 vedansh-5 May 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, yes earlier I was manually setting the timezoneOffset to -14 hours, which is incorrect, while working on this project, I learned a function getTimezoneOffset() and it serves our need.

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());
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));
Copy link
Member

Choose a reason for hiding this comment

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

Here, by naming utc, what do you mean? It sounds like you are creating a datetime at UTC, but the right-hand-side expression doesn't look like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intended the variable to have the utc value of user's local time, but this expression only considered for UTC-14 timezone, which is incorrect.
I have now updated the code with the correct solution.


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

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
Contributor

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));

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