Users autoblock feature #305
Conversation
entantoencuanto
force-pushed
the
users_autoblock
branch
from
542bf55 to
a9ebf8c
Compare
|
@microstudi we finished the job here some months ago. Anything pending to be able to add to next release of Awesome? |
This PR is marked as a Draft, also it does say "WIP" in the description and, with a quick look, it does not have any specs. Please fix things and I'll review it! |
Can you complete @entantoencuanto ? Tx! |
entantoencuanto
force-pushed
the
users_autoblock
branch
2 times, most recently
from
1b7982f to
bc0e0ee
Compare
Updated the README to reflect changes introduced with the PR for the Autoblock feature: decidim-ice#305, addressing issue decidim-ice#299.
|
Can you review @microstudi ? |
|
Sorry, I didn't notice this @aramollis. I see that you have a README updated in another place, can you port it here? meanwhile I'll test and review this between this and the next week |
I'm not sure where should be defined and I tried to add in the main readme of the module. |
ping @entantoencuanto @microstudi Tx! |
|
@entantoencuanto nice to see that you are finishing this, please try to merge main if you can. Let me know when ready ! |
|
Yes, I hope to get everything ready in the next few days. |
… autoblock from a task
entantoencuanto
force-pushed
the
users_autoblock
branch
from
ff873ba to
19047ac
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #305 +/- ##
===========================================
+ Coverage 60.06% 88.05% +27.98%
===========================================
Files 148 162 +14
Lines 3794 4328 +534
===========================================
+ Hits 2279 3811 +1532
+ Misses 1515 517 -998 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi, @microstudi , I have rebased the branch on main and added some tests and I think that the PR |
|
Is ready now @entantoencuanto can you put it ready to review? |
|
Yes, I'm going to add a few more tests but I don't expect to change the main code. |
|
@entantoencuanto I've deployed this to https://edge-awesome.dev.pokecode.net so we can test it! |
There was a problem hiding this comment.
Thanks for this, looks amazing.
I have some initial comments while we test it. Let me know what you think.
I would specially change the use of constants, instead let's use initializers or registry manifests to make this more expandable.
BTW, pleas add a CHANGELOG entry!
| end | ||
|
|
||
| def default_block_justification_message | ||
| DEFAULT_BLOCK_JUSTIFICATION_MESSAGE |
There was a problem hiding this comment.
can't we just use a default i18n key here?
| USERS_AUTOBLOCKS_TYPES = { | ||
| "about_blank" => { has_variable: false, default_blocklist: true }, | ||
| "activities_blank" => { has_variable: false, default_blocklist: true }, | ||
| "links_in_comments_or_about" => { has_variable: true, default_blocklist: true }, | ||
| "email_unconfirmed" => { has_variable: true, default_blocklist: true }, | ||
| "email_domain" => { has_variable: true, default_blocklist: true }, | ||
| "links_in_comments_or_about_with_domains" => { has_variable: true, default_blocklist: true } | ||
| }.freeze |
There was a problem hiding this comment.
I would prefer to put this in an initializer, this facilitates future expansions don't you think? It also allows implementator to remove some of the types if needed
| DEFAULT_ACTIVITIES = { | ||
| comments: ->(user) { Decidim::Comments::Comment.where(author: user).count }, | ||
| comment_votes: ->(user) { Decidim::Comments::CommentVote.where(author: user).count }, | ||
| proposals: ->(user) { Decidim::Proposals::Proposal.coauthored_by(user).count }, | ||
| proposals_votes: ->(user) { Decidim::Proposals::ProposalVote.where(author: user).where(author: user).count }, | ||
| collaborative_drafts: ->(user) { Decidim::Proposals::CollaborativeDraft.coauthored_by(user).count }, | ||
| amendments: ->(user) { Decidim::Amendment.where(amender: user).count }, | ||
| debates: ->(user) { Decidim::Debates::Debate.where(author: user).count }, | ||
| initiatives: ->(user) { Decidim::Initiative.where(author: user).count }, | ||
| initiatives_votes: ->(user) { Decidim::InitiativesVote.where(author: user).count }, | ||
| meetings: ->(user) { Decidim::Meetings::Meeting.where(author: user).count }, | ||
| meetings_answers: ->(user) { Decidim::Meetings::Answer.where(user:).count }, | ||
| budgets_orders: ->(user) { Decidim::Budgets::Order.where(user:).count }, | ||
| forms_answers: ->(user) { Decidim::Forms::Answer.where(user:).count }, | ||
| blog_posts: ->(user) { Decidim::Blogs::Post.where(author: user).count }, | ||
| follows: ->(user) { Decidim::Follow.where(user:).count }, | ||
| endorsements: ->(user) { Decidim::Endorsement.where(author: user).count }, | ||
| reports: ->(user) { Decidim::Report.where(user:).count } | ||
| }.freeze |
There was a problem hiding this comment.
As I see this is coupled with the available_keys.
I think a better approach for this would be to add a manifest where to register the user possible activities. This is for two reasons, first it will encapsulate the logic of detecting them in a separate, isolated place.
The other reason is that there are more external modules that can be applied here and it would be nice for these modules to incorporate their own resources here.
I imagine having an initializer in the engine.rb such as:
initializer "decidim_decidim_awesome.autoblock_users" do |_app|
if DecidimAwesome.enabled?(:users_autoblock)
if Decidim.module_installed?(:comments)
Decidim::DecidimAwesome.user_activities_registry.register(:comments, ->(user) { Decidim::Comments::Comment.where(author: user).count })
Decidim::DecidimAwesome.user_activities_registry.register(:comment_votess, ->(user) { Decidim::Comments::CommentVote.where(author: user).count })
end
end
endThen other plugins (or applications) could register other resources in a similar faishon:
Decidim::DecidimAwesome.configure do
Decidim::DecidimAwesome.user_activities_registry.register(:suggestion, ->(user) { Decidim::ParticipatoryDocuments::Suggestion.where(author: user).count })
end| @@ -66,6 +66,10 @@ module DecidimAwesome | |||
| true | |||
| end | |||
|
|
|||
| config_accessor :users_autoblocks do | |||
There was a problem hiding this comment.
We should add a description in here, also specify that this can be disabled as well :disabled
|
|
||
| initializer "decidim_decidim_awesome.override_blocked_users_notifications" do | ||
| config.to_prepare do | ||
| Decidim::BlockUserMailer.prepend(Decidim::DecidimAwesome::BlockUserMailerOverride) |
There was a problem hiding this comment.
| Decidim::BlockUserMailer.prepend(Decidim::DecidimAwesome::BlockUserMailerOverride) | |
| if DecidimAwesome.enabled?(:users_autoblock) | |
| Decidim::BlockUserMailer.prepend(Decidim::DecidimAwesome::BlockUserMailerOverride) | |
| end |
| decidim_admin_decidim_awesome.users_autoblocks_path, | ||
| position: 11, | ||
| icon_name: "user-unfollow-line", | ||
| if: menus[:users_autoblocks] |
There was a problem hiding this comment.
I think you are missing to specify the menus[:users_autoblock]
def menus
@menus ||= {
users_autoblock: config_enabled?(:users_autoblock),
Yes, you are right, It's much more flexible the use of initializers. I'm going to add all the changes you suggested. I'll let you know when they're all ready. |
This PR implements a feature which allows to detect spam users and block them based on a set of custom rules which generate a score for each user. By defining a threshold, the users who are candidates for blocking are classified.
The scores calculation generates a file that is used to generate a report which is sent to admins once generated and can be downloaded.
The page of the feature allows to manage the rules and configure different actions:
bundle exec rails decidim_decidim_awesome:autoblock_users:run_scheduled_block) that can be scheduled with cron jobs. The task inspects the configuration of the feature on each organization and runs the automatic block if this option was marked using the threshold, the users notification settings and the admin who saved the configuration (an admin is required to block users).A rule can be defined with several criteria:
Each rule, in addition to the criterion, has other attributes: