Fix: Show message when user have no activity for selected days#390
Fix: Show message when user have no activity for selected days#390Mateeb-Haider wants to merge 9 commits intofossasia:mainfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors scrum report generation to centralize building of the "last week" activity list and adds a fallback message when there is no activity in the selected period, ensuring both popup and email reports display a meaningful status instead of an empty list. Flow diagram for centralized activity list HTML generationflowchart TD
A[allIncluded called
outputTarget email or popup] --> B[Process data to populate
lastWeekArray and reviewedPrsArray]
B --> C{Subject_for_email?}
C -- yes --> D[Call buildActivityListHtml
for email lastWeekUl]
C -- no --> E[writeScrumBody for popup]
E --> F[Call buildActivityListHtml
for popup lastWeekUl]
subgraph Activity_List_Builder
G[buildActivityListHtml]
G --> H{lastWeekArray.length == 0
AND reviewedPrsArray.length == 0}
H -- yes --> I[Return '<ul><li>No contributions found for selected period.</li></ul>']
H -- no --> J[Initialize activityList = '<ul>']
J --> K[Append items from lastWeekArray]
K --> L[Append items from reviewedPrsArray]
L --> M[Close list '</ul>' and return activityList]
end
D --> N[Compose email body with lastWeekUl]
F --> O[Compose popup body with lastWeekUl]
N --> P[User views email report]
O --> Q[User views popup report]
File-Level Changes
Assessment against 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.
Pull request overview
Adds a user-facing fallback message when the selected day/range has no GitHub activity, preventing empty “What did I do…?” sections in both popup and email report generation.
Changes:
- Refactored “last week” HTML list generation into a shared helper (
buildActivityListHtml()). - Added a fallback
<li>message when bothlastWeekArrayandreviewedPrsArrayare empty.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/scripts/scrumHelper.js
Outdated
|
|
||
| function buildActivityListHtml() { | ||
| if (lastWeekArray.length === 0 && reviewedPrsArray.length === 0) { | ||
| return '<ul><li>No contributions found for selected period.</li></ul>'; |
There was a problem hiding this comment.
The fallback string is hard-coded to “selected period”, but the report header switches between “yesterday” and a concrete date range. This can lead to inconsistent/confusing wording for users (e.g., yesterday mode still says “selected period”). Consider deriving the message from yesterdayContribution / startingDate / endingDate so it matches the header context.
| return '<ul><li>No contributions found for selected period.</li></ul>'; | |
| let periodDescription; | |
| if (typeof yesterdayContribution !== 'undefined' && yesterdayContribution) { | |
| periodDescription = 'yesterday'; | |
| } else if (typeof startingDate !== 'undefined' && typeof endingDate !== 'undefined' && startingDate && endingDate) { | |
| periodDescription = `${formatDate(startingDate)} – ${formatDate(endingDate)}`; | |
| } else { | |
| periodDescription = 'the selected period'; | |
| } | |
| return `<ul><li>No contributions found for ${periodDescription}.</li></ul>`; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@vedansh-5, @hpdang, @mariobehling |
Saksham-Sirohi
left a comment
Saksham-Sirohi
left a comment
There was a problem hiding this comment.
It would be appreciated if this message did not appear as a list entry
@Saksham-Sirohi |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vedansh-5
left a comment
vedansh-5
left a comment
There was a problem hiding this comment.
Thanks @Mateeb-Haider for your contribution.
src/scripts/scrumHelper.js
Outdated
|
|
||
| function buildActivityListHtml() { | ||
| if (lastWeekArray.length === 0 && reviewedPrsArray.length === 0) { | ||
| return '<div style="padding: 0 12px;">No activity to report for this update.</div>'; |
There was a problem hiding this comment.
Please update this to no activity to report for the selected time period.
There was a problem hiding this comment.
Please update this to no activity to report for the selected time period.
I apply the required change now PR is ready to merge
thanku!
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function buildActivityListHtml() { | ||
| if (lastWeekArray.length === 0 && reviewedPrsArray.length === 0) { | ||
| return '<div style="padding: 0 12px;">No activity to report for this update.</div>'; | ||
| } | ||
|
|
||
| let activityList = '<ul>'; | ||
| for (let i = 0; i < lastWeekArray.length; i++) activityList += lastWeekArray[i]; | ||
| for (let i = 0; i < reviewedPrsArray.length; i++) activityList += reviewedPrsArray[i]; | ||
| activityList += '</ul>'; | ||
| return activityList; | ||
| } |
There was a problem hiding this comment.
Consider adding similar no-activity handling for the "next week" section. Currently, if nextWeekArray is empty, the code will render <ul></ul> (lines 1004-1006 and 1061-1063) which provides no user feedback. While the "next week" section represents plans rather than completed activities, an empty state might still benefit from a helpful message like "No plans added yet" or similar to maintain consistency with the improved UX for the activity section.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/scripts/scrumHelper.js
Outdated
| function buildBlockerTextHtml() { | ||
| return `<span style="display: inline-block; padding: 0 8px; margin: 0; line-height: 1.2;">${userReason}</span>`; |
There was a problem hiding this comment.
userReason is interpolated directly into an HTML string that later gets injected via innerHTML/email adapter. If userReason can contain user-provided text, this enables HTML/script injection. Escape/encode userReason before inserting it into markup (or inject it as text rather than HTML).
| function buildBlockerTextHtml() { | |
| return `<span style="display: inline-block; padding: 0 8px; margin: 0; line-height: 1.2;">${userReason}</span>`; | |
| function escapeHtml(str) { | |
| return String(str) | |
| .replace(/&/g, '&') | |
| .replace(/</g, '<') | |
| .replace(/>/g, '>') | |
| .replace(/"/g, '"') | |
| .replace(/'/g, '''); | |
| } | |
| function buildBlockerTextHtml() { | |
| return `<span style="display: inline-block; padding: 0 8px; margin: 0; line-height: 1.2;">${escapeHtml(userReason)}</span>`; |
| if (lastWeekArray.length === 0 && reviewedPrsArray.length === 0) { | ||
| return '<span style="display: inline-block; padding: 0 8px; margin: 0; line-height: 1.2;">No activity to report for the selected time period.</span>'; | ||
| } |
There was a problem hiding this comment.
The same long inline style="display: inline-block; padding: 0 8px; margin: 0; line-height: 1.2;" string is hard-coded in multiple helpers (activity, next-week, blockers). Consider extracting it into a constant or a shared wrapper helper (or a CSS class) to avoid duplication and keep formatting changes centralized.
@vedansh-5 I implemented all required changes by you and copilot as well and the output attached after the change please take a look when you have free time |
vedansh-5
left a comment
vedansh-5
left a comment
There was a problem hiding this comment.
Thanks @Mateeb-Haider for your contribution.
Please pull the latest changes from the base branch and rebase your PR. Also resolve the existing merge conflicts.
Other than that, this PR looks good to me.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/scripts/scrumHelper.js
Outdated
| for (let i = 0; i < lastWeekArray.length; i++) lastWeekUl += lastWeekArray[i]; | ||
| for (let i = 0; i < reviewedPrsArray.length; i++) lastWeekUl += reviewedPrsArray[i]; | ||
| lastWeekUl += '</ul>'; | ||
| if (!enableToggle) { |
There was a problem hiding this comment.
enableToggle is referenced here but it is not defined anywhere in scrumHelper.js (and a repo-wide search only finds it being removed from storage in popup.js). This will throw a ReferenceError when writeScrumBody() runs in the non-email path. Remove this guard or replace it with a locally-defined flag (e.g., read from storage) and a safe check that won’t crash when undefined.
| if (!enableToggle) { | |
| let isEnabled = true; | |
| if (typeof enableToggle !== 'undefined' && !enableToggle) { | |
| isEnabled = false; | |
| } | |
| if (!isEnabled) { |
|
@vedansh-5 |
📌 Fixes
Fixes #311
📝 Summary of Changes
📸 Screenshots / Demo (if UI-related)
Before
After
✅ Checklist
👀 Reviewer Notes
Please verify:
Summary by Sourcery
Show a meaningful fallback message in scrum reports when no activity exists for the selected period and reuse this behavior across email and popup outputs.
Bug Fixes:
Enhancements: