This PR adds customization and Template saving Features to extension. #103
Conversation
Scrum subject and body message are now correctly added in Outlook (fossasia#81)
merge the readme changes
Signed-off-by: Vedansh Saini <[email protected]>
Signed-off-by: Vedansh Saini <[email protected]>
new changes added to the master before 1 june
Updated repo june 2
Reviewer's GuideThis PR introduces a persistent customization layer—allowing users to toggle report sections and apply GitHub filters—alongside the ability to save and load layout templates, restructures the popup UI to include a dedicated Settings tab, refactors content-generation routines to respect those settings, applies new styling for tabs and templates, and adds a Release Drafter workflow for automated release notes. Sequence Diagram for Saving a Customization TemplatesequenceDiagram
actor User
participant SettingsUI as "Settings UI (settings.js)"
participant ScrumHelper as "scrumHelper.js"
participant Storage as "chrome.storage.local"
User->>SettingsUI: Modifies section toggles & filters
User->>SettingsUI: Enters template name ("My Layout")
User->>SettingsUI: Clicks "Save Current Layout"
SettingsUI->>ScrumHelper: saveTemplate("My Layout", currentSettings)
ScrumHelper->>Storage: get(["scrumTemplates"])
Storage-->>ScrumHelper: { scrumTemplates: existingTemplates }
ScrumHelper->>ScrumHelper: Add/Update "My Layout" in templates
ScrumHelper->>Storage: set({ scrumTemplates: updatedTemplates })
Storage-->>ScrumHelper: Promise resolves (save confirmation)
ScrumHelper-->>SettingsUI: Promise resolves
SettingsUI->>SettingsUI: refreshTemplateList()
SettingsUI->>ScrumHelper: loadSettings() // Called by refreshTemplateList
ScrumHelper->>Storage: get(["scrumSettings", "scrumTemplates"])
Storage-->>ScrumHelper: { settings, templates: updatedTemplates }
ScrumHelper-->>SettingsUI: { settings, templates }
SettingsUI->>User: Display updated template list
Sequence Diagram for Report Generation with Custom SettingssequenceDiagram
actor User
participant PopupUI as "Popup UI (popup.js)"
participant ScrumHelper as "scrumHelper.js"
participant Storage as "chrome.storage.local"
participant GitHubAPI as "GitHub API"
User->>PopupUI: Clicks "Generate Report"
PopupUI->>ScrumHelper: allIncluded("popup")
ScrumHelper->>ScrumHelper: loadSettings()
ScrumHelper->>Storage: get(["scrumSettings", "scrumTemplates"])
Storage-->>ScrumHelper: { settings, templates }
ScrumHelper->>GitHubAPI: fetchGithubData() (details omitted)
GitHubAPI-->>ScrumHelper: rawIssuesData, rawPrsReviewData
ScrumHelper->>ScrumHelper: writeGithubIssuesPrs(rawIssuesData, settings)
ScrumHelper->>ScrumHelper: filterGithubData(rawIssuesData, settings) // internal call
ScrumHelper->>ScrumHelper: writeGithubPrsReviews(rawPrsReviewData, settings)
ScrumHelper->>ScrumHelper: filterGithubData(rawPrsReviewData, settings) // internal call
ScrumHelper->>ScrumHelper: writeScrumBody(settings)
ScrumHelper-->>PopupUI: Formatted report content
PopupUI->>User: Display report reflecting settings
Entity Relationship Diagram for Settings and Templates StorageerDiagram
CHROME_STORAGE_LOCAL {
string key PK "Primary Key e.g., 'scrumSettings' or 'scrumTemplates'"
json value "Stored data as JSON string"
}
ScrumSettingsObject {
map sections "Key-value pairs for section visibility, e.g., {tasks: true, prs: false}"
map filters "Key-value pairs for GitHub filters, e.g., {openOnly: true, excludeDrafts: false}"
}
TemplateObject {
string name PK "User-defined template name"
map sections "Snapshot of section visibility settings"
map filters "Snapshot of GitHub filter settings"
}
CHROME_STORAGE_LOCAL ||--1 ScrumSettingsObject : "stores current settings (value for key 'scrumSettings')"
CHROME_STORAGE_LOCAL ||--1 TemplateCollection : "stores all templates (value for key 'scrumTemplates')"
TemplateCollection ||--o{ TemplateObject : "contains many (as a map of name:TemplateObject)"
Class Diagram for Settings and Template Management LogicclassDiagram
class ScrumHelper {
<<JavaScript: scrumHelper.js>>
+DEFAULT_SETTINGS : Object
+loadSettings() : Promise
+saveSettings(settings) : Promise
+saveTemplate(name, settings) : Promise
+loadTemplate(name) : Promise
+deleteTemplate(name) : Promise
+filterGithubData(data, settings) : Object
+allIncluded(outputTarget) : void
+writeScrumBody() : void
+writeGithubPrsReviews() : void
+writeGithubIssuesPrs() : void
}
class SettingsController {
<<JavaScript: settings.js>>
+initializeUI() : void
+refreshTemplateList() : void
+handleSaveTemplate() : void
+handleLoadTemplate(templateName) : void
+handleDeleteTemplate(templateName) : void
+handleSettingChange() : void
}
class ChromeStorageAPI {
<<Browser API: chrome.storage.local>>
+get(keys) : Promise
+set(items) : Promise
}
ScrumHelper ..> ChromeStorageAPI : Uses
SettingsController ..> ScrumHelper : Uses
SettingsController ..> ChromeStorageAPI : Uses (via ScrumHelper.loadSettings)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Preeti9764 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
tempDiv.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
reportElement.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - Concatenating raw item data directly into HTML risks XSS (link)
General comments:
- Consider caching settings and templates upon initialization and passing them through your data-processing functions instead of repeatedly calling chrome.storage.local.get in each step to improve performance and readability.
- The repeated promise wrappers around chrome.storage.local.get/set could be refactored into a single storage utility to reduce duplication and simplify future maintenance.
- The CSS approach for toggling between the main and settings tabs relies on complex sibling selectors—switching to explicit class toggles on the container might be more robust and easier to maintain.
Here's what I looked at during the review
- 🟡 General issues: 5 issues found
- 🔴 Security: 4 blocking issues
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| window.generateScrumReport(); | ||
|
|
||
| // Reset button state after a short delay | ||
| setTimeout(() => { |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid fixed timeout for resetting Generate button
Instead of using a hardcoded delay, reset the button state when generateScrumReport() completes—either by returning a promise or emitting an event and handling it accordingly.
Suggested implementation:
// Generate report and reset button state when done
Promise.resolve(window.generateScrumReport()).finally(() => {
this.innerHTML = '<i class="fa fa-refresh"></i> Generate Report';
this.disabled = false;
// Ensure the window stops scrolling
window.scrollTo(0, window.scrollY);
});- Ensure that
window.generateScrumReport()returns a Promise that resolves when the report generation is complete. If it does not, you will need to refactor that function accordingly.
src/scripts/scrumHelper.js
Outdated
| loadSettings().then(({ settings }) => { | ||
| setTimeout(() => { | ||
| // Generate content based on enabled sections | ||
| var lastWeekUl = '<ul>'; |
There was a problem hiding this comment.
🚨 suggestion (security): Concatenating raw item data directly into HTML risks XSS
Sanitize user-supplied values like item.title and project, or use DOM APIs to prevent XSS vulnerabilities from untrusted content.
Suggested implementation:
loadSettings().then(({ settings }) => {
setTimeout(() => {
// Utility function to escape HTML special characters
function escapeHtml(str) {
return String(str)
.replace(/&/g, '&')
.replace(/</g, '<')
.replace(/>/g, '>')
.replace(/"/g, '"')
.replace(/'/g, ''');
}
// Generate content based on enabled sectionsYou must now use escapeHtml() whenever you insert user-supplied values (such as item.title, project, etc.) into HTML strings. For example:
lastWeekUl += '<li>' + escapeHtml(item.title) + ' (' + escapeHtml(project) + ')</li>';Search for all such concatenations in this file and update them accordingly.
| }); | ||
|
|
||
| // Initialize template list | ||
| refreshTemplateList(templates); |
There was a problem hiding this comment.
nitpick: refreshTemplateList is called with an unused argument
Consider removing the unused argument from the call or updating the function to use it for clarity.
| document.getElementById('filter-open-only').checked = settings.filters.openOnly; | ||
| document.getElementById('filter-exclude-drafts').checked = settings.filters.excludeDrafts; |
There was a problem hiding this comment.
suggestion: Initial filter toggles aren’t reflected in label styling
Apply the same class toggling for filter labels during initialization as is done in handleOpenLabelChange to ensure consistent UI state.
| document.getElementById('filter-open-only').checked = settings.filters.openOnly; | |
| document.getElementById('filter-exclude-drafts').checked = settings.filters.excludeDrafts; | |
| document.getElementById('filter-open-only').checked = settings.filters.openOnly; | |
| document.getElementById('filter-exclude-drafts').checked = settings.filters.excludeDrafts; | |
| // Reflect initial filter state in label styling | |
| ['filter-open-only', 'filter-exclude-drafts'].forEach(filterId => { | |
| const filter = document.getElementById(filterId); | |
| const label = document.querySelector(`label[for="${filterId}"]`); | |
| if (label) { | |
| if (filter.checked) { | |
| label.classList.add('active'); | |
| } else { | |
| label.classList.remove('active'); | |
| } | |
| } | |
| }); |
src/scripts/scrumHelper.js
Outdated
| var repository_url = item.repository_url; | ||
| var project = repository_url.substr(repository_url.lastIndexOf('/') + 1); | ||
| var title = item.title; | ||
| var number = item.number; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
src/scripts/scrumHelper.js
Outdated
| var project = repository_url.substr(repository_url.lastIndexOf('/') + 1); | ||
| var title = item.title; | ||
| var number = item.number; | ||
| var html_url = item.html_url; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
src/scripts/scrumHelper.js
Outdated
| var obj = { | ||
| number: number, | ||
| html_url: html_url, | ||
| title: title, | ||
| state: item.state, | ||
| }; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
src/scripts/scrumHelper.js
Outdated
| prText1 += issue_opened_button; | ||
|
|
||
| for (var repo in githubPrsReviewDataProcessed) { | ||
| var repoLi = '<li><i>(' + repo + ')</i> - Reviewed '; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
src/scripts/scrumHelper.js
Outdated
|
|
||
| if (githubPrsReviewDataProcessed[repo].length <= 1) { | ||
| for (var pr in githubPrsReviewDataProcessed[repo]) { | ||
| var pr_arr = githubPrsReviewDataProcessed[repo][pr]; |
There was a problem hiding this comment.
issue (code-quality): Use const or let instead of var. (avoid-using-var)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
|
@hpdang @mariobehling I've implemented customisation and Template saving Features in the Scrum Helper extension. Suggest changes which i can do in Ui and your feedback on this, whether these features looks goods. For now i have added the codeheat tab only. Open to changes and happy to iterate based on your suggestions. |
Preeti9764
changed the title
|
hey, @vedansh-5 I have made changes for some of the issues you mentioned , the same name one is left..It would be great if you once check and give feedback. |
Yes, they are for testing purposes. |
Please elloborate about headers you want me to add. |
|
@hpdang I request you try the new features and share your feedback whenever you get time customisation.mp4 |
|
To do for this PRs: CodeHeat tab:
Setting tab
Change the top description to: |
okay thanks! will do these changes soon |
Initially, all checkboxes are selected, and the type of report includes PRs, issues, and reviews. After that, the user can filter it by going to the Template tab and also save the changes for future use, which will be saved in Templates.if no template is selected, the report will be generated according to the selected checkboxes. |
Fixes
Fixes #88
📝 Summary of Changes
✅ Checklist
Summary by Sourcery
Add persistent customization controls and template management to the Scrum Helper extension, update the report generation logic to honor user settings, enhance the popup UI with a Settings tab, and configure a Release Drafter workflow for automated release notes generation.
New Features:
Enhancements:
CI: