Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add account suspension features #174

Merged
merged 14 commits into from Nov 11, 2019

Conversation

@robjloranger
Copy link
Member

robjloranger commented Aug 29, 2019

This renders all requests for that user's posts, collections and related
ActivityPub endpoints with 404 responses.

While suspended, users may not create or edit posts or collections. A 403
is returned to the authenticated user.

User status is listed in the admin user page

Admin view of user details shows status and has a button to activate or
suspend a user.

Resolves T661.


  • I have signed the CLA
This renders all requests for that user's posts, collections and related
ActivityPub endpoints with 404 responses.

While suspended, users may not create or edit posts or collections.

User status is listed in the admin user page

Admin view of user details shows status and now has a button to activate
or suspend a user.
@robjloranger robjloranger requested a review from thebaer Aug 29, 2019
@thebaer thebaer added this to the 0.11 milestone Sep 4, 2019
Copy link
Member

thebaer left a comment

Looking pretty good overall -- just combed through everything, reading only. One major design change I'd suggest is creating something like an integer status column in the users table, instead of the boolean suspended column. That'll allow us to add other user states in the future without any database changes.

I'd look at the collVisibility type for prior art:

writefreely/collections.go

Lines 117 to 128 in 3759f16

// collVisibility represents the visibility level for the collection.
type collVisibility int
// Visibility levels. Values are bitmasks, stored in the database as
// decimal numbers. If adding types, append them to this list. If removing,
// replace the desired visibility with a new value.
const CollUnlisted collVisibility = 0
const (
CollPublic collVisibility = 1 << iota
CollPrivate
CollProtected
)

Of course, e.g. UserActive should be the 0 / default state, and then right now we'd just have UserSuspended as the second potential state.

migrations/v3.go Outdated Show resolved Hide resolved
activitypub.go Outdated Show resolved Hide resolved
activitypub.go Outdated Show resolved Hide resolved
activitypub.go Outdated Show resolved Hide resolved
activitypub.go Outdated Show resolved Hide resolved
posts.go Outdated Show resolved Hide resolved
routes.go Outdated Show resolved Hide resolved
schema.sql Outdated Show resolved Hide resolved
sqlite.sql Outdated Show resolved Hide resolved
templates/user/settings.tmpl Outdated Show resolved Hide resolved
@thebaer

This comment has been minimized.

Copy link
Member

thebaer commented Oct 24, 2019

Also resolved merge conflicts with develop, so be sure to pull.

- update error messages to be correct
- move suspended message into template and include for other pages
- check suspended status on all relevant pages and show message if
logged in user is suspended.
- fix possible nil pointer error
- remove changes to db schema files
- add version comment to migration
- add UserStatus type with UserActive and UserSuspended
- change database table to use status column instead of suspended
- update toggle suspended handler to be toggle status in prep for
possible future inclusion of further user statuses
@robjloranger robjloranger requested a review from thebaer Oct 25, 2019
robjloranger and others added 10 commits Oct 25, 2019
also fix logic bug in posts.go viewCollectionPost checking the page
owner
This adds a User.IsSuspended() method and uses it when displaying the
user's status on admin pages, instead of doing a magic number check.
This should also help in the future, in case this logic ever changes.

Ref T661
The link here is a little redundant, and might make people think that it
actually changes the status by clicking on it.
This also includes a bit of explanation about what suspending a user
actually does.

Ref T661
Previously it was above the header.

Ref T661
From u.Status == UserSuspended to u.IsSuspended()

Ref T661
This puts the verbiage more in line with what the feature does, and
leaves room for other moderation controls in the future.

NOTE: this includes no backend refactoring, which may be confusing. We
should rename things to fit ASAP.

Ref T661
Some of the work needed to have the backend match user-facing wording.

Ref T661
- Tagged posts
- Collection index

Ref T661
@thebaer

This comment has been minimized.

Copy link
Member

thebaer commented Nov 11, 2019

Made a few changes to the language, including renaming this function from Suspending a user to Silencing them. This more closely fits the function, leaves room for other kinds of moderation controls, and fits expectations admins should have with other platforms.

Since I'm going to be merging this relatively soon in order to get the release wrapped up, I'm going to note that this is experimental, and solicit bug reports and user feedback.

Some work still to do:

  • Rename variables and database method to match user-facing language
  • Total posts count shouldn't included suspended users' posts
  • Discuss future moderation controls
@thebaer thebaer merged commit bca678a into develop Nov 11, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.