Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-12176

No error shown when attempting to delete a founder

    Details

    • Type: Bug
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.12
    • Fix Version/s: 3.0.13-RC1, 3.1.0-b1
    • Component/s: None
    • Labels:
      None

      Description

      Problem: If you try to delete a founder the message returned is “User details updated.“ on the green success background. The user is not deleted, which is good, but there is no indication it actually failed other than later manually finding the user has not been deleted.

      Fix: Add a new error message key and value that will show when an attempt is made to delete a founder. Move the check for founder status from the $delete conditional to a separate check inside it, similar to the checks for deleting yourself or Anonymous.

        Activity

        Hide
        Oyabun1 Oyabun1 added a comment -

        Patch file

        Show
        Oyabun1 Oyabun1 added a comment - Patch file
        Hide
        bantu Andreas Fischer added a comment -

        Issue confirmed.

        Show
        bantu Andreas Fischer added a comment - Issue confirmed.
        Hide
        bantu Andreas Fischer added a comment -

        Patch rejected. Founders should be able to delete founders. The description for founders is "Founders have all administrative permissions and can never be banned, deleted or altered by non-founder members.". Deleting founders should only be forbidden for non-founders, i.e. regular user admins.

        Show
        bantu Andreas Fischer added a comment - Patch rejected. Founders should be able to delete founders. The description for founders is "Founders have all administrative permissions and can never be banned, deleted or altered by non-founder members.". Deleting founders should only be forbidden for non-founders, i.e. regular user admins.
        Hide
        bantu Andreas Fischer added a comment -

        I just noticed that "// You can't delete the founder" is part of current code, not of code that is being added. Patch is almost good.

        Show
        bantu Andreas Fischer added a comment - I just noticed that "// You can't delete the founder" is part of current code, not of code that is being added. Patch is almost good.
        Hide
        bantu Andreas Fischer added a comment - - edited

        The language "Founders have all administrative permissions and can never be banned, deleted or altered by non-founder members." actually isn't that clear. It can be read as "Founders have all administrative permissions and can never be (banned), (deleted) or (altered by non-founder members)." and "Founders have all administrative permissions and can never be (banned, deleted or altered) by non-founder members."

        Show
        bantu Andreas Fischer added a comment - - edited The language "Founders have all administrative permissions and can never be banned, deleted or altered by non-founder members." actually isn't that clear. It can be read as "Founders have all administrative permissions and can never be (banned), (deleted) or (altered by non-founder members)." and "Founders have all administrative permissions and can never be (banned, deleted or altered) by non-founder members."
        Hide
        bantu Andreas Fischer added a comment -

        Considering that founders can not be banned by anyone, I tend to apply the same to deletion.

        Show
        bantu Andreas Fischer added a comment - Considering that founders can not be banned by anyone, I tend to apply the same to deletion.
        Hide
        Oyabun1 Oyabun1 added a comment -

        I don't think founders can be banned, deleted or deactivated even by other founders. The status of the user first has to be changed to a non-founder, so I would suggest just removing the "by non-founder members" text because it is a bit misleading, but maybe add, "To make changes to a founder account founder status must first be removed."

        I don't like the language entry in the patch but made it like that to be consistent with the entries for CANNOT_BAN_FOUNDER and CANNOT_DEACTIVATE_FOUNDER. Maybe they should be combined into just one entry that gives a better explanation? something like:

        	'CANNOT_KILL_FOUNDER'			=> 'A founder cannot be banned, deactivated nor deleted. The founder status of the user must first be changed.',

        Requires a few more code changes, but may be better overall?

        Show
        Oyabun1 Oyabun1 added a comment - I don't think founders can be banned, deleted or deactivated even by other founders. The status of the user first has to be changed to a non-founder, so I would suggest just removing the "by non-founder members" text because it is a bit misleading, but maybe add, "To make changes to a founder account founder status must first be removed." I don't like the language entry in the patch but made it like that to be consistent with the entries for CANNOT_BAN_FOUNDER and CANNOT_DEACTIVATE_FOUNDER. Maybe they should be combined into just one entry that gives a better explanation? something like: 'CANNOT_KILL_FOUNDER' => 'A founder cannot be banned, deactivated nor deleted. The founder status of the user must first be changed.', Requires a few more code changes, but may be better overall?
        Hide
        bantu Andreas Fischer added a comment -

        The thing is that user details of a founder can be changed by other founders. Otherwise it would be impossible to change a founder to a non-founder.

        Show
        bantu Andreas Fischer added a comment - The thing is that user details of a founder can be changed by other founders. Otherwise it would be impossible to change a founder to a non-founder.

          People

          • Assignee:
            bantu Andreas Fischer
            Reporter:
            Oyabun1 Oyabun1
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development