Skip to content
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

Add Publish Date button to Post Settings #23333

Closed
wants to merge 5 commits into from

Conversation

kean
Copy link
Contributor

@kean kean commented Jun 7, 2024

Fixes #23326.

I tried to re-implement it on top of the new infrastructure and avoid the old issues. 99% of users will just go "Post Settings" → Set "Publish Date" → "Schedule" → "Schedule". Other scenarios – and it opens the door for dozens of these – are largely irrelevant.

For reference, here are the related previous issues (some might by misclassified): #12121, #13724, #14251, #18517, #17086, #15767, #16514, #13654.

Note

This fix targets 25.1.

To test:

Scenario 1

Scheduling a draft post using Post Settings.

  • Create a new draft
  • Open "Post Settings"
  • ✅ Verify that "Publish Date" has placeholder "Immediately"
  • Set "Publish Date" to the date in the future
  • Close "Post Settings"
  • ✅ Verify that the "Publish" button changes to "Schedule"
  • Tap "Schedule"
  • ✅ Verify that the "Publish Date" field displays the selected publish date and the primary button also says "Schedule"
  • Tap "Schedule"
  • ✅ Verify that the post got scheduled

Scenario 2

  • Create a new draft
  • Save the draft
  • Open the same draft on wp.com
  • Set "Publish Date" to the date in the future and save it
  • Return to the app and refresh the Post List
  • Open the same draft again
  • ✅ Verify that the "Publish" button changes to "Schedule"

Scenario 3

  • Create a new draft
  • Open "Post Settings"
  • Set "Publish Date" to the date in the future
  • Save draft
  • Open it again
  • ✅ Verify that the "Publish" button say "Schedule"
  • Tap "Schedule"
  • Open "Publish Date" and remove it
  • ✅ Verify that the value changes to "Immediately"
  • Tap "Shedule" to confirm publishing
  • ✅ Verify that the post actually got scheduled

Regression Notes

  1. Potential unintended areas of impact

  2. What I did to test those areas of impact (or what existing automated tests I relied on)

  3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@kean kean added this to the 25.1 milestone Jun 7, 2024
@kean kean requested a review from staskus June 7, 2024 20:57
// the app has to send a new value to override it.
parameters.date = post.shouldPublishImmediately() ? nil : Date()
}
parameters.date = options.publishDate ?? .now
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The safest option is to just to set to .now every time unless it's explicitly provided.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 7, 2024

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr23333-14e67ee
Version25.0
Bundle IDorg.wordpress.alpha
Commit14e67ee
App Center BuildWPiOS - One-Offs #10131
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jun 7, 2024

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr23333-14e67ee
Version25.0
Bundle IDcom.jetpack.alpha
Commit14e67ee
App Center Buildjetpack-installable-builds #9179
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Copy link
Contributor

@staskus staskus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scenario 3 doesn't work properly. Although after setting a post draft date it correctly turns into a "Schedule" state, the publish date is shown "Immediately". It only happens after initially saving the draft. The date is correctly shown after setting it in post settings after initially saving the draft.

Scenario.3.broken.mp4

@kean
Copy link
Contributor Author

kean commented Jun 10, 2024

Scenario 3 doesn't work properly.

I'm not sure it's feasible to make it work correctly in all scenarios considering how the API works.

The root cause is that if you pass date in the initial request to create a draft, the server thinks it's a "Created" date and not a "Publish" date and sets both date and modified to this value (if you want to know why, read the comment to shouldPublishImmediately) :

  "date": "2024-06-18T20:19:00+09:00",
  "modified": "2024-06-18T20:19:00+09:00",

There is another related issue: the "edited" date in the Post List is incorrect for these posts.

I added a workaround where the first time you save a draft, it doesn't send date to the server, but it should be addressed with a new API. The .org REST API doesn't have some of these problems, so maybe we could fix it in the future. Alternatively, I could add a more elaborate workaround where the app would skip sending date when creating the original draft and instead create a separate revision/diff to send it in a follow-up update, but that just sounds too flaky and complicated.

