WIP: Inline Moderation #120

wpillar · 2015-04-27T14:06:01Z

This PR contains the WIP of inline moderation for posts and topics. See the issue here #45 for the tools that it has implemented.

Would like some QA and feedback whilst I tidy things up, then I'm going to open a PR with a view to being merged. Once this work is merged I will crack on with the rest of the items in the above issue.

JN-Jones · 2015-04-28T13:16:50Z

app/Database/Repositories/Decorators/Forum/CachingDecorator.php

+	 */
+	public function moveTopicToForum(Topic $topic, Forum $forum)
+	{
+		return $this->decoratedRepository->moveTopicToForum($topic, $forum);


This needs to update the cache (simply call forget for now), otherwise the other functions will return invalid data.

Ok cool, thanks.

JN-Jones · 2015-04-28T14:25:58Z

Some other things I've noticed:

The checkbox column should be smaller
"Clear selection" doesn't change the checkbox status
Close/Open and Unapprove/Approve should only be shown if possible (eg hide "Open" if only open topics are selected)
The forum dropdown when moving should get some indention (like in 1.x)
Unapproved posts/topics should be ignored in the last discussions table
Restoring posts/topics should be added
When you unapprove a topic the firstpost is also unapproved but if you approve the firstpost the topic isn't approved which feels inconsistent
closing a topic should hide the quickreply and throw an error on the full reply page as long as the user doesn't have permission to reply to closed topics (which should at least show a warning)
I can't click on the moderations bar as long as it's over the quickreply textarea
The Merge link doesn't seem to work
The move modal should also accept a link and get the id out of it (there are laravel classes to do so, similar to the wio page does)
If you select one topic, click on "clear selection" and then select another topic the moderation bar doesn't pop up
The selected topics/posts should be saved in a cookie eg to merge two posts from different topics or to merge two posts on two pages

That's what I found while playing with the code a bit. Sorry if you're already aware of some of them ;)

wpillar · 2015-04-28T15:19:27Z

@JN-Jones Thanks man!

I'm not really worrying about the front-end tbh, it's not my area of expertise so I'm kinda making it work and not look horrendous on the basis that we will create separate issues to improve the front-end. What do you think?

Close/Open and Unapprove/Approve should only be shown if possible (eg hide "Open" if only open topics are selected)

Yup, I've thought about this but I don't think that blocks this PR, as above I think creating a new issue to improve the front-end is the best course of action once this is merged.

Unapproved posts/topics should be ignored in the last discussions table

Sure, don't think that is an issue for this PR though.

Restoring posts/topics should be added

Yup, will add to #45 for future PRs.

When you unapprove a topic the firstpost is also unapproved but if you approve the firstpost the topic isn't approved which feels inconsistent

Thanks, will fix that.

closing a topic should hide the quickreply and throw an error on the full reply page as long as the user doesn't have permission to reply to closed topics (which should at least show a warning)

Happy to this in this PR. Can the permissions system do this right now? Or will it have to wait?

The Merge link doesn't seem to work

What are the steps to reproduce?

The move modal should also accept a link and get the id out of it (there are laravel classes to do so, similar to the wio page does)

Actually I think we can be smarter and provide an autocomplete type-ahead feature for this. For now though I'd like to merge as is and tackle this in a separate issue / PR if that's ok?

If you select one topic, click on "clear selection" and then select another topic the moderation bar doesn't pop up

I'll have a look at the clear selection link in this PR, thanks.

The selected topics/posts should be saved in a cookie eg to merge two posts from different topics or to merge two posts on two pages

Agreed, would also like to tackle this in a separate PR.

The aim is to provide basic working inline moderation that we can iterate on with future PRs and issues. Keen to avoid a long-running branch that falls foul of conflicts over time.

@JN-Jones let me know what your thoughts are on what I've said, then I can put together a list of things to address in this PR and what we're happy to leave for future PRs. I will happily create the issues for future work so we do not forget the good ideas in this PR.

JN-Jones · 2015-04-28T15:26:14Z

I'm fine with seperate issues/PRs, did the same already with my PRs.

About styles: Feel free to add them on trello: https://trello.com/b/NFeiW7em/mybb-2-0-styles

Can the permissions system do this right now?

Yes. The permissions system has three types of permissions: yes/no where yes overrides no and never which is never overwritten. Simply add a new permission related to forums, 0 as default value and then add the admin role with value 1.

The Merge link doesn't seem to work

What are the steps to reproduce?

I've selected one post, selected the second, clicked on the link and nothing happened.

wpillar · 2015-04-28T16:11:30Z

