Fix issue labeling: solved opened and updated issues #230
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add logic to determine if an issue was created or updated in the selected date range - Issues created in date range show 'Opened Issue' - Issues updated but not created in date range show 'Updated Issue' - Mirrors existing PR logic that distinguishes 'Made PR' vs 'Existing PR' - Fixes confusion where all issues were always labeled as 'Opened Issue' Resolves the issue where users couldn't differentiate between newly created issues and existing issues that were just updated during the selected period.
Reviewer's GuideUpdates the issue labeling in scrumHelper.js by introducing date range filters and a new isNewIssue flag to dynamically label issues as opened or updated within the selected period. File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
bot
added
javascript
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Vikas4245 - I've reviewed your changes - here's some feedback:
- Consider extracting the date range calculation into a reusable helper to reduce duplication and improve readability.
- When labeling 'Updated Issue', also verify that the issue’s updated_at timestamp falls within the selected range to avoid tagging stale issues as updated.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the date range calculation into a reusable helper to reduce duplication and improve readability.
- When labeling 'Updated Issue', also verify that the issue’s updated_at timestamp falls within the selected range to avoid tagging stale issues as updated.
## Individual Comments
### Comment 1
<location> `src/scripts/scrumHelper.js:1518` </location>
<code_context>
+ if (yesterdayContribution) {
+ const today = new Date();
+ const yesterday = new Date(today.getTime() - 24 * 60 * 60 * 1000);
+ startDateFilter = new Date(yesterday.toISOString().split('T')[0] + 'T00:00:00Z');
+ endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z');
+ } else if (startingDate && endingDate) {
+ startDateFilter = new Date(startingDate + 'T00:00:00Z');
</code_context>
<issue_to_address>
Date construction using string manipulation may introduce timezone inconsistencies.
This approach may cause bugs when local dates cross midnight or during daylight saving changes. Use UTC-based methods or a date library for consistent results.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if (yesterdayContribution) {
const today = new Date();
const yesterday = new Date(today.getTime() - 24 * 60 * 60 * 1000);
startDateFilter = new Date(yesterday.toISOString().split('T')[0] + 'T00:00:00Z');
endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z');
} else if (startingDate && endingDate) {
startDateFilter = new Date(startingDate + 'T00:00:00Z');
endDateFilter = new Date(endingDate + 'T23:59:59Z');
} else {
// Default to last 7 days if no date range is set
const today = new Date();
const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7);
startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z');
endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z');
}
=======
if (yesterdayContribution) {
const now = new Date();
// Get UTC date parts for today and yesterday
const todayUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate()));
const yesterdayUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() - 1));
startDateFilter = new Date(Date.UTC(yesterdayUTC.getUTCFullYear(), yesterdayUTC.getUTCMonth(), yesterdayUTC.getUTCDate(), 0, 0, 0));
endDateFilter = new Date(Date.UTC(todayUTC.getUTCFullYear(), todayUTC.getUTCMonth(), todayUTC.getUTCDate(), 23, 59, 59));
} else if (startingDate && endingDate) {
// Parse startingDate and endingDate as UTC
const [startYear, startMonth, startDay] = startingDate.split('-').map(Number);
const [endYear, endMonth, endDay] = endingDate.split('-').map(Number);
startDateFilter = new Date(Date.UTC(startYear, startMonth - 1, startDay, 0, 0, 0));
endDateFilter = new Date(Date.UTC(endYear, endMonth - 1, endDay, 23, 59, 59));
} else {
// Default to last 7 days if no date range is set
const now = new Date();
const todayUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate()));
const lastWeekUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() - 7));
startDateFilter = new Date(Date.UTC(lastWeekUTC.getUTCFullYear(), lastWeekUTC.getUTCMonth(), lastWeekUTC.getUTCDate(), 0, 0, 0));
endDateFilter = new Date(Date.UTC(todayUTC.getUTCFullYear(), todayUTC.getUTCMonth(), todayUTC.getUTCDate(), 23, 59, 59));
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/scripts/scrumHelper.js:1526` </location>
<code_context>
+ } else {
+ // Default to last 7 days if no date range is set
+ const today = new Date();
+ const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7);
+ startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z');
+ endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z');
</code_context>
<issue_to_address>
Subtracting days using Date constructor can yield unexpected results at month boundaries.
Date arithmetic using the Date constructor can fail at month boundaries. Use getTime() subtraction for consistent results.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7);
startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z');
endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z');
=======
const lastWeek = new Date(today.getTime() - 7 * 24 * 60 * 60 * 1000);
startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z');
endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z');
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/scripts/scrumHelper.js:1534` </location>
<code_context>
+ const isNewIssue = issueCreatedDate >= startDateFilter && issueCreatedDate <= endDateFilter;
+ const issueAction = isNewIssue ? 'Opened Issue' : 'Updated Issue';
+
+ log(`[ISSUE DEBUG] Issue #${number} - isNewIssue: ${isNewIssue}, issueAction: ${issueAction}, state: ${item.state}, created: ${item.created_at}, updated: ${item.updated_at}`);
+
if (item.state === 'open') {
</code_context>
<issue_to_address>
Consider removing or gating debug logging for production environments.
Debug logs in production can expose sensitive data and create unnecessary noise. If these logs are only needed for development, use a conditional or appropriate logging level to prevent them from appearing in production.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
log(`[ISSUE DEBUG] Issue #${number} - isNewIssue: ${isNewIssue}, issueAction: ${issueAction}, state: ${item.state}, created: ${item.created_at}, updated: ${item.updated_at}`);
=======
if (process.env.NODE_ENV !== 'production') {
log(`[ISSUE DEBUG] Issue #${number} - isNewIssue: ${isNewIssue}, issueAction: ${issueAction}, state: ${item.state}, created: ${item.created_at}, updated: ${item.updated_at}`);
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/scripts/scrumHelper.js
Outdated
Comment on lines
1515
to
1529
| if (yesterdayContribution) { | ||
| const today = new Date(); | ||
| const yesterday = new Date(today.getTime() - 24 * 60 * 60 * 1000); | ||
| startDateFilter = new Date(yesterday.toISOString().split('T')[0] + 'T00:00:00Z'); | ||
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); | ||
| } else if (startingDate && endingDate) { | ||
| startDateFilter = new Date(startingDate + 'T00:00:00Z'); | ||
| endDateFilter = new Date(endingDate + 'T23:59:59Z'); | ||
| } else { | ||
| // Default to last 7 days if no date range is set | ||
| const today = new Date(); | ||
| const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7); | ||
| startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z'); | ||
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Date construction using string manipulation may introduce timezone inconsistencies.
This approach may cause bugs when local dates cross midnight or during daylight saving changes. Use UTC-based methods or a date library for consistent results.
Suggested change
| if (yesterdayContribution) { | |
| const today = new Date(); | |
| const yesterday = new Date(today.getTime() - 24 * 60 * 60 * 1000); | |
| startDateFilter = new Date(yesterday.toISOString().split('T')[0] + 'T00:00:00Z'); | |
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); | |
| } else if (startingDate && endingDate) { | |
| startDateFilter = new Date(startingDate + 'T00:00:00Z'); | |
| endDateFilter = new Date(endingDate + 'T23:59:59Z'); | |
| } else { | |
| // Default to last 7 days if no date range is set | |
| const today = new Date(); | |
| const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7); | |
| startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z'); | |
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); | |
| } | |
| if (yesterdayContribution) { | |
| const now = new Date(); | |
| // Get UTC date parts for today and yesterday | |
| const todayUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate())); | |
| const yesterdayUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() - 1)); | |
| startDateFilter = new Date(Date.UTC(yesterdayUTC.getUTCFullYear(), yesterdayUTC.getUTCMonth(), yesterdayUTC.getUTCDate(), 0, 0, 0)); | |
| endDateFilter = new Date(Date.UTC(todayUTC.getUTCFullYear(), todayUTC.getUTCMonth(), todayUTC.getUTCDate(), 23, 59, 59)); | |
| } else if (startingDate && endingDate) { | |
| // Parse startingDate and endingDate as UTC | |
| const [startYear, startMonth, startDay] = startingDate.split('-').map(Number); | |
| const [endYear, endMonth, endDay] = endingDate.split('-').map(Number); | |
| startDateFilter = new Date(Date.UTC(startYear, startMonth - 1, startDay, 0, 0, 0)); | |
| endDateFilter = new Date(Date.UTC(endYear, endMonth - 1, endDay, 23, 59, 59)); | |
| } else { | |
| // Default to last 7 days if no date range is set | |
| const now = new Date(); | |
| const todayUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate())); | |
| const lastWeekUTC = new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate() - 7)); | |
| startDateFilter = new Date(Date.UTC(lastWeekUTC.getUTCFullYear(), lastWeekUTC.getUTCMonth(), lastWeekUTC.getUTCDate(), 0, 0, 0)); | |
| endDateFilter = new Date(Date.UTC(todayUTC.getUTCFullYear(), todayUTC.getUTCMonth(), todayUTC.getUTCDate(), 23, 59, 59)); | |
| } |
src/scripts/scrumHelper.js
Outdated
Comment on lines
1526
to
1528
| const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7); | ||
| startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z'); | ||
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); |
There was a problem hiding this comment.
suggestion (bug_risk): Subtracting days using Date constructor can yield unexpected results at month boundaries.
Date arithmetic using the Date constructor can fail at month boundaries. Use getTime() subtraction for consistent results.
Suggested change
| const lastWeek = new Date(today.getFullYear(), today.getMonth(), today.getDate() - 7); | |
| startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z'); | |
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); | |
| const lastWeek = new Date(today.getTime() - 7 * 24 * 60 * 60 * 1000); | |
| startDateFilter = new Date(lastWeek.toISOString().split('T')[0] + 'T00:00:00Z'); | |
| endDateFilter = new Date(today.toISOString().split('T')[0] + 'T23:59:59Z'); |
src/scripts/scrumHelper.js
Outdated
| const isNewIssue = issueCreatedDate >= startDateFilter && issueCreatedDate <= endDateFilter; | ||
| const issueAction = isNewIssue ? 'Opened Issue' : 'Updated Issue'; | ||
|
|
||
| log(`[ISSUE DEBUG] Issue #${number} - isNewIssue: ${isNewIssue}, issueAction: ${issueAction}, state: ${item.state}, created: ${item.created_at}, updated: ${item.updated_at}`); |
There was a problem hiding this comment.
suggestion: Consider removing or gating debug logging for production environments.
Debug logs in production can expose sensitive data and create unnecessary noise. If these logs are only needed for development, use a conditional or appropriate logging level to prevent them from appearing in production.
Suggested change
| log(`[ISSUE DEBUG] Issue #${number} - isNewIssue: ${isNewIssue}, issueAction: ${issueAction}, state: ${item.state}, created: ${item.created_at}, updated: ${item.updated_at}`); | |
| if (process.env.NODE_ENV !== 'production') { | |
| log(`[ISSUE DEBUG] Issue #${number} - isNewIssue: ${isNewIssue}, issueAction: ${issueAction}, state: ${item.state}, created: ${item.created_at}, updated: ${item.updated_at}`); | |
| } |
- Extract date range calculation into reusable getDateRangeFilters() helper - Add proper updated_at timestamp validation for 'Updated Issue' labeling - Fix timezone issues with UTC-based date handling - Use time-based arithmetic to avoid month boundary bugs - Gate debug logging to prevent production noise - Eliminate code duplication between PR and issue processing - Add comprehensive validation to prevent stale issues being labeled as 'Updated' Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
@sourcery-ai Thank you for the excellent feedback! I've addressed all the suggestions: Changes Made:✅ Extracted date range calculation into a reusable "getDateRangeFilters()" helper function to eliminate duplication and improve readability ✅ Enhanced `updated_at "validation - "Updated Issue" label now requires the issue's "updated_at" timestamp to fall within the selected range, preventing stale issues from being incorrectly labeled ✅ Fixed timezone and date arithmetic issues by:
✅ Gated debug logging with "if (DEBUG)"conditionals to prevent production noise Additional Improvements:
The solution now handles all edge cases correctly, including issues that were created outside the date range but never updated within it (fallback to "Opened Issue"). All feedback has been implemented and tested. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Summary of Changes
Fixes #227
📝 Summary of Changes
Fix issue labeling: Distinguish between opened and updated issues
Problem
Issues were always labeled as "Opened Issue" regardless of whether they were created or just updated during the selected date range, causing user confusion.
Solution
Testing
Fixes the confusion where users couldn't differentiate between newly created issues and existing issues that were just updated during the selected period.
✅ Checklist
Summary by Sourcery
Distinguish between newly opened and updated issues when generating issue labels by checking the issue creation date against the selected date range and updating all issue templates to use the correct label.
Bug Fixes: