(event url)
{ - 'meet.jit.si' => { regex: %r{#{http}#{subdomain}meet.jit.si/#{suffix}}, type: 'direct' }, + 'meet.jit.si' => { regex: %r{#{http}#{subdomain}meet.jit.si/#{suffix}}, type: 'direct' }, 'meet.google.com' => { regex: %r{#{http}#{subdomain}meet.google.com/#{suffix}}, type: 'direct' }, - 'zoom.us' => { regex: %r{#{http}#{subdomain}zoom.us/j/#{suffix}}, type: 'direct' }, - 'facebook.com' => { regex: %r{#{http}#{subdomain}facebook.com/events/#{suffix}}, type: 'indirect' } + 'zoom.us' => { regex: %r{#{http}#{subdomain}zoom.us/j/#{suffix}}, type: 'direct' }, + 'facebook.com' => { regex: %r{#{http}#{subdomain}facebook.com/events/#{suffix}}, type: 'indirect' } } end end diff --git a/app/jobs/calendar_importer/events/manchester_uni_event.rb b/app/jobs/calendar_importer/events/manchester_uni_event.rb index e5fa93641..b04e3cfb1 100644 --- a/app/jobs/calendar_importer/events/manchester_uni_event.rb +++ b/app/jobs/calendar_importer/events/manchester_uni_event.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module CalendarImporter::Events class ManchesterUniEvent < Base def initialize(event) @@ -23,7 +25,7 @@ def dtstart time = @event.at_xpath('./ns:times[@type="local"] //ns:start //ns:time') DateTime.parse([date, time].join(', ')) rescue StandardError - nil + nil end def dtend @@ -31,7 +33,7 @@ def dtend time = @event.at_xpath('./ns:times[@type="local"] //ns:end //ns:time') DateTime.parse([date, time].join(', ')) rescue StandardError - nil + nil end def recurring_event? diff --git a/app/jobs/calendar_importer/events/meetup_event.rb b/app/jobs/calendar_importer/events/meetup_event.rb index 8b9edf7f0..bafe43b17 100644 --- a/app/jobs/calendar_importer/events/meetup_event.rb +++ b/app/jobs/calendar_importer/events/meetup_event.rb @@ -36,7 +36,7 @@ def location venue['city'], venue['localized_country_name'], venue['name'] # postcode? - ].map(&:to_s).map(&:strip).reject(&:blank?).join(', ') + ].map(&:to_s).map(&:strip).compact_blank.join(', ') end end diff --git a/app/jobs/calendar_importer/events/ticketsolve_event.rb b/app/jobs/calendar_importer/events/ticketsolve_event.rb index 848f92e9b..d2b1aae3e 100644 --- a/app/jobs/calendar_importer/events/ticketsolve_event.rb +++ b/app/jobs/calendar_importer/events/ticketsolve_event.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module CalendarImporter::Events class TicketsolveEvent < Base def uid diff --git a/app/jobs/calendar_importer/parsers/base.rb b/app/jobs/calendar_importer/parsers/base.rb index 1ab009e26..ae3a254d8 100644 --- a/app/jobs/calendar_importer/parsers/base.rb +++ b/app/jobs/calendar_importer/parsers/base.rb @@ -30,9 +30,7 @@ def calendar_to_events data = download_calendar checksum = digest(data) - if !@force_import && (@calendar.last_checksum == checksum) - return Output.new([], checksum) - end + return Output.new([], checksum) if !@force_import && (@calendar.last_checksum == checksum) Output.new(import_events_from(data), checksum) end diff --git a/app/jobs/calendar_importer/parsers/eventbrite.rb b/app/jobs/calendar_importer/parsers/eventbrite.rb index 2fe17fd6c..acb038a7a 100644 --- a/app/jobs/calendar_importer/parsers/eventbrite.rb +++ b/app/jobs/calendar_importer/parsers/eventbrite.rb @@ -8,10 +8,10 @@ module CalendarImporter::Parsers class Eventbrite < Base NAME = 'Eventbrite' KEY = 'eventbrite' - DOMAINS = %w[www.eventbrite.com www.eventbrite.co.uk] + DOMAINS = %w[www.eventbrite.com www.eventbrite.co.uk].freeze def self.whitelist_pattern - /^https:\/\/www.eventbrite\.(com|co.uk)\/o\/[A-Za-z0-9-]+/ + %r{^https://www.eventbrite\.(com|co.uk)/o/[A-Za-z0-9-]+} end def organizer_id @@ -20,7 +20,7 @@ def organizer_id end def download_calendar - EventbriteSDK.token = ENV['EVENTBRITE_TOKEN'] + EventbriteSDK.token = ENV.fetch('EVENTBRITE_TOKEN', nil) @events = [] results = EventbriteSDK::Organizer.retrieve(id: organizer_id).events.with_expansion(:venue).page(1) diff --git a/app/jobs/calendar_importer/parsers/ics.rb b/app/jobs/calendar_importer/parsers/ics.rb index 3482e0542..e00dde45f 100644 --- a/app/jobs/calendar_importer/parsers/ics.rb +++ b/app/jobs/calendar_importer/parsers/ics.rb @@ -15,7 +15,7 @@ class Ics < Base outlook.live.com ics.teamup.com webcal:// - ] + ].freeze def self.whitelist_pattern whitelists = { @@ -60,9 +60,8 @@ def digest(data) def parse_remote_calendars(data) Icalendar::Calendar.parse data - rescue StandardError => e - # FIXME this should set an error flag that is checked by the importer and + # FIXME: this should set an error flag that is checked by the importer and # logged to the calendar [] end diff --git a/app/jobs/calendar_importer/parsers/manchester_uni.rb b/app/jobs/calendar_importer/parsers/manchester_uni.rb index 545583d18..c1e32e5a2 100644 --- a/app/jobs/calendar_importer/parsers/manchester_uni.rb +++ b/app/jobs/calendar_importer/parsers/manchester_uni.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # In order for a parser to be recognized, it must be added # to the PARSERS constant list in app/models/calendar_parser.rb. # Parent parser classes should not be added. @@ -6,10 +8,10 @@ module CalendarImporter::Parsers class ManchesterUni < Xml PUBLIC = false NAME = 'Manchester University' - DOMAINS = %w[events.manchester.ac.uk] + DOMAINS = %w[events.manchester.ac.uk].freeze def self.whitelist_pattern - /^http(s)?:\/\/events.manchester.ac.uk\/f3vf\/calendar\/.*/ + %r{^http(s)?://events.manchester.ac.uk/f3vf/calendar/.*} end def import_events_from(data) @@ -21,6 +23,5 @@ def import_events_from(data) events end - end end diff --git a/app/jobs/calendar_importer/parsers/meetup.rb b/app/jobs/calendar_importer/parsers/meetup.rb index ae4f92131..05b77cc08 100644 --- a/app/jobs/calendar_importer/parsers/meetup.rb +++ b/app/jobs/calendar_importer/parsers/meetup.rb @@ -8,15 +8,15 @@ module CalendarImporter::Parsers class Meetup < Base NAME = 'Meetup' KEY = 'meetup' - DOMAINS = %w[www.meetup.com] + DOMAINS = %w[www.meetup.com].freeze def self.whitelist_pattern - /^https:\/\/www\.meetup\.com\/[^\/]*\/?$/ + %r{^https://www\.meetup\.com/[^/]*/?$} end def download_calendar - user_name = (@url =~ /^https:\/\/www\.meetup\.com\/([^\/]*)\/?$/) && $1 - return [] unless user_name.present? + user_name = (@url =~ %r{^https://www\.meetup\.com/([^/]*)/?$}) && Regexp.last_match(1) + return [] if user_name.blank? api_url = "https://api.meetup.com/#{user_name}/events" response = HTTParty.get(api_url).body diff --git a/app/jobs/calendar_importer/parsers/ticketsolve.rb b/app/jobs/calendar_importer/parsers/ticketsolve.rb index 10fd6dcc3..33abaea65 100644 --- a/app/jobs/calendar_importer/parsers/ticketsolve.rb +++ b/app/jobs/calendar_importer/parsers/ticketsolve.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + # In order for a parser to be recognized, it must be added # to the PARSERS constant list in app/models/calendar_parser.rb. # Parent parser classes should not be added. @@ -6,7 +8,7 @@ module CalendarImporter::Parsers class Ticketsolve < Xml NAME = 'Ticket Solve' KEY = 'ticket-solve' - DOMAINS = %w[*.ticketsolve.com] + DOMAINS = %w[*.ticketsolve.com].freeze def self.whitelist_pattern %r{^https?://([^.]*).ticketsolve.com/?} @@ -21,6 +23,5 @@ def import_events_from(data) @events end - end end diff --git a/app/jobs/calendar_importer_job.rb b/app/jobs/calendar_importer_job.rb index 41a38bfd0..1293ed9a3 100644 --- a/app/jobs/calendar_importer_job.rb +++ b/app/jobs/calendar_importer_job.rb @@ -1,17 +1,20 @@ +# frozen_string_literal: true + class CalendarImporterJob < ApplicationJob queue_as :default rescue_from CalendarImporter::CalendarImporter::UnsupportedFeed do |exception| - report_error exception, "Calendar URL is not supported" + report_error exception, 'Calendar URL is not supported' end rescue_from CalendarImporter::CalendarImporter::InaccessibleFeed do |exception| - report_error exception, "Calendar URL is not accessible" + report_error exception, 'Calendar URL is not accessible' end rescue_from ActiveRecord::ActiveRecordError do |exception| - raise exception if Rails.env != 'production' && @silence_db_exceptions == false - report_error exception, "Internal database error" + raise exception if !Rails.env.production? && @silence_db_exceptions == false + + report_error exception, 'Internal database error' end def calendar @@ -21,8 +24,7 @@ def calendar # Imports all events from a given calendar # @param calendar_id [int] The ID of the Calendar object to import from # @param from_date [Date] The Date from which to import from - def perform(calendar_id, from_date, force_import, silence_db_exceptions=false) - + def perform(calendar_id, from_date, force_import, silence_db_exceptions = false) Calendar.record_timestamps = false @silence_db_exceptions = silence_db_exceptions @@ -32,7 +34,7 @@ def perform(calendar_id, from_date, force_import, silence_db_exceptions=false) print "Importing events for calendar #{calendar.name} (ID #{calendar.id})" print " for place #{calendar.place.name} (ID #{calendar.place.id})" if calendar.place - print " is forced" if force_import + print ' is forced' if force_import print "\n" # calendar.import_events(from) diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 921a4fb86..16de3da18 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -1,5 +1,6 @@ +# frozen_string_literal: true + class ApplicationMailer < ActionMailer::Base default from: 'no-reply@placecal.org' layout 'mailer' end - diff --git a/app/mailers/join_mailer.rb b/app/mailers/join_mailer.rb index 26050a0fb..3d61560ce 100644 --- a/app/mailers/join_mailer.rb +++ b/app/mailers/join_mailer.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class JoinMailer < ApplicationMailer def join_us(join) @join = join diff --git a/app/models/address.rb b/app/models/address.rb index c156973cd..48098bf9c 100644 --- a/app/models/address.rb +++ b/app/models/address.rb @@ -2,8 +2,7 @@ # app/models/address.rb class Address < ApplicationRecord - - POSTCODE_REGEX = /\s*((GIR\s*0AA)|((([A-PR-UWYZ][0-9]{1,2})|(([A-PR-UWYZ][A-HK-Y][0-9]{1,2})|(([A-PR-UWYZ][0-9][A-HJKSTUW])|([A-PR-UWYZ][A-HK-Y][0-9][ABEHMNPRVWXY]))))\s*[0-9][ABD-HJLNP-UW-Z]{2}))\s*/i + POSTCODE_REGEX = /\s*((GIR\s*0AA)|((([A-PR-UWYZ][0-9]{1,2})|(([A-PR-UWYZ][A-HK-Y][0-9]{1,2})|(([A-PR-UWYZ][0-9][A-HJKSTUW])|([A-PR-UWYZ][A-HK-Y][0-9][ABEHMNPRVWXY]))))\s*[0-9][ABD-HJLNP-UW-Z]{2}))\s*/i.freeze validates :street_address, :postcode, :country_code, presence: true validates :postcode, format: { with: POSTCODE_REGEX, message: 'is invalid' } @@ -40,27 +39,24 @@ def first_address_line # Needed for schema.org outputs as streetAddress def full_street_address - [ street_address, - street_address2, - street_address3 - ].reject(&:blank?).join(', ') + [street_address, + street_address2, + street_address3].compact_blank.join(', ') end def other_address_lines - [ street_address2, - street_address3, - city, - postcode - ].reject(&:blank?) + [street_address2, + street_address3, + city, + postcode].compact_blank end def all_address_lines - [ street_address, - street_address2, - street_address3, - city, - postcode - ].reject(&:blank?) + [street_address, + street_address2, + street_address3, + city, + postcode].compact_blank end def last_line_of_address @@ -125,10 +121,10 @@ def build_from_components(components, postcode) return if components.blank? address = Address.new( - street_address: components[0]&.strip, + street_address: components[0]&.strip, street_address2: components[1]&.strip, street_address3: components[2]&.strip, - postcode: postcode + postcode: postcode ) address if address.save end @@ -139,6 +135,7 @@ def build_from_components(components, postcode) # before the final three characters. def standardised_postcode(pc) return unless pc + pc.gsub(/\s+/, '').strip.upcase.insert(-4, ' ') end end diff --git a/app/models/article.rb b/app/models/article.rb index c208f7d18..97ea58604 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -29,8 +29,8 @@ class Article < ApplicationRecord scope :with_partner_tag, lambda { |tag_id| joins('left outer join article_partners on articles.id=article_partners.article_id') - .joins('left outer join partner_tags on article_partners.partner_id = partner_tags.partner_id') - .where('partner_tags.tag_id = ?', tag_id) + .joins('left outer join partner_tags on article_partners.partner_id = partner_tags.partner_id') + .where('partner_tags.tag_id = ?', tag_id) } scope :with_tags, lambda { |tag_ids| @@ -38,7 +38,6 @@ class Article < ApplicationRecord } scope :for_site, lambda { |site| - # this is a bit complicated but necessary # the main problem to overcome is that we want articles by tag OR location # (emphasis on OR). @@ -75,9 +74,9 @@ class Article < ApplicationRecord if site_neighbourhood_ids.any? # TODO: service areas? scope = scope - .joins('LEFT OUTER JOIN article_partners ON articles.id=article_partners.article_id') - .joins('LEFT OUTER JOIN partners ON article_partners.partner_id = partners.id') - .joins('LEFT OUTER JOIN addresses ON partners.address_id = addresses.id') + .joins('LEFT OUTER JOIN article_partners ON articles.id=article_partners.article_id') + .joins('LEFT OUTER JOIN partners ON article_partners.partner_id = partners.id') + .joins('LEFT OUTER JOIN addresses ON partners.address_id = addresses.id') where_fragments << 'addresses.neighbourhood_id IN (?)' where_params << site_neighbourhood_ids end @@ -85,20 +84,20 @@ class Article < ApplicationRecord # articles by tag if site_tag_ids.any? scope = scope - .joins(' LEFT OUTER JOIN article_tags ON articles.id=article_tags.article_id') + .joins(' LEFT OUTER JOIN article_tags ON articles.id=article_tags.article_id') where_fragments << 'article_tags.tag_id IN (?)' where_params << site_tag_ids end # combine conditions with params to extend the scope scope = scope - .where("(#{where_fragments.join(' OR ')})", *where_params) + .where("(#{where_fragments.join(' OR ')})", *where_params) scope.distinct('articles.id') } def update_published_at - self.published_at = self.is_draft ? nil : DateTime.now + self.published_at = is_draft ? nil : DateTime.now end # This retrieves the author's name for use in the GQL output @@ -114,6 +113,6 @@ def highres_image end def slug_candidates - [ %i[title id] ] + [%i[title id]] end end diff --git a/app/models/article_partner.rb b/app/models/article_partner.rb index c7e483fef..48af9be66 100644 --- a/app/models/article_partner.rb +++ b/app/models/article_partner.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ArticlePartner < ApplicationRecord belongs_to :article belongs_to :partner diff --git a/app/models/article_tag.rb b/app/models/article_tag.rb index f4603e9a5..96ab77757 100644 --- a/app/models/article_tag.rb +++ b/app/models/article_tag.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ArticleTag < ApplicationRecord belongs_to :article belongs_to :tag diff --git a/app/models/calendar.rb b/app/models/calendar.rb index e9fa7930a..5d7c9e49e 100644 --- a/app/models/calendar.rb +++ b/app/models/calendar.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true + # app/models/calendar.rb class Calendar < ApplicationRecord include ActionView::Helpers::DateHelper include Validation extend Enumerize - CALENDAR_REGEX = /\A(?:(?:(https?|webcal)):\/\/)(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:\/[^\s]*)?\z/i + CALENDAR_REGEX = %r{\A(?:(?:(https?|webcal))://)(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:/[^\s]*)?\z}i.freeze self.inheritance_column = nil @@ -45,7 +46,6 @@ class Calendar < ApplicationRecord scope :where_idle, -> { where(calendar_state: :idle) } scope :where_errored, -> { where(calendar_state: :error) } - # We need a default location for some strategies def requires_default_location? %i[place room_number event_override].include? strategy.to_sym @@ -55,10 +55,10 @@ def requires_default_location? # This uses PaperTrail to get historical records of the Event models, including deletes # It does this to show a "event added" / "event removed" thing def recent_activity - versions = PaperTrail::Version.with_item_keys('Event', self.event_ids).where('created_at >= ?', 2.weeks.ago) + versions = PaperTrail::Version.with_item_keys('Event', event_ids).where('created_at >= ?', 2.weeks.ago) versions = versions.or(PaperTrail::Version.destroys .where("item_type = 'Event' AND object @> ? AND created_at >= ?", - { calendar_id: self.id }.to_json, 2.weeks.ago)) + { calendar_id: id }.to_json, 2.weeks.ago)) versions = versions.order(created_at: :desc).group_by { |version| version.created_at.to_date } end @@ -71,11 +71,11 @@ def events_this_week # Who should be contacted about this calendar? def contact_information if public_contact_email - [ public_contact_email, public_contact_name ] + [public_contact_email, public_contact_name] elsif partner&.public_email - [ partner.public_email, partner.public_name ] + [partner.public_email, partner.public_name] elsif place&.public_email - [ place.public_email, place.public_name ] + [place.public_email, place.public_name] else false end @@ -99,7 +99,6 @@ def source_supported # The calendar importer will raise an exception if the source # URL has a problem CalendarImporter::CalendarImporter.new(self) - rescue CalendarImporter::CalendarImporter::InaccessibleFeed, CalendarImporter::CalendarImporter::UnsupportedFeed => e flag_error_import_job! e.to_s end @@ -122,7 +121,7 @@ def queue_for_import!(force_import, from_date) update! calendar_state: :in_queue - CalendarImporterJob.perform_later self.id, from_date, force_import + CalendarImporterJob.perform_later id, from_date, force_import end end @@ -132,6 +131,7 @@ def queue_for_import!(force_import, from_date) def flag_start_import_job! transaction do return unless calendar_state.in_queue? + update! calendar_state: :in_worker end end diff --git a/app/models/concerns/html_render_cache.rb b/app/models/concerns/html_render_cache.rb index f112c5106..09d5f8041 100644 --- a/app/models/concerns/html_render_cache.rb +++ b/app/models/concerns/html_render_cache.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true module HtmlRenderCache extend ActiveSupport::Concern @@ -27,17 +28,17 @@ def render_html_fields if changed.include?(field_name.to_s) value = changes[field_name.to_s][1] html = Kramdown::Document.new(value).to_html - write_attribute cache_name, html + self[cache_name] = html next end # puts attributes.to_json - if @force_html_generation - value = attributes[field_name.to_s].to_s - html = Kramdown::Document.new(value).to_html - write_attribute cache_name, html - end + next unless @force_html_generation + + value = attributes[field_name.to_s].to_s + html = Kramdown::Document.new(value).to_html + self[cache_name] = html end end end diff --git a/app/models/concerns/validation.rb b/app/models/concerns/validation.rb index e9121faf5..b4b1ebab9 100644 --- a/app/models/concerns/validation.rb +++ b/app/models/concerns/validation.rb @@ -1,11 +1,13 @@ +# frozen_string_literal: true + module Validation - URL_REGEX = /\A(?:(?:https?):\/\/)(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:\/[^\s]*)?\z/i + URL_REGEX = %r{\A(?:(?:https?)://)(?:\S+(?::\S*)?@)?(?:(?!10(?:\.\d{1,3}){3})(?!127(?:\.\d{1,3}){3})(?!169\.254(?:\.\d{1,3}){2})(?!192\.168(?:\.\d{1,3}){2})(?!172\.(?:1[6-9]|2\d|3[0-1])(?:\.\d{1,3}){2})(?:[1-9]\d?|1\d\d|2[01]\d|22[0-3])(?:\.(?:1?\d{1,2}|2[0-4]\d|25[0-5])){2}(?:\.(?:[1-9]\d?|1\d\d|2[0-4]\d|25[0-4]))|(?:(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)(?:\.(?:[a-z\u00a1-\uffff0-9]+-?)*[a-z\u00a1-\uffff0-9]+)*(?:\.(?:[a-z\u00a1-\uffff]{2,})))(?::\d{2,5})?(?:/[^\s]*)?\z}i.freeze - TWITTER_REGEX = /\A@?(\w){1,15}\z/ + TWITTER_REGEX = /\A@?(\w){1,15}\z/.freeze - FACEBOOK_REGEX = /\A(\w){1,50}\z/ + FACEBOOK_REGEX = /\A(\w){1,50}\z/.freeze - UK_NUMBER_REGEX = /\A(?:(?:\(?(?:0(?:0|11)\)?[\s-]?\(?|\+)44\)?[\s-]?(?:\(?0\)?[\s-]?)?)|(?:\(?0))(?:(?:\d{5}\)?[\s-]?\d{4,5})|(?:\d{4}\)?[\s-]?(?:\d{5}|\d{3}[\s-]?\d{3}))|(?:\d{3}\)?[\s-]?\d{3}[\s-]?\d{3,4})|(?:\d{2}\)?[\s-]?\d{4}[\s-]?\d{4}))(?:[\s-]?(?:x|ext\.?|\#)\d{3,4})?\z/ + UK_NUMBER_REGEX = /\A(?:(?:\(?(?:0(?:0|11)\)?[\s-]?\(?|\+)44\)?[\s-]?(?:\(?0\)?[\s-]?)?)|(?:\(?0))(?:(?:\d{5}\)?[\s-]?\d{4,5})|(?:\d{4}\)?[\s-]?(?:\d{5}|\d{3}[\s-]?\d{3}))|(?:\d{3}\)?[\s-]?\d{3}[\s-]?\d{3,4})|(?:\d{2}\)?[\s-]?\d{4}[\s-]?\d{4}))(?:[\s-]?(?:x|ext\.?|\#)\d{3,4})?\z/.freeze - EMAIL_REGEX = /\A([\w+\-].?)+@[a-z\d\-]+(\.[a-z]+)*\.[a-z]+\z/i + EMAIL_REGEX = /\A([\w+\-].?)+@[a-z\d\-]+(\.[a-z]+)*\.[a-z]+\z/i.freeze end diff --git a/app/models/event.rb b/app/models/event.rb index 42ddd2c5b..70201a034 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -86,7 +86,7 @@ class Event < ApplicationRecord scope :sort_by_summary, -> { order(summary: :asc).order(:dtstart) } scope :sort_by_time, -> { order(dtstart: :asc).order(summary: :asc) } - scope :without_matching_times, ->(start_times, end_times) { + scope :without_matching_times, lambda { |start_times, end_times| where.not(dtstart: start_times).or(where.not(dtend: end_times)) } @@ -110,7 +110,7 @@ def sanitize_rrule def time if dtend - dtstart.strftime('%H:%M') + ' – ' + dtend.strftime('%H:%M') + "#{dtstart.strftime('%H:%M')} – #{dtend.strftime('%H:%M')}" else dtstart.strftime('%H:%M') end @@ -166,10 +166,10 @@ def require_location return if %w[event no_location online_only].include?(calendar&.strategy) # If we have an online address we don't need a physical one - return if self.online_address_id.present? + return if online_address_id.present? # If the address exists then the error doesn't apply - return unless self.address_id.blank? + return if address_id.present? errors.add(:base, 'No place or address could be created or found for ' \ "the event location: #{raw_location_from_source}") diff --git a/app/models/join.rb b/app/models/join.rb index ca49e6a88..70186cc27 100644 --- a/app/models/join.rb +++ b/app/models/join.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class Join include ActiveModel::Model @@ -9,10 +11,9 @@ class Join def submit if valid? JoinMailer.join_us(self).deliver - return true + true else - return false + false end end end - diff --git a/app/models/neighbourhood.rb b/app/models/neighbourhood.rb index 9bb88456b..c4a7dd5f3 100644 --- a/app/models/neighbourhood.rb +++ b/app/models/neighbourhood.rb @@ -10,9 +10,9 @@ class Neighbourhood < ApplicationRecord has_many :service_areas, dependent: :destroy has_many :service_area_partners, - through: :service_areas, - source: :partner, - class_name: 'Partner' + through: :service_areas, + source: :partner, + class_name: 'Partner' # validates :name, presence: true validates :unit_code_value, @@ -27,7 +27,7 @@ def shortname elsif name.present? name else - "[not set]" + '[not set]' end end @@ -57,7 +57,7 @@ def fullname # == Returns # A string of the name def abbreviated_name - name_abbr.present? ? name_abbr : name + name_abbr.presence || name end # normalize abbreviated names (usually coming from calendar @@ -72,7 +72,7 @@ def abbreviated_name def name_abbr=(value) value = value.to_s.strip - self['name_abbr'] = value.present? ? value : nil + self['name_abbr'] = value.presence end def to_s diff --git a/app/models/organisation_relationship.rb b/app/models/organisation_relationship.rb index aff6d1a51..a1e34667b 100644 --- a/app/models/organisation_relationship.rb +++ b/app/models/organisation_relationship.rb @@ -4,6 +4,7 @@ class OrganisationRelationship < ApplicationRecord self.table_name = 'organisation_relationships' belongs_to :subject, class_name: 'Partner' - extend Enumerize; enumerize :verb, in: %i[manages] + extend Enumerize + enumerize :verb, in: %i[manages] belongs_to :object, class_name: 'Partner' end diff --git a/app/models/partner.rb b/app/models/partner.rb index d61a7f3c7..af99ef814 100644 --- a/app/models/partner.rb +++ b/app/models/partner.rb @@ -23,9 +23,9 @@ class Partner < ApplicationRecord has_many :service_areas, dependent: :destroy has_many :service_area_neighbourhoods, - through: :service_areas, - source: :neighbourhood, - class_name: 'Neighbourhood' + through: :service_areas, + source: :neighbourhood, + class_name: 'Neighbourhood' validates_associated :service_areas @@ -65,7 +65,7 @@ class Partner < ApplicationRecord } validates :summary, presence: { - if: ->(p) { !p.description.blank? }, + if: ->(p) { p.description.present? }, message: 'cannot have a description without a summary' } validates :url, @@ -127,8 +127,8 @@ class Partner < ApplicationRecord site_tag_ids = site.tags.map(&:id) if site_tag_ids.any? query = query - .left_joins(:partner_tags) - .where("partner_tags.tag_id in (?)", site_tag_ids) + .left_joins(:partner_tags) + .where('partner_tags.tag_id in (?)', site_tag_ids) end # now look for addresses and service areas @@ -141,7 +141,8 @@ class Partner < ApplicationRecord .left_joins(:address, :service_areas) .where( '(service_areas.neighbourhood_id in (:neighbourhood_ids) OR addresses.neighbourhood_id in (:neighbourhood_ids))', - neighbourhood_ids: site_neighbourhood_ids) + neighbourhood_ids: site_neighbourhood_ids + ) .distinct } @@ -149,11 +150,11 @@ class Partner < ApplicationRecord return none if tag.nil? query = Partner - .select('"partners".*, LOWER("partners"."name") as sortable_name') + .select('"partners".*, LOWER("partners"."name") as sortable_name') query = query - .left_joins(:partner_tags) - .where(partner_tags: { tag: tag }) + .left_joins(:partner_tags) + .where(partner_tags: { tag: tag }) # now look for addresses and service areas site_neighbourhood_ids = site.owned_neighbourhoods.map(&:id) @@ -167,7 +168,8 @@ class Partner < ApplicationRecord .left_joins(:address, :service_areas) .where( '(service_areas.neighbourhood_id in (:neighbourhood_ids) OR addresses.neighbourhood_id in (:neighbourhood_ids))', - neighbourhood_ids: site_neighbourhood_ids) + neighbourhood_ids: site_neighbourhood_ids + ) .distinct .order('sortable_name') } @@ -176,15 +178,15 @@ class Partner < ApplicationRecord # # @param tags [ArrayThis is a meetup description!
' }, + name: { text: 'A summary' }, + description: { text: 'This is a meetup description!
' }, venue: nil, time: @start_date.to_i, utc_offset: (@end_date.to_i - @start_date.to_i), @@ -120,7 +122,7 @@ class EventsResolverTest < ActiveSupport::TestCase resolver = CalendarImporter::EventResolver.new(ics_event_data, calendar, [], @start_date) resolver.determine_online_location - assert resolver.data.online_address_id.present? + assert_predicate resolver.data.online_address_id, :present? online_address = OnlineAddress.find(resolver.data.online_address_id) assert_equal online_address.url, @fake_ics_event['custom_properties']['x_google_conference'].first @@ -136,7 +138,7 @@ class EventsResolverTest < ActiveSupport::TestCase resolver = CalendarImporter::EventResolver.new(ics_event_data, calendar, [], @start_date) resolver.determine_online_location - assert resolver.data.online_address_id.present? + assert_predicate resolver.data.online_address_id, :present? online_address = OnlineAddress.find(resolver.data.online_address_id) assert_equal online_address.url, jitsi_link @@ -152,7 +154,7 @@ class EventsResolverTest < ActiveSupport::TestCase resolver = CalendarImporter::EventResolver.new(ics_event_data, calendar, [], @start_date) resolver.determine_online_location - assert resolver.data.online_address_id.present? + assert_predicate resolver.data.online_address_id, :present? online_address = OnlineAddress.find(resolver.data.online_address_id) assert_equal online_address.url, meet_link @@ -168,7 +170,7 @@ class EventsResolverTest < ActiveSupport::TestCase resolver = CalendarImporter::EventResolver.new(ics_event_data, calendar, [], @start_date) resolver.determine_online_location - assert resolver.data.online_address_id.present? + assert_predicate resolver.data.online_address_id, :present? online_address = OnlineAddress.find(resolver.data.online_address_id) assert_equal online_address.url, zoom_link @@ -184,7 +186,7 @@ class EventsResolverTest < ActiveSupport::TestCase resolver = CalendarImporter::EventResolver.new(event_data, calendar, [], @start_date) resolver.determine_online_location - assert resolver.data.online_address_id.present? + assert_predicate resolver.data.online_address_id, :present? online_address = OnlineAddress.find(resolver.data.online_address_id) assert_equal online_address.url, eventbrite_link @@ -200,7 +202,7 @@ class EventsResolverTest < ActiveSupport::TestCase resolver = CalendarImporter::EventResolver.new(event_data, calendar, [], @start_date) resolver.determine_online_location - assert resolver.data.online_address_id.present? + assert_predicate resolver.data.online_address_id, :present? online_address = OnlineAddress.find(resolver.data.online_address_id) assert_equal online_address.url, meetup_link diff --git a/test/jobs/calendar_importer/events_import_test.rb b/test/jobs/calendar_importer/events_import_test.rb index cde42b6a9..9bc1a8b58 100644 --- a/test/jobs/calendar_importer/events_import_test.rb +++ b/test/jobs/calendar_importer/events_import_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class EventsImportTest < ActiveSupport::TestCase diff --git a/test/mailers/join_mailer_test.rb b/test/mailers/join_mailer_test.rb index e440fbece..e6c084003 100644 --- a/test/mailers/join_mailer_test.rb +++ b/test/mailers/join_mailer_test.rb @@ -1,4 +1,6 @@ -require "test_helper" +# frozen_string_literal: true + +require 'test_helper' describe JoinMailer do # it "does a thing" do diff --git a/test/mailers/previews/join_mailer_preview.rb b/test/mailers/previews/join_mailer_preview.rb index 50640ffe2..4426934c5 100644 --- a/test/mailers/previews/join_mailer_preview.rb +++ b/test/mailers/previews/join_mailer_preview.rb @@ -1,4 +1,5 @@ +# frozen_string_literal: true + # Preview all emails at http://localhost:3000/rails/mailers/contact_mailer class JoinMailerPreview < ActionMailer::Preview - end diff --git a/test/models/address_test.rb b/test/models/address_test.rb index d3a67f14f..fcf560e26 100644 --- a/test/models/address_test.rb +++ b/test/models/address_test.rb @@ -21,7 +21,7 @@ class AddressTest < ActiveSupport::TestCase ) address.save! - assert address.postcode == expected_output + assert_equal address.postcode, expected_output end end end diff --git a/test/models/article_partner_test.rb b/test/models/article_partner_test.rb index b42ae5cac..0f750c65c 100644 --- a/test/models/article_partner_test.rb +++ b/test/models/article_partner_test.rb @@ -1,4 +1,6 @@ -require "test_helper" +# frozen_string_literal: true + +require 'test_helper' class ArticlePartnerTest < ActiveSupport::TestCase setup do @@ -8,6 +10,6 @@ class ArticlePartnerTest < ActiveSupport::TestCase test 'create article for partner' do @partner.articles << @article - assert_equal @partner.articles.count, 1 + assert_equal(1, @partner.articles.count) end -end \ No newline at end of file +end diff --git a/test/models/article_test.rb b/test/models/article_test.rb index 48150066a..7fd5a3b79 100644 --- a/test/models/article_test.rb +++ b/test/models/article_test.rb @@ -10,21 +10,21 @@ class ArticleTest < ActiveSupport::TestCase end test 'article should be valid' do - assert @article.valid? + assert_predicate @article, :valid? end test 'title should be present' do @article.title = '' - refute @article.valid? + assert_not_predicate @article, :valid? end test 'body should be present' do @article.body = '' - refute @article.valid? + assert_not_predicate @article, :valid? end test 'published_at updates correctly when is_draft is set' do - assert @article_draft.published_at.nil? + assert_nil @article_draft.published_at @article_draft.is_draft = false assert @article_draft.save! @@ -40,7 +40,7 @@ class ArticleTest < ActiveSupport::TestCase 4.times do |n| Article.create!( title: "Article title no. #{n}", - body: "lorem ipsum ...", + body: 'lorem ipsum ...', author: user ) end @@ -49,7 +49,7 @@ class ArticleTest < ActiveSupport::TestCase 2.times do |n| article = Article.create!( title: "Article title no. #{n}", - body: "lorem ipsum ...", + body: 'lorem ipsum ...', author: user ) article.tags << tag @@ -69,7 +69,7 @@ class ArticleTest < ActiveSupport::TestCase 3.times do |n| Article.create!( title: "Article title no. #{n}", - body: "lorem ipsum ...", + body: 'lorem ipsum ...', author: user ) end @@ -78,7 +78,7 @@ class ArticleTest < ActiveSupport::TestCase 5.times do |n| article = Article.create!( title: "Article title no. #{n}", - body: "lorem ipsum ...", + body: 'lorem ipsum ...', author: user ) article.partners << partner @@ -203,6 +203,6 @@ class ArticleTest < ActiveSupport::TestCase art = create(:article) art.body = 'A body of text about something' art.save! - assert art.body_html.present? + assert_predicate art.body_html, :present? end end diff --git a/test/models/calendar_state_test.rb b/test/models/calendar_state_test.rb index f27ae4519..c449626f3 100644 --- a/test/models/calendar_state_test.rb +++ b/test/models/calendar_state_test.rb @@ -9,7 +9,7 @@ class CalendarStateTest < ActiveSupport::TestCase # idle test 'is idle by default' do - assert Calendar.new.calendar_state.idle? + assert_predicate Calendar.new.calendar_state, :idle? end # queueing @@ -17,7 +17,7 @@ class CalendarStateTest < ActiveSupport::TestCase assert_enqueued_jobs 0 calendar = create(:calendar) calendar.queue_for_import! false, Date.new(2000, 1, 1) - assert calendar.calendar_state.in_queue? + assert_predicate calendar.calendar_state, :in_queue? assert_enqueued_jobs 1 end @@ -27,38 +27,38 @@ class CalendarStateTest < ActiveSupport::TestCase calendar.queue_for_import! false, Date.new(2000, 1, 1) # this has not changed - assert calendar.calendar_state.in_queue? + assert_predicate calendar.calendar_state, :in_queue? # not in queue assert_enqueued_jobs 0 end - test "can move into working state" do + test 'can move into working state' do VCR.use_cassette(:import_test_calendar) do calendar = create(:calendar, calendar_state: :in_queue, source: SOURCE_URL) calendar.flag_start_import_job! - assert calendar.calendar_state.in_worker? + assert_predicate calendar.calendar_state, :in_worker? end end # in worker - test "can move into idle state" do + test 'can move into idle state' do VCR.use_cassette(:import_test_calendar) do calendar = create(:calendar, calendar_state: :in_worker, source: SOURCE_URL) calendar.flag_complete_import_job! [], 0, 'ical' - assert calendar.calendar_state.idle? + assert_predicate calendar.calendar_state, :idle? # are we close enough? # deal with weird database encoding serialisations assert_in_delta Time.now.to_i, calendar.last_import_at.to_i, 24.hours end end - test "can move into error state" do + test 'can move into error state' do VCR.use_cassette(:import_test_calendar) do bad_message = 'A description of the error' calendar = create(:calendar, calendar_state: :in_worker, source: SOURCE_URL) calendar.flag_error_import_job! bad_message - assert calendar.calendar_state.error? + assert_predicate calendar.calendar_state, :error? assert_equal bad_message, calendar.critical_error end end @@ -80,25 +80,25 @@ class CalendarStateTest < ActiveSupport::TestCase # set some invalid state calendar.name = nil - assert (calendar.valid? == false), 'Calendar should be invalid' + assert_predicate calendar, :invalid?, 'Calendar should be invalid' + # assert_equal (calendar.valid?, false), 'Calendar should be invalid' # flag bad thing calendar.flag_error_import_job! bad_message # check the calendar is back to how it was assert_equal name, calendar.name - assert calendar.valid?, 'Calendar should be valid now' + assert_predicate calendar, :valid?, 'Calendar should be valid now' # check the error was saved to calendar - assert calendar.calendar_state.error? + assert_predicate calendar.calendar_state, :error? assert_equal bad_message, calendar.critical_error end end # error'd - test "can be tested for" do + test 'can be tested for' do calendar = create(:calendar, calendar_state: :error) - assert calendar.calendar_state.error? + assert_predicate calendar.calendar_state, :error? end - end diff --git a/test/models/calendar_test.rb b/test/models/calendar_test.rb index ee40e215d..8a4785628 100644 --- a/test/models/calendar_test.rb +++ b/test/models/calendar_test.rb @@ -10,50 +10,47 @@ class CalendarTest < ActiveSupport::TestCase test 'has required fields' do # Must have a name and source URL - refute @calendar.valid? - @calendar.name = "A name for the calendar" - refute @calendar.valid? - @calendar.source = "https://my-calendar.com" - refute @calendar.valid? + assert_not_predicate @calendar, :valid? + @calendar.name = 'A name for the calendar' + assert_not_predicate @calendar, :valid? + @calendar.source = 'https://my-calendar.com' + assert_not_predicate @calendar, :valid? @calendar.partner = create(:partner) - refute @calendar.valid? + assert_not_predicate @calendar, :valid? @calendar.place = create(:partner) - assert @calendar.valid? + assert_predicate @calendar, :valid? @calendar.save # Sources must be unique @existing_calendar = create(:calendar) - @existing_calendar.update(source: "https://my-calendar.com") - refute @existing_calendar.valid? - assert_equal ["calendar source already in use"], @existing_calendar.errors[:source] + @existing_calendar.update(source: 'https://my-calendar.com') + assert_not_predicate @existing_calendar, :valid? + assert_equal ['calendar source already in use'], @existing_calendar.errors[:source] end test 'gets a contact for each calendar' do @calendar = create(:calendar) - assert @calendar.valid? + assert_predicate @calendar, :valid? # If calendar contact listed, show that - assert_equal [ @calendar.public_contact_email, - @calendar.public_contact_name - ], @calendar.contact_information + assert_equal [@calendar.public_contact_email, + @calendar.public_contact_name], @calendar.contact_information # Otherwise, show the partner public contact if possible @calendar.update(public_contact_email: nil) - assert_equal [ @calendar.partner.public_email, - @calendar.partner.public_name - ], @calendar.contact_information + assert_equal [@calendar.partner.public_email, + @calendar.partner.public_name], @calendar.contact_information # Otherwise, show the default location contact if possible @calendar.partner.update(public_email: nil) - assert_equal [ @calendar.place.public_email, - @calendar.place.public_name - ], @calendar.contact_information + assert_equal [@calendar.place.public_email, + @calendar.place.public_name], @calendar.contact_information # Otherwise, return false @calendar.place.update(public_email: nil) - assert_equal false, @calendar.contact_information + assert_not @calendar.contact_information end - test "notices get counted when saved" do - messages = [ - "alpha", - "beta", - "cappa" + test 'notices get counted when saved' do + messages = %w[ + alpha + beta + cappa ] calendar = build(:calendar) @@ -61,14 +58,13 @@ class CalendarTest < ActiveSupport::TestCase calendar.save! assert_equal 3, calendar.notice_count - end - test "notices are not counted if notices have not changed value" do - messages = [ - "alpha", - "beta", - "cappa" + test 'notices are not counted if notices have not changed value' do + messages = %w[ + alpha + beta + cappa ] calendar = create(:calendar, notices: messages) diff --git a/test/models/events_test.rb b/test/models/events_test.rb index 8d410fc04..4db31e85c 100644 --- a/test/models/events_test.rb +++ b/test/models/events_test.rb @@ -25,7 +25,7 @@ class EventTest < ActiveSupport::TestCase **@event_data } assert Event.new(event_hash).save - refute Event.new(event_hash).valid? + assert_not_predicate Event.new(event_hash), :valid? assert_raises ActiveRecord::RecordInvalid do Event.new(event_hash).save! @@ -38,7 +38,7 @@ class EventTest < ActiveSupport::TestCase assert a.save b = Event.new(summary: 'foonlys get together :,)', **event_hash) - assert b.valid? + assert_predicate b, :valid? end test 'can create two events with the same summary and calendar but diff start' do @@ -46,8 +46,8 @@ class EventTest < ActiveSupport::TestCase a = Event.new(dtstart: DateTime.now - 1.hour, **event_hash) assert a.save - b = Event.new(dtstart: DateTime.now - 3.hour, **event_hash) - assert b.valid? + b = Event.new(dtstart: DateTime.now - 3.hours, **event_hash) + assert_predicate b, :valid? end test 'can create two events with the same summary and dtstart but diff calendar' do @@ -56,7 +56,7 @@ class EventTest < ActiveSupport::TestCase assert a.save b = Event.new(calendar: create(:calendar), **event_hash) - assert b.valid? + assert_predicate b, :valid? end test 'has blank location with no addresses at all' do @@ -105,7 +105,7 @@ class EventTest < ActiveSupport::TestCase events = Event.all.find_by_day(Date.today) - assert_equal events.length, 1 + assert_equal(1, events.length) assert_equal events.first.dtstart, todays_event.dtstart end @@ -123,7 +123,7 @@ class EventTest < ActiveSupport::TestCase events = Event.all.find_by_day(Date.today) - assert_equal events.length, 1 + assert_equal(1, events.length) assert_equal events.first.dtstart, todays_event.dtstart end @@ -140,7 +140,7 @@ class EventTest < ActiveSupport::TestCase events = Event.all.find_by_day(Date.today) - assert_equal events.length, 1 + assert_equal(1, events.length) assert_equal events.first.dtstart, tomorrows_event.dtstart end end diff --git a/test/models/neighbourhoods_test.rb b/test/models/neighbourhoods_test.rb index dbb9116a9..15ac6e567 100644 --- a/test/models/neighbourhoods_test.rb +++ b/test/models/neighbourhoods_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class NeighbourhoodsAncestryTest < ActiveSupport::TestCase @@ -13,35 +15,34 @@ class NeighbourhoodsAncestryTest < ActiveSupport::TestCase assert @neighbourhood.district assert @neighbourhood.country - assert @ashton_neighbourhood.district.name == 'Tameside' - assert @ashton_neighbourhood.district.unit == 'district' + assert_equal('Tameside', @ashton_neighbourhood.district.name) + assert_equal('district', @ashton_neighbourhood.district.unit) - assert @ashton_neighbourhood.county.name == 'Greater Manchester' - assert @ashton_neighbourhood.county.unit == 'county' + assert_equal('Greater Manchester', @ashton_neighbourhood.county.name) + assert_equal('county', @ashton_neighbourhood.county.unit) - assert @ashton_neighbourhood.region.name == 'North West' - assert @ashton_neighbourhood.region.unit == 'region' + assert_equal('North West', @ashton_neighbourhood.region.name) + assert_equal('region', @ashton_neighbourhood.region.unit) - assert @ashton_neighbourhood.country.name == 'England' - assert @ashton_neighbourhood.country.unit == 'country' + assert_equal('England', @ashton_neighbourhood.country.name) + assert_equal('country', @ashton_neighbourhood.country.unit) end test 'uses name if name_abbr is missing' do # missing altogether hood = Neighbourhood.new(name: 'neighbourhood') - assert hood.abbreviated_name == 'neighbourhood' + assert_equal('neighbourhood', hood.abbreviated_name) # is empty string hood = Neighbourhood.new(name: 'neighbourhood', name_abbr: '') - assert hood.abbreviated_name == 'neighbourhood' + assert_equal('neighbourhood', hood.abbreviated_name) # is a blank string hood = Neighbourhood.new(name: 'neighbourhood', name_abbr: ' ') - assert hood.abbreviated_name == 'neighbourhood' + assert_equal('neighbourhood', hood.abbreviated_name) # is fine with a value hood = Neighbourhood.new(name: 'neighbourhood', name_abbr: 'hood') - assert hood.abbreviated_name == 'hood' + assert_equal('hood', hood.abbreviated_name) end - end diff --git a/test/models/partner_address_or_service_area_permission_test.rb b/test/models/partner_address_or_service_area_permission_test.rb index b47010a4a..a90c944ee 100644 --- a/test/models/partner_address_or_service_area_permission_test.rb +++ b/test/models/partner_address_or_service_area_permission_test.rb @@ -5,32 +5,32 @@ class PartnerAddressOrServiceAreaPermissionsTest < ActiveSupport::TestCase setup do @user = create(:user) - @user_neighbourhood = neighbourhoods(:one) - + @user_neighbourhood = neighbourhoods(:one) + @user.neighbourhoods << @user_neighbourhood @new_partner = build( - :partner, + :partner, address: nil, accessed_by_user: @user ) end - test "valid if address is in user ward" do + test 'valid if address is in user ward' do @new_partner.address = create(:address, neighbourhood: @user_neighbourhood) @new_partner.save! - assert @new_partner.valid? + assert_predicate @new_partner, :valid? end - test "verify: with service area in user neighbourhoods" do + test 'verify: with service area in user neighbourhoods' do @new_partner.service_area_neighbourhoods << @user_neighbourhood @new_partner.save! - assert @new_partner.valid? + assert_predicate @new_partner, :valid? end - test "verify: with service area contained within users neighbourhood subtrees" do + test 'verify: with service area contained within users neighbourhood subtrees' do child_neighbourhood = create(:neighbourhood) parent_neighbourhood = child_neighbourhood.parent @@ -39,7 +39,7 @@ class PartnerAddressOrServiceAreaPermissionsTest < ActiveSupport::TestCase @new_partner.service_area_neighbourhoods << child_neighbourhood @new_partner.save! - assert @new_partner.valid? + assert_predicate @new_partner, :valid? end test "with a service area not in user's ward set" do @@ -48,6 +48,6 @@ class PartnerAddressOrServiceAreaPermissionsTest < ActiveSupport::TestCase @new_partner.service_area_neighbourhoods << @user_neighbourhood @new_partner.service_area_neighbourhoods << other_neighbourhood - assert @new_partner.valid? == false, 'Users cannot create service areas outside of their neighbourhoods' + assert_not_predicate(@new_partner, :valid?, 'Users cannot create service areas outside of their neighbourhoods') end end diff --git a/test/models/partner_address_or_service_area_presence_test.rb b/test/models/partner_address_or_service_area_presence_test.rb index 6df034d26..73dd7c854 100644 --- a/test/models/partner_address_or_service_area_presence_test.rb +++ b/test/models/partner_address_or_service_area_presence_test.rb @@ -3,7 +3,6 @@ require 'test_helper' class PartnerAddressOrServiceAreaPresenceTest < ActiveSupport::TestCase - setup do @user = create(:root) @neighbourhood = neighbourhoods(:one) @@ -16,21 +15,20 @@ class PartnerAddressOrServiceAreaPresenceTest < ActiveSupport::TestCase ) end - test "is invalid if both service area and address not present" do - + test 'is invalid if both service area and address not present' do @new_partner.validate - assert @new_partner.valid? == false, 'Partner should be invalid' - + assert_not_predicate(@new_partner, :valid?, 'Partner should be invalid') + base_errors = @new_partner.errors[:base] - assert base_errors.length > 0 + assert_predicate base_errors.length, :positive? end test 'is valid with service_area set' do @new_partner.service_areas.build neighbourhood: @neighbourhood @new_partner.validate - assert @new_partner.valid? == true, 'Partner should be valid' + assert_predicate(@new_partner, :valid?, 'Partner should be valid') end test 'is valid with address set' do @@ -39,7 +37,7 @@ class PartnerAddressOrServiceAreaPresenceTest < ActiveSupport::TestCase @new_partner.address = address @new_partner.save! - assert @new_partner.valid? == true, 'Partner should be valid' + assert_predicate(@new_partner, :valid?, 'Partner should be valid') end test 'is valid with both service_area and address set' do @@ -49,6 +47,6 @@ class PartnerAddressOrServiceAreaPresenceTest < ActiveSupport::TestCase @new_partner.service_areas.build neighbourhood: @neighbourhood @new_partner.validate - assert @new_partner.valid? == true, 'Partner should valid' + assert_predicate(@new_partner, :valid?, 'Partner should valid') end end diff --git a/test/models/partner_service_area_test.rb b/test/models/partner_service_area_test.rb index 819b4d86b..48846b4bd 100644 --- a/test/models/partner_service_area_test.rb +++ b/test/models/partner_service_area_test.rb @@ -15,13 +15,13 @@ class PartnerServiceAreaTest < ActiveSupport::TestCase @partner.address = create(:address, neighbourhood: @neighbourhood) @partner.save! - assert @partner.valid?, 'Partner (without service_area) is not valid' + assert_predicate @partner, :valid?, 'Partner (without service_area) is not valid' end test 'is valid when set, can be accessed' do model = build(:ashton_service_area_partner) model.save! - assert model.valid? + assert_predicate model, :valid? service_areas = model.service_areas assert_equal 1, service_areas.count @@ -32,7 +32,7 @@ class PartnerServiceAreaTest < ActiveSupport::TestCase @partner.service_area_neighbourhoods << @neighbourhood @partner.save! - assert @partner.valid?, 'Partner (with service_area) is not valid' + assert_predicate @partner, :valid?, 'Partner (with service_area) is not valid' neighbourhood_count = @partner.service_area_neighbourhoods.count assert_equal 1, neighbourhood_count, 'count neighbourhoods' @@ -42,7 +42,7 @@ class PartnerServiceAreaTest < ActiveSupport::TestCase @partner.address = create(:address, neighbourhood: @neighbourhood) @partner.save! - assert_raises ActiveRecord::RecordInvalid do + assert_raises ActiveRecord::RecordInvalid do @partner.service_areas.create!(neighbourhood: @neighbourhood) @partner.service_areas.create!(neighbourhood: @neighbourhood) end @@ -58,7 +58,7 @@ class PartnerServiceAreaTest < ActiveSupport::TestCase @partner.service_areas.create! neighbourhood: other_neighbourhood neighbourhoods = @partner.service_area_neighbourhoods.order('neighbourhoods.name').all - assert neighbourhoods.count == 2, 'Failed to count neighbourhoods' + assert_equal(2, neighbourhoods.count, 'Failed to count neighbourhoods') n1 = neighbourhoods[0] assert_equal 'Ashton Hurst', n1.name @@ -71,7 +71,7 @@ class PartnerServiceAreaTest < ActiveSupport::TestCase @partner.service_areas.build neighbourhood: create(:moss_side_neighbourhood) @partner.validate - assert @partner.valid? == false, 'Partner should not be valid' + assert_not_predicate(@partner, :valid?, 'Partner should not be valid') end test 'can be set by root users' do @@ -82,7 +82,7 @@ class PartnerServiceAreaTest < ActiveSupport::TestCase @partner.service_area_neighbourhoods << other_neighbourhood @partner.save! - assert @partner.valid?, 'Partner (with service_area) should be valid' + assert_predicate @partner, :valid?, 'Partner (with service_area) should be valid' neighbourhood_count = @partner.service_area_neighbourhoods.count assert_equal 1, neighbourhood_count, 'count neighbourhoods' diff --git a/test/models/partner_site_scope_test.rb b/test/models/partner_site_scope_test.rb index 20c489121..239aa49bd 100644 --- a/test/models/partner_site_scope_test.rb +++ b/test/models/partner_site_scope_test.rb @@ -3,7 +3,6 @@ require 'test_helper' class PartnerSiteScopeTest < ActiveSupport::TestCase - # this verifies that partner#for_site is behaving # NOTE: these MUST match up with the geocoder response @@ -39,12 +38,12 @@ def address_one Neighbourhood.destroy_all end - test "empty site returns nothing" do + test 'empty site returns nothing' do output = Partner.for_site(site) - assert output.empty?, 'site should be empty' + assert_empty output, 'site should be empty' end - test "can find partners in site with address" do + test 'can find partners in site with address' do neighbourhood = geocodable_neighbourhood_one site.neighbourhoods << neighbourhood @@ -54,7 +53,7 @@ def address_one assert_equal 5, output.count # number of partners with addresses in site end - test "can find partners in site with service areas (without duplicates)" do + test 'can find partners in site with service areas (without duplicates)' do neighbourhood_a = create(:bare_neighbourhood) site.neighbourhoods << neighbourhood_a @@ -73,7 +72,7 @@ def address_one assert_equal 5, output.count end - test "can find partners by address and service_area" do + test 'can find partners by address and service_area' do neighbourhood_a = create(:bare_neighbourhood) site.neighbourhoods << neighbourhood_a @@ -92,7 +91,7 @@ def address_one assert_equal 2, output.count end - test "ignores partners on other sites" do + test 'ignores partners on other sites' do neighbourhood_a = create(:bare_neighbourhood) site.neighbourhoods << neighbourhood_a @@ -135,7 +134,7 @@ def address_one assert_equal 10, output.count end - test "only finds partners with tags if site has tags" do + test 'only finds partners with tags if site has tags' do neighbourhood = geocodable_neighbourhood_one tag = create(:tag) other_tag = create(:tag) diff --git a/test/models/partner_site_with_tag_scope_test.rb b/test/models/partner_site_with_tag_scope_test.rb index a8e25f04d..3a8959225 100644 --- a/test/models/partner_site_with_tag_scope_test.rb +++ b/test/models/partner_site_with_tag_scope_test.rb @@ -1,10 +1,8 @@ - # frozen_string_literal: true require 'test_helper' class PartnerSiteWithTagScopeTest < ActiveSupport::TestCase - # this verifies that partner#for_site is behaving # NOTE: these MUST match up with the geocoder response @@ -40,13 +38,13 @@ def address_one Neighbourhood.destroy_all end - test "empty site/tag returns nothing" do + test 'empty site/tag returns nothing' do tag = nil output = Partner.for_site_with_tag(site, tag) - assert output.empty?, 'site should be empty' + assert_empty output, 'site should be empty' end - test "finds partners with tag" do + test 'finds partners with tag' do tag = create(:tag) other_tag = create(:tag) @@ -72,5 +70,4 @@ def address_one output = Partner.for_site_with_tag(site, tag) assert_equal 4, output.all.length end - end diff --git a/test/models/partner_test.rb b/test/models/partner_test.rb index 366aeab5b..8b617c440 100644 --- a/test/models/partner_test.rb +++ b/test/models/partner_test.rb @@ -12,7 +12,7 @@ class PartnerTest < ActiveSupport::TestCase test 'updates user roles when saved' do @new_partner.users << @user @new_partner.save - assert @user.partner_admin? + assert_predicate @user, :partner_admin? end test 'can change postcode of partner' do @@ -21,15 +21,15 @@ class PartnerTest < ActiveSupport::TestCase new_postcode = 'OL6 8BH' new_params = { address_attributes: { - 'street_address': @new_address.street_address, - 'postcode': new_postcode + street_address: @new_address.street_address, + postcode: new_postcode } } @new_partner.update! new_params @new_partner.reload - assert @new_partner.address.postcode == new_postcode + assert_equal @new_partner.address.postcode, new_postcode end test 'it recycles addresses if it can' do @@ -38,14 +38,14 @@ class PartnerTest < ActiveSupport::TestCase other_partner = build(:partner, address: nil, accessed_by_user: @user) other_params = { address_attributes: { - 'street_address': @new_address.street_address, - 'postcode': @new_address.postcode + street_address: @new_address.street_address, + postcode: @new_address.postcode } } other_partner.update! other_params other_partner.reload - assert @new_partner.address_id == other_partner.address_id, 'should have same address ID' + assert_equal @new_partner.address_id, other_partner.address_id, 'should have same address ID' end test 'validate name length' do @@ -66,12 +66,12 @@ class PartnerTest < ActiveSupport::TestCase test 'validate no summary or description' do @new_partner.update(name: 'Test Partner', summary: '', description: '') - assert @new_partner.errors.empty? + assert_empty @new_partner.errors end test 'validate summary without description' do @new_partner.update(name: 'Test Partner', summary: 'This is a test partner used for testing :)', description: '') - assert @new_partner.errors.empty?, 'Should be able to submit a summary' + assert_empty @new_partner.errors, 'Should be able to submit a summary' end test 'validate description without summary' do @@ -86,17 +86,16 @@ class PartnerTest < ActiveSupport::TestCase end test 'validate description and summary' do - @new_partner.update(name: 'Test Partner', - summary: 'This is a test summary wheee :)', - description: 'This is a test new_partner used for testing :)') - assert @new_partner.errors.empty?, 'Should be able to submit a summary and description' + summary: 'This is a test summary wheee :)', + description: 'This is a test new_partner used for testing :)') + assert_empty @new_partner.errors, 'Should be able to submit a summary and description' end test 'validate summary length' do # We can submit a 200 character summary @new_partner.update(name: 'Test Partner', summary: ''.ljust(200, 'a')) - assert @new_partner.errors.empty?, '200 character summary should be valid' + assert_empty @new_partner.errors, '200 character summary should be valid' # But not a 201 character summary @new_partner.update(name: 'Test Partner', summary: ''.ljust(201, 'a')) @@ -108,15 +107,15 @@ class PartnerTest < ActiveSupport::TestCase @new_partner.update(url: 'htp://bad-domain.co') assert_equal ['is invalid'], @new_partner.errors[:url], 'Partner must have a valid url' @new_partner.update(url: 'https://good-domain.com') - refute @new_partner.errors.key?(:url), 'Valid URL not saved' + assert_not @new_partner.errors.key?(:url), 'Valid URL not saved' end test 'validate twitter' do # Twitter must be valid @new_partner.update(twitter_handle: '@asdf') - refute @new_partner.errors.key?(:twitter_handle), 'Valid twitter not saved' + assert_not @new_partner.errors.key?(:twitter_handle), 'Valid twitter not saved' @new_partner.update(twitter_handle: 'asdf') - refute @new_partner.errors.key?(:twitter_handle), 'Valid twitter not saved' + assert_not @new_partner.errors.key?(:twitter_handle), 'Valid twitter not saved' @new_partner.update(twitter_handle: 'https://twitter.com/asdf') assert @new_partner.errors.key?(:twitter_handle), 'Should be account name not full URL' @new_partner.update(twitter_handle: 'asd£$%dsa') @@ -130,14 +129,14 @@ class PartnerTest < ActiveSupport::TestCase @new_partner.update(facebook_link: 'Group-Name') assert @new_partner.errors.key?(:facebook_link), 'invalid Facebook page name saved' @new_partner.update(facebook_link: 'GroupName') - refute @new_partner.errors.key?(:facebook_link), 'Valid page name not saved' + assert_not @new_partner.errors.key?(:facebook_link), 'Valid page name not saved' end test 'deals with badly formatted opening times' do partner = build(:partner) partner.opening_times = '{{ $data.openingHoursSpecifications }}' - assert_equal [], partner.human_readable_opening_times + assert_empty partner.human_readable_opening_times end test 'opening_times can be unset' do @@ -150,7 +149,7 @@ class PartnerTest < ActiveSupport::TestCase opening_times_payload = [ { opens: '', closes: '' }, { opens: '', closes: '' }, - { opens: '', closes: '' }, + { opens: '', closes: '' } ].to_json p = Partner.new(opening_times: opening_times_payload) diff --git a/test/models/service_area_test.rb b/test/models/service_area_test.rb index a54724c55..ae8c9748b 100644 --- a/test/models/service_area_test.rb +++ b/test/models/service_area_test.rb @@ -1,4 +1,6 @@ -require "test_helper" +# frozen_string_literal: true + +require 'test_helper' describe ServiceArea do # it "does a thing" do diff --git a/test/models/site_test.rb b/test/models/site_test.rb index 4e676bd10..7f8b26fb9 100644 --- a/test/models/site_test.rb +++ b/test/models/site_test.rb @@ -7,7 +7,7 @@ class SiteTest < ActiveSupport::TestCase @site = create(:site) end - test "gets correct stylesheet link" do + test 'gets correct stylesheet link' do assert_equal 'themes/pink', @site.stylesheet_link @site.theme = :custom @site.slug = 'my-town' @@ -18,7 +18,6 @@ class SiteTest < ActiveSupport::TestCase end class SitePartnerTest < ActiveSupport::TestCase - test 'can find sites by partner' do address_neighbourhood = FactoryBot.create(:neighbourhood) service_area_neighbourhood = FactoryBot.create(:rusholme_neighbourhood) @@ -44,7 +43,6 @@ class SitePartnerTest < ActiveSupport::TestCase found = Site.sites_that_contain_partner(partner).order(:id) - assert found == [address_site, service_area_site, tag_site] + assert_equal found, [address_site, service_area_site, tag_site] end end - diff --git a/test/models/supporter_test.rb b/test/models/supporter_test.rb index 8956b5953..9365137e2 100644 --- a/test/models/supporter_test.rb +++ b/test/models/supporter_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class SupporterTest < ActiveSupport::TestCase diff --git a/test/models/tag_test.rb b/test/models/tag_test.rb index 48c4028f9..a67f40f7b 100644 --- a/test/models/tag_test.rb +++ b/test/models/tag_test.rb @@ -12,14 +12,14 @@ class TagTest < ActiveSupport::TestCase test 'updates user roles when saved' do @tag.users << @user @tag.save - assert @user.tag_admin? + assert_predicate @user, :tag_admin? end test 'updates partners tags when saved' do @tag.partners << @partner @tag.save - assert @tag.partners.length > 0 + assert_predicate @tag.partners.length, :positive? end test 'system_tags cannot modify name or slug' do @@ -27,9 +27,9 @@ class TagTest < ActiveSupport::TestCase @tag.name = 'This is a new name' @tag.slug = 'a-new-tag-slug' - refute @tag.validate + assert_not @tag.validate - assert @tag.errors.has_key?(:name) - assert @tag.errors.has_key?(:slug) + assert @tag.errors.key?(:name) + assert @tag.errors.key?(:slug) end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index c0456a025..db173d278 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -10,19 +10,19 @@ class UserTest < ActiveSupport::TestCase test 'owned neighbourhoods returns all descendants' do # Does the admin only own one region neighbourhood? - assert @neighbourhood_region_admin.neighbourhoods.length == 1 - assert @neighbourhood_region_admin.neighbourhoods.first.unit == 'region' + assert_equal(1, @neighbourhood_region_admin.neighbourhoods.length) + assert_equal('region', @neighbourhood_region_admin.neighbourhoods.first.unit) # Does it return the region, the districts, and the wards? # We have five counties with one district each, all parenting one ward, plus the region # (See: factories/user.rb - neighbourhood_region_admin) owned_length = 1 + (5 * 3) - assert @neighbourhood_region_admin.owned_neighbourhoods.to_a.length == owned_length + assert_equal @neighbourhood_region_admin.owned_neighbourhoods.to_a.length, owned_length # does it actually return both the districts and the wards? - assert @neighbourhood_region_admin.owned_neighbourhoods.find_all { |u| u.unit == 'county' }.length == 5 - assert @neighbourhood_region_admin.owned_neighbourhoods.find_all { |u| u.unit == 'district' }.length == 5 - assert @neighbourhood_region_admin.owned_neighbourhoods.find_all { |u| u.unit == 'ward' }.length == 5 + assert_equal(5, @neighbourhood_region_admin.owned_neighbourhoods.count { |u| u.unit == 'county' }) + assert_equal(5, @neighbourhood_region_admin.owned_neighbourhoods.count { |u| u.unit == 'district' }) + assert_equal(5, @neighbourhood_region_admin.owned_neighbourhoods.count { |u| u.unit == 'ward' }) end test 'can edit neighourhoods' do @@ -30,12 +30,12 @@ class UserTest < ActiveSupport::TestCase owned_neighbourhoods = @neighbourhood_region_admin.owned_neighbourhoods - county = owned_neighbourhoods.find_all { |u| u.unit == 'county' }.first - district = owned_neighbourhoods.find_all { |u| u.unit == 'district' }.first - ward = owned_neighbourhoods.find_all { |u| u.unit == 'ward' }.first + county = owned_neighbourhoods.find { |u| u.unit == 'county' } + district = owned_neighbourhoods.find { |u| u.unit == 'district' } + ward = owned_neighbourhoods.find { |u| u.unit == 'ward' } # We do not have permissions to edit the country! - assert (@neighbourhood_region_admin.can_alter_neighbourhood_by_id? region.parent.id) == false + assert_not((@neighbourhood_region_admin.can_alter_neighbourhood_by_id? region.parent.id)) # We should have permissions to edit a county, district, or ward neighbourhood assert @neighbourhood_region_admin.can_alter_neighbourhood_by_id?(county.id) @@ -55,16 +55,16 @@ class UserTest < ActiveSupport::TestCase # Does this person manage at least one partner? @user.partners << create(:partner) @user.save - assert @user.partner_admin? + assert_predicate @user, :partner_admin? # Does this person manage at least one tag? @user.tags << create(:tag) @user.save - assert @user.tag_admin? + assert_predicate @user, :tag_admin? # Is this person a root? If they are, they're also a secretary @user.update(role: :root) - assert @user.root? + assert_predicate @user, :root? end test 'full name method gives sensible responses' do @@ -87,13 +87,12 @@ class UserTest < ActiveSupport::TestCase test 'without skip_password_validation' do user = User.new(email: 'test@test.com', role: 'citizen') - assert_equal false, user.valid? + assert_not_predicate user, :valid? end test 'with skip_password_valiation' do user = User.new(email: 'test@test.com', role: 'citizen') user.skip_password_validation = true - assert_equal true, user.valid? - + assert_predicate user, :valid? end end diff --git a/test/policies/article_policy_test.rb b/test/policies/article_policy_test.rb index 29db94980..7ef518360 100644 --- a/test/policies/article_policy_test.rb +++ b/test/policies/article_policy_test.rb @@ -16,7 +16,7 @@ def setup partner.address.save! end # Double check that it's been looped together properly - assert @neighbourhood_admin.owned_neighbourhood_ids.include?(@partner.address.neighbourhood.id) + assert_includes @neighbourhood_admin.owned_neighbourhood_ids, @partner.address.neighbourhood.id @partner_admin = create(:partner_admin) do |user| user.partners << @partner diff --git a/test/policies/neighbourhoods_policy_test.rb b/test/policies/neighbourhoods_policy_test.rb index a3990dcaf..6cd8b50e8 100644 --- a/test/policies/neighbourhoods_policy_test.rb +++ b/test/policies/neighbourhoods_policy_test.rb @@ -11,7 +11,7 @@ class NeighbourhoodPolicyTest < ActiveSupport::TestCase end def test_scope - assert_equal(permitted_records(@citizen, Neighbourhood), []) + assert_empty(permitted_records(@citizen, Neighbourhood)) assert_equal(permitted_records(@neighbourhood_admin, Neighbourhood), @neighbourhood_admin.neighbourhoods) # Enforcing ordering is annoying, just compare the length, let the user test ensure that owned_neighbourhoods works assert_equal(permitted_records(@neighbourhood_region_admin, Neighbourhood).length, diff --git a/test/policies/partner_policy_test.rb b/test/policies/partner_policy_test.rb index 48192da89..407ca3f59 100644 --- a/test/policies/partner_policy_test.rb +++ b/test/policies/partner_policy_test.rb @@ -7,7 +7,7 @@ class PartnerPolicyTest < ActiveSupport::TestCase # Make some user accounts # ----------------------- @citizen = create(:citizen) - + @correct_partner_admin = create(:partner_admin) @wrong_partner_admin = create(:partner_admin) @@ -24,7 +24,7 @@ class PartnerPolicyTest < ActiveSupport::TestCase ## Set up partner we want to test for and make sure it's in the right regions # --------------------------------------------------------------------------- - + @partner = @correct_partner_admin.partners.first @correct_ward_admin.neighbourhoods << @partner.address.neighbourhood @correct_district_admin.neighbourhoods << @partner.address.neighbourhood.district @@ -38,7 +38,7 @@ class PartnerPolicyTest < ActiveSupport::TestCase # puts "B: Partner address ward parental ID: #{parent.id}; Child ID: #{@partner.address.neighbourhood.id}" # is_childed = parent.children.map(&:subtree).flatten.include?(@partner.address.neighbourhood) # puts "B: Partner address ward parent contains ward: #{is_childed}" - + # @multi_admin = create(:neighbourhood_admin) # @multi_admin.neighbourhoods << @partner.address.neighbourhood @@ -47,7 +47,7 @@ class PartnerPolicyTest < ActiveSupport::TestCase end # Everyone except guess can view list - def test_index + def test_index assert denies_access(@citizen, Partner, :index) assert allows_access(@root, Partner, :index) @@ -137,10 +137,9 @@ def test_update_with_partner_permissions # denies user with no partners assert denies_access(user, partner, :update) - + # can update partners user has access to user.partners << partner assert allows_access(user, partner, :update) end end - diff --git a/test/policies/partner_scope_test.rb b/test/policies/partner_scope_test.rb index 7f27f5ffd..94f9c6f6c 100644 --- a/test/policies/partner_scope_test.rb +++ b/test/policies/partner_scope_test.rb @@ -8,11 +8,11 @@ class PartnerScopeTest < ActiveSupport::TestCase @basic_partner = create(:partner) end - test "returns nothing" do - assert permitted_records(@normal_user, Partner) == [] + test 'returns nothing' do + assert_empty(permitted_records(@normal_user, Partner)) end - test "scope on ownership" do # test_scope_for_ownership + test 'scope on ownership' do # test_scope_for_ownership # user doesn't own this other_neighbourhood = neighbourhoods(:two) not_user_address = create(:address, neighbourhood: other_neighbourhood) @@ -28,11 +28,10 @@ class PartnerScopeTest < ActiveSupport::TestCase # now we should see all the partners the user owns found_partners = permitted_records(@normal_user, Partner) - assert found_partners.count == 3 + assert_equal(3, found_partners.count) end - test "scope on address" do # test_scope_for_address - + test 'scope on address' do # test_scope_for_address # give the user a neighbourhood to admin neighbourhood = neighbourhoods(:one) @normal_user.neighbourhoods << neighbourhood @@ -47,10 +46,10 @@ class PartnerScopeTest < ActiveSupport::TestCase # now we should get all the partners in this users neighbourhoods found_partners = permitted_records(@normal_user, Partner) - assert found_partners.count == 4 + assert_equal(4, found_partners.count) end - test "scope on service areas" do # test_scope_for_service_areas + test 'scope on service areas' do # test_scope_for_service_areas # give the user a neighbourhood to admin neighbourhood = neighbourhoods(:one) @normal_user.neighbourhoods << neighbourhood @@ -67,7 +66,6 @@ class PartnerScopeTest < ActiveSupport::TestCase # we should be able to see all the partners with service areas # in the users' neighbourhoods found_partners = permitted_records(@normal_user, Partner) - assert found_partners.count == 5 + assert_equal(5, found_partners.count) end end - diff --git a/test/support/email_helper.rb b/test/support/email_helper.rb index 55007115b..2bf3b6bcf 100644 --- a/test/support/email_helper.rb +++ b/test/support/email_helper.rb @@ -1,5 +1,6 @@ -module EmailHelper +# frozen_string_literal: true +module EmailHelper # get the last email sent by ActionMailer # # Returns @@ -25,6 +26,6 @@ def extract_link_from(email) body_text = html_body.decoded end - body_text =~ %r{ 'M15 5DD', 'quality' => 1, 'eastings' => 383_417, - 'northings' => 395_997, + 'northings' => 395_997, 'country' => 'England', 'nhs_ha' => 'North West', 'longitude' => -2.251226, @@ -86,8 +87,8 @@ { 'postcode' => 'M16 7BA', 'quality' => 1, - 'eastings' => 383321, - 'northings' => 395843, + 'eastings' => 383_321, + 'northings' => 395_843, 'country' => 'England', 'nhs_ha' => 'North West', 'longitude' => -2.252664, diff --git a/test/system/admin/partner_test.rb b/test/system/admin/partner_test.rb index 9b34d7c26..12ad4ac57 100644 --- a/test/system/admin/partner_test.rb +++ b/test/system/admin/partner_test.rb @@ -12,8 +12,8 @@ class AdminPartnerTest < ApplicationSystemTestCase @partner = create :ashton_partner @tag = create :tag @tag_pub = create :tag_public - @neighbourhood_one = neighbourhoods[1].to_s.gsub('w', 'W') - @neighbourhood_two = neighbourhoods[2].to_s.gsub('w', 'W') + @neighbourhood_one = neighbourhoods[1].to_s.tr('w', 'W') + @neighbourhood_two = neighbourhoods[2].to_s.tr('w', 'W') # logging in as root user visit '/users/sign_in' @@ -83,12 +83,12 @@ class AdminPartnerTest < ApplicationSystemTestCase # check it's in the list assert all(:css, '.list-group-item')[-1].text.starts_with?('Sunday all day') # check it's added to the text area - assert data.include?(new_time) + assert_includes data, new_time # remove the event all(:css, '.list-group-item')[-1].click_button('Remove') data = find('[data-opening-times-target="textarea"]', visible: :hidden).value # check time is removed from the text area - assert !data.include?(new_time) + assert_not_includes data, new_time end end diff --git a/test/system/admin/site_test.rb b/test/system/admin/site_test.rb index cda1e3f19..7e91fcfc7 100644 --- a/test/system/admin/site_test.rb +++ b/test/system/admin/site_test.rb @@ -13,8 +13,8 @@ class AdminSiteTest < ApplicationSystemTestCase @tag_pub = create :tag_public @site = create :site - @neighbourhood_one = neighbourhoods[1].to_s.gsub('w', 'W') - @neighbourhood_two = neighbourhoods[2].to_s.gsub('w', 'W') + @neighbourhood_one = neighbourhoods[1].to_s.tr('w', 'W') + @neighbourhood_two = neighbourhoods[2].to_s.tr('w', 'W') @sites_neighbourhood = create(:sites_neighbourhood, site: @site, @@ -80,7 +80,7 @@ class AdminSiteTest < ApplicationSystemTestCase click_link @site.name await_select2 service_areas = all_cocoon_select2_nodes 'sites_neighbourhoods' - msg = '@site should only have a primary neighbourhood, if this fails either this is now rendering where it should\'t or another neighborhood has been added at setup and the test should be adjusted' - assert service_areas.length.zero?, msg + msg = '@site should only have a primary neighbourhood, if this fails either this is now rendering where it should\'t or another neighborhood has been added at setup and the test should be adjusted' + assert_predicate service_areas.length, :zero?, msg end end diff --git a/test/system/admin/user_test.rb b/test/system/admin/user_test.rb index 036cf73f7..8b48b1fad 100644 --- a/test/system/admin/user_test.rb +++ b/test/system/admin/user_test.rb @@ -17,9 +17,9 @@ class AdminUserTest < ApplicationSystemTestCase @tag = create :tag @tag_pub = create :tag_public - - @neighbourhood_one = neighbourhoods[1].to_s.gsub('w', 'W') - @neighbourhood_two = neighbourhoods[2].to_s.gsub('w', 'W') + + @neighbourhood_one = neighbourhoods[1].to_s.tr('w', 'W') + @neighbourhood_two = neighbourhoods[2].to_s.tr('w', 'W') # logging in as root user visit '/users/sign_in' diff --git a/test/system/application_system_test_case.rb b/test/system/application_system_test_case.rb index cb4a6b456..3f2b4993d 100644 --- a/test/system/application_system_test_case.rb +++ b/test/system/application_system_test_case.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'system_test_helper' # For system tests so we can test our JS frontend is working @@ -6,4 +8,3 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase Capybara.server = :puma, { Silent: true } Selenium::WebDriver.logger.ignore(:browser_options) end - diff --git a/test/system/collections_test.rb b/test/system/collections_test.rb index 943ce4e97..45232f817 100644 --- a/test/system/collections_test.rb +++ b/test/system/collections_test.rb @@ -1,11 +1,12 @@ +# frozen_string_literal: true ## frozen_string_literal: true # -#require 'application_system_test_case' +# require 'application_system_test_case' # -#class CollectionsTest < ApplicationSystemTestCase +# class CollectionsTest < ApplicationSystemTestCase # # test "visiting the index" do # # visit collections_url # # # # assert_selector "h1", text: "Collection" # # end -#end +# end diff --git a/test/system/create_admin_users_test.rb b/test/system/create_admin_users_test.rb index ddfce6c6a..05213462a 100644 --- a/test/system/create_admin_users_test.rb +++ b/test/system/create_admin_users_test.rb @@ -1,4 +1,6 @@ -require_relative "./application_system_test_case" +# frozen_string_literal: true + +require_relative './application_system_test_case' class CreateAdminUsersTest < ApplicationSystemTestCase setup do @@ -12,8 +14,7 @@ class CreateAdminUsersTest < ApplicationSystemTestCase app_routes.default_url_options[:protocol] = 'http' end - test "visiting the index" do - + test 'visiting the index' do # set up given_a_root_user_exists given_the_default_site_exists @@ -79,6 +80,6 @@ def last_email_delivered def extract_invitation_link_from(email) body = email.body.parts.first.body.raw_source - body =~ /^(https?:\/\/.*)$/ && $1 + body =~ %r{^(https?://.*)$} && Regexp.last_match(1) end end diff --git a/test/system/graphql/graphql_test.rb b/test/system/graphql/graphql_test.rb index 696c30e9e..590dc5ab2 100644 --- a/test/system/graphql/graphql_test.rb +++ b/test/system/graphql/graphql_test.rb @@ -1,29 +1,28 @@ -require_relative "../application_system_test_case" - -=begin - -This is basically a stand alone script that pokes the GraphQL endpoint -running on the server through a network connection (just as how users -will use the endpoint on staging/production). It does not use any -internal code like /test/integration/graphql/* - -We could be being overly paranoid here but it may become useful for -catching the kind of bugs that only show up on a real network -connection. - -NOTE: we are mainly interested in testing the endpoints that -the Trans Dimension elm app uses. -=end +# frozen_string_literal: true + +require_relative '../application_system_test_case' + +# +# This is basically a stand alone script that pokes the GraphQL endpoint +# running on the server through a network connection (just as how users +# will use the endpoint on staging/production). It does not use any +# internal code like /test/integration/graphql/* +# +# We could be being overly paranoid here but it may become useful for +# catching the kind of bugs that only show up on a real network +# connection. +# +# NOTE: we are mainly interested in testing the endpoints that +# the Trans Dimension elm app uses. require 'graphql/client' require 'graphql/client/http' - module PlaceCalApi extend self PING_QUERY_STRING = - "query { ping }" + 'query { ping }' ARTICLE_CONNECTION_QUERY_STRING = <<-QUERY query { articleConnection { edges { @@ -51,9 +50,8 @@ module PlaceCalApi } } QUERY - def configure(default_url_options) - @url = URI::HTTP::build(default_url_options) + @url = URI::HTTP.build(default_url_options) @url.path = '/api/v1/graphql' @http = GraphQL::Client::HTTP.new(@url.to_s) @@ -77,18 +75,18 @@ def do_article_connection_query end def do_partners_by_tag_query(tag_id) - @client.query PartnersByTagQuery, variables: { 'tagId': tag_id } + @client.query PartnersByTagQuery, variables: { tagId: tag_id } end private def define_constant(name, value) return if const_defined?(name) + const_set name, value end end - class GraphQLTest < ApplicationSystemTestCase # This tests a number of small (critical) bits of the GraphQL # system so its name is bit generic @@ -111,11 +109,11 @@ class GraphQLTest < ApplicationSystemTestCase PlaceCalApi.configure app_routes.default_url_options end - test "ping works" do + test 'ping works' do result = PlaceCalApi.do_ping_query - assert result.errors.empty? + assert_empty result.errors assert result.data.ping =~ /^Hello World! The time is \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}/, - 'missing PING data response' + 'missing PING data response' end test 'news article connection with image url' do @@ -145,4 +143,3 @@ class GraphQLTest < ApplicationSystemTestCase assert url =~ %r{\Ahttps://#{@server.host}:#{@server.port}/}, 'partner logo is not full URL' end end - diff --git a/test/system_test_helper.rb b/test/system_test_helper.rb index 87c03ac75..087541d02 100644 --- a/test/system_test_helper.rb +++ b/test/system_test_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' Capybara.app_host = 'http://lvh.me' @@ -14,10 +16,10 @@ ) Capybara::Selenium::Driver.new app, - browser: :chrome, - desired_capabilities: capabilities + browser: :chrome, + desired_capabilities: capabilities end -#Dir.glob(File.join(Rails.root, 'test/system/**/*.rb')) do |path| +# Dir.glob(File.join(Rails.root, 'test/system/**/*.rb')) do |path| # require path -#end +# end diff --git a/test/test_helper.rb b/test/test_helper.rb index ae0f9c10d..317eaa2e1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -21,10 +21,10 @@ JsonMatchers.schema_root = 'test/support/api/schemas' include JsonMatchers::Minitest::Assertions -require "capybara/rails" -require "capybara/minitest" +require 'capybara/rails' +require 'capybara/minitest' -Dir.glob(File.join(Rails.root, 'test/support/**/*.rb')) do |path| +Dir.glob(File.join(Rails.root, 'test/support/**/*.rb')).sort.each do |path| require path end @@ -114,7 +114,6 @@ def create_default_site create(:site, slug: 'default-site') end - # Some helpers for working with JS and Capybara def click_sidebar(href) @@ -159,7 +158,7 @@ def assert_select2_multiple(options_array, node) # If we are left with nothing then the options and stored data match within :xpath, node.path do assert_selector '.select2-selection__choice', count: options_array.length - rendered = find(:css, '.select2-selection__rendered').text.gsub('×', '').gsub("\n", '') + rendered = find(:css, '.select2-selection__rendered').text.delete('×').delete("\n") options_array.each do |opt| rendered = rendered.gsub(opt, '') end @@ -202,7 +201,7 @@ def assert_field(obj, key, message = nil) def refute_field(obj, key, message = nil) message ||= "Field '#{key}' exists in: #{obj}" - refute obj.key?(key), message # obj.key? returns false if an item is nil, ergo... + assert_not obj.key?(key), message # obj.key? returns false if an item is nil, ergo... obj[key] end