maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Zappat0n/add rate limit model everywhere 11500 (#14609)

Browse source 
* Create showModalAfterError function

* Show modal when comments rate limit is reached

* Show modal when article reaction rate limit is reached

* Show modal when follow user rate limit is reached

* Show form error when listing creation rate limit is reached

* Show form error when feedback messages rate limit is reached

* Rename functions for listings rate limit checks

* Show modal when notifications reaction rate limit is reached

* Show modal when picture upload rate limit reached

* Show modal for reactable objects when rate limit reached

* Add and modify tests

* empty commit

* Match modal messages to tests

* Fix error updating Modals

Co-authored-by: Dan Uber <dan@forem.com>
This commit is contained in:
Angel Barros 2021-09-23 10:49:19 +01:00 • committed by GitHub
parent 48e327dae0
commit bb8b32bdf2
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 222 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
app/assets/javascripts/initializers/initNotifications.js
View file

@ -1,4 +1,4 @@
/* global checkUserLoggedIn, instantClick, InstantClick, sendHapticMessage */
/* global checkUserLoggedIn, instantClick, InstantClick, sendHapticMessage, showModalAfterError */
function initNotifications() {
fetchNotificationsCount();
@ -94,6 +94,13 @@ function initReactions() {
.then(function (response) {
if (response.status === 200) {
response.json().then(successCb);
} else {
showModalAfterError({
response,
element: 'reaction',
action_ing: 'updating',
action_past: 'updated',
});
}
});
};

15
app/assets/javascripts/initializers/initializeArticleReactions.js
View file

@ -1,4 +1,4 @@
/* global sendHapticMessage, showLoginModal */
/* global sendHapticMessage, showLoginModal, showModalAfterError */
// Set reaction count to correct number
function setReactionCount(reactionName, newCount) {
@ -88,10 +88,17 @@ function reactToArticle(articleId, reaction) {
return response.json().then(() => {
document.getElementById('reaction-butt-' + reaction).disabled = false;
});
} else {
toggleReaction();
document.getElementById('reaction-butt-' + reaction).disabled = false;
showModalAfterError({
response,
element: 'reaction',
action_ing: 'updating',
action_past: 'updated',
});
return undefined;
}
toggleReaction();
document.getElementById('reaction-butt-' + reaction).disabled = false;
return undefined;
})
.catch((error) => {
toggleReaction();

28
app/assets/javascripts/initializers/initializeCommentsPage.js.erb
View file

@ -148,6 +148,13 @@ function initializeCommentsPage() {
thisButt.disabled = false;
if (response.status === 200) {
response.json().then(successCb);
} else {
showModalAfterError({
response,
element: 'reaction',
action_ing: 'making',
action_past: 'made',
});
}
});
};
@ -297,15 +304,11 @@ function handleCommentSubmit(event) {
})
} else {
form.classList.remove('submitting');
var responseStatus = response.status;
response.json().then(function parseError(errorReponse) {
if (responseStatus === 429) {
showRateLimitModal('made a comment', 'making another comment')
} else {
showUserAlertModal('Error posting comment', 'Your comment could not be posted due to an error: ' + errorReponse.error, 'OK')
}
}).catch(function parseError(error){
showUserAlertModal('Error posting comment', 'Your comment could not be posted due to a server error', 'OK')
showModalAfterError({
response,
element: 'comment',
action_ing: 'posting',
action_past: 'posted',
});
return false;
}
@ -528,6 +531,13 @@ function handleImageUpload(event, randomIdNumber) {
messageContainer.style.top = "5px";
}
);
} else if (response.status === 429) {
showRateLimitModal({
response,
element: 'image',
action_ing: 'uploading',
action_past: 'uploaded',
});
} else {
response.json().then(function(responseBody) {
var errorMessage = responseBody.error || 'Invalid file!';

100
app/assets/javascripts/utilities/showUserAlertModal.js
View file

@ -30,24 +30,78 @@ function showUserAlertModal(title, text, confirm_text) {
* Displays a user rate limit alert modal letting the user know what they did that exceeded a rate limit,
* and gives them links to explain why they must wait
*
* @function showUserAlertModal
* @param {string} action_text Description of the action taken by the user
* @param {string} next_action_text Description of the next action that can be taken
* @function showRateLimitModal
* @param {string} element Description of the element that throw the error
* @param {string} action_ing The -ing form of the action taken by the user
* @param {string} action_past The past tense of the action taken by the user
* @param {string} timeframe Description of the time that we need to wait
*
* @example
* showRateLimitModal('Made a comment', 'comment again')
*/
function showRateLimitModal(action_text, next_action_text) {
let rateLimitText = buildRateLimitText(action_text, next_action_text);
function showRateLimitModal({
element,
action_ing,
action_past,
timeframe = 'a moment',
}) {
let rateLimitText = buildRateLimitText({
element,
action_ing,
action_past,
timeframe,
});
let rateLimitLink = '/faq';
showUserAlertModal(
'Wait a moment...',
`Wait ${timeframe}...`,
rateLimitText,
'Got it',
rateLimitLink,
'Why do I have to wait?',
);
}
/**
* Displays the corresponding modal after an error.
*
* @function showModalAfterError
* @param {Object} response The response from the API
* @param {string} element Description of the element that throw the error
* @param {string} action_ing The -ing form of the action taken by the user
* @param {string} action_past The past tense of the action taken by the user
* @param {string} timeframe Description of the time that we need to wait
*
* @example
* showModalAfterError(response, 'made a comment', 'making another comment', 'a moment');
*/
function showModalAfterError({
response,
element,
action_ing,
action_past,
timeframe = 'a moment',
}) {
response
.json()
.then(function parseError(errorReponse) {
if (response.status === 429) {
showRateLimitModal({ element, action_ing, action_past, timeframe });
} else {
showUserAlertModal(
`Error ${action_ing} ${element}`,
`Your ${element} could not be ${action_past} due to an error: ` +
errorReponse.error,
'OK',
);
}
})
.catch(function parseError(error) {
showUserAlertModal(
`Error ${action_ing} ${element}`,
`Your ${element} could not be ${action_past} due to a server error`,
'OK',
);
});
}
/**
* HTML template for modal
@ -61,17 +115,17 @@ function showRateLimitModal(action_text, next_action_text) {
* @returns {string} HTML for the modal
*/
const getModalHtml = (text, confirm_text) => `
<div id="${modalId}" hidden>
<div class="flex flex-col">
<p class="color-base-70">
${text}
</p>
<button class="crayons-btn mt-4 ml-auto" type="button" onClick="window.Forem.closeModal()">
${confirm_text}
</button>
</div>
</div>
`;
<div id="${modalId}" hidden>
<div class="flex flex-col">
<p class="color-base-70">
${text}
</p>
<button class="crayons-btn mt-4 ml-auto" type="button" onClick="window.Forem.closeModal()">
${confirm_text}
</button>
</div>
</div>
`;
/**
* Constructs wording for rate limit modals
@ -79,13 +133,15 @@ const getModalHtml = (text, confirm_text) => `
* @private
* @function buildRateLimitText
*
* @param {string} action_text Description of the action taken by the user
* @param {string} next_action_text Description of the next action that can be taken
* @param {string} element Description of the element that throw the error
* @param {string} action_ing The -ing form of the action taken by the user
* @param {string} action_past The past tense of the action taken by the user
* @param {string} timeframe Description of the time that we need to wait
*
* @returns {string} Formatted body text for a rate limit modal
*/
function buildRateLimitText(action_text, next_action_text) {
return `Since you recently ${action_text}, you’ll need to wait a moment before ${next_action_text}.`;
function buildRateLimitText({ element, action_ing, action_past, timeframe }) {
return `Since you recently ${action_past} a ${element}, you’ll need to wait ${timeframe} before ${action_ing} another ${element}.`;
}
/**
@ -104,6 +160,8 @@ function buildModalDiv(text, confirm_text) {
if (!modalDiv) {
modalDiv = getModal(text, confirm_text);
document.body.appendChild(modalDiv);
} else {
modalDiv.outerHTML = getModal(text, confirm_text).outerHTML;
}
return modalDiv;
}

12
app/controllers/concerns/listings_toolkit.rb
View file

@ -36,6 +36,16 @@ module ListingsToolkit
@listing = Listing.find(params[:id])
end
def rate_limit?
begin
rate_limit!(:listing_creation)
rescue StandardError => e
@listing.errors.add(:listing_creation, e.message)
return true
end
false
end
def create
@listing = Listing.new(listing_params)
organization_id = @listing.organization_id
@ -51,7 +61,7 @@ module ListingsToolkit
return
end
unless @listing.valid?
if !@listing.valid? || rate_limit?
@credits = current_user.credits.unspent
process_unsuccessful_creation
return

13
app/controllers/feedback_messages_controller.rb
View file

@ -6,13 +6,12 @@ class FeedbackMessagesController < ApplicationController
def create
flash.clear
rate_limit!(:feedback_message_creation)
params = feedback_message_params.merge(reporter_id: current_user&.id)
@feedback_message = FeedbackMessage.new(params)
recaptcha_enabled = ReCaptcha::CheckEnabled.call(current_user)
if (!recaptcha_enabled || recaptcha_verified? || connect_feedback?) && @feedback_message.save
if (!recaptcha_enabled || recaptcha_verified? || connect_feedback?) && !rate_limit? && @feedback_message.save
Slack::Messengers::Feedback.call(
user: current_user,
type: feedback_message_params[:feedback_type],
@ -65,4 +64,14 @@ class FeedbackMessagesController < ApplicationController
allowed_params = %i[message feedback_type category reported_url offender_id]
params.require(:feedback_message).permit(allowed_params)
end
def rate_limit?
begin
rate_limit!(:feedback_message_creation)
rescue StandardError => e
@feedback_message.errors.add(:feedback_message_creation, e.message)
return true
end
false
end
end

5
app/controllers/listings_controller.rb
View file

@ -27,7 +27,6 @@ class ListingsController < ApplicationController
# actions `create` and `update` are defined in the module `ListingsToolkit`,
# we thus silence Rubocop lexical scope filter cop: https://rails.rubystyle.guide/#lexically-scoped-action-filter
# rubocop:disable Rails/LexicallyScopedActionFilter
before_action :check_limit, only: [:create]
before_action :set_listing, only: %i[edit update destroy]
before_action :set_cache_control_headers, only: %i[index]
before_action :raise_suspended, only: %i[new create update]
@ -142,10 +141,6 @@ class ListingsController < ApplicationController
redirect_to "/listings/dashboard"
end
def check_limit
rate_limit!(:listing_creation)
end
# This is a convenience method to query listings for use in the index view in
# the index action and process_after_update method
def listings_for_index_view

16
app/javascript/packs/followButtons.js
View file

@ -1,6 +1,6 @@
import { getInstantClick } from '../topNavigation/utilities';
/* global showLoginModal userData */
/* global showLoginModal userData showModalAfterError*/
/**
* Sets the text content of the button to the correct 'Follow' state
@ -223,7 +223,19 @@ function handleFollowButtonClick({ target }) {
formData.append('followable_type', className);
formData.append('followable_id', id);
formData.append('verb', verb);
getCsrfToken().then(sendFetch('follow-creation', formData));
getCsrfToken()
.then(sendFetch('follow-creation', formData))
.then((response) => {
if (response.status !== 200) {
showModalAfterError({
response,
element: 'user',
action_ing: 'following',
action_past: 'followed',
timeframe: 'for a day',
});
}
});
}
}

2
cypress/integration/seededFlows/articleFlows/commentOnArticle.spec.js
View file

@ -479,7 +479,7 @@ describe('Comment on articles', () => {
cy.findByRole('button', { name: /Close/ }).should('have.focus');
cy.findByRole('heading', { name: 'Wait a moment...' }).should('exist');
cy.findByText(
'Since you recently made a comment, you’ll need to wait a moment before making another comment.',
'Since you recently posted a comment, you’ll need to wait a moment before posting another comment.',
);
cy.findByRole('button', { name: 'Got it' }).click();
});

4
spec/requests/feedback_messages_spec.rb
View file

@ -108,8 +108,8 @@ RSpec.describe "feedback_messages", type: :request do
allow(RateLimitChecker).to receive(:new) { limiter }
allow(limiter).to receive(:limit_by_action).and_return(true)
post "/feedback_messages", params: valid_abuse_report_params, headers: headers
expect(response.status).to eq(429)
post "/feedback_messages.json", params: valid_abuse_report_params, headers: headers
expect(response.parsed_body["status"]).to eq("bad_request")
end
end

3
spec/requests/listings_spec.rb
View file

@ -359,8 +359,7 @@ RSpec.describe "/listings", type: :request do
it "returns a 429 status when rate limit is reached" do
allow(rate_limiter).to receive(:limit_by_action).and_return(true)
post "/listings", params: listing_params
expect(response.status).to eq(429)
expect(response.body).to include("Rate limit reached")
end
end
end

20
spec/system/articles/user_creates_an_article_spec.rb
View file

@ -77,4 +77,24 @@ RSpec.describe "Creating an article with the editor", type: :system do
expect_runkit_tag_to_be_active
end
end
context "when user creates too many articles" do
let(:rate_limit_checker) { RateLimitChecker.new(user) }
before do
# avoid hitting new user rate limit check
allow(user).to receive(:created_at).and_return(1.week.ago)
allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker)
allow(rate_limit_checker).to receive(:limit_by_action)
.with(:published_article_creation)
.and_return(true)
end
it "displays a rate limit warning", :flaky, js: true do
visit new_path
fill_in "article_body_markdown", with: template
click_button "Save changes"
expect(page).to have_text("Rate limit reached")
end
end
end

20
spec/system/articles/user_edits_an_article_spec.rb
View file

@ -36,4 +36,24 @@ RSpec.describe "Editing with an editor", type: :system, js: true do
click_button("Save changes")
expect(page).to have_text("Unpublished Post.")
end
context "when user edits too many articles" do
let(:rate_limit_checker) { RateLimitChecker.new(user) }
before do
# avoid hitting new user rate limit check
allow(user).to receive(:created_at).and_return(1.week.ago)
allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker)
allow(rate_limit_checker).to receive(:limit_by_action)
.with(:article_update)
.and_return(true)
end
it "displays a rate limit warning", :flaky, js: true do
visit "/#{user.username}/#{article.slug}/edit"
fill_in "article_body_markdown", with: template.gsub("Suspendisse", "Yooo")
click_button "Save changes"
expect(page).to have_text("Rate limit reached")
end
end
end

25
spec/system/feedback_message_spec.rb
View file

@ -2,6 +2,8 @@ require "rails_helper"
RSpec.describe "Feedback report by chat channel messages", type: :system do
let(:user) { create(:user) }
let(:message) { Faker::Lorem.paragraph }
let(:url) { Faker::Lorem.sentence }
context "when user create a report abuse feedback message" do
before do
@ -21,4 +23,27 @@ RSpec.describe "Feedback report by chat channel messages", type: :system do
end.to change(FeedbackMessage, :count).by(1)
end
end
context "when user creates too many report abuse feedback messages" do
let(:rate_limit_checker) { RateLimitChecker.new(user) }
before do
# avoid hitting new user rate limit check
allow(user).to receive(:created_at).and_return(1.week.ago)
allow(RateLimitChecker).to receive(:new).and_return(rate_limit_checker)
allow(rate_limit_checker).to receive(:limit_by_action)
.with(:feedback_message_creation)
.and_return(true)
end
it "displays a rate limit warning", :flaky, js: true do
visit report_abuse_path
choose("Other")
fill_in "feedback_message_message", with: message
fill_in "feedback_message_reported_url", with: url
click_button "Send Feedback"
expect(page).to have_current_path("/feedback_messages")
expect(page).to have_text("Rate limit reached")
end
end
end