Opened 11 years ago
Closed 10 years ago
#2528 closed defect (bug) (fixed)
Topic Merge Freshness
Reported by: | netweb | Owned by: | netweb |
---|---|---|---|
Milestone: | 2.6 | Priority: | normal |
Severity: | normal | Version: | 2.5 |
Component: | Component - Topics | Keywords: | needs-patch needs-unit-tests |
Cc: |
Description
Merge Topic Fresness
- Merge 'TopicB 2:00pm' topic into 'TopicA 1:00pm'
- 'TopicA' freshness is now 2:00pm
- Merge 'TopicC 3:00pm' topic into 'TopicA 1:00pm'
- 'TopicA 1:00pm' topic freshness remains at 2:00pm
Related: #2329
See: bbp_merge_topic_handler
and bbp_merge_topic_count
Attachments (2)
Change History (11)
#2
@
10 years ago
- Keywords needs-patch needs-unit-tests added
- Milestone changed from Future Release to Awaiting Review
Merging two topics:
- 1st topic 10am (post_date is 10am
2014-05-30 10:00:00
) - 2nd topic 11am (post_date is 11am
2014-05-30 11:00:00
)
If we merge the 11am topic into the 10am topic then $destination_topic
of 10am is older than the $source_topic
of 11am so the $destination_topic
date should be updated to 2014-05-30 10:59:59
Expected: Older topic (10am) post_date should now be updated to 10:59:59
Actual: No change to either topics post_date
If you merge the 10am topic into the 11am topic then the 11am topic post_date is updated to 2014-05-30 09:59:59
, so we are doing it backwards to what we actually want.
Relevant function bbp_merge_topic_handler
<?php // Check if the destination topic is older than the source topic if ( strtotime( $source_topic->post_date ) < strtotime( $destination_topic->post_date ) ) { // Set destination topic post_date to 1 second before source topic $destination_post_date = date( 'Y-m-d H:i:s', strtotime( $source_topic->post_date ) - 1 ); // Update destination topic wp_update_post( array( 'ID' => $destination_topic_id, 'post_date' => $destination_post_date, 'post_date_gmt' => get_gmt_from_date( $destination_post_date ) ) ); }
Currently we are actually "Checking if the source topic less than than the destination topic"
<?php // Check if the destination topic is older than the source topic if ( strtotime( $source_topic->post_date ) < strtotime( $destination_topic->post_date ) ) {
It should be "Checking if the source topic is greater than than the destination topic"
<?php // Check if the destination topic is older than the source topic if ( strtotime( $source_topic->post_date ) > strtotime( $destination_topic->post_date ) ) {
Or maybe a clearer way to write the function based on the comment verbiage:
<?php // Check if the destination topic is older than the source topic if ( strtotime( $destination_topic->post_date ) < strtotime( $source_topic->post_date ) ) {
Now in theory if we merge two topics, the 1:20pm into 1:10pm per:
- #25596 May 30, 2014 at 1:10 pm Topic One Topic 1:10pm
- #25597 May 30, 2014 at 1:30 pm Topic One Reply 1:30pm
- #25598 May 30, 2014 at 1:50 pm Topic One Reply 1:50pm
- #25599 May 30, 2014 at 1:20 pm Topic Two Topic 1:20pm
- #25600 May 30, 2014 at 1:40 pm Topic Two Reply 1:40pm
- #25601 May 30, 2014 at 2:00 pm Topic Two Topic 2:00pm
Expected Results:
- #25596 May 30, 2014 at 1:19 pm Topic One Topic 1:10pm
- #25599 May 30, 2014 at 1:20 pm Topic Two Topic 1:20pm
- #25597 May 30, 2014 at 1:30 pm Topic One Reply 1:30pm
- #25600 May 30, 2014 at 1:40 pm Topic Two Reply 1:40pm
- #25598 May 30, 2014 at 1:50 pm Topic One Reply 1:50pm
- #25601 May 30, 2014 at 2:00 pm Topic Two Topic 2:00pm
Actual results:
- #25596 May 30, 2014 at 1:19 pm Topic One Topic 1:10pm
- #25597 May 30, 2014 at 1:30 pm Topic One Reply 1:30pm
- #25598 May 30, 2014 at 1:50 pm Topic One Reply 1:50pm
- #25599 May 30, 2014 at 1:20 pm Topic Two Topic 1:20pm
- #25600 May 30, 2014 at 1:40 pm Topic Two Reply 1:40pm
- #25601 May 30, 2014 at 2:00 pm Topic Two Topic 2:00pm
It looks like the replies from the source topic are not getting updated menu_order
ID | post_date | post_content | post_name | post_type | post_parent | menu_order | |
25596 | 2014-05-30 13:19:59 | Topic One Topic 1:10pm | topic-one-topic-110pm | topic | 25451 | 0 | |
25597 | 2014-05-30 13:30:00 | Topic One Reply 1:30pm | 25597 | reply | 25596 | 1 | |
25598 | 2014-05-30 13:50:00 | Topic One Reply 1:50pm | 25598 | reply | 25596 | 2 | |
25599 | 2014-05-30 13:20:00 | Topic Two Topic 1:20pm | reply-to-topic-one-topic-110pm | reply | 25596 | 3 | |
25600 | 2014-05-30 13:40:00 | Topic Two Reply 1:40pm | reply-to-topic-one-topic-110pm-2 | reply | 25596 | 1 | <- Source Reply |
25601 | 2014-05-30 14:00:00 | Topic Two Topic 2:00pm | reply-to-topic-one-topic-110pm-3 | reply | 25596 | 2 | <- Source Reply |
#5
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
This is still not quite right:
- Merged replies and source topic topic are assigned a post title
- Merged replies and source topic topic are assigned a post name
- Reply
menu_order
is assigned the old source topicmenu_order
rather than destinationmenu_order
- (i.e. Don't update
menu_order
until after thepost_parent
has been updated)
- (i.e. Don't update
Attached patch 2528.1.diff addresses the above and also removes source topic's last and count meta data.
The above patch only fully works alongside patch 2623.1.diff in #2623, specifics details are in that ticket, without 2623.1.diff reply_to
values are created of themselves in post meta for merged replies.
Related: #2623
Also because our source topic post_name
after merging get's an updated post meta key _wp_old_slug
that is a WordPress 301 redirect using the old slug to the new slug this should allow us to forward the URL of reply notifications from the old topic per #2329 if #2424 is implemented.
Similar issues to the above are also needed for the 'Split Topic' function, see #2624
#6
@
10 years ago
In 2528.2.diff refreshes 2528.1.diff with the following additions:
- Reverts r5404
strtotime
- Antiprops netweb. (I blame the epoch's, long detailed description available if req.) - Removes the
bbp_update_reply_to()
"adjust reply to values" reply to updates, a threaded reply is always a reply to a reply, if we move a reply as part of a merge any reply to another reply will still be a reply to the same reply ID.
Patch 2528.2.diff fully tested with 'non-threaded' and "thread replies" -> https://cloudup.com/cvnzD1f85HG
#7
@
10 years ago
In 2528.2.diff why set the post_name
to $reply->ID
? We seem to set it to false everywhere else, and let wp_update_post()
take care of the slug for us.
Not a deal breaker, moving to future release pending a patch, then can be moved into an active milestone.
Workaround: Use the repair tools to recalculate freshness.