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
Merged

Add account suspension features #174

merged 14 commits into from Nov 11, 2019

Conversation

ghost
Copy link

@ghost ghost 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.
@ghost ghost requested a review from thebaer August 29, 2019 16:13
@thebaer thebaer added this to the 0.11 milestone Sep 4, 2019
Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: https://github.com/writeas/writefreely/blob/3759f16ed3ff30bf1bb4277b4d8ef7f15bcfc043/collections.go#L117-L128

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
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
@ghost ghost requested a review from thebaer October 25, 2019 19:10
Rob Loranger and others added 10 commits October 25, 2019 13:40
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
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant