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

Updates in response to initial feedback after reopening #1112

Merged
merged 5 commits into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 27 additions & 11 deletions app/assets/stylesheets/petitions/views/_home.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,39 @@
}

.actioned-petitions {
@extend %contain-floats;
ul {
display: flex;
margin: 30px 0 0 0;
}

li {
float: left;
margin: 0;
padding: 0;
width: 50%;
@include box-sizing(border-box);
@include media(tablet) {
padding-right: $gutter*3;
}
}
li:first-child {
padding-right: $gutter-half;
}

.count {
display: block;
font-size: 36px;
line-height: 40/36;
font-weight: 700;
font-weight: bold;
line-height: 1;
margin-bottom: 5px;

@include media(tablet) {
font-size: 48px;
}
}

.suffix {
display: block;
font-size: 14px;
padding-right: 10px;
text-wrap: balance;

@include media(tablet) {
font-size: 19px;
padding-right: 20px;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/rate_limits_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def rate_limit_attributes
geoblocking_enabled countries country_rate_limits_enabled
country_burst_rate country_sustained_rate trending_items_notification_url
threshold_for_logging_trending_items threshold_for_notifying_trending_items
enable_logging_of_trending_items ignored_domains
enable_logging_of_trending_items ignored_domains blocked_emails
]
end

Expand Down
5 changes: 5 additions & 0 deletions app/controllers/parliaments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class ParliamentsController < ApplicationController
before_action :set_cors_headers, if: :json_request?
before_action :fetch_parliaments, only: [:index]
before_action :fetch_parliament, only: [:show]
before_action :fetch_constituencies, only: [:show]

def index
expires_in 1.hour, public: true,
Expand Down Expand Up @@ -32,4 +33,8 @@ def fetch_parliaments
def fetch_parliament
@parliament = Parliament.archived.find_by!(period: params[:period])
end

def fetch_constituencies
@constituencies = @parliament.constituencies.by_ons_code
end
end
38 changes: 38 additions & 0 deletions app/models/rate_limit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class RateLimit < ActiveRecord::Base
SINGLE_GLOB = "*."
SINGLE_PATTERN = "(?:[-a-z0-9]+\\.)"
DOMAIN_PATTERN = /^(?:[a-z0-9][a-z0-9-]{0,61}[a-z0-9]\.)+[a-z]{2,}$/
EMAIL_PATTERN = /\A[^@\s]+@[^@\s]+\z/

validates :burst_rate, presence: true, numericality: { only_integer: true, greater_than: 0 }
validates :burst_period, presence: true, numericality: { only_integer: true, greater_than: 0 }
Expand All @@ -26,6 +27,7 @@ class RateLimit < ActiveRecord::Base
validates :threshold_for_notifying_trending_items, presence: true, numericality: { only_integer: true, greater_than: 0 }
validates :trending_items_notification_url, length: { maximum: 100, allow_blank: true }
validates :ignored_domains, length: { maximum: 10000, allow_blank: true }
validates :blocked_emails, length: { maximum: 50000, allow_blank: true }

validate do
unless sustained_rate.nil? || burst_rate.nil?
Expand Down Expand Up @@ -75,6 +77,12 @@ class RateLimit < ActiveRecord::Base
rescue StandardError => e
errors.add :ignored_domains, :invalid
end

begin
blocked_emails_list
rescue StandardError => e
errors.add :blocked_emails, :invalid
end
end

def reload
Expand All @@ -84,6 +92,7 @@ def reload
@blocked_ips_list = nil
@allowed_countries = nil
@ignored_domains_list = nil
@blocked_emails_list = nil

super
end
Expand All @@ -92,6 +101,7 @@ def exceeded?(signature)
return true if ip_blocked?(signature.ip_address)
return true if ip_geoblocked?(signature.ip_address)
return true if domain_blocked?(signature.domain)
return true if email_blocked?(signature.email)
return false if ip_allowed?(signature.ip_address)
return false if domain_allowed?(signature.domain)

Expand Down Expand Up @@ -170,6 +180,15 @@ def ignored_domains_list
@ignored_domains_list ||= build_ignored_domains
end

def blocked_emails=(value)
@blocked_emails_list
super(normalize_lines(value))
end

def blocked_emails_list
@blocked_emails_list || build_blocked_emails
end

private

def strip_comments(list)
Expand Down Expand Up @@ -228,6 +247,17 @@ def build_ignored_domains
strip_blank_lines(strip_comments(ignored_domains)).map { |d| validate_domain!(d.strip) }
end

def build_blocked_emails
strip_blank_lines(strip_comments(blocked_emails)).map { |e| validate_email!(e.strip) }
end

def email_blocked?(email)
return false if email.blank?

canonical_email = Domain.normalize(email)
blocked_emails_list.any?{ |e| e == canonical_email }
end

def build_allowed_countries
strip_blank_lines(strip_comments(countries)).map(&:strip)
end
Expand Down Expand Up @@ -272,6 +302,14 @@ def validate_domain!(domain)
end
end

def validate_email!(email)
if email =~ EMAIL_PATTERN
email
else
raise ArgumentError, "Invalid email: #{email.inspect}"
end
end

def normalize_lines(value)
value.to_s.strip.gsub(/\r\n|\r/, "\n")
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/site.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ def signature_count_updated_at!(timestamp = Time.current)
end

def moderation_overdue_in_days
7.days
10.days
end

def moderation_near_overdue_in_days
5.days
8.days
end

def maximum_number_of_signatures
Expand Down
6 changes: 3 additions & 3 deletions app/views/admin/admin/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
<% unless recently_in_moderation_untagged_count.zero? %>
/ <%= recently_in_moderation_untagged_count %>
<% end %>
<span class="label">0-5 days</span>
<span class="label">0-7 days</span>
<% end %>
</div>

Expand All @@ -125,7 +125,7 @@
<% unless nearly_overdue_in_moderation_untagged_count.zero? %>
/ <%= nearly_overdue_in_moderation_untagged_count %>
<% end %>
<span class="label">6-7 days</span>
<span class="label">8-10 days</span>
<% end %>
</div>

Expand All @@ -135,7 +135,7 @@
<% unless overdue_in_moderation_untagged_count.zero? %>
/ <%= overdue_in_moderation_untagged_count %>
<% end %>
<span class="label">&gt; 7 days</span>
<span class="label">&gt; 10 days</span>
<% end %>
</div>

Expand Down
7 changes: 7 additions & 0 deletions app/views/admin/rate_limits/_blocked_emails.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<%= hidden_field_tag :tab, "blocked_emails" %>

<%= form_row for: [form.object, :blocked_emails] do %>
<%= error_messages_for_field @rate_limit, :blocked_emails %>
<%= form.text_area :blocked_emails, tabindex: increment, rows: 15, class: 'form-control' %>
<p><small>normalize the email address according to the <%= link_to "domain rules", admin_domains_path %></small><p>
<% end %>
18 changes: 17 additions & 1 deletion app/views/admin/rate_limits/_tabs.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
<p class="tabs">
<% if params[:tab] == "allowed_domains" %>
<% if params[:tab] == "blocked_emails" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
Blocked Emails |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
<%= link_to "Blocked IPs", edit_admin_rate_limits_path(tab: 'blocked_ips') %> |
<%= link_to "Countries", edit_admin_rate_limits_path(tab: 'countries') %> |
<%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %>
<% elsif params[:tab] == "allowed_domains" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
Allowed Domains |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
Expand All @@ -9,6 +19,7 @@
<%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %>
<% elsif params[:tab] == "blocked_domains" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
Blocked Domains |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
Expand All @@ -17,6 +28,7 @@
<%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %>
<% elsif params[:tab] == "allowed_ips" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
Allowed IPs |
Expand All @@ -25,6 +37,7 @@
<%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %>
<% elsif params[:tab] == "blocked_ips" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
Expand All @@ -33,6 +46,7 @@
<%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %>
<% elsif params[:tab] == "countries" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
Expand All @@ -41,6 +55,7 @@
<%= link_to "Trending Items", edit_admin_rate_limits_path(tab: 'trending_items') %>
<% elsif params[:tab] == "trending_items" %>
<%= link_to "Rate Limits", edit_admin_rate_limits_path %> |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
Expand All @@ -49,6 +64,7 @@
Trending Items
<% else %>
Rate Limits |
<%= link_to "Blocked Emails", edit_admin_rate_limits_path(tab: 'blocked_emails') %> |
<%= link_to "Allowed Domains", edit_admin_rate_limits_path(tab: 'allowed_domains') %> |
<%= link_to "Blocked Domains", edit_admin_rate_limits_path(tab: 'blocked_domains') %> |
<%= link_to "Allowed IPs", edit_admin_rate_limits_path(tab: 'allowed_ips') %> |
Expand Down
4 changes: 3 additions & 1 deletion app/views/admin/rate_limits/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
</div>

<div class="column-two-thirds extra-gutter">
<% if params[:tab] == "allowed_domains" %>
<% if params[:tab] == "blocked_emails" %>
<%= render "form", tab: "blocked_emails" %>
<% elsif params[:tab] == "allowed_domains" %>
<%= render "form", tab: "allowed_domains" %>
<% elsif params[:tab] == "blocked_domains" %>
<%= render "form", tab: "blocked_domains" %>
Expand Down
22 changes: 12 additions & 10 deletions app/views/parliaments/show.json.jbuilder
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
json.period @parliament.period
json.dissolution_at @parliament.dissolution_at
json.government @parliament.government
json.response_threshold @parliament.threshold_for_response
json.debate_threshold @parliament.threshold_for_debate

json.attributes do
json.government @parliament.government
json.response_threshold @parliament.threshold_for_response
json.debate_threshold @parliament.threshold_for_debate
end
json.constituencies @parliament.constituencies.each do |constituency|
json.constituency constituency.name
json.ons_code constituency.ons_code
json.start_date constituency.start_date
json.end_date constituency.end_date
json.constituencies do
@constituencies.each do |constituency|
json.set! constituency.ons_code do
json.constituency constituency.name
json.ons_code constituency.ons_code
json.start_date constituency.start_date
json.end_date constituency.end_date
end
end
end
8 changes: 0 additions & 8 deletions app/views/trackers/show.gif.ruby

This file was deleted.

2 changes: 2 additions & 0 deletions config/locales/activerecord.en-GB.yml
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ en-GB:
invalid: "Blocked IPs list is invalid"
ignored_domains:
invalid: "Ignored domains list is invalid"
blocked_emails:
invalid: "Blocked emails list is invalid"

tag:
attributes:
Expand Down
16 changes: 8 additions & 8 deletions config/locales/petitions.en-GB.yml
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,20 @@ en-GB:
counts:
awaiting_response:
html:
one: "<span class=\"count\">%{formatted_count}</span> petition is waiting for a response from the Government"
other: "<span class=\"count\">%{formatted_count}</span> petitions are waiting for responses from the Government"
one: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petition from the current Parliament is waiting for a response from the Government</span>"
other: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petitions from the current Parliament are waiting for responses from the Government</span>"
with_response:
html:
one: "<span class=\"count\">%{formatted_count}</span> petition got a response from the Government"
other: "<span class=\"count\">%{formatted_count}</span> petitions got a response from the Government"
one: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petition from the current Parliament got a response from the Government</span>"
other: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petitions from the current Parliament got a response from the Government</span>"
awaiting_debate:
html:
one: "<span class=\"count\">%{formatted_count}</span> petition is waiting to be considered for debate"
other: "<span class=\"count\">%{formatted_count}</span> petitions are waiting to be considered for debate"
one: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petition from the current Parliament is waiting to be considered for debate</span>"
other: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petitions from the current Parliament are waiting to be considered for debate</span>"
with_debated_outcome:
html:
one: "<span class=\"count\">%{formatted_count}</span> petition was debated in the House of Commons"
other: "<span class=\"count\">%{formatted_count}</span> petitions were debated in the House of Commons"
one: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petition from the current Parliament was debated in the House of Commons</span>"
other: "<span class=\"count\">%{formatted_count}</span> <span class=\"suffix\">petitions from the current Parliament were debated in the House of Commons</span>"
awaiting_response_explanation:
html: "See all petitions waiting for a government response (%{formatted_count})"
with_response_explanation:
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20241221160446_add_blocked_emails_to_rate_limit.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddBlockedEmailsToRateLimit < ActiveRecord::Migration[7.2]
def change
add_column :rate_limits, :blocked_emails, :string, limit: 50000, null: false, default: ""
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2024_10_21_142904) do
ActiveRecord::Schema[7.2].define(version: 2024_12_21_160446) do
# These are extensions that must be enabled in order to support this database
enable_extension "intarray"
enable_extension "plpgsql"
Expand Down Expand Up @@ -617,6 +617,7 @@
t.integer "creator_rate", default: 2, null: false
t.integer "sponsor_rate", default: 5, null: false
t.integer "feedback_rate", default: 2, null: false
t.string "blocked_emails", limit: 50000, default: "", null: false
end

create_table "regions", force: :cascade do |t|
Expand Down
Loading