Magic string checks such as "has the god mode permission" are bad. Let's turn those into dedicated methods.

CommentFileSizeAuthor
#8 group-3259144-8.patch32.87 KBkristiaanvandeneynde
#6 group-3259144-6.patch32.98 KBkristiaanvandeneynde
#4 group-3259144-4.patch22.39 KBkristiaanvandeneynde
#2 group-3259144-2.patch21.91 KBkristiaanvandeneynde

Comments

kristiaanvandeneynde created an issue. See original summary.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Active » Needs review
StatusFileSize
new group-3259144-2.patch21.91 KB

Interested to find out how many tests this breaks. It also turns existing god mode roles into actual admin roles, even if they are anon/outsider/member. We can fix the fact that you can't edit those in #3204083: Rework group roles into a scoped system and adjust permission calculation where we will allow these special roles to be deleted.

Status: Needs review » Needs work

The last submitted patch, 2: group-3259144-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs work » Needs review
StatusFileSize
new group-3259144-4.patch22.39 KB

Forgot to update the schema.

Status: Needs review » Needs work

The last submitted patch, 4: group-3259144-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs work » Needs review
StatusFileSize
new group-3259144-6.patch32.98 KB

Adjusted tests and removed the 'administer group' group permission as it serves no purpose now.

Status: Needs review » Needs work

The last submitted patch, 6: group-3259144-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs work » Needs review
StatusFileSize
new group-3259144-8.patch32.87 KB

Deleted a necessary admin check by accident.

kristiaanvandeneynde’s picture

kristiaanvandeneynde
Primary language Dutch
Location Antwerp, Belgium
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.