-
-
Notifications
You must be signed in to change notification settings - Fork 133
feat: Add support for scheduled posts #2222
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?
Conversation
We do something similar with |
Database Schema Changes - Removed isScheduled boolean field from Item model - Kept only scheduledAt timestamp field - Removed unnecessary indexes for isScheduled - Updated stored functions to use scheduledAt IS NULL instead of isScheduled = false API Changes - Updated scheduledOrMine() function to check scheduledAt IS NULL - Updated scheduled items query to use scheduledAt IS NOT NULL - Updated mutations to only set/unset scheduledAt field - Kept isScheduled as computed GraphQL field that returns !!item.scheduledAt Item Creation Logic - Removed isScheduled variable and logic - Only set scheduledAt field during item creation and updates Worker Functions - Updated to check !item.scheduledAt instead of !item.isScheduled - Only update scheduledAt field when publishing/canceling The refactoring maintains the same functionality while eliminating the redundant boolean field. The isScheduled GraphQL field remains available for frontend convenience as a computed property.
I've been struggling with the time update thing for a few days but I'm getting closer. We can't just do the same as with the invoice paid logic because scheduled posts run in the backend and don't have access to the apollo cache directly. I guess we'll need some kind of cache update system where the frontend polls the backend for these kind of updates. Open for ideas though. |
This makes things cleaner and shows the right “time since” for scheduled posts.
Finally fixed it! Had to change a bit the implementation, it's more clear now and “time since” works as expected. |
The commits can (and probably should) be squashed. |
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.
It can schedule a post, but it doesn't take into account all the side effects that come with posting.
If you mention someone, we store this in the Mention table with the relative itemId. The problem is, the scheduled Item is excluded from queries until it's live, but notifications don’t know about it, try to access that Item, and it crashes the notifications page for the mentioned user.
The same applies for other types of notifications tied to an Item, like user subscriptions, territory subscriptions, and so on.
While scheduledOrMine
can look like an easy way to do scheduled posts, it can become really messy really quickly.
Take another look ^^
isScheduled: async (item, args, { me, models }) => { | ||
return !!item.scheduledAt |
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.
I think you forgot isScheduled
around the code, as you can now just do !!item.scheduledAt
when you need it.
<Link href={`/items/${item.id}`} title={item.scheduledAt || item.invoicePaidAt || item.createdAt} className='text-reset' suppressHydrationWarning> | ||
{timeSince(new Date(item.scheduledAt || item.invoicePaidAt || item.createdAt))} |
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.
A nitpick: I think it would be better UX if the countdown says "in 5m" rather than "5m". Or something else that signals that it's a scheduled, not-yet-live post.
export const hasScheduleMention = (text) => scheduleMentionPattern.test(text ?? '') | ||
|
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.
Did you want to add this to lib/form.js
to toast a successful scheduling?
@@ -733,6 +749,36 @@ export default { | |||
homeMaxBoost: homeAgg._max.boost || 0, | |||
subMaxBoost: subAgg?._max.boost || 0 | |||
} | |||
}, | |||
scheduledItems: async (parent, { cursor, limit = LIMIT }, { me, models }) => { |
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.
I don't think you used this anywhere. It was a good direction though, it would've been nice to access a list of my scheduled posts.
note: I didn't look much into this query
Would appreciate your QA on this @huumn |
Can you fix the conflicts? Did you resolve the issues @Soxasora raised in #2222 (review)? |
Would be great to get as much input as possible before doing another round. Also to know if this is a priority at the moment or not. Maybe also some ideas on how to fix @Soxasora concern about the general approach, since that will require a full rewrite I think it's it's better to discuss it first. |
I'm sorry, I didn't want to come across as cryptic.
It was implied in my review that fully creating and hiding a post until the set date triggers a series of side effects that need to be taken into account, such as mentions. But I didn't say it was a bad concept! The purpose of my review wasn't to tell you what to do, but to make you aware of what could happen (and happened). And I get that you may feel lost. I recommend you look at your options, see how you would implement them, and choose what you think is the one that will cause fewer side effects - and then address them. No need to put the PR ready for review if it's not really finished yet, we'll reply to your questions as soon as we can, even if it's a draft. |
There are many side effects to posts that revolve around
There are probably many others. Is this a priority? No. What are priorities: bug fixes, performance fixes, stuff from ek/sox that they QA to death, features/enhancements from FOSS contributors that are well encapsulated and simple to review and QA. |
I'm willing to work on this whenever it's a priority again since I think the QA might be “expensive”. Until then happy to help with other stuff. |
Description
This PR implements scheduled posts functionality, allowing users to schedule content for future publication using the
@schedule in X time
syntax.Closes #740
Core Features:
@schedule in X time
command parsing (supports seconds, minutes, hours, days, weeks, months, years)scheduledAt
field on Item model with proper indexingscheduledItems
query for users to view their scheduled content (not exposed on the UI)Technical Implementation:
COALESCE("Item"."scheduledAt", "Item"."invoicePaidAt", "Item".created_at)
for timestamp handlingscheduledAt
for published scheduled postsUser Experience:
@schedule in 2 hours
syntax are automatically scheduledScreenshots
Note that anon users can see that this account has 6 total items but only 5 are listed (one scheduled post not shown but counted).
Checklist
Are your changes backwards compatible? Please answer below:
Yes. The implementation is fully backwards compatible:
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
9/10. Comprehensive testing includes:
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
No frontend changes.
Did you introduce any new environment variables? If so, call them out explicitly here:
No new environment variables introduced. The feature uses existing database infrastructure with efficient query-based filtering.
TODO
Figure out a way to refresh the item's "created at" time that's cached by Apollo and does not get updated even after the scheduled publishing happens.✅ RESOLVED: Implemented proper Apollo cache handling and timestamp display logic. Scheduled posts now show their
scheduledAt
timestamp in the UI once published, providing users with the intended publication time rather than the original creation time.