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

Prevent users from creating more than 5 forum threads per day #1161 #1162

Merged
merged 6 commits into from
Nov 1, 2024

Conversation

iam-subho
Copy link
Contributor

No description provided.

Copy link
Member

@driesvints driesvints left a comment

Choose a reason for hiding this comment

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

Thanks for your effort! I think ideally we'd like the user to just be redirected back when the user hits the create controller action and shown the warning message.

});

// Check if the user has reached the limit
if ($threadCount >= getenv('APP_MAX_THREAD_COUNT')) {
Copy link
Member

Choose a reason for hiding this comment

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

This can be a fixed value. No need to pull from a config or env.

return Thread::where('author_id', $userId)
->where('created_at', '>=', now()->startOfDay())
->count();
});
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't cache the results tbh. Maybe we can simply get the count by default with withCount: https://laravel.com/docs/11.x/eloquent-relationships#counting-related-models

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@driesvints can i use default alert of javascript

Copy link
Member

Choose a reason for hiding this comment

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

In an ideal world I'd add a new "warning" method to SendsAlerts that gets implemented in the UI as well. But let's use $this->error( for now.

@driesvints driesvints merged commit f556b4a into laravelio:main Nov 1, 2024
1 check passed
@driesvints
Copy link
Member

Thanks a bunch! Did a couple of changes myself here.

@tvbeek
Copy link
Contributor

tvbeek commented Nov 1, 2024

@iam-subho Thanks for this PR, this will prevent an amount of spam that we need to remove 😃

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.

3 participants