Thanks buddy, I'll add a to-do list to the PR tonight and work through it this week.

JN-Jones · 2015-05-08T10:42:45Z

About the merge link:

http://localhost/moderate Failed to load resource: the server responded with a status of 500 (Internal Server Error)

(Un)approving any post will also change the status of the topic.

JN-Jones · 2015-05-08T10:44:16Z

app/Http/Controllers/TopicController.php

@@ -209,6 +210,10 @@ public function reply($slug, $id, Request $request, $postId = null)
 			throw new TopicNotFoundException;
 		}

+		if ($topic->closed) {
+			throw new \Exception("This topic has been closed");


This should be translatable and a real exception class instead of throwing a basic exception

JN-Jones · 2015-05-10T08:49:56Z

The file headers need to be updated now ;)

…ersa

wpillar · 2015-05-10T13:36:36Z

Closing for refurbishment.

wpillar added t:feature s:in-progress labels Apr 27, 2015

JN-Jones reviewed Apr 28, 2015
View reviewed changes

JN-Jones reviewed May 8, 2015
View reviewed changes

wpillar added 12 commits May 10, 2015 14:18

Basic moderation architecture

3152bf1

Tidying up how inline moderation bar is rendered

0ba1740

Frontend handling for inline moderation

817031f

Making ModerationController more generic

3377358

Add ModerationRequest to handle lots of stuff for us

86dd8a1

MovePost moderation and support for moderation options and forms

3f6e73f

Add ModerationInterface::visible()

03bf647

Adding MergePosts moderation

c419ead

Make array moderations only appear when more than one post is selected

718fb8d

Delete post moderation

54d5a55

Adding approval of topic from forum show view

7bdd0ef

When approving a topic, do the same to the first post too

1fbfa32

wpillar added 18 commits May 10, 2015 14:18

Dynamic name of what is being selected

f676994

DeleteTopic moderation

d85f7c9

Make sure only checkboxes in topic-lists are counted

d150f4a

Support for multiple fields to be specified by a moderation presenter

655c8d5

Adding close/open moderation to topics

a87af60

MoveTopic moderation

a6707d2

Fixing a few issues and compiling the front-end

b714ab7

Default posts and topics to being approved:

09f7c4d

Forget the caches when moving a topic to a forum

2c6423e

Update the last post of a forum when moving a topic to a forum

16b6026

Add new lines around the horizontal rule separating merged posts

a877c66

Inject a forum repository rather that invoking app()

54aea10

Ensure first post approval/unapproval applies to the topic and vice v…

5388323

…ersa

Fixing the clear selection link in the moderation bar

1101f68

Don't allow replies to a closed topic

d0ee30f

Adding mybb/standards as a dev dependency

2b272cf

Fixing standards issues

0a09a04

Adding composer commands for the sniffer and sniff fixer

1c32f7f

wpillar force-pushed the moderation branch from 791c59e to 1c32f7f Compare May 10, 2015 13:19

wpillar added 5 commits May 10, 2015 14:22

Adding Licence headers

b44309a

Updating mybb/standards dependency

e83893f

Fixing some standards issues and the sniffer

cda476b

Add default attribute to closed column on the forums and topics tables:

94a1840

Removing phpcs and standards install from cicle config

7d9c386

wpillar closed this May 10, 2015

JN-Jones mentioned this pull request Aug 25, 2015

Cannot use MyBB\Core\Database\Models\Topic as Topic because the name is already in use #173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Inline Moderation #120

WIP: Inline Moderation #120

wpillar commented Apr 27, 2015

JN-Jones Apr 28, 2015

wpillar Apr 28, 2015

JN-Jones commented Apr 28, 2015

wpillar commented Apr 28, 2015

JN-Jones commented Apr 28, 2015

wpillar commented Apr 28, 2015

JN-Jones commented May 8, 2015

JN-Jones May 8, 2015

JN-Jones commented May 10, 2015

wpillar commented May 10, 2015

WIP: Inline Moderation #120

WIP: Inline Moderation #120

Conversation

wpillar commented Apr 27, 2015

JN-Jones Apr 28, 2015

Choose a reason for hiding this comment

wpillar Apr 28, 2015

Choose a reason for hiding this comment

JN-Jones commented Apr 28, 2015

wpillar commented Apr 28, 2015

JN-Jones commented Apr 28, 2015

wpillar commented Apr 28, 2015

JN-Jones commented May 8, 2015

JN-Jones May 8, 2015

Choose a reason for hiding this comment

JN-Jones commented May 10, 2015

wpillar commented May 10, 2015