Skip to content

added Standard closed label for issues #196

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

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

Conversation

Preeti9764
Copy link
Contributor

@Preeti9764 Preeti9764 commented Jul 11, 2025

📌 Fixes

Fixes #186


📝 Summary of Changes

  • Added separate label for different types of issue completion such as completed , not planned accorfing to the github satndards.

📸 Screenshots / Demo (if UI-related)

image

✅ Checklist

  • I’ve tested my changes locally
  • I’ve added tests (if applicable)
  • I’ve updated documentation (if applicable)
  • My code follows the project’s code style guidelines

👀 Reviewer Notes

Add any special notes for the reviewer here

Summary by Sourcery

Introduce distinct labels for closed issues, add a toggle to display commits on open PRs, remove the blocker input field, and update UI and storage logic accordingly

New Features:

  • Add a checkbox to show commits on open PRs with a tooltip explaining GitHub token requirement

Enhancements:

  • Render closed issues with specific ‘completed’ and ‘not planned’ labels based on state_reason
  • Update the issue label toggle to "Show Open/Merged/Closed Labels"
  • Remove the blocker reason input from the UI and default to a 'No Blocker at the moment' message
  • Clean up related local storage handling by dropping userReason
  • Apply minor HTML markup and whitespace fixes in the popup

Copy link
Contributor

sourcery-ai bot commented Jul 11, 2025

Reviewer's Guide

This PR extends the issue-status labeling by introducing two new closed-state buttons, updates the popup UI to reflect Open/Merged/Closed options (including a new commit-display toggle), and removes the now-obsolete user blocker input along with its storage and event handling.

Class diagram for updated scrumHelper.js data handling

classDiagram
    class StorageItems {
      +string githubUsername
      +string projectName
      +string githubToken
      +string lastWeekContribution
      +string yesterdayContribution
      +string showOpenLabel
      +string showCommits
      -string userReason
    }
    class allIncluded {
      +allIncluded(outputTarget)
      +Handles: githubUsername, projectName, githubToken, lastWeekContribution, yesterdayContribution, showOpenLabel, showCommits
      -userReason = 'No Blocker at the moment'
    }
    StorageItems <|-- allIncluded
Loading

Class diagram for new issue state label rendering

classDiagram
    class IssueLabelRenderer {
      +issue_closed_button
      +issue_opened_button
      +issue_closed_completed_button
      +issue_closed_notplanned_button
      +renderIssueLabel(item)
    }
    class IssueItem {
      +string state
      +string state_reason
      +string html_url
      +string title
      +string number
      +string project
    }
    IssueItem --> IssueLabelRenderer: used by
Loading

Class diagram for main.js UI event handling changes

classDiagram
    class MainUI {
      +showOpenLabelElement
      +showCommitsElement
      -userReasonElement // removed
      +handleBodyOnLoad()
      +handleOpenLabelChange()
      +handleShowCommitsChange()
      -handleUserReasonChange() // removed
    }
Loading

File-Level Changes

Change Details Files
UI updates in popup.html
  • Changed open/closed label checkbox text to include merged
  • Added a "Show commits on open PRs" checkbox with tooltip
  • Removed the user-reason input field
  • Adjusted tooltip and paragraph wrapping for readability
src/popup.html
New closed-state labels in issue rendering
  • Defined 'completed' and 'not planned' button HTML fragments
  • Updated closed-issue rendering branch to use state_reason for label selection
src/scripts/scrumHelper.js
Removed userReason storage and handlers
  • Dropped userReason reads/writes in storage
  • Removed userReason DOM references and event listeners
  • Defaulted blocker text to a constant and cleared stale storage
src/scripts/scrumHelper.js
src/scripts/main.js
src/scripts/popup.js

Assessment against linked issues

Issue Objective Addressed Explanation
#186 Issue labels should indicate the status of the issue (open, closed as planned, closed not as planned).
#186 Closed issues should have a purple label if completed, and grey if not closed as planned.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @Preeti9764 - I've reviewed your changes - here's some feedback:

  • Consolidate the inline badge HTML in scrumHelper.js into a helper or CSS classes to avoid repetitive styling across status badges.
  • Remove all residual userReason code and references (storage, comments, docs) now that the input has been removed to keep the codebase clean.
  • The updated “Show Open/Merged/Closed Labels” setting implies merged PR support, but the logic only handles open/closed states—ensure merged PRs are properly detected and labeled.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consolidate the inline badge HTML in scrumHelper.js into a helper or CSS classes to avoid repetitive styling across status badges.
- Remove all residual `userReason` code and references (storage, comments, docs) now that the input has been removed to keep the codebase clean.
- The updated “Show Open/Merged/Closed Labels” setting implies merged PR support, but the logic only handles open/closed states—ensure merged PRs are properly detected and labeled.

## Individual Comments

### Comment 1
<location> `src/scripts/main.js:11` </location>
<code_context>
 let startingDateElement = document.getElementById('startingDate');
 let endingDateElement = document.getElementById('endingDate');
 let showOpenLabelElement = document.getElementById('showOpenLabel');
-let userReasonElement = document.getElementById('userReason');
+let userReasonElement = null; // userReason element removed from UI
 let showCommitsElement = document.getElementById('showCommits');

</code_context>

<issue_to_address>
Consider removing all references to userReasonElement for clarity.

Since the UI element has been removed, deleting the variable and related code will help prevent confusion and reduce the risk of future errors.

Suggested implementation:

```javascript
let startingDateElement = document.getElementById('startingDate');
let endingDateElement = document.getElementById('endingDate');
let showOpenLabelElement = document.getElementById('showOpenLabel');
let showCommitsElement = document.getElementById('showCommits');

```

You should also search the rest of `src/scripts/main.js` for any other references to `userReasonElement` (such as assignments, event listeners, or logic using this variable) and remove those lines as well. If any functions or logic blocks depend on `userReasonElement`, refactor or remove them as appropriate to ensure the code remains clean and functional.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Preeti9764
Copy link
Contributor Author

Preeti9764 commented Jul 11, 2025

@hpdang does the label seems good ,please share your review .Thanks!

@Preeti9764 Preeti9764 requested review from hpdang and vedansh-5 July 11, 2025 11:27
@hpdang
Copy link
Member

hpdang commented Jul 11, 2025

@Preeti9764 a bit confusing to me: in your screenshot, in the beginning, it showed Opened Issue (#176) then the end, it showed not planned, so that issue is open or closed?

@Preeti9764
Copy link
Contributor Author

Preeti9764 commented Jul 11, 2025

@Preeti9764 a bit confusing to me: in your screenshot, in the beginning, it showed Opened Issue (#176) then the end, it showed not planned, so that issue is open or closed?

It is closed as not planned. I just closed it as a not planned issue for testing the labels. As we decided to have separate labels for different types of issue closing, it is one of those types.Thanks!

@vedansh-5
Copy link
Contributor

📸 Screenshots / Demo (if UI-related)

image

My suggestion would be to go with grey colored label as it is now with closed text instead of not planned.
This PR works fine for me, lgtm, Please change the not planned to closed. Thankyou.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement]: Issue labels
3 participants