This is one of the many reasons we removed this field...

@kean kean requested a review from staskus June 10, 2024 11:55
@kean kean force-pushed the task/add-publish-date-back-to-settings branch from 709b449 to 14e67ee Compare June 10, 2024 11:57
@staskus
Copy link
Contributor

staskus commented Jun 10, 2024

Thanks for providing context, @kean. 🤔

The root cause is that if you pass date in the initial request to create a draft, the server thinks it's a "Created" date and not a "Publish" date and sets both date and modified to this value

Okay... It explains the weird behavior I was seeing. The date picker shows "Immediately" but once I opened it, the date was set to the future date I set before saving the draft.

I added a workaround where the first time you save a draft, it doesn't send date to the server, but it should be addressed with a new API. The .org REST API doesn't have some of these problems, so maybe we could fix it in the future.

As I understand, this workaround is not expected to work with WP.com API for now? And how does WP.com solve this problem? I cannot reproduce the same issue. Setting the publish date and saving the draft works well.

This is one of the many reasons we removed this field...

Yes, I understand now why it was removed. It should be fixed from the backend side.

@kean
Copy link
Contributor Author

kean commented Jun 10, 2024

And how does WP.com solve this problem?

wp.com creates a post on the server the moment you open a new draft (and this is when it sets the initial date aka "Created" data) and they also keep auto-saving drafts automatically as you edit.

@staskus
Copy link
Contributor

staskus commented Jun 10, 2024

Okay, @kean.

So it does sound a bit like another workaround you proposed:

Alternatively, I could add a more elaborate workaround where the app would skip sending date when creating the original draft and instead create a separate revision/diff to send it in a follow-up update, but that just sounds too flaky and complicated.

What I dislike about the current state is that it leaves many contradictions. For example, for a draft that had a publish date set to the next day, we:

  • Show Schedule button
  • After tapping schedule, we show "Immediately"
  • In post settings, we show "Immediately" but after tapping we show tomorrow's date in the date picker
Simulator.Screen.Recording.-.iPhone.15.-.2024-06-10.at.15.47.59.mp4

If we don't support publish dates for initial drafts but want to show the publish date in post settings for immediate scheduling, could we remove the created date when the initial draft is saved? At least then the UI will be consistent and display "Publish" instead of "Schedule". 🤔 I realize that none of these options is ideal.

@dvdchr
Copy link
Contributor

dvdchr commented Jun 10, 2024

Hey folks 👋 , I'm going to run code freeze now. Once it's ready, feel free to update the base branch to release/25.1. For task-keeping purposes, I'll set the milestone to Pending for now.

@dvdchr dvdchr modified the milestones: 25.1, Pending Jun 10, 2024
@kean
Copy link
Contributor Author

kean commented Jun 10, 2024

the publish date in post settings for immediate scheduling, could we remove the created date when the initial draft is saved?

I think that's what I ended up doing If I understood your suggestion correctly.

Once it's ready, feel free to update the base branch to release/25.1

Sure!

@staskus
Copy link
Contributor

staskus commented Jun 10, 2024

I think that's what I ended up doing If I understood your suggestion correctly.

Yes, you did. I didn't realize that there is a different flow triggered when selecting "Save Draft" from the top bar menu. I was testing it and this workaround wasn't working. Let's apply the same workaround here as well, if possible, and move on with this PR.

image

@dvdchr
Copy link
Contributor

dvdchr commented Jun 10, 2024

👋 Code freeze for 25.1 has been completed. You can now target the release/25.1 branch. 🙂

@dvdchr dvdchr modified the milestones: Pending, 25.1 ❄️ Jun 10, 2024
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is assigned to the milestone 25.1 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@kean
Copy link
Contributor Author

kean commented Jun 11, 2024

Tracks show that the "prepublishing flow" works just fine. I'm going to close it for now.

@kean kean closed this Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feedback Collection: Recent Change to Schedule Post Option
5 participants