Conversation
Reviewer's GuideRefactors duplicated date and GitLab mapping logic into a shared utils module (scripts/utils.js), wires it into popup/main/scrum helper flows, and performs minor formatting/consistency cleanups, without changing core behavior. Sequence diagram for building Scrum subject using scrumUtilssequenceDiagram
actor User
participant Popup as popup_js
participant scrumUtils
participant EmailAdapter as emailClientAdapter
User->>Popup: trigger report generation
Popup->>Popup: read projectName from input
Popup->>scrumUtils: buildScrumSubject(projectName)
scrumUtils-->>Popup: subject
Popup->>EmailAdapter: sendReport(subject)
EmailAdapter-->>User: report composed with standardized subject
Sequence diagram for yesterdayContribution date handling with scrumUtilssequenceDiagram
actor User
participant Popup as popup_js
participant scrumUtils
participant Storage as browser_storage
User->>Popup: select yesterdayContribution timeframe
Popup->>scrumUtils: getYesterdayDateString()
scrumUtils-->>Popup: yyyy_mm_dd_yesterday
Popup->>scrumUtils: getTodayDateString()
scrumUtils-->>Popup: yyyy_mm_dd_today
Popup->>Popup: set startDateInput and endDateInput
Popup->>Popup: set inputs readOnly
Popup->>Storage: save yesterdayContribution flag
Class diagram for new scrumUtils module and consumersclassDiagram
class scrumUtils {
+getTodayDateString(now)
+getYesterdayDateString(now)
+getScrumDateCode(now)
+buildScrumSubject(projectName, now)
+mapGitLabItem(item, projects, type)
}
class scrumHelper_js {
+allIncluded(outputTarget)
+renderErrorMessage(container, key, fallback, args)
+handleYesterdayContributionChange()
+processGithubData(mappedData, isGitLab, subjectForEmail)
}
class popup_js {
+buildScrumSubjectFromPopup()
+toggleRadio(radio)
+setupButtonTooltips()
+applyI18n()
}
class main_js {
+handleYesterdayContributionChange()
+handlePlatformUsernameChange()
}
class gitlabHelper_js {
+fetchGitLabData(username, startingDate, endingDate, gitlabToken)
}
scrumHelper_js ..> scrumUtils : uses getTodayDateString
scrumHelper_js ..> scrumUtils : uses getYesterdayDateString
scrumHelper_js ..> scrumUtils : uses buildScrumSubject
scrumHelper_js ..> scrumUtils : uses mapGitLabItem
popup_js ..> scrumUtils : uses getTodayDateString
popup_js ..> scrumUtils : uses getYesterdayDateString
popup_js ..> scrumUtils : uses buildScrumSubject
main_js ..> scrumUtils : uses getTodayDateString
main_js ..> scrumUtils : uses getYesterdayDateString
gitlabHelper_js ..> scrumUtils : no direct use
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
utils.js,mapGitLabItemhardcodes the GitLab API base URL (https://gitlab.com/api/v4/...), which diverges from the configurablebaseUrlused inGitLabHelper; consider wiringmapGitLabItemto use the same base or accept it as a parameter so self‑hosted GitLab instances work consistently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `utils.js`, `mapGitLabItem` hardcodes the GitLab API base URL (`https://gitlab.com/api/v4/...`), which diverges from the configurable `baseUrl` used in `GitLabHelper`; consider wiring `mapGitLabItem` to use the same base or accept it as a parameter so self‑hosted GitLab instances work consistently.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Refactors common date/subject formatting and GitLab item mapping logic into a shared utils.js, then updates the popup and content-script codepaths to use these shared helpers to reduce duplication (Fixes #519).
Changes:
- Added
src/scripts/utils.jsexposing shared helpers underwindow.scrumUtils. - Replaced duplicated “today/yesterday”, scrum subject building, and GitLab item mapping logic in
popup.js,main.js, andscrumHelper.jswithscrumUtilscalls. - Ensured
utils.jsis loaded inpopup.htmland added to Chrome/Firefox manifest content scripts; minor formatting cleanups in several files.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scripts/utils.js | Introduces shared date/subject helpers and a centralized GitLab item mapping function. |
| src/scripts/scrumHelper.js | Uses scrumUtils for GitLab mapping, subject generation, and date helpers; formatting-only changes in several places. |
| src/scripts/popup.js | Removes local date helpers and uses scrumUtils for “yesterday/today” and subject generation; formatting tweaks. |
| src/scripts/main.js | Uses scrumUtils for “yesterday/today” date values instead of local helper functions. |
| src/scripts/gitlabHelper.js | Line-wrapping/formatting adjustments to thrown error construction. |
| src/popup.html | Loads scripts/utils.js before scripts that reference window.scrumUtils. |
| src/manifests/chrome.json | Adds scripts/utils.js to content script list before scrumHelper.js. |
| src/manifests/firefox.json | Adds scripts/utils.js to content script list before scrumHelper.js. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mariobehling
left a comment
There was a problem hiding this comment.
Please address AI reviews or comment if not relevant.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@mariobehling On closer review, I found that the original functions in main.js (getToday, getYesterday) already used toISOString() (UTC), and scrumHelper.js also interprets date strings as UTC boundaries (e.g. new Date(startDate + 'T00:00:00Z')). So mixing local getters with toISOString() output was actually a pre-existing inconsistency. I've fixed this by moving fully to UTC throughout utils.js: Before (mixed): After (consistent UTC): This also applies to getScrumDateCode, which now uses getUTCFullYear/Month/Date for the same reason. |
📌 Fixes
Fixes # (Use "Fixes", "Closes", or "Resolves" for automatic closing)
Fixes: #519
📝 Summary of Changes
Refactor codebase reduce duplication and introduce utils.js for helper/utils functions
📸 Screenshots / Demo (if UI-related)
Add screenshots, video, or link to deployed preview if applicable
✅ Checklist
👀 Reviewer Notes
Add any special notes for the reviewer here
Summary by Sourcery
Extract shared date/subject formatting and GitLab item mapping logic into a new reusable utils module and update existing scripts to use it.
Enhancements: