-
Notifications
You must be signed in to change notification settings - Fork 78
Report-preview #82
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
base: master
Are you sure you want to change the base?
Report-preview #82
Conversation
…s with your FOSSASIA contributions. Users can input their GitHub username, project details, and specify the date range to fetch contributions. The tool also provides an option to display open/closed labels for issues and PRs, and allows the user to share what might be stopping them from completing their work. After entering the details, users can preview and copy the generated SCRUM report to their clipboard.
Reviewer's Guide by SourceryThis pull request introduces a preview feature for the scrum report, refactors date handling, improves code readability, and enhances the user experience with visual feedback and error handling. The preview feature allows users to view the generated report before copying it, ensuring accuracy and relevance. Date handling has been refactored to use a common formatting function, improving maintainability. Code readability has been improved by removing redundant checks and simplifying conditional assignments. User experience has been enhanced with visual feedback and error handling. Sequence diagram for generating and previewing the scrum reportsequenceDiagram
participant User
participant Popup
participant GitHubAPI
participant PreviewModal
User->>Popup: Enters details (username, project, dates, reason)
User->>Popup: Clicks 'Preview Scrum Report'
Popup->>Popup: Validates input
alt Input invalid
Popup-->>User: Shows error message
else Input valid
Popup->>GitHubAPI: Fetches GitHub data (issues, PRs)
GitHubAPI-->>Popup: Returns data
Popup->>Popup: Generates scrum report
Popup->>PreviewModal: Displays report in modal
PreviewModal-->>User: Shows preview
end
User->>PreviewModal: Clicks 'Copy to Clipboard'
PreviewModal->>Popup: Copies report to clipboard
Popup-->>User: Shows success message
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Relacosm - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a loading indicator while fetching GitHub data to improve user experience.
- The date formatting logic could be extracted into a reusable utility function.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 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.
Materialize.toast('Copied to clipboard!', 2000); | ||
} | ||
|
||
function initializeModal() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Duplicate modal initialization in main.js and popup.html may lead to redundant calls.
Both main.js and an inline script in popup.html initialize modals. Consider centralizing the modal initialization logic to avoid potential conflicts or unnecessary double initialization.
Suggested implementation:
Since popup.html already initializes the modals through its inline script, make sure this centralized initialization does not need to be replicated in main.js. If you prefer to centralize modal initialization in main.js instead, remove the inline initialization from popup.html to avoid redundancy.
document.getElementById('codeheatTab').addEventListener('click', handleCodeheatClick); | ||
document.getElementById('gsocTab').addEventListener('click', handleGsocClick); | ||
previewButtonElement.addEventListener('click', handlePreviewClick); | ||
copyPreviewElement.addEventListener('click', handleCopyPreview); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Event handler for copyPreview is bound in both main.js and popup.html.
There are two separate event bindings for the copy preview button. Consolidating them in one place would reduce potential duplicate executions.
@@ -150,6 +134,7 @@ function handleCodeheatClick() { | |||
chrome.storage.local.set({ gsoc: 0 }); | |||
handleLastWeekContributionChange(); | |||
} | |||
|
|||
function handleGsocClick() { |
There was a problem hiding this comment.
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 report generation logic into a separate module to improve readability and testability by using an array of HTML fragments and template literals to construct the HTML report, and then importing and using this module in the main file to keep concerns separated and the code more maintainable and testable, which reduces complexity significantly by improving modularity and readability..
Consider extracting the report generation logic from the UI handling and simplifying the nested HTML construction. For example, you can refactor the generateScrumReport
function to use an array of HTML fragments and template literals, which improves readability. You might also extract the report-building logic to a separate module if it grows further. Here’s a small focused example:
// In a separate module (e.g., reportBuilder.js)
export function generateScrumReport(data, projectName, showLabels, reason) {
const items = data.items || [];
const prs = items.filter(item => item.pull_request);
const issues = items.filter(item => !item.pull_request);
const parts = [
`<div><strong>Project:</strong> ${projectName || 'N/A'}</div>`,
`<div><strong>What did I do last week?</strong></div>`,
`<ul>`
];
if (prs.length) {
parts.push('<li>Worked on Pull Requests:<ul>');
prs.forEach(pr => {
const status = pr.state === 'open'
? `<span class="label-open">[OPEN]</span>`
: `<span class="label-closed">[CLOSED]</span>`;
parts.push(
`<li><a href="${pr.html_url}" target="_blank">${pr.title}</a>${showLabels ? ' ' + status : ''}</li>`
);
});
parts.push('</ul></li>');
}
if (issues.length) {
parts.push('<li>Worked on Issues:<ul>');
issues.forEach(issue => {
const status = issue.state === 'open'
? `<span class="label-open">[OPEN]</span>`
: `<span class="label-closed">[CLOSED]</span>`;
parts.push(
`<li><a href="${issue.html_url}" target="_blank">${issue.title}</a>${showLabels ? ' ' + status : ''}</li>`
);
});
parts.push('</ul></li>');
}
parts.push(`</ul>
<div><strong>What am I going to do this week?</strong></div>
<ul><li>Continue working on open PRs and issues</li></ul>
<div><strong>What is stopping me?</strong></div>
<ul><li>${reason || 'Nothing'}</li></ul>`);
return parts.join('');
}
Then, in your main file, import and use the new function:
// In your main file
import { generateScrumReport } from './reportBuilder.js';
function handlePreviewClick() {
const username = githubUsernameElement.value;
const projectName = projectNameElement.value;
const startDate = startingDateElement.value;
const endDate = endingDateElement.value;
const showLabels = showOpenLabelElement.checked;
const reason = userReasonElement.value;
if (!username) {
Materialize.toast('Please enter your GitHub username', 3000);
return;
}
$('#previewModal').modal('open');
$('#previewContent').html('<p>Loading your GitHub data...</p>');
fetchGitHubData(username, startDate, endDate)
.then(data => {
const preview = generateScrumReport(data, projectName, showLabels, reason);
$('#previewContent').html(preview);
})
.catch(error => {
$('#previewContent').html('<p>Error loading data: ' + error.message + '</p>');
});
}
This separation not only improves readability but also makes unit testing the report generation isolated and simpler.
u should check it actually
Summary by Sourcery
Add a preview feature for generating GitHub activity-based Scrum reports in the browser extension
New Features:
Enhancements:
Chores: