Make interval between posts configurable

[timozander]

Fixes flarum/issue-archive#302

Changes proposed in this pull request:

• Add post_flood_interval setting to manually configure the interval users have to wait before posting again
• Add integration tests for both this feature and Add permission to bypass throttling #1938
• Add Advanced settings page with a Security section to contain this and possibly further settings

Reviewers should focus on:
Use of Mockery: It was already required but hasn't been used anywhere else. I didn't found any guidelines regarding this.

Screenshot

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).

Required changes:

• Related core extension PRs: flarum/lang-english

[askvortsov1]

You're going to want to rebase and adjust the unit tests, we've been moving them away from the old controller test style to more accurately model real interactions with the site

[askvortsov1]

I like the ability to configure flood time! However, I think that some discussion as to the 'Advanced' page is warranted to make sure that the core team wants it. I personally think it's a good idea, but it might also encourage disorganized grouping of settings.

[timozander]

Definitely! I was also a little hesitant with that one. I could imagine to group Permissions and this one into some kind of Security tab

I'll adjust the tests

[timozander]

I rebased and changed the test accordingly

[clarkwinkelmann]

What happened to the diff of this PR ? Seems like the merge/rebase didn't work as expected.

I have trouble decrypting what was actually added by the PR.

Based on the original post, I don't think naming a dashboard tab "advanced" makes sense. It's just like calling it "miscellaneous", it does not indicate any purpose and would just become an excuse to not organize settings properly.

For these kind of settings, I'd recommend making them extendable, and then use another extension (third-party at first) to make them customizable. That way "time between posts" can become a setting inside an extension's settings modal.

[askvortsov1]

Yeah it looks like you might have accidentally merged instead of rebasing.

I agree with making stuff like this extendable, but I'm also not sure that just because it doesn't fit into the admin panes we have not means it should be an extension feature: ie, I don't think that the criteria we use to determine what should and shouldn't be an extension should be how it fits into the admin UI. For this case I think it's fine (the defaults will probably work for most people), but as a general practice I don't think that is how we should approach this issue.

I think that the admin dashboard UX definitely needs to be revisited, but this is probably something that can wait past stable.

[timozander]

Yes, looks like I butchered the branch. I fixed that.

In my opinion, all built-in features should also be configurable using a vanilla Flarum installation. This is where my idea of "advanced" or "expert" settings originate from, suggesting that these settings are not meant for the 90% users but for those who want to excessively customize their installation.

Using an extension to configurate this built-in behaviour seems counter-intuitive for me.

[franzliedke]

Welcome @timozander and thanks for this awesome first contribution!

As you can see, we have a lot of open PRs currently. Our top goal right now is to keep our focus on the stable release, and our planned path to get there.

As this issue (#1302) was originally planned for a later release, it might take a while until we get around to further reviews (and merging it). Now that so much work has already been done, that does not mean we cannot merge a good PR if we find the time, but I wanted to explain what we're trying to keep in mind when choosing what to review next, in order to avoid confusion or disappointment. We are trying hard not to get sidetracked in our focus, and our resources are limited.

Thanks for your understanding - and your contribution! 🤗

[franzliedke]

Because we now auto-format our JS code with Prettier, this branch now has conflicts with master. Sorry about that. 😉

Please take the steps outlined in the forum to resolve the conflicts.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum.
In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

[stale]

We are closing this issue as it seems to have grown stale. If you still encounter this problem with the latest version, feel free to re-open it.

[askvortsov1]

My viewpoint on this is shifting a bit. I think we could benefit from an "advanced settings" system, where various stuff like this could be collected into a searchable page for advanced configuration. For many cases (e.g. trust levels) an extension with a solution-focused admin page would be ideal: I'd like to avoid turning completely into a Discourse-like "all settings are text inputs in a giant list" type of UI, but for some advanced "tweak" settings, a setting with an input is probably preferable to a magic number.

I'm very sorry this got lost for so long. It needs some more discussion, but I think this might make sense to have.

[askvortsov1]

Despite the above comment, I'm going to close the above PR as part of our ongoing cleanup to reduce outdated issues/PRs. We're still very much interested in eventually having something like this though. Sorry again for the communication issues on our part.