[ticket/12230] Disable newly registered group when post limit is set to 0

[Elsensee]

Old PR: #3553
Older PR: #3536

Now all users are just removed from the group when the limit is set to 0. :)

PHPBB3-12230

[nickvergessen]

!empty

[bantu]

Not sure this behaviour is desired. Any reviews please make sure to have read #3553 (comment)

[Nicofuma]

@bantu which behaviour would you have instead?

[Elsensee]

bump?

[Elsensee]

bump? :/

[marc1706]

@bantu pls answer the above question ;)

[gn36]

I believe you can find his answer in his linked comment above, since it is basically the same problem: If you remove all users from the newly registered group, this makes it hard on Administrators who simply want to manage the users in that group themselves, instead of having it automatically managed. They will have to re-add all users they want in this group.

In that sense, the 0 would only disable the automatic adding and removing feature, not remove all users from the group as well.

I think you could argue for both cases, because a lot of administrators setting this to 0 will probably want to remove all users from that group at the same time, but whatever the choice is, administrators should be informed about this behaviour in the description.

[marc1706]

I think that most admins will expect the newly introduced behavior for setting this to 0. The explain string should however clearly state that this will remove all members from the newly registered users group.

[rxu]

I'd better just add a conditional around https://github.com/phpbb/phpbb/blob/3.1.x/phpBB/phpbb/session.php#L1586-L1589 to carry out removing user from the NEWLY_REGISTERED if $this->data['user_new'] && $config['new_member_post_limit'] == 0.
This way we won't change default behavior and user will be removed from newly registered on his 1st visit after disabling the feature.

[gn36]

I think that most admins will expect the newly introduced behavior for setting this to 0.

I agree.

user will be removed from newly registered on his 1st visit after disabling the feature.

I think removing the users after disabling is changing the default behaviour far more than removing them all at once. In that case, the newly registered users group cannot be used by the administrator at all, while now, they can simply manage users in that group themselves.

[rxu]

I think removing the users after disabling is changing the default behaviour far more

This is just how it works currently for the limit >0. And it can work same way for the limit = 0.

In that case, the newly registered users group cannot be used by the administrator at all

Why is that?

[gn36]

Because users that are manually added to the group by the administrator are immediately removed again automatically whenever they log in if the setting is 0.

[rxu]

They won't be removed because $this->data['user_new'] is not set for them 😉

P.S. I guess it's as simple as removing && !empty($config['new_member_post_limit']) from the condition.

[Elsensee]

Please see PR #3553. I suggested a similar change there.

[Nicofuma]

@Elsensee what's up here?

[Elsensee]

@Nicofuma to be honest, I don't really know. There is also a patch #3553 which fixes this as @rxu has described it.
Maybe we should ask guys over at area51 how they think we should do it. (and if we should do it at all, but I can assume what their reaction will be ;) )

Maybe our new TL @marc1706 has anything to say about this?

[marc1706]

@Elsensee I think I actually clearly stated the expected behavior here? Feel free to chat me up on IRC to clear anything up

[Elsensee]

<Elsensee> Marc: regarding this: https://github.com/phpbb/phpbb/pull/3659 the question is, should we remove all users at once from the group after the admin has set this setting to 0 or should the users be removed from the group, the next time they visit the board?
<Elsensee> they are very similar and personally I'd just go for the second option as there are less mistakes to be made
<Marc> The latter one, especially due to the large query option 1 might cause
<Elsensee> Marc: good, me thinks the same :)

[marc1706]

Reopened as I think you clicked on close and comment by accident?

[Elsensee]

Okay, we now got the same solution as in #3553. The administrator can still manage the group by himself because, as @rxu pointed it out, user_new is only set for really new users.

[Elsensee]

bump?

[Elsensee]

Bump?