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

Not any last poster name in forumdisplay after splitting a/some post(s) #870

Closed
Starpaul20 opened this issue Jul 5, 2014 · 19 comments · Fixed by #873
Closed

Not any last poster name in forumdisplay after splitting a/some post(s) #870

Starpaul20 opened this issue Jul 5, 2014 · 19 comments · Fixed by #873
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:bug Type: Bug. An issue causing error / flaw / malfunction
Milestone

Comments

@Starpaul20
Copy link
Member

If in one thread i use split posts, after check last poster in forumdisplay not apper any user in last post.

I suggested to @jvdabz to create one thread only with this issue... but him not created. http://community.mybb.com/thread-154453-...pid1078247
So i created this thread.
So all credits to @jvdabz.

Original thread: Not any last poster name in forumdisplay after splitting a/some post(s)

@Destroy666x
Copy link
Contributor

There is a similar bug in 1.6 (added label), see my first comment in the merged PR. My and martec's code fixes it.

@Destroy666x Destroy666x added the 1.6 label Jul 9, 2014
@DiogoParrinha
Copy link
Contributor

@Destroy666x I am confused. Where's the PR for 1.6?

@Destroy666x
Copy link
Contributor

@PirataNervo, there is no PR for 1.6, that's why I added a comment and a label.

@DiogoParrinha
Copy link
Contributor

Right, I'll add it then.

@DiogoParrinha
Copy link
Contributor

PR created, should fix it. I've used the same things as the PR for 1.8 except for a line which was already there.

DiogoParrinha pushed a commit that referenced this issue Jul 18, 2014
Fixes #870 Not any last poster name in forumdisplay after splitting a/some post(s) (master)
Destroy666x pushed a commit to Destroy666x/mybb that referenced this issue Jul 26, 2014
Fixes mybb#870 Not any last poster name in forumdisplay after splitting
a/some post(s)
@Stefan-MyBB
Copy link
Contributor

Replaced the fix by c5feb8b which fixes the original code instead of adding new code.
#873 has several issues:

  • end($pids) assumes $pids is sorted by dateline (which is not even guaranteed within the MyBB code)
  • update_thread_data($tid) makes all the other changes useless as it overwrites the lastposter data added to the array.

@PirataNervo I recommend to review the fix for 1.6 as it seams to have the same issues. Actually I wasn't able to reproduce the issue at all.

@Stefan-MyBB Stefan-MyBB reopened this Jul 27, 2014
@martec
Copy link
Contributor

martec commented Jul 27, 2014

@stefan-st
update_thread_data($tid), used only to initial thread and new thread created by splitting posts is not afected by this, without update_thread_data($tid) initial thread not updated (No update of last post timestamp, No update of last author (uid + name))

@Stefan-MyBB
Copy link
Contributor

@martec

(No update of last post timestamp, No update of last author (uid + name))

update_last_post($tid) does that?

@martec
Copy link
Contributor

martec commented Jul 27, 2014

@stefan-st
i not tested with update_last_post($tid) only with update_last_post($post['tid']) that not work...
i will test with update_last_post($tid) later

@Destroy666x
Copy link
Contributor

@stefan-st, your fix works and is better but could you tell me when $pids aren't sorted by dateline ASC? They always seemed to be for me.

@Stefan-MyBB
Copy link
Contributor

@Destroy666x It's up to the database engine as the query in moderation.php doesn't specify any sorting:

$query = $db->simple_select("posts", "pid", "tid='$tid'");

Mostly the database returns the data in chronological order but it's not safe to rely on that. Using the merge system or some database operations may result in a different order.

Not specifying the order has already caused issues in the past: http://dev.mybb.com/issues/2017

@Destroy666x
Copy link
Contributor

Oh ok, thanks for explantation.

EDIT: but why is $pids[0] used for first post then? Doesn't the same apply to this? I'm confused.

@Stefan-MyBB
Copy link
Contributor

@Destroy666x I don't know. :P But it may cause (minor) problems in some rare cases.

@PirataNervo @JordanMussi @Destroy666x Can anybody tell me how to reproduce this issue in 1.6.14? I can't and the fix (#934) makes not much sense to me.

@Destroy666x
Copy link
Contributor

@Stefan-S, why wasn't that fixed too then?

As for 1.6, I'm quite positive it showed me wrong lastposter in new thread when I splitted posts that weren't made by old thread's last poster. But I couldn't reproduce it on http://community.mybb.com now so I may have missed something.

@DiogoParrinha
Copy link
Contributor

@stefan-st I can't remember to be honest..

Stefan-MyBB pushed a commit that referenced this issue Jul 28, 2014
@Stefan-MyBB
Copy link
Contributor

@Destroy666x It wasn't fixed because nobody ever noticed the problem. ;) Feel free to create a ticket for it.

I've reverted #934 now as long as nobody can provide steps to reproduce the issue.

@martec
Copy link
Contributor

martec commented Jul 28, 2014

@stefan-st
i tested now patch of 1.8... all ok...
work very well...
thanks!

@DiogoParrinha
Copy link
Contributor

I can't seem to understand if both PRs were reverted.

@Destroy666x
Copy link
Contributor

They were and a simplier fix was applied by Stefan. Which seems to work fine in 1.6 and 1.8 so this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:bug Type: Bug. An issue causing error / flaw / malfunction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants