Skip to:
Content

Opened 6 years ago

Last modified 2 years ago

#1989 new enhancement

Only show a single revision message

Reported by: alexvorn2 Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.1.2
Component: Component - Any/All Keywords:
Cc: stephen@…, jared@…, wordpress@…

Description

http://wpimpact.com/wp-content/uploads/2012/10/Untitled-3.png

  • If a user edits a topic or a reply multiple times than log info is added. I think it would be nice to show ONLY the last one.

Attachments (4)

revision-log.patch (1.4 KB) - added by MZAWeb 6 years ago.
1989.patch (17.8 KB) - added by MZAWeb 6 years ago.
1989-no-whitespace.patch (5.2 KB) - added by MZAWeb 6 years ago.
1989.2.patch (5.7 KB) - added by johnjamesjacoby 6 years ago.
Fix indentation from 1989-no-whitespace.patch

Download all attachments as: .zip

Change History (25)

#1 @johnjamesjacoby
6 years ago

  • Component changed from General to Back-end
  • Milestone changed from Awaiting Review to Future Release

Agree it would be nice to be able to edit this meta directly; moving to future release.

#2 @netweb
6 years ago

  • Cc stephen@… added

#3 @johnjamesjacoby
6 years ago

  • Milestone changed from Future Release to 2.4

#4 @jaredatch
6 years ago

  • Cc jared@… added

jjj - what are you thinking? Possibly a filter that when set to try only shows the most recent one?

@MZAWeb
6 years ago

#5 @MZAWeb
6 years ago

  • Cc wordpress@… added
  • Keywords has-patch added

Added patch. Introduced 'bbp_reply_revision_log_limit' filter.

#6 @johnjamesjacoby
6 years ago

  • Milestone changed from 2.4 to 2.5

Patch works, but hoping for something a bit more sexy (maybe a cumulative count of total edits?) Moving to 2.5.

#7 @MZAWEb
6 years ago

Working on a patch already.

Show 3 + total count + ajax loader to see all. Will try to post this week.

@MZAWeb
6 years ago

#8 @MZAWeb
6 years ago

Can't believe how lazy I was with the other patch.

1989.patch does it better, both for topics and replies. Show 1 revision (with a filter to modify) and a link "Show %d more" that will expand. Didn't implement the AJAX loading yet, so for now it reloads the page (and will work on themes without jQuery). Want to validate the approach, get it fixed (or rejected) first, then moving it to AJAX is super easy.

Because there's a way simpler approach, but it's jQuery dependent: just echo everything in a hided div and collapse/expand with jQuery.

#9 follow-up: @MZAWeb
6 years ago

Same patch, without whitespace diff :)
Note that in latest trunk there are a few wrongly indented functions in those files.

@johnjamesjacoby
6 years ago

Fix indentation from 1989-no-whitespace.patch

#10 in reply to: ↑ 9 @johnjamesjacoby
6 years ago

  • Keywords early added

Replying to MZAWeb:

Same patch, without whitespace diff :)
Note that in latest trunk there are a few wrongly indented functions in those files.

Thanks for this. This is a pretty good iteration. Let's try to work on this early for 2.5.

#11 follow-up: @johnjamesjacoby
6 years ago

Some ideas while looking at this code.

  • The markup could be separated out into a template part used by both topics and replies.
  • Always hate adding new parameters, since it means committing to those changes forever.
  • Since this is a developer option (without a ui for tweaking the limits) we have an opportunity to really improve this significantly.
  • The $total = $limit = - 1; bit is happening before $limit is set. This should probably be somewhere else?

#12 in reply to: ↑ 11 @MZAWeb
6 years ago

Replying to johnjamesjacoby:

  • The markup could be separated out into a template part used by both topics and replies.

Great idea. It's was bothering me a bit that much duplicated code. Will do.

  • Always hate adding new parameters, since it means committing to those changes forever.

If you mean the the params I added to bbp_get_*_revisions, the only other option is to grab always the full results and chop off the array, which seem inefficient.

If you mean the $_GET params, I'm not sure of any other approach other than going only the AJAX way.

For any of those, I'd love to hear suggestions. Maybe I'm just missing something obvious.

  • Since this is a developer option (without a ui for tweaking the limits) we have an opportunity to really improve this significantly.

Sorry, don't follow. What do you want me to improve?

  • The $total = $limit = - 1; bit is happening before $limit is set. This should probably be somewhere else?

The filter is inside the IF. If the user clicked to expand and we have the $_GET param, we don't want to run the filter, we want to do -1 to grab all, right? Or do you want me to filter the -1 too? Seems weird...

#13 follow-up: @johnjamesjacoby
5 years ago

  • Milestone changed from 2.5 to 2.6

There are variables in your patch that are being referenced that have not been defined, triggering PHP notices. I'm uncertain where that data is suppose to come from, so I can't call this a functioning or complete patch to give accurate feedback.

Do you mind taking a look at mine, and improving on it? Moving to 2.6 so we can take our time with this one.

#14 @johnjamesjacoby
5 years ago

  • Milestone changed from 2.6 to 2.7

Bump to 2.7.

#15 in reply to: ↑ 13 @MZAWeb
5 years ago

Replying to johnjamesjacoby:

There are variables in your patch that are being referenced that have not been defined, triggering PHP notices. I'm uncertain where that data is suppose to come from, so I can't call this a functioning or complete patch to give accurate feedback.

Uhm weird. Maybe I need to refresh it to the latest trunk.

Do you mind taking a look at mine, and improving on it? Moving to 2.6 so we can take our time with this one.

Sure. Will do that asap.

#16 @MZAWeb
5 years ago

@johnjamesjacoby:

I just applied your patch to the latest trunk and the feature works as expected, I don't see any PHP notice in browser or logs and my IDE is not showing me any unreferenced variable. Can you give me a hint on how to reproduce?

Edit: Weee

Edit: Weee 2

Last edited 5 years ago by MZAWeb (previous) (diff)

#17 @johnjamesjacoby
4 years ago

  • Milestone changed from 2.7 to 2.8

Bumping all 2.7 to 2.8 milestone.

This ticket was mentioned in Slack in #bbpress by netweb. View the logs.


4 years ago

#19 @netweb
4 years ago

  • Component changed from Back-end to Component - Any/All
  • Keywords has-patch early removed
  • Summary changed from show only one modification message to Only show a single revision messagae

#20 @Clorith
2 years ago

Using the beast that is the w.org forums as my basis here (https://wordpress.org/support/topic/forum-bugs-and-broken-things/ the edit list is... fun)

Although comment:16 is a nice approach if all you have are edits to the post content, this does not account for other scenarios such as edit logs of tag modifications, topic movement between forums and such.

I'm far more in favor of the approach from comment:8 where the full list can be expanded with a click and only seeing X ones by default.

Maybe making the modified text a link to see the diff as suggested in comment:16, but that is if the edit is a text one and not a meta one.

#21 @netweb
2 years ago

  • Summary changed from Only show a single revision messagae to Only show a single revision message

Related: #meta2232

Note: See TracTickets for help on using tickets.