Repository navigation
Shubham Jakhete - Phase 1: Finishing followup fixes for PR 4558+1953: Fix Dark Mode, Date Selection, Time Selection and Scheduling the post for Mastodon (frontend) - #5593
Conversation
The Mastodon composer called /api/mastodon/... on the frontend's own address, so requests never reached the backend and scheduling always failed with an error popup. - Add Mastodon endpoints to URL.js and call them with axios, which sends the logged-in user's token - Show the backend's error message when a request fails - When editing a scheduled post, delete the old version only after the new one is saved - Label the schedule date and time inputs for screen readers - Add tests for scheduling and loading scheduled posts and history
The composer imported its CSS module but used plain class names, so none of its styles were applied: the sub-tabs ran together as one line of text and dark mode never worked. - Use the CSS module classes throughout the composer - Switch dark mode on from the app's dark mode setting instead of the operating system's - Draw the date and time pickers in dark colors in dark mode - Make placeholder text readable in dark mode - Add tests for the tab classes and dark mode
The preferences box on the Details tab used an inline white background, so in dark mode its light text was unreadable. Move its styles into the CSS module with a dark variant.
Editing a scheduled post read the date in UTC but the time in local time. For evening posts in Pacific time the UTC date is already the next day, so the post moved forward a day when saved. - Read the date and time together in local time - Add tests for evening times and leading zeros
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
one-community
added
the
High Priority - Please Review First
AaditTrivedi
left a comment
AaditTrivedi
left a comment
There was a problem hiding this comment.
Reviewed by Aadit Trivedi.
How I verified
Checked out shubham-mastodon-scheduling and tested locally against the staging backend as Admin and Volunteer, reviewed the diff, and ran the new tests. I did not test Post Now or scheduling a real post end to end: that needs backend #2373 and the Mastodon test account, and staging does not have #2373.
Verified working
- Scope matches the description: 7 files, all in the Mastodon composer, its CSS module, its tests, and
URL.js. - Tests:
npx vitest run src/components/Announcements/platforms/socialpasses 11/11 across the three new test files. - Composer, Scheduled, History, and Details show as separate, styled tabs.
- The date and time pickers work.
- Scheduling with a past date shows "Scheduled time must be in the future."
- Dark mode follows the app setting: all four tabs, the date and time popups, and the Details preferences box are readable.
- Volunteer accounts do not see Send Emails in the menu.
- Time zone handling:
toScheduleInputValuesreads the saved time with local getters, and scheduling builds the time withnew Date(\${scheduleDate}T${scheduleTime}`)before converting withtoISOString()`, so both reading and saving use local time correctly. - On staging without #2373, History shows "Failed to load post history," as expected.
Pointers, not blocking
- Test step 1 still says "HGNRest#BACKEND"; it should reference HGNRest #2373.
- The past-time check runs in the browser before any request (line 258), so the message comes from the frontend, not the backend as test step 5 describes. Same wording, so users see the same thing.
- When editing a scheduled post, the new version is saved and then the old one is deleted. If the delete fails after the save succeeds, both versions remain scheduled and both would publish. Consider telling the user or retrying the delete.
- On staging, History shows a load error but Scheduled shows "No scheduled posts yet." If the scheduled request can fail the same way, the Scheduled tab may be showing an empty list instead of an error. Worth confirming.
Evidence
Tabs and composer, light mode:
Dark mode with the date picker open:
Past-time error:
Approving.
adit24dhaya
left a comment
adit24dhaya
left a comment
There was a problem hiding this comment.
I reviewed 2896010 with its explicitly paired backend #2373 at 9fb3ee10. The frontend request routing, CSS-module binding, app-controlled dark mode and local-time conversion check out. I haven't repeated AaditTrivedi's existing edit/delete and scheduled-load feedback.
I'm holding approval of the paired scheduling feature for a separate backend integration blocker: #2373 starts the existing minute scheduler in server.js, but overlapping callbacks can publish the same due record twice. A successful publication followed by a failed delete also leaves it eligible for a second publication on the next tick. Both paths reproduce with the real scheduler, while the frontend browser fixture sends just one correctly formed scheduling request. The fix belongs in #2373; this is not a second diagnosis of the frontend edit/delete issue.
Please address the paired scheduler's claim/idempotency and post-success recovery before this scheduling feature is considered merge-ready. I've put the concrete source-level feedback on the backend review rather than repeating an inline comment here.
Validation: 11/11 existing frontend tests in three files passed on Node 20.19.4. I checked the actual composer in light/dark browser fixtures: separate tab labels, app-controlled dark class, dark native date control, one request to /api/mastodon/schedule, correct content and local-to-UTC time. Frontend build passed with 9,176 modules; changed-JS ESLint zero errors/three warnings, CSS Stylelint and git diff --check passed. Paired backend: 13/13 existing tests and ten additional scheduler reproduction/control assertions; Babel compiled 773 files and ESLint had zero errors/18 warnings. Requests, persistence and external publications were mocked. No backend server, database write or real Mastodon post was used; live account/scopes and authenticated role navigation remain unverified.
Commit audit: the four commits are focused, imperative, under 72 characters, without trailing periods, and have blank-separated what/why bodies. Optional documentation cleanup: add the relevant issue/PR reference to future commit bodies; the placeholder #BACKEND test-step issue is already noted by Aadit.
RichaSapre
left a comment
RichaSapre
left a comment
There was a problem hiding this comment.
Tested locally together with backend PR #2373. The main Mastodon flows worked as expected: Composer/Scheduled/History/Details tabs and styling, light/dark mode, date/time pickers, Post Now, scheduled posting, past-time validation, editing an evening scheduled post without shifting the date, failed-edit preservation of the original scheduled post, deletion, and Volunteer access restrictions.
I found two code-review concerns:
- The newly added console.error calls are being flagged by the PR check as unexpected console statements.
- During an edit, the new scheduled post is created before the old scheduled post is deleted. If creation succeeds but deletion of the old post fails, it looks possible for both versions to remain scheduled while the UI reports “Failed to schedule post.” Could that partial-failure case be handled separately or recovered from?
Testing/setup note: following the Mastodon setup instructions with only write:statuses and write:media caused an “outside the authorised scope” error when opening History. Adding the required read scope resolved it. It would be helpful to update the testing instructions with the complete required scopes.
Attached screenshots from local testing showing the Mastodon UI in light/dark mode and successful posting/scheduling behavior.
| setScheduledPosts(data || []); | ||
| } catch (err) { | ||
| if (process.env.NODE_ENV === 'development') { | ||
| console.error('Error loading scheduled posts:', err); |
There was a problem hiding this comment.
Could we remove this console.error or use the project’s existing logging approach instead? The PR check is currently flagging it as an unexpected console statement. Same applies to the similar console.error below.
| // so a failed save never loses the original post. | ||
| if (editingPostId) { | ||
| await fetch(`/api/mastodon/schedule/${editingPostId}`, { method: 'DELETE' }); | ||
| await axios.delete(ENDPOINTS.MASTODON_SCHEDULED_POST_BY_ID(editingPostId)); |
There was a problem hiding this comment.
What happens if the new scheduled post is saved successfully but deleting the old post fails? It looks like both versions could remain scheduled, while the catch block reports “Failed to schedule post.” Should the delete failure be handled separately so we avoid duplicate scheduled posts or provide a more accurate recovery/error state?
iAbhi001
left a comment
iAbhi001
left a comment
There was a problem hiding this comment.
PR Review & Testing Feedback
I tested this locally alongside backend PR #2373, and the core Mastodon flows worked as expected.
✅ Verified Working Flows
- Tabs & Styling: Composer, Scheduled, History, and Details tabs, along with styling and light/dark mode.
- Functionality: Date/time pickers, Post Now, scheduled posting, past-time validation, and deletion.
- Edge Cases: Editing an evening scheduled post without shifting the date, preserving the original scheduled post on failed edits, and Volunteer access restrictions.
🔍 Code-Review Concerns & Questions
- Unexpected Console Statements: The newly added
console.errorcalls are currently being flagged by PR checks. - Scheduled Post Edit Race Condition / Partial Failure: During an edit, the new scheduled post is created before the old one is deleted. If creation succeeds but deletion fails, both versions could potentially remain scheduled while the UI reports a "Failed to schedule post" error. Can this partial-failure case be handled separately or rolled back/recovered from?
📝 Documentation Note
Following the Mastodon setup instructions with only write:statuses and write:media triggered an “outside the authorised scope” error when opening the History tab. Adding the required read scope resolved the issue.
Suggestion: It would be helpful to update the setup instructions to include the complete set of required scopes.
(Attached screenshots from local testing showing the Mastodon UI in light/dark mode and successful posting/scheduling behavior.)
The scheduler found due posts, published each one and then deleted it, with no claim. This PR starts the scheduler on every server process, so an overlapping run or a second server could publish the same post again, and a failed delete after a successful post left it due for the next run. - Claim each due post atomically with findOneAndUpdate, moving it from pending to publishing with a lease, so only one worker sends it - Record a successful post as posted with its Mastodon status ID instead of deleting it - Mark a post failed, not resent, if its claim expires before delivery is recorded or the request times out - Retry clear Mastodon rejections (429, 5xx) on later runs, up to three attempts - Send a stable Idempotency-Key with each post - Hide posted records from the Scheduled list and only allow deleting pending or failed posts - Add tests for overlapping runs, two workers and a failed save after a successful post Related to OneCommunityGlobal/HighestGoodNetworkApp#5593
There was a problem hiding this comment.
Reviewed by Prajwal.
Tested locally on Windows with backend PR #2373. Works well; requesting one small fix to the testing steps.
What I verified
- Tabs and styling. Composer, Scheduled, History, and Details render as separate styled tabs, and the DOM shows CSS Module classes applied (for example _tab-button_16wgl_59, _active_16wgl_103).
html
<div class="_tabs-container_16wgl_41">
<button class="_tab-button_16wgl_59 ">Composer</button>
<button class="_tab-button_16wgl_59 _active_16wgl_103">Scheduled</button>
...
</div>
-
Requests reach the backend with login. Calls go to localhost:4500/api/mastodon/... with the Authorization header.
-
Functional flows work: Post Now, scheduling, editing, and deleting.
-
Error handling: scheduling in the past shows the backend's message; stopping the backend during an edit shows an error and keeps the original post; empty, whitespace only, and over 500 characters show clear messages.
-
Duplicates: rapid repeated edits and double clicks produce a single scheduled post and one Mastodon post.
-
Dark mode: works on all tabs, both pickers, the Details box, and modals, and switches live without a reload.
-
No CSS leakage: other pages look unchanged after using the composer.
-
Tests pass: 4 files, 14 tests.
Test Files 4 passed (4)
Tests 14 passed (14)
Required change
Placeholder in How to test. Step 1 says HGNRest#BACKEND; please replace it with #2373.
Note :
The History tab shows "This action is outside the authorized scopes". This is a Mastodon token scope issue in the backend setup steps, documented on backend PR #2373, not a frontend bug.
Two failure cases could still publish a scheduled post twice or after it was deleted (review on #2373): - A slow image upload could outlast the claim. Another run then marked the post failed, the user could delete it, and the original worker still published it once the upload finished. - A gateway or server error (5xx) can come back after Mastodon has accepted the post. It was put back in pending and retried, so a retry sent after the Idempotency-Key expired (Mastodon keeps it for up to one hour) could publish it a second time. Changes: - Renew the claim right before the status request and skip the post if the claim was lost, so an expired or deleted post is never sent - Pass the 30-second request timeout to the image upload and alt text requests, which had no timeout - Retry only 429 responses, where nothing was posted. Mark 5xx responses failed with lastError, like timeouts, so the user can reschedule instead of risking a duplicate - Add regression tests for the expired-and-deleted post, each 5xx status, a retry after the key window, and the 429 retry limit Refs #2373, OneCommunityGlobal/HighestGoodNetworkApp#5593
Follow-up to review on #5593 and the scheduler changes in OneCommunityGlobal/HGNRest#2373. - Editing a scheduled post saves the new version, then deletes the old one. If the delete failed, both stayed scheduled while the UI said "Failed to schedule post." Report that case on its own: the changes were saved and the older version must be deleted - Show an error on the Scheduled tab when the list fails to load, instead of "No scheduled posts yet" - The backend now marks a post failed when delivery can't be confirmed and does not retry it. Show failed posts with their error so the user can reschedule, and disable actions on a post that is being published - Remove the console.error calls flagged by the lint check - Add tests for the edit cases, the load error, and failed and publishing posts Refs #5593, OneCommunityGlobal/HGNRest#2373
|
|
Pushed to address the review feedback:
17/17 tests pass in the Mastodon composer suites. The new tests mock the API calls. |
Niket07pathak
left a comment
Niket07pathak
left a comment
There was a problem hiding this comment.
Reviewed by Niket Pathak
How I verified
Checked out frontend PR #5593 and tested the Mastodon composer locally with the required frontend/backend setup. Tested the functionality using Admin/Owner and Volunteer roles, verified post creation and scheduling, date/time selection, scheduled post editing, validation scenarios, and UI behavior in light and dark modes.
Verified working
- Composer, Scheduled, History, and Details tabs are properly styled, separated, and functional.
- Calendar and time pickers work correctly and retain selected values.
- Light and dark modes display correctly across all tabs, input fields, calendar/clock popups, and Confirmation Preferences.
- Future posts can be scheduled successfully, appear in the Scheduled tab, and publish at the scheduled time.
- Editing an evening scheduled post preserves the correct local date and time, and changes persist.
- Scheduling a post in the past displays the appropriate backend validation message.
- Missing date/time inputs are handled with proper validation without crashes.
- Failed edits do not delete or overwrite the original scheduled post.
- Volunteer users cannot access the restricted Send Emails page or Mastodon routes, including through direct URLs.
Issues found
No issues were observed during the completed tests.
Evidence
Required action
No changes requested based on the completed tests.
Approved
adit24dhaya
left a comment
adit24dhaya
left a comment
There was a problem hiding this comment.
Thanks, Shubham. I rechecked a9ac10a7 with backend #2373 at 86f04fc5. The scheduler blockers from my earlier review are fixed.
The frontend now gives a clear warning when the new version saves but the original cannot be deleted, and keeps the original when saving fails. Scheduled-list failures show an error, failed posts show the delivery message, and publishing posts have their actions disabled. Evening edits keep the same local date and time.
Validation: 17/17 frontend tests passed, plus 10 actual-component browser scenarios with 78 assertions across light and dark modes. The build, changed-file ESLint, Stylelint and whitespace checks passed. Paired backend: 27 tests and 10 independent scheduler scenarios passed. API calls and persistence were mocked; no real posts or shared database writes were made, and this was not a live authenticated account test.
Approving. The partial-edit warning still requires the user to remove the older version; it does not automatically roll back the edit.
Commit audit: the follow-up is focused, has a 62-character imperative subject without a final period, and explains what/why in a blank-separated body with references to both PRs.
iAbhi001
left a comment
iAbhi001
left a comment
There was a problem hiding this comment.
Rechecked the branch alongside backend PR #2373, and the scheduler blockers from before are resolved.
The frontend handles save failures cleanly now, showing a clear warning if an older version cannot be removed while preserving the original when saves fail. Error messaging on the scheduled list looks good, failed posts display the delivery note properly, publishing states lock action buttons, and evening edits keep the right local timestamps.
All frontend and backend unit tests pass, browser checks hold up across light and dark themes, and linting passes with zero issues. Tested everything with mocked APIs so dev data stayed untouched.
Quick note that the partial edit alert prompts the user to delete the previous version manually rather than auto reverting, but the flow itself is solid. Approving! Thanks, Shubham.
AaditTrivedi
left a comment
AaditTrivedi
left a comment
There was a problem hiding this comment.
Re-reviewed by Aadit Trivedi.
The Oct 8 commit "Report partial edits and failed scheduled posts" addresses two pointers from my earlier review:
- When loading scheduled posts fails, the Scheduled tab now shows an error (
scheduledLoadError) instead of an empty list that could be mistaken for having no scheduled posts. - Edits now report failures, including the partial case where the new version saves but the old one isn't removed.
Ran npx vitest run src/components/Announcements/platforms/social: 17/17 passing, including the new tests for these cases. Everything else from my earlier approval still holds.
Approving.
|
Thank you all, merging! |
8 participants
Description
Bug list: "Phase 1: Finishing Followup fixes for the PR 4558+1953 Fix Dark Mode, Date Selection, Time Selection and Scheduling the post for Mastodon" (priority high, Yeshwanth).
Issues found when testing the Mastodon composer (Other Links → Send Emails → Mastodon):
Causes on the frontend:
fetch('/api/mastodon/...')on the frontend's own address, so requests never reached the backend (locally the Vite proxy also strips/api; on Netlify the request returnsindex.html).SocialMediaComposer.module.cssbut used plain class names, so none of its styles were applied: the sub-tabs ran together as "ComposerScheduledHistoryDetails", the inputs were unstyled, and the existing dark styles never applied. They also only followed the operating system's dark mode, not the app's.Related PRS (if any):
This frontend PR is related to backend PR OneCommunityGlobal/HGNRest#2373.
To test this frontend PR you need to check out backend PR OneCommunityGlobal/HGNRest#2373.
Builds on #4558 and OneCommunityGlobal/HGNRest#1953.
Main changes explained:
src/utils/URL.js: addMASTODON_POST,MASTODON_SCHEDULED_POSTS,MASTODON_SCHEDULED_POST_BY_ID, andMASTODON_POST_HISTORY, following the LinkedIn entries.SocialMediaComposer.jsx:fetch('/api/mastodon/...')calls withaxiosand the new endpoints, which sends the logged-in user's token.styles['post-card'], etc.). Bootstrap button classes are unchanged.state.theme.darkMode) and add acomposer-darkclass.SocialMediaComposer.module.css:prefers-color-scheme: darkblock to.composer-darkrules so dark mode follows the app's setting.color-scheme: darkon the date and time inputs so the calendar and clock popups and icons are drawn in dark colors.scheduleTime.js: new helper that reads a scheduled post's date and time in local time for editing.How to test:
.envsetup).npm install, thennpm run start:local.Screenshots or videos of changes: