WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 22 months ago

#32386 assigned defect (bug)

Draft's Last Modified date incorrect if it is Scheduled

Reported by: lastnode Owned by: obenland
Milestone: Future Release Priority: normal
Severity: normal Version: 4.2.2
Component: Date/Time Keywords:
Focuses: ui, administration Cc:
PR Number:

Description

Once a Draft is Scheduled, the date of it its most recent revision is not shown under the Date column at Posts > All Posts.

Instead, the Draft's scheduled publication date is shown in the Date column, alongside the label 'Last Modified'.

Steps to recreate:
1) Create and save a Draft
2) Set a scheduled publication date and time for the Draft
3) Save the Draft
4) Check the 'Date' column at Posts > All Posts

If you schedule a Draft for the future, for example, a Last Modified date from the future is shown under Date.

Attachments (5)

last_modified_date_1.diff (1.6 KB) - added by lastnode 4 years ago.
A patch that uses $post->post_modified for Drafts instead of $post->post_date.
32386.diff (941 bytes) - added by yoavf 4 years ago.
32386.1.diff (1.8 KB) - added by lastnode 4 years ago.
This bug should be fixed in excerpt mode as well now. Also made sure that Drafts are run through 'human_time_diff' as well.
32386.2.diff (1.7 KB) - added by lastnode 4 years ago.
Cleaned up code and fixed trailing spaces. Thank you for the pointers, Yoav!
32386.3.diff (2.5 KB) - added by obenland 4 years ago.

Download all attachments as: .zip

Change History (25)

#1 @valendesigns
4 years ago

  • Component changed from Posts, Post Types to Date/Time
  • Keywords 2nd-opinion added

I feel like I've seen this issue before in Trac, but I can't find the ticket. Anyone else experiencing déjà vu?

Also, this seems like one of those issues that is based on opinion and could go either way. An argument for why it should show the actual last time you made changes and why it should show the future date for a scheduled post both have merit. If I had to venture a guess though, it's likely the way it is by design.

@lastnode
4 years ago

A patch that uses $post->post_modified for Drafts instead of $post->post_date.

#2 @lastnode
4 years ago

Sorry, I think my ticket could have been clearer. Hopefully this screenshot will show you exactly what I mean -

https://cldup.com/OsnDDXAyEi-2000x2000.png

https://cloudup.com/co-gRsi8KsX

I may be way off base here, but a Post being able to have a 'Last Modified' date in the future seems somewhat counter-intuitive to me.

Should we show the scheduled Publish date even though the user hasn't yet hit Schedule?

And even if we do that, should the text there then say 'Last Modified'?

I think my above patch fixes this issue, but any feedback is welcome. :) Thank you!

Last edited 4 years ago by lastnode (previous) (diff)

#3 @lastnode
4 years ago

  • Keywords has-patch added

@yoavf
4 years ago

#4 @yoavf
4 years ago

  • Keywords 2nd-opinion removed

Totally makes sense to me - last modified shouldn't be a future date for drafts. I've just cleaned up the patch as per the php coding standards in 32386.diff

Last edited 4 years ago by yoavf (previous) (diff)

#5 @obenland
4 years ago

  • Milestone changed from Awaiting Review to 4.3
  • Owner set to obenland
  • Status changed from new to assigned

#6 @obenland
4 years ago

  • Keywords needs-patch added; has-patch removed

Looking at the entire code for the date output, we should probably select our times based on the post status.

The provided patch also only fixes it for the list mode, in excerpt mode that code is not even used.

#7 @obenland
4 years ago

Some history: [6856], [9110]

#8 @SergeyBiryukov
4 years ago

While we're at it, let's also replace $t_time, $m_time, etc. with more descriptive names.

@lastnode
4 years ago

This bug should be fixed in excerpt mode as well now. Also made sure that Drafts are run through 'human_time_diff' as well.

#9 @lastnode
4 years ago

Thanks for taking a look at this, Yoav and Konstantin!

Attempted to fix the time in excerpt mode in 32386.2.diff. Not sure if this was the most elegant way to do it, though! :)

Also fixed times for Drafts not being run through human_time_diff().

Tried to make the variable names a little more meaningful as well, but left $h_time and $t_time alone, as I'm not sure if those are used elsewhere and should remain the same.

Last edited 4 years ago by lastnode (previous) (diff)

@lastnode
4 years ago

Cleaned up code and fixed trailing spaces. Thank you for the pointers, Yoav!

@obenland
4 years ago

#10 @obenland
4 years ago

  • Keywords has-patch added; needs-patch removed

32386.3.diff looks worse than it actually is. It just uses modified time functions for drafts, cleans up some logic, and uses saner variable names. With this we also get relative times back for drafts.

This ticket was mentioned in Slack in #design by obenland. View the logs.


4 years ago

#12 @afercia
4 years ago

Please consider with the patch applied the whole idea of "sort by Date column" gets a bit confusing. When you click on the Date column header, you're still getting an order by post_date while the column will show dates that are not ordered, because we're showing different data type in the same column.

https://cldup.com/Ffbq9RfJ1Z.png

#13 follow-up: @SergeyBiryukov
4 years ago

32386.3.diff looks clean. The only thing I noticed is that __( 'Unpublished' ) is no longer passed to the 'post_date_column_time' filter, seems like a potential back compat issue.

Not sure how to address comment:12 yet.

#14 in reply to: ↑ 13 ; follow-up: @obenland
4 years ago

Replying to SergeyBiryukov:

Not sure how to address comment:12 yet.

Me neither. I have a hard time re-creating it.
@afercia, can you go through the steps you took to get there?

#15 in reply to: ↑ 14 @afercia
4 years ago

Replying to obenland:

can you go through the steps you took to get there?

The "Sticky draft" post in the screenshot above is a post that was previously "published" in date 2015/01/17 then I've changed its status in "pending" and saved as pending. It's still ordered by the "2015/01/17" date but the Date column it's showing its last modified date.

This ticket was mentioned in Slack in #design by helen. View the logs.


4 years ago

#17 @helen
4 years ago

I hate potentially rabbitholing tickets by asking us to pull back, but I think this warrants it. There is clearly a bug here, in that this is a subpar and rather inaccurate column of data. What data should this column contain and in what situation?

If we operate off the premise that data displayed in the list table is to help users make decisions about what to do next (is this the draft I'm looking for, do I need to delete this thing that is past its prime), I don't think the date column is doing a great job. I have no idea what happens when you go to publish a draft post with a date you've set in the past. I'd even love to know when a published/scheduled post was last modified. And why does a private post show last modified even though it appears on the front-end with its original post date? Is potentially two dates too much to have in that column?

We can fix the draft modified date itself on its own, as the date sort thing is really already a problem once we're in this situation. I just don't think it's our only problem here.

This ticket was mentioned in Slack in #core by michael.ecklund. View the logs.


4 years ago

#19 @obenland
4 years ago

  • Keywords has-patch removed
  • Milestone changed from 4.3 to Future Release

I don't think we'll come to a conclusion on this in what's left in 4.3.

#20 @swissspidy
22 months ago

#43014 was marked as a duplicate.

Note: See TracTickets for help on using tickets.