diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..cc32da4b --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1 @@ +inherit_from: .rubocop_todo.yml diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 00000000..dc77f4bf --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,253 @@ +Style/PercentLiteralDelimiters: + Enabled: false + +# kind_of? is a good way to check a type +Style/ClassCheck: + EnforcedStyle: kind_of? + +Style/FrozenStringLiteralComment: + Enabled: false + +# This doesn't work with older versions of Ruby (pre 2.4.0) +Style/SafeNavigation: + Enabled: false + +# This doesn't work with older versions of Ruby (pre 2.4.0) +Performance/RegexpMatch: + Enabled: false + +# This suggests use of `tr` instead of `gsub`. While this might be more performant, +# these methods are not at all interchangable, and behave very differently. This can +# lead to people making the substitution without considering the differences. +Performance/StringReplacement: + Enabled: false + +# .length == 0 is also good, we don't always want .zero? +Style/NumericPredicate: + Enabled: false + +# this would cause errors with long lanes +Metrics/BlockLength: + Enabled: false + +# this is a bit buggy +Metrics/ModuleLength: + Enabled: false + +# certificate_1 is an okay variable name +Style/VariableNumber: + Enabled: false + +# This is used a lot across the fastlane code base for config files +Style/MethodMissing: + Enabled: false + +# +# File.chmod(0777, f) +# +# is easier to read than +# +# File.chmod(0o777, f) +# +Style/NumericLiteralPrefix: + Enabled: false + +# +# command = (!clean_expired.nil? || !clean_pattern.nil?) ? CLEANUP : LIST +# +# is easier to read than +# +# command = !clean_expired.nil? || !clean_pattern.nil? ? CLEANUP : LIST +# +Style/TernaryParentheses: + Enabled: false + +# sometimes it is useful to have those empty methods +Style/EmptyMethod: + Enabled: false + +# It's better to be more explicit about the type +Style/BracesAroundHashParameters: + Enabled: false + +# specs sometimes have useless assignments, which is fine +Lint/UselessAssignment: + Exclude: + - '**/spec/**/*' + +# We could potentially enable the 2 below: +Style/IndentHash: + Enabled: false + +Style/AlignHash: + Enabled: false + +# HoundCI doesn't like this rule +Style/DotPosition: + Enabled: false + +# We allow !! as it's an easy way to convert ot boolean +Style/DoubleNegation: + Enabled: false + +# Prevent to replace [] into %i +Style/SymbolArray: + Enabled: false + +# We still support Ruby 2.0.0 +Style/IndentHeredoc: + Enabled: false + +# This cop would not work fine with rspec +Style/MixinGrouping: + Exclude: + - '**/spec/**/*' + +# Sometimes we allow a rescue block that doesn't contain code +Lint/HandleExceptions: + Enabled: false + +# Cop supports --auto-correct. +Lint/UnusedBlockArgument: + Enabled: false + +Lint/AmbiguousBlockAssociation: + Enabled: false + +# Needed for $verbose +Style/GlobalVars: + Enabled: false + +# We want to allow class Fastlane::Class +Style/ClassAndModuleChildren: + Enabled: false + +# $? Exit +Style/SpecialGlobalVars: + Enabled: false + +Metrics/AbcSize: + Enabled: false + +Metrics/MethodLength: + Enabled: false + +Metrics/CyclomaticComplexity: + Enabled: false + +# The %w might be confusing for new users +Style/WordArray: + MinSize: 19 + +# raise and fail are both okay +Style/SignalException: + Enabled: false + +# Better too much 'return' than one missing +Style/RedundantReturn: + Enabled: false + +# Having if in the same line might not always be good +Style/IfUnlessModifier: + Enabled: false + +# and and or is okay +Style/AndOr: + Enabled: false + +# Configuration parameters: CountComments. +Metrics/ClassLength: + Max: 320 + + +# Configuration parameters: AllowURI, URISchemes. +Metrics/LineLength: + Max: 370 + +# Configuration parameters: CountKeywordArgs. +Metrics/ParameterLists: + Max: 17 + +Metrics/PerceivedComplexity: + Max: 18 + +# Sometimes it's easier to read without guards +Style/GuardClause: + Enabled: false + +# We allow both " and ' +Style/StringLiterals: + Enabled: false + +# something = if something_else +# that's confusing +Style/ConditionalAssignment: + Enabled: false + +# Better to have too much self than missing a self +Style/RedundantSelf: + Enabled: false + +# e.g. +# def self.is_supported?(platform) +# we may never use `platform` +Lint/UnusedMethodArgument: + Enabled: false + +# the let(:key) { ... } +Lint/ParenthesesAsGroupedExpression: + Exclude: + - '**/spec/**/*' + +# This would reject is_ in front of methods +# We use `is_supported?` everywhere already +Style/PredicateName: + Enabled: false + +# We allow the $ +Style/PerlBackrefs: + Enabled: false + +# Disable '+ should be surrounded with a single space' for xcodebuild_spec.rb +Style/SpaceAroundOperators: + Exclude: + - '**/spec/actions_specs/xcodebuild_spec.rb' + +AllCops: + Include: + - '**/fastlane/Fastfile' + Exclude: + - '**/lib/assets/custom_action_template.rb' + - './vendor/**/*' + +# They have not to be snake_case +Style/FileName: + Exclude: + - '**/Dangerfile' + - '**/Brewfile' + - '**/Gemfile' + - '**/Podfile' + - '**/Rakefile' + - '**/Fastfile' + - '**/Deliverfile' + - '**/Snapfile' + +# We're not there yet +Style/Documentation: + Enabled: false + +# Added after upgrade to 0.38.0 +Style/MutableConstant: + Enabled: false + +# length > 0 is good +Style/ZeroLengthPredicate: + Enabled: false + +# Adds complexity +Style/IfInsideElse: + Enabled: false + +# Sometimes we just want to 'collect' +Style/CollectionMethods: + Enabled: false diff --git a/Gemfile b/Gemfile index b3ada65e..a6ea52e6 100644 --- a/Gemfile +++ b/Gemfile @@ -49,6 +49,9 @@ group :development, :test do # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem 'spring' + + # make sure our code is spick and span + gem 'rubocop', require: false end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 2b2c6d64..5bdd2940 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -322,6 +322,7 @@ DEPENDENCIES rack (>= 1.6.11) rails (~> 5.0.0) rails_12factor + rubocop sass-rails (~> 5.0) spring uglifier (>= 1.3.0) diff --git a/app/controllers/invite_controller.rb b/app/controllers/invite_controller.rb index ec8b6af1..b89bc5c9 100644 --- a/app/controllers/invite_controller.rb +++ b/app/controllers/invite_controller.rb @@ -4,7 +4,7 @@ class InviteController < ApplicationController before_action :check_disabled_text before_action :check_imprint_url - skip_before_filter :verify_authenticity_token + skip_before_action :verify_authenticity_token def index if boarding_service.user and boarding_service.password @@ -30,7 +30,7 @@ def submit render :index return end - + email = params[:email] first_name = params[:first_name] last_name = params[:last_name] @@ -41,7 +41,7 @@ def submit if domains.count == 1 @message = "Sorry! Early access is currently restricted to people within the #{domains.first} domain." else - @message = "Sorry! Early access is currently restricted to people within the following domains: (#{domains.join(", ")})" + @message = "Sorry! Early access is currently restricted to people within the following domains: (#{domains.join(', ')})" end @type = "warning" render :index @@ -57,7 +57,7 @@ def submit return end end - + if email.length == 0 render :index return @@ -89,47 +89,45 @@ def submit end private - def create_and_add_tester(email, first_name, last_name) - add_tester_response = boarding_service.add_tester(email, first_name, last_name) - @message = add_tester_response.message - @type = add_tester_response.type - end - def boarding_service - BOARDING_SERVICE - end + def create_and_add_tester(email, first_name, last_name) + add_tester_response = boarding_service.add_tester(email, first_name, last_name) + @message = add_tester_response.message + @type = add_tester_response.type + end - def app_metadata - Rails.cache.fetch('appMetadata', expires_in: 10.minutes) do - { - icon_url: boarding_service.app.app_icon_preview_url, - title: boarding_service.app.name - } - end - end + def boarding_service + BOARDING_SERVICE + end - def set_app_details - @metadata = app_metadata - @title = @metadata[:title] + def set_app_details + @metadata = app_metadata + @title = @metadata[:title] - tabIcon = ENV["TAB_ICON"].to_s - if tabIcon.to_s.length == 0 - @tabIcon = "/BoardingIcon.ico" - else - @tabIcon = tabIcon - end + tabIcon = ENV["TAB_ICON"].to_s + if tabIcon.to_s.length == 0 + @tabIcon = "/BoardingIcon.ico" + else + @tabIcon = tabIcon end + end - def check_disabled_text - if boarding_service.itc_closed_text - @message = boarding_service.itc_closed_text - @type = "warning" - end + def set_app_details + @metadata = app_metadata + @title = @metadata[:title] + end + + def check_disabled_text + if boarding_service.itc_closed_text + @message = boarding_service.itc_closed_text + @type = "warning" end + end - def check_imprint_url - if boarding_service.imprint_url - @imprint_url = boarding_service.imprint_url - end + def check_imprint_url + if boarding_service.imprint_url + @imprint_url = boarding_service.imprint_url end + end + end diff --git a/app/services/boarding_service.rb b/app/services/boarding_service.rb index b7fc1b7c..fed5debe 100644 --- a/app/services/boarding_service.rb +++ b/app/services/boarding_service.rb @@ -3,7 +3,7 @@ class AddTesterResponse attr_accessor :message attr_accessor :type -end +end class BoardingService include AbstractController::Translation @@ -20,9 +20,9 @@ class BoardingService attr_accessor :imprint_url def initialize(app_id: ENV["ITC_APP_ID"], - user: ENV["ITC_USER"] || ENV["FASTLANE_USER"], - password: ENV["ITC_PASSWORD"] || ENV["FASTLANE_PASSWORD"], - tester_group_names: ENV["ITC_APP_TESTER_GROUPS"]) + user: ENV["ITC_USER"] || ENV["FASTLANE_USER"], + password: ENV["ITC_PASSWORD"] || ENV["FASTLANE_PASSWORD"], + tester_group_names: ENV["ITC_APP_TESTER_GROUPS"]) @app_id = app_id @user = user @password = password @@ -67,34 +67,55 @@ def add_tester(email, first_name, last_name) end private + def ensure_values + error_message = [] - def ensure_values - error_message = [] - - error_message << "Environment variable `ITC_APP_ID` required" if @app_id.to_s.length == 0 - error_message << "Environment variable `ITC_USER` or `FASTLANE_USER` required" if @user.to_s.length == 0 - error_message << "Environment variable `ITC_PASSWORD` or `FASTLANE_PASSWORD`" if @password.to_s.length == 0 - raise error_message.join("\n") if error_message.length > 0 + error_message << "Environment variable `ITC_APP_ID` required" if @app_id.to_s.length == 0 + error_message << "Environment variable `ITC_USER` or `FASTLANE_USER` required" if @user.to_s.length == 0 + error_message << "Environment variable `ITC_PASSWORD` or `FASTLANE_PASSWORD`" if @password.to_s.length == 0 + raise error_message.join("\n") if error_message.length > 0 - spaceship = Spaceship::Tunes.login(@user, @password) - spaceship.select_team + spaceship = Spaceship::Tunes.login(@user, @password) + spaceship.select_team - @app ||= Spaceship::Tunes::Application.find(@app_id) - raise "Could not find app with ID #{app_id}" if @app.nil? + @app ||= Spaceship::Tunes::Application.find(@app_id) + raise "Could not find app with ID #{app_id}" if @app.nil? - if tester_group_names - @test_flight_groups = Spaceship::ConnectAPI.get_beta_groups(filter: { app: @app.apple_id }).select do |group| - tester_group_names.include?(group.name) - end + if tester_group_names + @test_flight_groups = Spaceship::ConnectAPI.get_beta_groups(filter: { app: @app.apple_id }).select do |group| + tester_group_names.include?(group.name) + end - test_flight_group_names = @test_flight_groups.map { |group| group.name }.to_set - tester_group_names.select do |group_name| - next if test_flight_group_names.include?(group_name) - error_message << "TestFlight missing group `#{group_name}`, You need to first create this group in App Store Connect." - end + test_flight_group_names = @test_flight_groups.map { |group| group.name }.to_set + tester_group_names.select do |group_name| + next if test_flight_group_names.include?(group_name) + error_message << "TestFlight missing group `#{group_name}`, You need to first create this group in App Store Connect." end + end + return tester + end - raise error_message.join("\n") if error_message.length > 0 + def ensure_values + error_message = [] + + error_message << "Environment variable `ITC_APP_ID` required" if @app_id.to_s.length == 0 + error_message << "Environment variable `ITC_USER` or `FASTLANE_USER` required" if @user.to_s.length == 0 + error_message << "Environment variable `ITC_PASSWORD` or `FASTLANE_PASSWORD`" if @password.to_s.length == 0 + raise error_message.join("\n") if error_message.length > 0 + + spaceship = Spaceship::Tunes.login(@user, @password) + spaceship.select_team + + @app ||= Spaceship::Tunes::Application.find(@app_id) + raise "Could not find app with ID #{app_id}" if @app.nil? + + if tester_group_names + test_flight_groups = Spaceship::TestFlight::Group.filter_groups(app_id: @app.apple_id) + test_flight_group_names = test_flight_groups.map(&:name).to_set + tester_group_names.select do |group_name| + next if test_flight_group_names.include?(group_name) + error_message << "TestFlight missing group `#{group_name}`, You need to first create this group in iTunes Connect." + end end def testing_is_live? # TODO: clean this when Spaceship::TestFlight::BuildTrains has more attributes @@ -107,6 +128,7 @@ def testing_is_live? # TODO: clean this when Spaceship::TestFlight::BuildTrains # end # end end - return false end + return false + end end diff --git a/bin/setup b/bin/setup index acdb2c13..d3bb9976 100755 --- a/bin/setup +++ b/bin/setup @@ -2,7 +2,7 @@ require 'pathname' # path to your application root. -APP_ROOT = Pathname.new File.expand_path('../../', __FILE__) +APP_ROOT = Pathname.new File.expand_path('../../', __FILE__) Dir.chdir APP_ROOT do # This script is a starting point to setup your application. diff --git a/config/application.rb b/config/application.rb index 1c3f5f3c..20f9f7d2 100644 --- a/config/application.rb +++ b/config/application.rb @@ -22,7 +22,7 @@ class Application < Rails::Application # The default locale is :en and all translations from config/locales/*.rb,yml are auto loaded. # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] # config.i18n.default_locale = :de - + # Allows iframe from other origins config.action_dispatch.default_headers = { 'X-Frame-Options' => 'ALLOWALL'