Improve split functions #44
Conversation
|
Thanks to @simon1222 for helping with this. He wrote a function for moving posts that works even with discussions that have a large number of posts (even 120k) |
|
one quick question, if a discussion has only one post and we move it into a different discussion will the old discussion will be soft-deleted or the old post stays no posts? |
There was a problem hiding this comment.
I had a thought a while back to drop fof/split and fof/merge-discussions, and create a new fof/split-and-merge extension in their place. Seeing this PR has reignited that thought process.
Anyhow, the biggest issue with this PR is the raw sql use - this has to be changed
Yes, there is some duplication of code on the frontend and it would be worth merging them. Tonight or tomorrow I'll try to find time to fix the SQL Injection problem. |
Co-authored-by: Ian Morland <ian@morland.me>
Co-authored-by: Ian Morland <ian@morland.me>
Co-Authored-By: simon1222 <67348152+simon1222@users.noreply.github.com>
| // Persist the new statistics. | ||
| $discussion->save(); | ||
|
|
||
| return Discussion::find($discussion->id); |
There was a problem hiding this comment.
Why do you need to find the discussion again?? The methods called earlier modify the original $discussion object, so theoretically both will equal the same thing.
If it is to update a relation, you can unset it by using $model->unsetRelation(<name>).
| ]; | ||
| } | ||
|
|
||
| disabled = () => { |
There was a problem hiding this comment.
Both disabled and select should be methods, not class properties.
| $db->statement('SET @maxNumber = (SELECT MAX(number) FROM posts WHERE discussion_id = ?);', [$discussion->id]); | ||
| $db->statement('SET @rank = @maxNumber;'); | ||
| $db->statement('UPDATE posts SET number=@rank:=@rank+1 WHERE discussion_id = ? ORDER BY created_at;', [$discussion->id]); | ||
| $db->statement('UPDATE posts SET number=number-@maxNumber WHERE discussion_id = ?;', [$discussion->id]); |
There was a problem hiding this comment.
This raw SQL will not work with table prefixes. I'm also not good at SQL so I'm not going to comment on the statements themselves, performance, etc.
|
Due to lack of time I unfortunately cannot continue to develop this PR further. I already maintain my own version of this extension on my forum anyway. If anyone has the time and willingness then please help to complete this PR. |
|
The Move Posts extension was released, so there is no point in continuing this PR |
Changes proposed in this pull request:
This PR adds the ability to split posts to an existing discussion instead of creating a new one.
Additionally, this PR adds:
.prettierrcScreenshot
Confirmed