From eb9698df9d331142c142e4d3e75b0cd64b8998b8 Mon Sep 17 00:00:00 2001 From: Alexander Riese Date: Fri, 18 Aug 2017 13:43:04 +0200 Subject: [PATCH] Add coding style guide #636 (#676) * add rubocop config files * add rubocop.yml including rails style guide * update gems to use the rubocop rails extension * adjust color format in lib/pdf_generation/badges_pdf.rb * rubocop autofix * update rubocop todo file * reference rubocop todo file * downgrade rubocop to 0.46 for codeclimate * remove fixme * Fix style error * fix missing space after comma --- .rubocop.yml | 12 + .rubocop_todo.yml | 136 +++++++++++ Gemfile | 38 ++- Gemfile.lock | 170 +++++++------- .../agreement_letters_controller.rb | 2 +- app/controllers/application_controller.rb | 27 +-- .../application_letters_controller.rb | 73 +++--- .../application_notes_controller.rb | 3 +- app/controllers/emails_controller.rb | 17 +- app/controllers/events_controller.rb | 152 ++++++------ .../participant_groups_controller.rb | 21 +- app/controllers/profiles_controller.rb | 24 +- app/controllers/requests_controller.rb | 45 ++-- .../users/omniauth_callbacks_controller.rb | 6 +- .../users/registrations_controller.rb | 11 +- app/controllers/users_controller.rb | 13 +- app/helpers/applicants_overview_helper.rb | 14 +- app/helpers/application_helper.rb | 43 ++-- app/helpers/participants_overview_helper.rb | 12 +- app/mailers/portal_mailer.rb | 4 +- app/models/ability.rb | 41 ++-- app/models/agreement_letter.rb | 75 +++--- app/models/application_letter.rb | 65 +++--- app/models/date_range.rb | 1 - app/models/email.rb | 10 +- app/models/email_template.rb | 3 +- app/models/event.rb | 96 ++++---- app/models/participant_group.rb | 4 +- app/models/profile.rb | 27 ++- app/models/request.rb | 7 +- app/models/user.rb | 40 ++-- app/services/mailer.rb | 5 +- app/uploaders/event_image_uploader.rb | 5 +- lib/pdf_generation/applications_pdf.rb | 219 +++++++++--------- lib/pdf_generation/badges_pdf.rb | 177 +++++++------- lib/pdf_generation/participants_pdf.rb | 114 +++++---- lib/tasks/sample_data.rake | 2 +- .../application_letters_controller_spec.rb | 9 +- 38 files changed, 920 insertions(+), 803 deletions(-) create mode 100644 .rubocop.yml create mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..07ad66c6 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,12 @@ +inherit_from: .rubocop_todo.yml + +AllCops: + TargetRubyVersion: 2.2 + Exclude: + - 'spec/**/*' + - 'db/**/*' + - 'config/**/*' + - 'bin/*' + - 'Rakefile' + - 'Vagrantfile' + - 'vendor/**/*' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 00000000..e3fa86ae --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,136 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2017-08-09 18:05:47 +0200 using RuboCop version 0.46.0. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# Offense count: 1 +Lint/IneffectiveAccessModifier: + Exclude: + - 'app/models/agreement_letter.rb' + +# Offense count: 2 +Lint/UselessAssignment: + Exclude: + - 'lib/pdf_generation/applications_pdf.rb' + +# Offense count: 41 +Metrics/AbcSize: + Max: 71 + +# Offense count: 6 +# Configuration parameters: CountComments. +Metrics/ClassLength: + Max: 235 + +# Offense count: 6 +Metrics/CyclomaticComplexity: + Max: 13 + +# Offense count: 233 +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. +# URISchemes: http, https +Metrics/LineLength: + Max: 274 + +# Offense count: 31 +# Configuration parameters: CountComments. +Metrics/MethodLength: + Max: 48 + +# Offense count: 3 +# Configuration parameters: CountKeywordArgs. +Metrics/ParameterLists: + Max: 6 + +# Offense count: 4 +Metrics/PerceivedComplexity: + Max: 13 + +# Offense count: 5 +Style/AccessorMethodName: + Exclude: + - 'app/controllers/emails_controller.rb' + - 'app/models/agreement_letter.rb' + - 'app/models/application_letter.rb' + - 'app/models/event.rb' + +# Offense count: 2 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: nested, compact +Style/ClassAndModuleChildren: + Exclude: + - 'app/controllers/users/omniauth_callbacks_controller.rb' + - 'app/controllers/users/registrations_controller.rb' + +# Offense count: 28 +Style/Documentation: + Enabled: false + +# Offense count: 1 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: for, each +Style/For: + Exclude: + - 'app/models/event.rb' + +# Offense count: 19 +# Configuration parameters: MinBodyLength. +Style/GuardClause: + Exclude: + - 'app/controllers/application_controller.rb' + - 'app/controllers/application_letters_controller.rb' + - 'app/controllers/events_controller.rb' + - 'app/controllers/users/omniauth_callbacks_controller.rb' + - 'app/controllers/users_controller.rb' + - 'app/helpers/applicants_overview_helper.rb' + - 'app/models/ability.rb' + - 'app/models/application_letter.rb' + - 'app/models/date_range.rb' + - 'app/models/event.rb' + - 'app/models/profile.rb' + - 'app/uploaders/event_image_uploader.rb' + - 'lib/pdf_generation/applications_pdf.rb' + - 'lib/pdf_generation/participants_pdf.rb' + +# Offense count: 1 +Style/MultilineTernaryOperator: + Exclude: + - 'app/helpers/application_helper.rb' + +# Offense count: 1 +# Cop supports --auto-correct. +Style/MutableConstant: + Exclude: + - 'app/models/participant_group.rb' + +# Offense count: 2 +# Cop supports --auto-correct. +# Configuration parameters: AutoCorrect, EnforcedStyle, SupportedStyles. +# SupportedStyles: predicate, comparison +Style/NumericPredicate: + Exclude: + - 'spec/**/*' + - 'app/controllers/events_controller.rb' + - 'lib/pdf_generation/applications_pdf.rb' + +# Offense count: 6 +# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist. +# NamePrefix: is_, has_, have_ +# NamePrefixBlacklist: is_, has_, have_ +# NameWhitelist: is_a? +Style/PredicateName: + Exclude: + - 'spec/**/*' + - 'app/controllers/events_controller.rb' + - 'app/models/agreement_letter.rb' + - 'app/models/event.rb' + +# Offense count: 1 +# Configuration parameters: Methods. +# Methods: {"reduce"=>["acc", "elem"]}, {"inject"=>["acc", "elem"]} +Style/SingleLineBlockParams: + Exclude: + - 'app/models/event.rb' diff --git a/Gemfile b/Gemfile index 113575f7..caef6190 100644 --- a/Gemfile +++ b/Gemfile @@ -35,20 +35,20 @@ gem 'jquery-ui-rails' # Authentication gem 'devise' # openID Authentication -#gem 'devise_openid_authenticatable' +# gem 'devise_openid_authenticatable' gem 'omniauth' gem 'omniauth-openid' # Use Bootstrap (app/assets/stylesheets) for app styling # Also provides some nifty helpers: # https://github.com/seyhunak/twitter-bootstrap-rails#using-helpers -gem "therubyracer" -#gem 'less-rails-bootstrap' +gem 'therubyracer' +# gem 'less-rails-bootstrap' # less-rails requires an older sprockets -gem "sprockets", '3.6.3' -gem "less-rails" -gem 'twitter-bootstrap-rails' +gem 'less-rails' gem 'ref' +gem 'sprockets', '3.6.3' +gem 'twitter-bootstrap-rails' # Boostrap-syle view for devise gem 'devise-bootstrap-views' # Integrates Bootstrap Tooltip library with Rails asset pipeline @@ -63,10 +63,10 @@ gem 'bootstrap-datepicker-rails' gem 'airbrake' # to parse date parameters from ui -gem "delocalize" +gem 'delocalize' # American style month/day/year parsing for ruby 1.9 # https://github.com/jeremyevans/ruby-american_date -gem "american_date" +gem 'american_date' # Authorisation library, define who is allowed what # See https://github.com/CanCanCommunity/cancancan @@ -77,7 +77,7 @@ gem 'cancancan' gem 'has_scope' # Static code analysis -gem 'rubocop', '~> 0.29.1' +gem 'rubocop', '0.46' # DSL for building forms # See https://github.com/plataformatec/simple_form @@ -91,15 +91,15 @@ gem 'owlcarousel-rails', github: 'acrogenesis/owlcarousel-rails', branch: 'OwlCa gem 'coveralls', require: false # pdf generation +gem 'combine_pdf' gem 'prawn' gem 'prawn-table' -gem 'combine_pdf' -#zip generation +# zip generation gem 'rubyzip', require: 'zip' # pdf inspection -gem 'pdf-inspector', require: "pdf/inspector" +gem 'pdf-inspector', require: 'pdf/inspector' # Simple, Heroku-friendly Rails app configuration using ENV and a single YAML file gem 'figaro' @@ -107,7 +107,7 @@ gem 'figaro' # Allow ORM functionality in plain ruby models gem 'active_attr' -#to only display a limited number of items on an index page +# to only display a limited number of items on an index page gem 'will_paginate-bootstrap' # Markdown renderer @@ -123,11 +123,11 @@ gem 'icalendar' group :development, :test do # Call 'byebug' anywhere in the code to stop execution and get a debugger console gem 'byebug' - gem 'rspec-rails', '~> 3.6' - gem 'rspec-steps', '~> 2.1.1' gem 'capybara', '~> 2.5' - gem 'poltergeist' gem 'database_cleaner' + gem 'poltergeist' + gem 'rspec-rails', '~> 3.6' + gem 'rspec-steps', '~> 2.1.1' # gem 'database_cleaner' gem 'factory_girl_rails' end @@ -160,18 +160,16 @@ end group :test do # Coverage information + gem 'codeclimate-test-reporter', '~> 1.0.0' gem 'simplecov' - gem "codeclimate-test-reporter", "~> 1.0.0" # Explicitly set parser version, to remove warnings # Might lead to problems with other gems that require higher parser versions # In that case, update to a newer Ruby version - gem 'parser', '~> 2.2.2.5' + gem 'parser', '~> 2.3.3.1' # Stubbing external calls by blocking traffic with WebMock.disable_net_connect! or allow: # gem 'webmock' - end - group :production do # Use Puma web server # https://devcenter.heroku.com/articles/deploying-rails-applications-with-the-puma-web-server diff --git a/Gemfile.lock b/Gemfile.lock index 50de6b58..236f1488 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -29,9 +29,9 @@ GEM erubis (~> 2.7.0) rails-dom-testing (~> 1.0, >= 1.0.5) rails-html-sanitizer (~> 1.0, >= 1.0.2) - active_attr (0.9.0) - activemodel (>= 3.0.2, < 5.1) - activesupport (>= 3.0.2, < 5.1) + active_attr (0.10.2) + activemodel (>= 3.0.2, < 5.2) + activesupport (>= 3.0.2, < 5.2) activejob (4.2.7.1) activesupport (= 4.2.7.1) globalid (>= 0.3.0) @@ -48,20 +48,18 @@ GEM minitest (~> 5.1) thread_safe (~> 0.3, >= 0.3.4) tzinfo (~> 1.1) - addressable (2.5.0) + addressable (2.5.1) public_suffix (~> 2.0, >= 2.0.2) afm (0.2.2) - airbrake (5.6.1) - airbrake-ruby (~> 1.6) - airbrake-ruby (1.6.0) + airbrake (6.2.1) + airbrake-ruby (~> 2.3, >= 2.3.1) + airbrake-ruby (2.3.2) american_date (1.1.1) - annotate (2.7.1) + annotate (2.7.2) activerecord (>= 3.2, < 6.0) - rake (>= 10.4, < 12.0) - arel (6.0.3) + rake (>= 10.4, < 13.0) + arel (6.0.4) ast (2.3.0) - astrolabe (1.3.1) - parser (~> 2.2) bcrypt (3.1.11) better_errors (2.1.1) coderay (>= 1.0.0) @@ -73,64 +71,64 @@ GEM railties (>= 3.0) builder (3.2.3) byebug (9.0.6) - cancancan (1.15.0) - capybara (2.11.0) + cancancan (2.0.0) + capybara (2.15.1) addressable - mime-types (>= 1.16) + mini_mime (>= 0.1.3) nokogiri (>= 1.3.3) rack (>= 1.0.0) rack-test (>= 0.5.4) xpath (~> 2.0) - carrierwave (1.0.0) + carrierwave (1.1.0) activemodel (>= 4.0.0) activesupport (>= 4.0.0) mime-types (>= 1.16) cliver (0.3.2) - codeclimate-test-reporter (1.0.3) - simplecov + codeclimate-test-reporter (1.0.8) + simplecov (<= 0.13) coderay (1.1.1) - combine_pdf (0.2.32) + combine_pdf (1.0.6) ruby-rc4 (>= 0.1.5) commonjs (0.2.7) - concurrent-ruby (1.0.4) - coveralls (0.8.16) + concurrent-ruby (1.0.5) + coveralls (0.8.19) json (>= 1.8, < 3) simplecov (~> 0.12.0) - term-ansicolor (~> 1.3.0) + term-ansicolor (~> 1.3) thor (~> 0.19.1) - tins (>= 1.6.0, < 2) - database_cleaner (1.5.3) - debug_inspector (0.0.2) - delocalize (1.1.0) + tins (~> 1.6) + database_cleaner (1.6.1) + debug_inspector (0.0.3) + delocalize (1.2.0) rails (>= 2) - devise (4.2.0) + devise (4.3.0) bcrypt (~> 3.0) orm_adapter (~> 0.1) - railties (>= 4.1.0, < 5.1) + railties (>= 4.1.0, < 5.2) responders warden (~> 1.2.3) - devise-bootstrap-views (0.0.9) + devise-bootstrap-views (0.0.11) diff-lcs (1.3) docile (1.1.5) erubis (2.7.0) execjs (2.7.0) - factory_girl (4.7.0) + factory_girl (4.8.0) activesupport (>= 3.0.0) - factory_girl_rails (4.7.0) - factory_girl (~> 4.7.0) + factory_girl_rails (4.8.0) + factory_girl (~> 4.8.0) railties (>= 3.0.0) figaro (1.1.1) thor (~> 0.14) - globalid (0.3.7) - activesupport (>= 4.1.0) - has_scope (0.7.0) - actionpack (>= 4.1, < 5.1) - activesupport (>= 4.1, < 5.1) + globalid (0.4.0) + activesupport (>= 4.2.0) + has_scope (0.7.1) + actionpack (>= 4.1, < 5.2) + activesupport (>= 4.1, < 5.2) hashery (2.1.2) - hashie (3.5.5) - i18n (0.8.4) + hashie (3.5.6) + i18n (0.8.6) icalendar (2.4.1) - jquery-rails (4.2.1) + jquery-rails (4.3.1) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) @@ -147,18 +145,19 @@ GEM less (~> 2.6.0) sprockets (> 2, < 4) tilt - libv8 (3.16.14.17) + libv8 (3.16.14.19) loofah (2.0.3) nokogiri (>= 1.5.9) - mail (2.6.4) + mail (2.6.6) mime-types (>= 1.16, < 4) method_source (0.8.2) mime-types (3.1) mime-types-data (~> 3.2015) mime-types-data (3.2016.0521) - mini_magick (4.6.0) + mini_magick (4.8.0) + mini_mime (0.1.3) mini_portile2 (2.2.0) - minitest (5.10.2) + minitest (5.10.3) nokogiri (1.8.0) mini_portile2 (~> 2.2.0) omniauth (1.6.1) @@ -168,36 +167,36 @@ GEM omniauth (~> 1.0) rack-openid (~> 1.3.1) orm_adapter (0.5.0) - parser (2.2.2.6) - ast (>= 1.1, < 3.0) - pdf-core (0.6.1) - pdf-inspector (1.2.1) - pdf-reader (~> 1.0) - pdf-reader (1.4.0) + parser (2.3.3.1) + ast (~> 2.2) + pdf-core (0.7.0) + pdf-inspector (1.3.0) + pdf-reader (>= 1.0, < 3.0.a) + pdf-reader (2.0.0) Ascii85 (~> 1.0.0) afm (~> 0.2.1) hashery (~> 2.0) ruby-rc4 ttfunk - pg (0.19.0) - poltergeist (1.11.0) + pg (0.21.0) + poltergeist (1.15.0) capybara (~> 2.1) cliver (~> 0.3.1) websocket-driver (>= 0.2.0) powerpack (0.1.1) - prawn (2.1.0) - pdf-core (~> 0.6.1) - ttfunk (~> 1.4.0) + prawn (2.2.2) + pdf-core (~> 0.7.0) + ttfunk (~> 1.5) prawn-table (0.2.2) prawn (>= 1.3.0, < 3.0.0) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) slop (~> 3.4) - pry-rails (0.3.4) - pry (>= 0.9.10) - public_suffix (2.0.4) - puma (3.6.2) + pry-rails (0.3.6) + pry (>= 0.10.4) + public_suffix (2.0.5) + puma (3.9.1) rack (1.6.8) rack-openid (1.3.1) rack (>= 1.1.0) @@ -233,12 +232,14 @@ GEM activesupport (= 4.2.7.1) rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) - rainbow (2.1.0) - rake (11.3.0) + rainbow (2.2.2) + rake + rake (12.0.0) redcarpet (3.4.0) ref (2.0.0) - responders (2.3.0) - railties (>= 4.2.0, < 5.1) + responders (2.4.0) + actionpack (>= 4.2.0, < 5.3) + railties (>= 4.2.0, < 5.3) rspec (3.6.0) rspec-core (~> 3.6.0) rspec-expectations (~> 3.6.0) @@ -262,23 +263,23 @@ GEM rspec-steps (2.1.1) rspec (>= 3.0, < 3.99) rspec-support (3.6.0) - rubocop (0.29.1) - astrolabe (~> 1.3) - parser (>= 2.2.0.1, < 3.0) + rubocop (0.46.0) + parser (>= 2.3.1.1, < 3.0) powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) - ruby-progressbar (~> 1.4) + ruby-progressbar (~> 1.7) + unicode-display_width (~> 1.0, >= 1.0.1) ruby-openid (2.7.0) ruby-progressbar (1.8.1) ruby-rc4 (0.1.5) - rubyzip (1.2.0) + rubyzip (1.2.1) simplecov (0.12.0) docile (~> 1.1.0) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) - simplecov-html (0.10.0) + simplecov-html (0.10.1) slop (3.6.0) - spring (2.0.0) + spring (2.0.2) activesupport (>= 4.2) sprockets (3.6.3) concurrent-ruby (~> 1.0) @@ -287,20 +288,20 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - sqlite3 (1.3.12) - term-ansicolor (1.3.2) + sqlite3 (1.3.13) + term-ansicolor (1.6.0) tins (~> 1.0) - therubyracer (0.12.2) - libv8 (~> 3.16.14.0) + therubyracer (0.12.3) + libv8 (~> 3.16.14.15) ref thor (0.19.4) thread_safe (0.3.6) - tilt (2.0.5) - tins (1.13.0) - ttfunk (1.4.0) + tilt (2.0.8) + tins (1.15.0) + ttfunk (1.5.1) turbolinks (5.0.1) turbolinks-source (~> 5) - turbolinks-source (5.0.0) + turbolinks-source (5.0.3) twitter-bootstrap-rails (3.2.2) actionpack (>= 3.1) execjs (>= 2.2.2, >= 2.2) @@ -308,20 +309,21 @@ GEM railties (>= 3.1) tzinfo (1.2.3) thread_safe (~> 0.1) - warden (1.2.6) + unicode-display_width (1.3.0) + warden (1.2.7) rack (>= 1.0) web-console (2.3.0) activemodel (>= 4.0) binding_of_caller (>= 0.7.2) railties (>= 4.0) sprockets-rails (>= 2.0, < 4.0) - websocket-driver (0.6.4) + websocket-driver (0.6.5) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) - will_paginate (3.1.5) + will_paginate (3.1.6) will_paginate-bootstrap (1.0.1) will_paginate (>= 3.0.3) - xpath (2.0.0) + xpath (2.1.0) nokogiri (~> 1.3) PLATFORMS @@ -358,7 +360,7 @@ DEPENDENCIES omniauth omniauth-openid owlcarousel-rails! - parser (~> 2.2.2.5) + parser (~> 2.3.3.1) pdf-inspector pg poltergeist @@ -373,7 +375,7 @@ DEPENDENCIES ref rspec-rails (~> 3.6) rspec-steps (~> 2.1.1) - rubocop (~> 0.29.1) + rubocop (= 0.46) rubyzip simplecov spring diff --git a/app/controllers/agreement_letters_controller.rb b/app/controllers/agreement_letters_controller.rb index 35999bc6..7cd0147c 100644 --- a/app/controllers/agreement_letters_controller.rb +++ b/app/controllers/agreement_letters_controller.rb @@ -15,7 +15,7 @@ def create if @agreement_letter.save && @agreement_letter.save_file(file) redirect_to check_application_letter_path(application_letter), - notice: t("agreement_letters.upload_success") + notice: t('agreement_letters.upload_success') else redirect_to check_application_letter_path(application_letter), alert: @agreement_letter.errors.full_messages diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7662412c..48532cae 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,47 +3,47 @@ class ApplicationController < ActionController::Base # For APIs, you may want to use :null_session instead. protect_from_forgery with: :exception before_action :configure_permitted_parameters, if: :devise_controller? - before_action :store_current_location, :unless => :devise_controller? + before_action :store_current_location, unless: :devise_controller? before_action :add_missing_permission_flashes rescue_from CanCan::AccessDenied do |exception| respond_to do |format| format.json { head :forbidden } - format.html { redirect_to main_app.root_url, :alert => exception.message } + format.html { redirect_to main_app.root_url, alert: exception.message } end end def index - # FIXME application-side filtering isn't all that great, but - # unless we want to write custom SQL joins (which - # we should if this becomes a perf problem), there is no - # other solution + # Application-side filtering isn't all that great, but + # unless we want to write custom SQL joins (which + # we should if this becomes a perf problem), there is no + # other solution @events = Event.sorted_by_start_date(true) - .select { |a| a.start_date > Time.zone.now.yesterday } - .first(3) + .select { |a| a.start_date > Time.zone.now.yesterday } + .first(3) render 'index', locals: { full_width: true } end - def imprint - end + def imprint; end def add_missing_permission_flashes - if current_user - flash.now[:warning] ||= [] + flash.now[:warning] ||= [] current_user.events_with_missing_agreement_letters.each do |event| application_letter = ApplicationLetter.where(user: current_user, event: event).first path = check_application_letter_path(application_letter) + next unless current_user.older_than_required_age_at_start_date_of_event?(event, current_user.profile.age) && application_letter.accepted? && event.acceptances_have_been_sent? flash.now[:warning] << "#{t('agreement_letters.please_upload', event: event.name)} #{t('agreement_letters.upload')} - ".html_safe if current_user.older_than_required_age_at_start_date_of_event?(event, current_user.profile.age) && application_letter.accepted? && event.acceptances_have_been_sent? + ".html_safe end end end protected + def configure_permitted_parameters devise_parameter_sanitizer.permit(:sign_up) do |user_params| user_params.permit(:email, :name, :password, :password_confirmation, :role) @@ -54,6 +54,7 @@ def configure_permitted_parameters end private + # override the devise helper to store the current location so we can # redirect to it after logging in or out. This override makes signing in # and signing up work automatically. diff --git a/app/controllers/application_letters_controller.rb b/app/controllers/application_letters_controller.rb index 04d928b7..ec75697f 100644 --- a/app/controllers/application_letters_controller.rb +++ b/app/controllers/application_letters_controller.rb @@ -1,8 +1,8 @@ class ApplicationLettersController < ApplicationController load_and_authorize_resource param_method: :application_params - skip_authorize_resource :only => :new + skip_authorize_resource only: :new - before_action :set_application, only: [:show, :edit, :update, :destroy, :check] + before_action :set_application, only: %i(show edit update destroy check) # GET /applications def index @@ -17,23 +17,23 @@ def show # GET /applications/new def new - if not current_user + if !current_user message = I18n.t('application_letters.login_before_creation') flash[:event_id] = params[:event_id] flash.keep(:event_id) - return redirect_to user_session_path, :alert => message - elsif not current_user.profile.present? + return redirect_to user_session_path, alert: message + elsif !current_user.profile.present? message = I18n.t('application_letters.fill_in_profile_before_creation') flash[:event_id] = params[:event_id] flash.keep(:event_id) - return redirect_to new_profile_path, :alert => message + return redirect_to new_profile_path, alert: message end @application_letter = ApplicationLetter.new - last_application_letter = ApplicationLetter.where(user: current_user).order("created_at").last + last_application_letter = ApplicationLetter.where(user: current_user).order('created_at').last if last_application_letter attrs_to_fill_in = last_application_letter.attributes - .slice("emergency_number", "organisation", "vegetarian", "vegan", "allergies") + .slice('emergency_number', 'organisation', 'vegetarian', 'vegan', 'allergies') @application_letter.attributes = attrs_to_fill_in flash.now[:notice] = I18n.t('application_letters.fields_filled_in') end @@ -66,7 +66,7 @@ def edit # POST /applications def create @application_letter = ApplicationLetter.new(application_params) - #event must be param to new_application_letter_path + # event must be param to new_application_letter_path seminar_name = '' if params[:event_id] @application_letter.event_id = params[:event_id] @@ -80,15 +80,15 @@ def create # Send Confirmation E-Mail email_params = { - :hide_recipients => true, - :recipients => current_user.email, - :reply_to => Rails.configuration.reply_to_address, - :subject => I18n.t('controllers.application_letters.confirmation_mail.subject'), - :content => I18n.t("controllers.application_letters.confirmation_mail.content_#{current_user.profile.gender}", - :seminar_name => seminar_name, - :first_name => current_user.profile.first_name, - :last_name => current_user.profile.last_name, - :event_link => application_letters_url) + hide_recipients: true, + recipients: current_user.email, + reply_to: Rails.configuration.reply_to_address, + subject: I18n.t('controllers.application_letters.confirmation_mail.subject'), + content: I18n.t("controllers.application_letters.confirmation_mail.content_#{current_user.profile.gender}", + seminar_name: seminar_name, + first_name: current_user.profile.first_name, + last_name: current_user.profile.last_name, + event_link: application_letters_url) } @email = Email.new(email_params) Mailer.send_generic_email(@email.hide_recipients, @email.recipients, @email.reply_to, @email.subject, @email.content) @@ -121,7 +121,11 @@ def update_status mail_tooltip: @application_letter.event.send_mails_tooltip } else - redirect_to :back, notice: I18n.t('application_letters.successful_update') rescue ActionController::RedirectBackError redirect_to root_path + begin + redirect_to :back, notice: I18n.t('application_letters.successful_update') + rescue ActionController::RedirectBackError + redirect_to root_path + end end else render :edit @@ -135,21 +139,22 @@ def destroy end private - # Use callbacks to share common setup or constraints between actions. - def set_application - @application_letter = ApplicationLetter.find(params[:id]) - end - # Only allow a trusted parameter "white list" through. - # Don't allow user_id as you shouldn't be able to set the user from outside of create/update. - def application_params - params.require(:application_letter).permit(:motivation, :emergency_number, :organisation, - :vegetarian, :vegan, :allergies, :annotation, :user_id, :event_id) - .merge({:custom_application_fields => params[:custom_application_fields]}) - end + # Use callbacks to share common setup or constraints between actions. + def set_application + @application_letter = ApplicationLetter.find(params[:id]) + end - # Only allow to update the status - def application_status_param - params.require(:application_letter).permit(:status) - end + # Only allow a trusted parameter "white list" through. + # Don't allow user_id as you shouldn't be able to set the user from outside of create/update. + def application_params + params.require(:application_letter).permit(:motivation, :emergency_number, :organisation, + :vegetarian, :vegan, :allergies, :annotation, :user_id, :event_id) + .merge(custom_application_fields: params[:custom_application_fields]) + end + + # Only allow to update the status + def application_status_param + params.require(:application_letter).permit(:status) + end end diff --git a/app/controllers/application_notes_controller.rb b/app/controllers/application_notes_controller.rb index 74ade9dd..f921918f 100644 --- a/app/controllers/application_notes_controller.rb +++ b/app/controllers/application_notes_controller.rb @@ -5,11 +5,12 @@ def create if @application_note.valid? redirect_to application_letter_path(@application_letter) else - render "application_letters/show" + render 'application_letters/show' end end private + def notes_params params.require(:application_note).permit(:note) end diff --git a/app/controllers/emails_controller.rb b/app/controllers/emails_controller.rb index 92a3dfe2..05746581 100644 --- a/app/controllers/emails_controller.rb +++ b/app/controllers/emails_controller.rb @@ -1,5 +1,4 @@ class EmailsController < ApplicationController - def show authorize! :send_email, Email @event = Event.find(params[:event_id]) @@ -8,12 +7,12 @@ def show @templates = EmailTemplate.with_status(status) if status == :acceptance @addresses = @event.email_addresses_of_type_without_notification_sent(:accepted) - elsif (status == :rejection) + elsif status == :rejection @addresses = @event.email_addresses_of_type_without_notification_sent(:rejected) if @event.has_participants_without_status_notification?(:alternative) @addresses.append(@event.email_addresses_of_type_without_notification_sent(:alternative)) - end - else + end + else @addresses = [] end @@ -36,9 +35,7 @@ def submit_generic authorize! :send_email, Email @templates = [] @event = Event.find(params[:id]) - if params[:send] - send_generic - end + send_generic if params[:send] end private @@ -83,8 +80,8 @@ def send_generic def save_template @email = Email.new(email_params) - @template = EmailTemplate.new({ status: get_status, hide_recipients: @email.hide_recipients, - subject: @email.subject, content: @email.content }) + @template = EmailTemplate.new(status: get_status, hide_recipients: @email.hide_recipients, + subject: @email.subject, content: @email.content) if @email.validates_presence_of(:subject, :content) && @template.save flash.now[:success] = t('emails.submit.saving_successful') @@ -103,7 +100,7 @@ def update_event(event) if status == :acceptance event.set_status_notification_flag_for_applications_with_status(:accepted) event.acceptances_have_been_sent = true - if not (event.has_participants_without_status_notification?(:rejected) || @event.has_participants_without_status_notification?(:alternative)) + unless event.has_participants_without_status_notification?(:rejected) || @event.has_participants_without_status_notification?(:alternative) event.rejections_have_been_sent = true end elsif get_status == :rejection diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index e9ce560f..4092edd4 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -4,15 +4,14 @@ require 'rubygems' require 'zip' require 'carrierwave' -require "event_image_upload_helper" +require 'event_image_upload_helper' class EventsController < ApplicationController include EventImageUploadHelper load_and_authorize_resource - skip_authorize_resource :only => [:badges, :download_agreement_letters, :send_participants_email] - before_action :set_event, only: [:show, :edit, :update, :destroy, :participants, - :participants_pdf, :print_applications, :print_applications_eating_habits, :badges, :print_badges] - + skip_authorize_resource only: %i(badges download_agreement_letters send_participants_email) + before_action :set_event, only: %i(show edit update destroy participants + participants_pdf print_applications print_applications_eating_habits badges print_badges) # GET /events def index @@ -25,8 +24,8 @@ def archive # GET /events/1 def show - if @event.hidden and !can? :view_hidden, Event - redirect_to new_application_letter_path(:event_id => @event.id) + if @event.hidden && !can?(:view_hidden, Event) + redirect_to new_application_letter_path(event_id: @event.id) end @free_places = @event.compute_free_places @occupied_places = @event.compute_occupied_places @@ -91,12 +90,12 @@ def print_badges selected_participants &= @participants if selected_participants.empty? flash.now[:error] = I18n.t('events.badges.no_users_selected') - render 'badges' and return + render('badges') && return end begin pdf = BadgesPDF.generate(@event, selected_participants, name_format, show_color, show_organisation, logo) - send_data pdf, filename: "badges.pdf", type: "application/pdf", disposition: "inline" + send_data pdf, filename: 'badges.pdf', type: 'application/pdf', disposition: 'inline' rescue Prawn::Errors::UnsupportedImageType flash.now[:error] = I18n.t('events.badges.wrong_file_format') render 'badges' @@ -112,12 +111,12 @@ def participants # GET /events/1/print_applications def print_applications pdf = ApplicationsPDF.generate(@event) - send_data pdf, filename: "applications_#{@event.name}_#{Date.today}.pdf", type: "application/pdf", disposition: "inline" + send_data pdf, filename: "applications_#{@event.name}_#{Date.today}.pdf", type: 'application/pdf', disposition: 'inline' end def print_applications_eating_habits pdf = ParticipantsPDF.generate(@event) - send_data pdf, filename: "applications_eating_habits_#{@event.name}_#{Date.today}.pdf", type: "application/pdf", disposition: "inline" + send_data pdf, filename: "applications_eating_habits_#{@event.name}_#{Date.today}.pdf", type: 'application/pdf', disposition: 'inline' end # GET /events/1/accept-all-applicants @@ -131,21 +130,21 @@ def accept_all_applicants def send_participants_email authorize! :send_email, Email event = Event.find(params[:id]) - @email = event.generate_participants_email(params[:all],params[:groups], params[:users]) + @email = event.generate_participants_email(params[:all], params[:groups], params[:users]) @templates = [] @send_generic = true render '/emails/email' end - #POST /events/1/participants/agreement_letters + # POST /events/1/participants/agreement_letters # creates either a zip or a pdf containing all agreement letters for all selected participants def download_agreement_letters @event = Event.find(params[:id]) - if not params.has_key?(:selected_participants) - redirect_to event_participants_url(@event), notice: I18n.t('events.agreement_letters_download.notices.no_participants_selected') and return + unless params.key?(:selected_participants) + redirect_to(event_participants_url(@event), notice: I18n.t('events.agreement_letters_download.notices.no_participants_selected')) && return end authorize! :print_agreement_letters, @event - if params[:download_type] == "zip" + if params[:download_type] == 'zip' filename = "agreement_letters_#{@event.name}_#{Date.today}.zip" temp_file = Tempfile.new(filename) number_of_files = 0 @@ -158,23 +157,23 @@ def download_agreement_letters unless agreement_letter.nil? number_of_files += 1 - zipfile.add(number_of_files.to_s + "_" + user.name + ".pdf", agreement_letter.path) + zipfile.add(number_of_files.to_s + '_' + user.name + '.pdf', agreement_letter.path) end end end zip_data = File.read(temp_file.path) if number_of_files != 0 - send_data(zip_data, :type => 'application/zip', :filename => filename) + send_data(zip_data, type: 'application/zip', filename: filename) end ensure temp_file.close temp_file.unlink end if number_of_files == 0 - redirect_to event_participants_url(@event), notice: I18n.t('events.agreement_letters_download.notices.no_agreement_letters') and return + redirect_to(event_participants_url(@event), notice: I18n.t('events.agreement_letters_download.notices.no_agreement_letters')) && return end end - if params[:download_type] == "pdf" + if params[:download_type] == 'pdf' empty = true pdf = CombinePDF.new params[:selected_participants].each do |participant_id| @@ -185,9 +184,9 @@ def download_agreement_letters end end if empty - redirect_to event_participants_url(@event), notice: I18n.t('events.agreement_letters_download.notices.no_agreement_letters') and return + redirect_to(event_participants_url(@event), notice: I18n.t('events.agreement_letters_download.notices.no_agreement_letters')) && return end - send_data pdf.to_pdf, filename: "agreement_letters_#{@event.name}_#{Date.today}.pdf", type: "application/pdf", disposition: "inline" unless pdf.nil? + send_data pdf.to_pdf, filename: "agreement_letters_#{@event.name}_#{Date.today}.pdf", type: 'application/pdf', disposition: 'inline' unless pdf.nil? end end @@ -195,29 +194,29 @@ def download_agreement_letters def upload_material event = Event.find(params[:event_id]) material_path = event.material_path - Dir.mkdir(material_path) unless File.exists?(material_path) + Dir.mkdir(material_path) unless File.exist?(material_path) file = params[:file_upload] unless is_file?(file) - redirect_to event_path(event), alert: t("events.material_area.no_file_given") + redirect_to event_path(event), alert: t('events.material_area.no_file_given') return false end begin - File.write(File.join(material_path, file.original_filename), file.read, mode: "wb") + File.write(File.join(material_path, file.original_filename), file.read, mode: 'wb') rescue IOError - redirect_to event_path(event), alert: I18n.t("events.material_area.saving_fails") + redirect_to event_path(event), alert: I18n.t('events.material_area.saving_fails') return false end - redirect_to event_path(event), notice: I18n.t("events.material_area.success_message") + redirect_to event_path(event), notice: I18n.t('events.material_area.success_message') end # GET /event/1/participants_pdf def participants_pdf - default = {:order_by => 'email', :order_direction => 'asc'} + default = { order_by: 'email', order_direction: 'asc' } default = default.merge(params) @application_letters = @event.application_letters_ordered(default[:order_by], default[:order_direction]) - .where(:status => ApplicationLetter.statuses[:accepted]) + .where(status: ApplicationLetter.statuses[:accepted]) data = @application_letters.collect do |application_letter| [ @@ -229,74 +228,75 @@ def participants_pdf end data.unshift([ - I18n.t('controllers.events.participants_pdf.first_name'), - I18n.t('controllers.events.participants_pdf.last_name'), - I18n.t('controllers.events.participants_pdf.date_of_birth'), - I18n.t('controllers.events.participants_pdf.allergies') - ]) + I18n.t('controllers.events.participants_pdf.first_name'), + I18n.t('controllers.events.participants_pdf.last_name'), + I18n.t('controllers.events.participants_pdf.date_of_birth'), + I18n.t('controllers.events.participants_pdf.allergies') + ]) name = @event.name - doc = Prawn::Document.new(:page_size => 'A4') do - text "Teilnehmerliste - " + name + doc = Prawn::Document.new(page_size: 'A4') do + text 'Teilnehmerliste - ' + name table(data, width: bounds.width) end - send_data doc.render, :filename => "participants.pdf", :type => "application/pdf", disposition: "inline" + send_data doc.render, filename: 'participants.pdf', type: 'application/pdf', disposition: 'inline' end # POST /events/1/download_material def download_material event = Event.find(params[:event_id]) - unless params.has_key?(:file) - redirect_to event_path(event), alert: I18n.t('events.material_area.no_file_given') and return + unless params.key?(:file) + redirect_to(event_path(event), alert: I18n.t('events.material_area.no_file_given')) && return end file_full_path = File.join(event.material_path, params[:file]) - unless File.exists?(file_full_path) - redirect_to event_path(event), alert: t("events.material_area.download_file_not_found") and return + unless File.exist?(file_full_path) + redirect_to(event_path(event), alert: t('events.material_area.download_file_not_found')) && return end - send_file file_full_path, :x_sendfile => true + send_file file_full_path, x_sendfile: true end private - def set_event - @event = Event.find(params[:id]) - end - def event_params - params.require(:event).permit(:name, :description, :image, :custom_image, :custom_image_cache, :max_participants, :organizer, :knowledge_level, :application_deadline, :published, :hidden, :custom_application_fields => [], date_ranges_attributes: [:start_date, :end_date, :id]) - end + def set_event + @event = Event.find(params[:id]) + end - def add_event_query_conditions(query) - conditions = {} - conditions[:hidden] = false unless can? :view_hidden, Event - conditions[:published] = true unless can? :view_unpublished, Event - query.where(conditions) - end + def event_params + params.require(:event).permit(:name, :description, :image, :custom_image, :custom_image_cache, :max_participants, :organizer, :knowledge_level, :application_deadline, :published, :hidden, custom_application_fields: [], date_ranges_attributes: %i(start_date end_date id)) + end - def filter_application_letters(application_letters) - application_letters = application_letters.to_a - filters = (params[:filter] || {}).select { |k, v| v == '1' }.map{ |k, v| k.to_s } - if filters.count > 0 # skip filtering if no filters have been set - application_letters.keep_if { |l| filters.include?(l.status) } - end - application_letters - end + def add_event_query_conditions(query) + conditions = {} + conditions[:hidden] = false unless can? :view_hidden, Event + conditions[:published] = true unless can? :view_unpublished, Event + query.where(conditions) + end - # Checks if a file is valid and not empty - # - # @param [ActionDispatch::Http::UploadedFile] is a file object - # @return [Boolean] whether @file is a valid file - def is_file?(file) - file.respond_to?(:open) && file.respond_to?(:content_type) && file.respond_to?(:size) + def filter_application_letters(application_letters) + application_letters = application_letters.to_a + filters = (params[:filter] || {}).select { |_k, v| v == '1' }.map { |k, _v| k.to_s } + if filters.count > 0 # skip filtering if no filters have been set + application_letters.keep_if { |l| filters.include?(l.status) } end + application_letters + end - # Gets all file names stored in the material storage of the event - # - # @param [Event] - # @return [Array of Strings] - def get_material_files(event) - material_path = event.material_path - File.exists?(material_path) ? Dir.glob(File.join(material_path, "*")) : [] - end + # Checks if a file is valid and not empty + # + # @param [ActionDispatch::Http::UploadedFile] is a file object + # @return [Boolean] whether @file is a valid file + def is_file?(file) + file.respond_to?(:open) && file.respond_to?(:content_type) && file.respond_to?(:size) + end + + # Gets all file names stored in the material storage of the event + # + # @param [Event] + # @return [Array of Strings] + def get_material_files(event) + material_path = event.material_path + File.exist?(material_path) ? Dir.glob(File.join(material_path, '*')) : [] + end end diff --git a/app/controllers/participant_groups_controller.rb b/app/controllers/participant_groups_controller.rb index bfefa393..a743edaf 100644 --- a/app/controllers/participant_groups_controller.rb +++ b/app/controllers/participant_groups_controller.rb @@ -1,23 +1,20 @@ class ParticipantGroupsController < ApplicationController - load_and_authorize_resource # PATCH/PUT /participant_group/1/group def update - begin - if @participant_group.update_attributes(participant_group_params) - redirect_to :back, notice: I18n.t('participant_groups.update.successful') - else - redirect_to :back, alert: I18n.t('participant_groups.update.failed') - end - rescue ActionController::RedirectBackError - redirect_to root_path + if @participant_group.update_attributes(participant_group_params) + redirect_to :back, notice: I18n.t('participant_groups.update.successful') + else + redirect_to :back, alert: I18n.t('participant_groups.update.failed') end + rescue ActionController::RedirectBackError + redirect_to root_path end private - def participant_group_params - params.require(:participant_group).permit(:group) - end + def participant_group_params + params.require(:participant_group).permit(:group) + end end diff --git a/app/controllers/profiles_controller.rb b/app/controllers/profiles_controller.rb index 2dd095c0..a997b0bf 100644 --- a/app/controllers/profiles_controller.rb +++ b/app/controllers/profiles_controller.rb @@ -1,11 +1,10 @@ class ProfilesController < ApplicationController load_and_authorize_resource - before_action :set_profile, only: [:show, :edit, :update, :destroy] + before_action :set_profile, only: %i(show edit update destroy) # GET /profiles/1 - def show - end + def show; end # GET /profiles/new def new @@ -31,7 +30,7 @@ def create if @profile.save if flash[:event_id] - redirect_to new_application_letter_path(:event_id => flash[:event_id]), notice: I18n.t('profiles.successful_creation') + redirect_to new_application_letter_path(event_id: flash[:event_id]), notice: I18n.t('profiles.successful_creation') else redirect_to edit_user_registration_path, notice: I18n.t('profiles.successful_creation') end @@ -55,13 +54,14 @@ def update end private - # Use callbacks to share common setup or constraints between actions. - def set_profile - @profile = Profile.find(params[:id]) - end - # Only allow a trusted parameter "white list" through. - def profile_params - params.require(:profile).permit(Profile.allowed_params) - end + # Use callbacks to share common setup or constraints between actions. + def set_profile + @profile = Profile.find(params[:id]) + end + + # Only allow a trusted parameter "white list" through. + def profile_params + params.require(:profile).permit(Profile.allowed_params) + end end diff --git a/app/controllers/requests_controller.rb b/app/controllers/requests_controller.rb index 366da9b7..8a341713 100644 --- a/app/controllers/requests_controller.rb +++ b/app/controllers/requests_controller.rb @@ -1,7 +1,7 @@ class RequestsController < ApplicationController load_and_authorize_resource - skip_authorize_resource only: [:new, :create] - before_action :set_request, only: [:show, :edit, :update, :destroy, :accept] + skip_authorize_resource only: %i(new create) + before_action :set_request, only: %i(show edit update destroy accept) # GET /requests def index @@ -10,8 +10,7 @@ def index end # GET /requests/1 - def show - end + def show; end # GET /requests/new def new @@ -19,8 +18,7 @@ def new end # GET /requests/1/edit - def edit - end + def edit; end # POST /requests def create @@ -46,7 +44,7 @@ def update def set_contact_person @request = Request.find(params[:request_id]) update_params = contact_person_params - if !update_params[:contact_person].nil? and @request.update(update_params) + if !update_params[:contact_person].nil? && @request.update(update_params) redirect_to @request, notice: I18n.t('requests.notice.was_updated') else render :show @@ -56,7 +54,7 @@ def set_contact_person def set_notes @request = Request.find(params[:request_id]) update_params = notes_params - if !update_params[:notes].nil? and @request.update(update_params) + if !update_params[:notes].nil? && @request.update(update_params) redirect_to @request, notice: I18n.t('requests.notice.was_updated') else render :show @@ -66,7 +64,7 @@ def set_notes # DELETE /requests/1 def destroy @request.destroy - redirect_to requests_url, notice:I18n.t('requests.notice.was_deleted') + redirect_to requests_url, notice: I18n.t('requests.notice.was_deleted') end def accept @@ -77,21 +75,22 @@ def accept end private - # Use callbacks to share common setup or constraints between actions. - def set_request - @request = Request.find(params[:id]) - end - # Only allow a trusted parameter "white list" through. - def request_params - params.require(:request).permit(:form_of_address, :first_name, :last_name, :phone_number, :street, :zip_code_city, :topic_of_workshop, :time_period, :email, :number_of_participants, :knowledge_level, :annotations) - end + # Use callbacks to share common setup or constraints between actions. + def set_request + @request = Request.find(params[:id]) + end - def contact_person_params - params.require(:request).permit(:contact_person) - end + # Only allow a trusted parameter "white list" through. + def request_params + params.require(:request).permit(:form_of_address, :first_name, :last_name, :phone_number, :street, :zip_code_city, :topic_of_workshop, :time_period, :email, :number_of_participants, :knowledge_level, :annotations) + end - def notes_params - params.require(:request).permit(:notes) - end + def contact_person_params + params.require(:request).permit(:contact_person) + end + + def notes_params + params.require(:request).permit(:notes) + end end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index 44eb03b1..4c4b7109 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -2,10 +2,10 @@ class Users::OmniauthCallbacksController < Devise::OmniauthCallbacksController skip_before_action :verify_authenticity_token, only: :hpiopenid def hpiopenid - @user = User.from_omniauth(request.env["omniauth.auth"]) + @user = User.from_omniauth(request.env['omniauth.auth']) if @user.persisted? - sign_in_and_redirect @user, :event => :authentication #this will throw if @user is not activated - set_flash_message(:notice, :success, :kind => "HPI OpenID") if is_navigational_format? + sign_in_and_redirect @user, event: :authentication # this will throw if @user is not activated + set_flash_message(:notice, :success, kind: 'HPI OpenID') if is_navigational_format? end end end diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index a82f89aa..6cc574f2 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -1,6 +1,6 @@ class Users::RegistrationsController < Devise::RegistrationsController -# before_action :configure_sign_up_params, only: [:create] -# before_action :configure_account_update_params, only: [:update] + # before_action :configure_sign_up_params, only: [:create] + # before_action :configure_account_update_params, only: [:update] # GET /resource/sign_up # def new @@ -22,7 +22,7 @@ def create else clean_up_passwords resource set_minimum_password_length - resource.errors.full_messages.each {|x| flash["error"] = x} + resource.errors.full_messages.each { |x| flash['error'] = x } redirect_to new_user_session_path(resource) end end @@ -34,7 +34,7 @@ def create # PUT /resource def update - unless params.fetch(:user, false) and params[:user].fetch(:profile, false) + unless params.fetch(:user, false) && params[:user].fetch(:profile, false) return super end @user = resource_class.to_adapter.get!(send(:"current_#{resource_name}").to_key) @@ -47,7 +47,6 @@ def update end end - # DELETE /resource # def destroy # super @@ -83,7 +82,7 @@ def after_sign_up_path_for(resource) stored_location = stored_location_for(resource) if stored_location referrer = Rails.application.routes.recognize_path(stored_location) - if referrer[:controller] == "application_letters" and referrer[:action] == "new" + if referrer[:controller] == 'application_letters' && referrer[:action] == 'new' # This comes from a application letter creation page -> redirect user there. return stored_location end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 09271c64..dd56ded7 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,22 +1,19 @@ class UsersController < ApplicationController - before_action :set_user, only: [:update_role] # GET /users def index authorize! :index, User - @users = User.with_profiles.paginate(:page => params[:page], :per_page => 20) + @users = User.with_profiles.paginate(page: params[:page], per_page: 20) if params[:search] - @users = User.search(params[:search]).paginate(:page => params[:page], :per_page => 20) + @users = User.search(params[:search]).paginate(page: params[:page], per_page: 20) end end # PATCH/PUT /users/1/role def update_role authorize! :update_role, @user - if user_params[:role] == "admin" - authorize! :update_role_to_admin, @user - end + authorize! :update_role_to_admin, @user if user_params[:role] == 'admin' if @user.update(user_params) redirect_to :back, notice: I18n.t('users.successful_role_update') @@ -28,8 +25,8 @@ def user_params end private + def set_user @user = User.find(params[:id]) end - -end \ No newline at end of file +end diff --git a/app/helpers/applicants_overview_helper.rb b/app/helpers/applicants_overview_helper.rb index b61375e6..dee090f9 100644 --- a/app/helpers/applicants_overview_helper.rb +++ b/app/helpers/applicants_overview_helper.rb @@ -1,9 +1,9 @@ module ApplicantsOverviewHelper def sort_caret(label, attr) is_sorted_ascending = (params[:sort] == attr.to_s) && params[:order] != 'descending' - url = "?sort=#{attr.to_s}" + - "#{'&order=descending' if is_sorted_ascending}" + - "#{'&' + params[:filter].map { |k,v| "filter[#{h(k)}]=#{h(v)}" }.join('&') if params[:filter]}" + url = "?sort=#{attr}" \ + "#{'&order=descending' if is_sorted_ascending}" \ + "#{'&' + params[:filter].map { |k, v| "filter[#{h(k)}]=#{h(v)}" }.join('&') if params[:filter]}" " #{label} @@ -11,16 +11,14 @@ def sort_caret(label, attr) end def sort_application_letters - if params[:sort] - unless Profile.allowed_sort_methods.include? params[:sort].to_sym - raise CanCan::AccessDenied + if Profile.allowed_sort_methods.include? params[:sort].to_sym + @application_letters.sort_by! { |l| l.user.profile.send(params[:sort]) } else - @application_letters.sort_by! {|l| l.user.profile.send(params[:sort]) } + raise CanCan::AccessDenied end end @application_letters.reverse! if params[:order] == 'descending' - end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 45b768dc..5b00bd05 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -5,27 +5,26 @@ # except paragraphs class MarkdownRenderTruncate < Redcarpet::Render::Base def paragraph(text) - CGI::escapeHTML(text) + ' ' + CGI.escapeHTML(text) + ' ' end - def link(link, title, content) - CGI::escapeHTML(content) + def link(_link, _title, content) + CGI.escapeHTML(content) end end module ApplicationHelper def menu_items (menu_item t(:events, scope: 'navbar'), events_path) + - request_menu_item + request_menu_item end def request_menu_item if can? :index, Request - item = (menu_item t(:requests, scope: 'navbar'), requests_path) + (menu_item t(:requests, scope: 'navbar'), requests_path) else - item = (menu_item t(:new_request, scope: 'navbar'), new_request_path) + (menu_item t(:new_request, scope: 'navbar'), new_request_path) end - item end # Render the given string as markdown @@ -36,21 +35,17 @@ def request_menu_item def markdown(text, truncatable = false) renderer = truncatable ? MarkdownRenderTruncate.new : - Redcarpet::Render::HTML.new({ - filter_html: true, - hard_wrap: true, - tables: true, - strikethrough: true, - link_attributes: { rel: 'nofollow', target: "_blank" }, - space_after_headers: true, - fenced_code_blocks: true - }) + Redcarpet::Render::HTML.new(filter_html: true, + hard_wrap: true, + tables: true, + strikethrough: true, + link_attributes: { rel: 'nofollow', target: '_blank' }, + space_after_headers: true, + fenced_code_blocks: true) - Redcarpet::Markdown.new(renderer, { - autolink: true, - superscript: true, - disable_indented_code_blocks: true - }).render(text).html_safe + Redcarpet::Markdown.new(renderer, autolink: true, + superscript: true, + disable_indented_code_blocks: true).render(text).html_safe end def dropdown_items @@ -62,15 +57,15 @@ def dropdown_items o << (menu_item t(:create_profile, scope: 'navbar'), new_profile_path) end # pupils get their applications - if current_user.role == "pupil" + if current_user.role == 'pupil' o << (menu_item t(:my_events, scope: 'navbar'), application_letters_path) end # admins get user management - if current_user.role == "admin" || current_user.role == "organizer" + if current_user.role == 'admin' || current_user.role == 'organizer' o << (menu_item t(:user_management, scope: 'navbar'), users_path) end # everyone gets logout - o << (menu_item t(:logout, scope: 'navbar'), destroy_user_session_path, :method => :delete) + o << (menu_item t(:logout, scope: 'navbar'), destroy_user_session_path, method: :delete) o.html_safe end diff --git a/app/helpers/participants_overview_helper.rb b/app/helpers/participants_overview_helper.rb index 32213893..53b02c90 100644 --- a/app/helpers/participants_overview_helper.rb +++ b/app/helpers/participants_overview_helper.rb @@ -1,17 +1,15 @@ module ParticipantsOverviewHelper - def sort_participants - @participants.sort_by! {|p| @event.participant_group_for(p).send('group') } if params[:sort] == 'group' + @participants.sort_by! { |p| @event.participant_group_for(p).send('group') } if params[:sort] == 'group' if params[:sort] == 'eating-habits' - - @participants.sort! do |a,b| + + @participants.sort! do |a, b| a = ApplicationLetter.find_by(user_id: a.id, event_id: @event.id) b = ApplicationLetter.find_by(user_id: b.id, event_id: @event.id) - a.get_eating_habit_state <=> b.get_eating_habit_state + a.get_eating_habit_state <=> b.get_eating_habit_state end end - + @participants.reverse! if params[:order] == 'descending' end - end diff --git a/app/mailers/portal_mailer.rb b/app/mailers/portal_mailer.rb index 42f1cc9f..69509920 100644 --- a/app/mailers/portal_mailer.rb +++ b/app/mailers/portal_mailer.rb @@ -8,10 +8,10 @@ class PortalMailer < ApplicationMailer # @param some_attachments - array of hashes with name and content # @return [ActionMailer::MessageDelivery] a mail object with the given parameters. def generic_email(recipients, reply_to, subject, content, attached_files = []) - attached_files.each do | attachment | + attached_files.each do |attachment| attachments[attachment[:name]] = attachment[:content] end - mail(to: recipients, reply_to: reply_to, subject: subject) do | format | + mail(to: recipients, reply_to: reply_to, subject: subject) do |format| format.html { markdown content } format.text { content } end diff --git a/app/models/ability.rb b/app/models/ability.rb index 5ddfaae2..3213ed12 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -34,55 +34,52 @@ def initialize(user) # Even guests can see the apply button # This is revoked for coaches and organizers below. can :view_apply_button, Event - can [:show, :index, :archive], Event - + can %i(show index archive), Event if user.role? :pupil # Pupils can only edit their own profiles - can [:new, :create], Profile - can [:index, :show, :edit, :update, :destroy], Profile, user: { id: user.id } + can %i(new create), Profile + can %i(index show edit update destroy), Profile, user: { id: user.id } # Pupils can only edit their own applications - if user.profile.present? - can [:new, :create], ApplicationLetter - end - can [:index, :show, :edit, :update, :check, :destroy], ApplicationLetter, user: { id: user.id } + can %i(new create), ApplicationLetter if user.profile.present? + can %i(index show edit update check destroy), ApplicationLetter, user: { id: user.id } # Pupils can upload their letters of agreement can [:create], AgreementLetter - can [:new, :create], Request + can %i(new create), Request cannot :view_personal_details, ApplicationLetter, user: { id: !user.id } end if user.role? :coach # Coaches can view Applications and participants for and view, upload and download materials for Event - can [:view_applicants, :view_participants, :view_material, :upload_material, :print_applications, :download_material], Event - can [:view_and_add_notes, :show], ApplicationLetter - can [:show, :index], Request + can %i(view_applicants view_participants view_material upload_material print_applications download_material), Event + can %i(view_and_add_notes show), ApplicationLetter + can %i(show index), Request cannot :view_apply_button, Event cannot :check, ApplicationLetter end if user.role? :organizer - can [:index, :show], Profile - can [:index, :show, :view_and_add_notes, :update_status], ApplicationLetter + can %i(index show), Profile + can %i(index show view_and_add_notes update_status), ApplicationLetter cannot :update, ApplicationLetter - can [:view_applicants, :edit_applicants, :view_participants, :print_applications, - :manage, :view_material, :upload_material, :print_agreement_letters, :download_material, - :view_unpublished, :show_eating_habits, :print_applications_eating_habits, :view_hidden], Event + can %i(view_applicants edit_applicants view_participants print_applications + manage view_material upload_material print_agreement_letters download_material + view_unpublished show_eating_habits print_applications_eating_habits view_hidden), Event can :send_email, Email - can [:manage, :set_contact_person, :set_notes], Request + can %i(manage set_contact_person set_notes), Request cannot :apply, Event cannot :view_apply_button, Event - can [:edit, :update, :destroy], Event + can %i(edit update destroy), Event can [:update], ParticipantGroup # Organizers can update user roles of pupil, coach and organizer, but cannot manage admins and cannot update a role to admin - can :manage, User, role: ["pupil", "coach", "organizer"] - cannot :update_role, User, role: "admin" + can :manage, User, role: %w(pupil coach organizer) + cannot :update_role, User, role: 'admin' cannot :update_role_to_admin, User end if user.role? :admin can :manage, :all can :view_delete_button, ApplicationLetter - cannot [:edit, :update], ApplicationLetter + cannot %i(edit update), ApplicationLetter end end end diff --git a/app/models/agreement_letter.rb b/app/models/agreement_letter.rb index 0d403a47..60723b2e 100644 --- a/app/models/agreement_letter.rb +++ b/app/models/agreement_letter.rb @@ -20,7 +20,7 @@ class AgreementLetter < ActiveRecord::Base belongs_to :event validates_presence_of :user, :event, :path MAX_SIZE = 300_000_000 - ALLOWED_MIMETYPE = "application/pdf" + ALLOWED_MIMETYPE = 'application/pdf'.freeze STORAGE_DIR = Rails.root.join('storage', 'agreement_letters') # Generates a unique filename for an AgreementLetter's file @@ -38,14 +38,14 @@ def filename # @return [Boolean] whether saving the file was saved def save_file(file) unless valid_file?(file) - self.destroy + destroy return false end begin File.write(path, file.read, mode: 'wb') true rescue IOError - self.destroy + destroy errors.add(I18n.t('agreement_letters.file_error'), I18n.t('agreement_letters.write_failed')) false end @@ -70,49 +70,48 @@ def set_path end private - def valid_file?(file) - if !is_file?(file) - errors.add(I18n.t('agreement_letters.file_error'), I18n.t("agreement_letters.not_a_file")) - false - elsif too_big?(file) - errors.add(I18n.t('agreement_letters.file_error'), I18n.t("agreement_letters.file_too_big")) - false - elsif wrong_filetype?(file) - errors.add(I18n.t('agreement_letters.file_error'), I18n.t("agreement_letters.wrong_filetype")) - false - elsif unable_to_open?(file) - errors.add(I18n.t('agreement_letters.file_error'), I18n.t("agreement_letters.corrupt_document")) - false - else - true - end - end - - def is_file?(file) - file.respond_to?(:open) && file.respond_to?(:content_type) && file.respond_to?(:size) - end - - def too_big?(file) - file.size > MAX_SIZE - end - - def wrong_filetype?(file) - file.content_type != ALLOWED_MIMETYPE - end - def unable_to_open? file - begin - PDF::Inspector::Page.analyze_file(file.open) + def valid_file?(file) + if !is_file?(file) + errors.add(I18n.t('agreement_letters.file_error'), I18n.t('agreement_letters.not_a_file')) + false + elsif too_big?(file) + errors.add(I18n.t('agreement_letters.file_error'), I18n.t('agreement_letters.file_too_big')) + false + elsif wrong_filetype?(file) + errors.add(I18n.t('agreement_letters.file_error'), I18n.t('agreement_letters.wrong_filetype')) + false + elsif unable_to_open?(file) + errors.add(I18n.t('agreement_letters.file_error'), I18n.t('agreement_letters.corrupt_document')) false - rescue PDF::Reader::UnsupportedFeatureError, PDF::Reader::MalformedPDFError + else true end end + def is_file?(file) + file.respond_to?(:open) && file.respond_to?(:content_type) && file.respond_to?(:size) + end + + def too_big?(file) + file.size > MAX_SIZE + end + + def wrong_filetype?(file) + file.content_type != ALLOWED_MIMETYPE + end + + def unable_to_open?(file) + PDF::Inspector::Page.analyze_file(file.open) + false + rescue PDF::Reader::UnsupportedFeatureError, PDF::Reader::MalformedPDFError + true + end + def self.get_attachment { - name: (I18n.t 'emails.agreement_letter_attachment'), - content: File.read(Rails.configuration.empty_agreement_letter_path) + name: (I18n.t 'emails.agreement_letter_attachment'), + content: File.read(Rails.configuration.empty_agreement_letter_path) } end end diff --git a/app/models/application_letter.rb b/app/models/application_letter.rb index 41ca3dce..eae843d8 100644 --- a/app/models/application_letter.rb +++ b/app/models/application_letter.rb @@ -32,23 +32,22 @@ class ApplicationLetter < ActiveRecord::Base serialize :custom_application_fields, Array validates_presence_of :user, :event, :motivation, :emergency_number, :organisation - #Use 0 as default for hidden event applications + # Use 0 as default for hidden event applications validates :vegetarian, :vegan, :allergic, inclusion: { in: [true, false] } validates :vegetarian, :vegan, :allergic, exclusion: { in: [nil] } - validate :deadline_cannot_be_in_the_past, :if => Proc.new { |letter| !(letter.status_changed? || letter.status_notification_sent_changed?) } - validate :status_notification_sent_cannot_be_changed, :if => Proc.new { |letter| letter.status_notification_sent_changed? } - validate :status_cannot_be_changed, :if => Proc.new { |letter| letter.status_changed? } + validate :deadline_cannot_be_in_the_past, if: proc { |letter| !(letter.status_changed? || letter.status_notification_sent_changed?) } + validate :status_notification_sent_cannot_be_changed, if: proc { |letter| letter.status_notification_sent_changed? } + validate :status_cannot_be_changed, if: proc { |letter| letter.status_changed? } - enum status: {accepted: 1, rejected: 0, pending: 2, alternative: 3, canceled: 4} + enum status: { accepted: 1, rejected: 0, pending: 2, alternative: 3, canceled: 4 } validates :status, inclusion: { in: statuses.keys } - # Returns an array of selectable statuses # # @param none # @return [Array ] array of selectable statuses def self.selectable_statuses - ["accepted","rejected","pending","alternative"] + %w(accepted rejected pending alternative) end # Checks if the deadline is over @@ -86,21 +85,21 @@ def status_notification_sent_change_allowed? # Adds error def deadline_cannot_be_in_the_past if after_deadline? - errors.add(:event, I18n.t("application_letters.form.warning")) + errors.add(:event, I18n.t('application_letters.form.warning')) end end - - # Since EatingHabits are persited in booleans we need to generate a + + # Since EatingHabits are persited in booleans we need to generate a # EatingHabitStateCode to allow sorting # US 28_4.5 - - def get_eating_habit_state - eating_habit_state = 0 - eating_habit_state += 4 if vegetarian - eating_habit_state += 5 if vegan - eating_habit_state += 99 if allergic - return eating_habit_state - end + + def get_eating_habit_state + eating_habit_state = 0 + eating_habit_state += 4 if vegetarian + eating_habit_state += 5 if vegan + eating_habit_state += 99 if allergic + eating_habit_state + end # Chooses right status based on status and event deadline # @@ -108,20 +107,20 @@ def get_eating_habit_state # @return [String] to display on the website def status_type case ApplicationLetter.statuses[status] - when ApplicationLetter.statuses[:accepted] - return I18n.t("application_status.accepted") - when ApplicationLetter.statuses[:rejected] - return I18n.t("application_status.rejected") - when ApplicationLetter.statuses[:pending] - if after_deadline? - return I18n.t("application_status.pending_after_deadline") - else - return I18n.t("application_status.pending_before_deadline") - end - when ApplicationLetter.statuses[:canceled] - return I18n.t("application_status.canceled") + when ApplicationLetter.statuses[:accepted] + I18n.t('application_status.accepted') + when ApplicationLetter.statuses[:rejected] + I18n.t('application_status.rejected') + when ApplicationLetter.statuses[:pending] + if after_deadline? + I18n.t('application_status.pending_after_deadline') else - return I18n.t("application_status.alternative") + I18n.t('application_status.pending_before_deadline') + end + when ApplicationLetter.statuses[:canceled] + I18n.t('application_status.canceled') + else + I18n.t('application_status.alternative') end end @@ -154,7 +153,7 @@ def applicant_age_when_event_starts # @param none # @return [Array ] array of eating habits, empty if none def eating_habits - habits = Array.new + habits = [] habits.push(ApplicationLetter.human_attribute_name(:vegetarian)) if vegetarian habits.push(ApplicationLetter.human_attribute_name(:vegan)) if vegan habits.push(ApplicationLetter.human_attribute_name(:allergies)) if allergic @@ -162,6 +161,6 @@ def eating_habits end def allergic - not allergies.empty? + !allergies.empty? end end diff --git a/app/models/date_range.rb b/app/models/date_range.rb index a67356e3..fe39736b 100644 --- a/app/models/date_range.rb +++ b/app/models/date_range.rb @@ -16,7 +16,6 @@ # class DateRange < ActiveRecord::Base - belongs_to :event validate :validate_end_not_before_start diff --git a/app/models/email.rb b/app/models/email.rb index 2f8ac34f..b67b8263 100644 --- a/app/models/email.rb +++ b/app/models/email.rb @@ -12,11 +12,11 @@ class Email include ActiveModel::Conversion extend ActiveModel::Naming - attribute :hide_recipients, :type => Boolean - attribute :recipients, :type => String - attribute :reply_to, :type => String - attribute :subject, :type => String - attribute :content, :type => String + attribute :hide_recipients, type: Boolean + attribute :recipients, type: String + attribute :reply_to, type: String + attribute :subject, type: String + attribute :content, type: String validates_presence_of :recipients, :reply_to, :subject, :content validates_inclusion_of :hide_recipients, in: [true, false] diff --git a/app/models/email_template.rb b/app/models/email_template.rb index b778bfca..446f58ba 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -10,11 +10,10 @@ # class EmailTemplate < ActiveRecord::Base - enum status: { default: 0, acceptance: 1, rejection: 2 } validates_inclusion_of :status, in: statuses.keys - validates_inclusion_of :hide_recipients, in: [ true, false ] + validates_inclusion_of :hide_recipients, in: [true, false] validates_presence_of :subject, :content scope :with_status, ->(status) { where(status: statuses[status]).to_a } diff --git a/app/models/event.rb b/app/models/event.rb index 75edbfd6..d692823e 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -48,7 +48,7 @@ class Event < ActiveRecord::Base # if we uploaded a custom image, we want it to be synced to the "official" # image slot. only do this if we actually uploaded one and that image is valid def update_image - if custom_image.filename.present? and errors[:custom_image].empty? + if custom_image.filename.present? && errors[:custom_image].empty? self.image = '/' + custom_image.list_view.store_path end end @@ -59,12 +59,11 @@ def check_image_dimensions if custom_image.upload_width.present? && custom_image.upload_height.present? && (custom_image.upload_width < 200 || custom_image.upload_height < 155) - errors.add(:custom_image, I18n.t("events.errors.image_too_small")) + errors.add(:custom_image, I18n.t('events.errors.image_too_small')) custom_image.remove! end end - # Returns all participants for this event in following order: # 1. All participants that have to submit an letter of agreement but did not yet do so, ordered by name. # 2. All participants that have to submit an letter of agreement and did do so, ordered by name. @@ -73,8 +72,8 @@ def check_image_dimensions # @param none # @return [Array] the event's participants in that order. def participants_by_agreement_letter - @participants = self.participants - @participants.sort { |x, y| self.compare_participants_by_agreement(x,y) } + @participants = participants + @participants.sort { |x, y| compare_participants_by_agreement(x, y) } end # Checks if the participant selection is locked @@ -87,12 +86,12 @@ def participant_selection_locked # @return the minimum start_date over all date ranges def start_date - (date_ranges.min { |a,b| a.start_date <=> b.start_date }).start_date + date_ranges.min_by(&:start_date).start_date end # @return the minimum end_date over all date ranges def end_date - (date_ranges.max { |a,b| a.end_date <=> b.end_date }).end_date + date_ranges.max_by(&:end_date).end_date end # @return whether this event appears unreasonably long as defined by @@ -106,7 +105,7 @@ def has_date_ranges errors.add(:date_ranges, I18n.t('date_range.errors.no_timespan')) if date_ranges.blank? end - #validate that application deadline is before the start of the event + # validate that application deadline is before the start of the event def application_deadline_before_start_of_event errors.add(:application_deadline, I18n.t('events.errors.application_deadline_before_start_of_event')) if application_deadline.present? && !date_ranges.blank? && application_deadline > start_date end @@ -125,7 +124,7 @@ def after_deadline? # @return [Array] the event's participants def participants accepted_applications = application_letters.where(status: ApplicationLetter.statuses[:accepted]) - accepted_applications.collect { |a| a.user } + accepted_applications.collect(&:user) end # Returns the participant group for this event for a given participant (user). If it doesn't exist, it is created @@ -133,7 +132,7 @@ def participants # @param user [User] the user whose participant group we want # @return [ParticipantGroup] the user's participant group def participant_group_for(user) - participant_group = self.participant_groups.find_by(user: user) + participant_group = participant_groups.find_by(user: user) if participant_group.nil? participant_group = ParticipantGroup.create(event: self, user: user, group: ParticipantGroup::GROUPS.default) end @@ -145,7 +144,7 @@ def participant_group_for(user) # @param user [User] the user whose agreement letter we want # @return [AgreementLetter, nil] the user's agreement letter or nil def agreement_letter_for(user) - self.agreement_letters.where(user: user).take + agreement_letters.where(user: user).take end # Returns whether all application_letters are classified or not @@ -160,12 +159,10 @@ def applications_classified? # # @return [String] the translated tooltip text or nil if mails can be sent def send_mails_tooltip - if not applications_classified? + if !applications_classified? I18n.t 'events.applicants_overview.unclassified_applications_left' elsif compute_free_places < 0 I18n.t 'events.applicants_overview.maximum_number_of_participants_exeeded' - else - nil end end @@ -182,7 +179,7 @@ def accept_all_application_letters # @param status [Type] the desired application status the flag should be set for # @return none def set_status_notification_flag_for_applications_with_status(status) - applications = application_letters.select {|application| application.status == status.to_s} + applications = application_letters.select { |application| application.status == status.to_s } applications.each do |application_letter| application_letter.update(status_notification_sent: true) end @@ -205,11 +202,11 @@ def email_addresses_of_type_without_notification_sent(type) # @return [Email] new email def generate_participants_email(all, groups, users) Email.new( - :hide_recipients => false, - :recipients => email_addresses_of_participants(all, groups, users), - :reply_to => '', - :subject => '', - :content => '' + hide_recipients: false, + recipients: email_addresses_of_participants(all, groups, users), + reply_to: '', + subject: '', + content: '' ) end @@ -217,17 +214,17 @@ def generate_participants_email(all, groups, users) # # @return [Array>] def participants_with_id - participants.map{|participant| [participant.profile.name, participant.id]} + participants.map { |participant| [participant.profile.name, participant.id] } end # Returns a list of group names and their id # # @return Array> def groups_with_id - existing = ParticipantGroup::GROUPS.select do |group_id,_| - group_id != 0 && participant_groups.where(:group => group_id).any? + existing = ParticipantGroup::GROUPS.select do |group_id, _| + group_id != 0 && participant_groups.where(group: group_id).any? end - existing.map do |group_id,color_code| + existing.map do |group_id, color_code| [I18n.t("participant_groups.options.#{color_code}"), group_id] end end @@ -261,7 +258,7 @@ def has_participants_without_status_notification?(status) # @param none # @return [Symbol] state def phase - return :draft if !published + return :draft unless published return :application if published && !after_deadline? return :selection if published && after_deadline? && !(acceptances_have_been_sent && rejections_have_been_sent) return :execution if published && after_deadline? && acceptances_have_been_sent && rejections_have_been_sent @@ -281,7 +278,7 @@ def has_alternative_application_letters? # @return string containing the label or nil def application_deadline_label days = (application_deadline - Date.current).to_i - return I18n.t('events.notices.deadline_approaching', count: days) if days <= 7 and days >= 0 + return I18n.t('events.notices.deadline_approaching', count: days) if days <= 7 && days >= 0 end # Uses the start date to determine whether or not this event is in the past (or more @@ -289,7 +286,7 @@ def application_deadline_label # # @return boolean if it's in the past def is_past - return start_date < Date.current + start_date < Date.current end # Returns a label that describes the duration of the event in days, @@ -317,12 +314,12 @@ def duration_label # @return [ApplicationLetter] the application letters found def application_letters_ordered(field, order_by) field = case field - when "email" - "users.email" - when "birth_date", "first_name", "last_name" - "profiles." + field - else - "users.email" + when 'email' + 'users.email' + when 'birth_date', 'first_name', 'last_name' + 'profiles.' + field + else + 'users.email' end order_by = 'asc' unless order_by == 'asc' || order_by == 'desc' application_letters.joins(user: :profile).order(field + ' ' + order_by) @@ -331,7 +328,7 @@ def application_letters_ordered(field, order_by) # Make sure any assignment coming from the controller # replaces all date ranges instead of adding new ones def date_ranges_attributes=(*args) - self.date_ranges.clear + date_ranges.clear super(*args) end @@ -339,7 +336,7 @@ def date_ranges_attributes=(*args) # # @return [String] path in the material storage def material_path - File.join("storage/materials/", self.id.to_s + "_event") + File.join('storage/materials/', id.to_s + '_event') end # Make sure we add errors from our date_range children @@ -353,8 +350,8 @@ def material_path end end - scope :draft_is, ->(status) { where("not published = ?", status) } - scope :hidden_is, ->(status) { where("hidden = ?", status) } + scope :draft_is, ->(status) { where('not published = ?', status) } + scope :hidden_is, ->(status) { where('hidden = ?', status) } scope :with_date_ranges, -> { joins(:date_ranges).group('events.id').order('MIN(start_date)') } scope :future, -> { with_date_ranges.having('date(MAX(end_date)) > ?', Time.zone.yesterday.end_of_day) } scope :past, -> { with_date_ranges.having('date(MAX(end_date)) < ?', Time.zone.now.end_of_day) } @@ -386,7 +383,7 @@ def get_ical_attachment end end - {name: (I18n.t 'emails.ical_attachment'), content: cal.to_ical} + { name: (I18n.t 'emails.ical_attachment'), content: cal.to_ical } end protected @@ -396,30 +393,28 @@ def get_ical_attachment # @param none # @return [String] Concatenation of all email addresses of accepted applications, seperated by ',' def email_addresses_of_accepted_applicants - participants.collect { |user| user.email }.join(',') + participants.collect(&:email).join(',') end # Returns a list of email addresses of all participants that are in one of the given groups # # @return [String] def email_addresses_of_groups(groups) - if groups.nil? - groups = [] - end - groups.reduce([]) {|addresses, group| addresses + email_addresses_of_group(group)}.uniq + groups = [] if groups.nil? + groups.reduce([]) { |addresses, group| addresses + email_addresses_of_group(group) }.uniq end # Returns a list of email addresses of all participants that are in this exact group # # @return [String] def email_addresses_of_group(group) - participant_groups.where(:group => group).map {|participant_group| participant_group.user.email} + participant_groups.where(group: group).map { |participant_group| participant_group.user.email } end # Returns a list of email addresses def email_addresses_of_users(users) user = User.where(id: users) - user.map{ |user| user.email } + user.map(&:email) end # Returns all email addresses of user that fit the given criteria @@ -447,19 +442,14 @@ def compare_participants_by_agreement(participant1, participant2) end return 1 end - unless participant2.requires_agreement_letter_for_event?(self) - return -1 - end + return -1 unless participant2.requires_agreement_letter_for_event?(self) if participant1.agreement_letter_for_event?(self) if participant2.agreement_letter_for_event?(self) return participant1.email <=> participant2.email end return 1 end - if participant2.agreement_letter_for_event?(self) - return -1 - end - return participant1.email <=> participant2.email + return -1 if participant2.agreement_letter_for_event?(self) + participant1.email <=> participant2.email end - end diff --git a/app/models/participant_group.rb b/app/models/participant_group.rb index 29aacca0..f76e928a 100644 --- a/app/models/participant_group.rb +++ b/app/models/participant_group.rb @@ -14,13 +14,11 @@ # class ParticipantGroup < ActiveRecord::Base - belongs_to :user belongs_to :event GROUPS = { 0 => '0', 1 => '00008B', 2 => 'FF0000', 3 => 'FFFF00', 4 => 'FFA500', 5 => '90EE90', 6 => '800080', 7 => 'FF1493', 8 => '006400', 9 => 'ADD8E6', 10 => '000000' } GROUPS.default = 0 - validates_inclusion_of :group, :in => GROUPS.keys - + validates_inclusion_of :group, in: GROUPS.keys end diff --git a/app/models/profile.rb b/app/models/profile.rb index 3bd71b54..5d013e8d 100644 --- a/app/models/profile.rb +++ b/app/models/profile.rb @@ -23,21 +23,20 @@ # class Profile < ActiveRecord::Base - POSSIBLE_GENDERS = ['male', 'female', 'other'] - + POSSIBLE_GENDERS = %w(male female other).freeze + belongs_to :user validates_presence_of :user, :first_name, :last_name, :gender, :birth_date, :street_name, :zip_code, :city, :state, :country validate :birthdate_not_in_future validates_inclusion_of :gender, in: POSSIBLE_GENDERS - # Returns true if the user is 18 years old or older # # @param none # @return [Boolean] whether the user is an adult - def adult?() - self.birth_date.in_time_zone <= 18.years.ago + def adult? + birth_date.in_time_zone <= 18.years.ago end # Returns the age of the user based on the current date @@ -45,16 +44,16 @@ def adult?() # @param none # @return [Int] for age as number of years def age - return age_at_time(Time.zone.now) + age_at_time(Time.zone.now) end # Returns the age of the user based on the given date # # @param none # @return [Int] for age as number of years - def age_at_time (given_date) + def age_at_time(given_date) given_date_is_before_birth_date = given_date.month > birth_date.month || (given_date.month == birth_date.month && given_date.day >= birth_date.day) - return given_date.year - birth_date.year - (given_date_is_before_birth_date ? 0 : 1) + given_date.year - birth_date.year - (given_date_is_before_birth_date ? 0 : 1) end # Returns the Full Name of the user @@ -62,7 +61,7 @@ def age_at_time (given_date) # @param none # @return [String] of name def name - first_name + " " + last_name + first_name + ' ' + last_name end # Returns the address of the user @@ -71,7 +70,7 @@ def name # @param none # @return [String] of address def address - street_name + ", " + zip_code + " " + city + ", " + state + ", " + country + street_name + ', ' + zip_code + ' ' + city + ', ' + state + ', ' + country end # Returns a list of allowed parameters. @@ -79,21 +78,21 @@ def address # @param none # @return [Symbol] List of parameters def self.allowed_params - [:first_name, :last_name, :gender, :birth_date, :street_name, :zip_code, :city, :state, :country, :discovery_of_site] + %i(first_name last_name gender birth_date street_name zip_code city state country discovery_of_site) end - # Returns an array containing the allowed methods to sort by # # @param none # @return [Symbol] List of methods def self.allowed_sort_methods - Profile.allowed_params + [:address, :name, :age] + Profile.allowed_params + %i(address name age) end private + def birthdate_not_in_future - if birth_date.present? and birth_date.in_time_zone > Date.current + if birth_date.present? && birth_date.in_time_zone > Date.current errors.add(:birth_date, I18n.t('profiles.validation.birthday_in_future')) end end diff --git a/app/models/request.rb b/app/models/request.rb index 22550e83..e492d446 100644 --- a/app/models/request.rb +++ b/app/models/request.rb @@ -23,13 +23,12 @@ # class Request < ActiveRecord::Base - validates_presence_of :form_of_address, :last_name, :first_name, :phone_number, :street, :zip_code_city, :email, :topic_of_workshop validates :number_of_participants, numericality: { only_integer: true, greater_than: 0 }, allow_nil: :true - validates_format_of :email, :with => Devise::email_regexp + validates_format_of :email, with: Devise.email_regexp - enum form_of_address: [:mr, :mrs, :prefer_to_omit] - enum status: [:open, :accepted, :declined] # per database declaration, the first value is default + enum form_of_address: %i(mr mrs prefer_to_omit) + enum status: %i(open accepted declined) # per database declaration, the first value is default def name "#{first_name} #{last_name}" diff --git a/app/models/user.rb b/app/models/user.rb index 3ed6c50a..92f67628 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -30,7 +30,7 @@ class User < ActiveRecord::Base # :confirmable, :lockable, :timeoutable and :omniauthable devise :database_authenticatable, :registerable, :recoverable, :rememberable, :trackable, :validatable, - :omniauthable, :omniauth_providers => [:hpiopenid] + :omniauthable, omniauth_providers: [:hpiopenid] has_one :profile has_many :agreement_letters @@ -39,7 +39,7 @@ class User < ActiveRecord::Base before_create :set_default_role - ROLES = %i[pupil coach organizer admin] + ROLES = %i(pupil coach organizer admin).freeze def role?(base_role) return false unless role @@ -54,7 +54,7 @@ def set_default_role def self.from_omniauth(auth) where(provider: auth.provider, uid: auth.uid).first_or_create! do |user| user.email = auth.info.email - user.password = Devise.friendly_token[0,20] + user.password = Devise.friendly_token[0, 20] user.role = :pupil # user.name = auth.info.first_name + " " + auth.info.last_name @@ -75,7 +75,7 @@ def name # @return [Array] the user's events def events accepted_applications = application_letters.where(status: ApplicationLetter.statuses[:accepted]) - accepted_applications.collect { |a| a.event } + accepted_applications.collect(&:event) end # Returns true iff. user has submitted an application letter for the given event @@ -83,7 +83,7 @@ def events # @param [Event] given_event # @return [Boolean] def application_letter_for_event?(given_event) - return !self.application_letter_for_event(given_event).nil? + !application_letter_for_event(given_event).nil? end # Returns the application letter the user has submitted for given_event. Returns Nil if no such letter exists. @@ -91,15 +91,15 @@ def application_letter_for_event?(given_event) # @param [Event] given_event # @return [ApplicationLetter, Nil] def application_letter_for_event(given_event) - return self.application_letters.find{ |letter| letter.event == given_event } + application_letters.find { |letter| letter.event == given_event } end - #Returns the events, for which a user has not uploaded an agreement letter + # Returns the events, for which a user has not uploaded an agreement letter # # @param none # @return [Array] def events_with_missing_agreement_letters - events.select{ |e| (AgreementLetter.where(user_id: self.id, event_id: e.id).blank? and requires_agreement_letter_for_event?(e))} + events.select { |e| (AgreementLetter.where(user_id: id, event_id: e.id).blank? && requires_agreement_letter_for_event?(e)) } end # Returns true iff. user has submitted an agreement_letter for the given event @@ -107,7 +107,7 @@ def events_with_missing_agreement_letters # @param [Event] given_event # @return [Boolean] def agreement_letter_for_event?(given_event) - return !self.agreement_letter_for_event(given_event).nil? + !agreement_letter_for_event(given_event).nil? end # Returns the agreement letter the user has submitted for given_event. Returns Nil if no such letter exists. @@ -115,7 +115,7 @@ def agreement_letter_for_event?(given_event) # @param [Event] given_event # @return [AgreementLetter, Nil] def agreement_letter_for_event(given_event) - return self.agreement_letters.find{ |letter| letter.event == given_event } + agreement_letters.find { |letter| letter.event == given_event } end # Returns true iff. user is at least 18 years old at the start date of given_event @@ -123,7 +123,7 @@ def agreement_letter_for_event(given_event) # @param [Event] given_event # @return [Boolean] def requires_agreement_letter_for_event?(given_event) - return (not self.older_than_required_age_at_start_date_of_event?(given_event, 18)) + !older_than_required_age_at_start_date_of_event?(given_event, 18) end # Returns true iff. the age of user is age or more at the start_date of given_event. Returns false if age of user is unknown. @@ -131,9 +131,9 @@ def requires_agreement_letter_for_event?(given_event) # @param given_event [Event], age [Integer] # @return [Boolean] def older_than_required_age_at_start_date_of_event?(given_event, age) - return false unless self.profile - age_at_event_start = self.profile.age_at_time(given_event.start_date) - return age_at_event_start >= age + return false unless profile + age_at_event_start = profile.age_at_time(given_event.start_date) + age_at_event_start >= age end # Returns the number of accepted applications from the user without counting status of current event application @@ -142,7 +142,7 @@ def older_than_required_age_at_start_date_of_event?(given_event, age) # @return [Int] of number of currently accepted applications def accepted_applications_count(event) ApplicationLetter.where(user_id: id, status: ApplicationLetter.statuses[:accepted]) - .where.not(event: event).count() + .where.not(event: event).count end # Returns the number of rejected applications from the user without counting status of current event application @@ -150,7 +150,7 @@ def accepted_applications_count(event) # @param current event (which application status will be excluded) # @return [Int] of number of currently rejected applications def rejected_applications_count(event) - ApplicationLetter.where(user_id: id, status: ApplicationLetter.statuses[:rejected]).where.not(event: event).count() + ApplicationLetter.where(user_id: id, status: ApplicationLetter.statuses[:rejected]).where.not(event: event).count end # Searches all users with last/first_name containing pattern @@ -158,15 +158,15 @@ def rejected_applications_count(event) # @param pattern to search for # @return [Array] all users with pattern in their name def self.search(pattern) - with_profiles.where("profiles.first_name LIKE ? or profiles.last_name LIKE ?", "%#{pattern}%", "%#{pattern}%") + with_profiles.where('profiles.first_name LIKE ? or profiles.last_name LIKE ?', "%#{pattern}%", "%#{pattern}%") end # Provides access to profile information # and orders users by first, last name and email (if user has no profile) # # @return [Array] all users including their profile information - def self.with_profiles() - joins("LEFT JOIN profiles ON users.id = profiles.user_id") - .order('profiles.first_name, profiles.last_name, users.email ASC') + def self.with_profiles + joins('LEFT JOIN profiles ON users.id = profiles.user_id') + .order('profiles.first_name, profiles.last_name, users.email ASC') end end diff --git a/app/services/mailer.rb b/app/services/mailer.rb index bc899523..0e1ddf56 100644 --- a/app/services/mailer.rb +++ b/app/services/mailer.rb @@ -1,5 +1,4 @@ class Mailer - # @param hide_recipients - a boolean. `true` will send an email to the recipients one by one, `false` will send an email to all at once # @param recipients [Array] - email addresses of recipients - can be a string of comma separated email adresses too # @param reply_to [Array] - email addresses of recipient of the answer - can be a string of comma separated email adresses too @@ -9,9 +8,7 @@ class Mailer # @return [ActionMailer::MessageDelivery] a mail object with the given parameters. def self.send_generic_email(hide_recipients, recipients, reply_to, subject, content, attachments = []) if hide_recipients - if recipients.is_a? String - recipients = recipients.split(',') - end + recipients = recipients.split(',') if recipients.is_a? String recipients.each do |recipient| PortalMailer.generic_email(recipient, reply_to, subject, content, attachments).deliver_now end diff --git a/app/uploaders/event_image_uploader.rb b/app/uploaders/event_image_uploader.rb index 83fcda99..c84b7fd1 100644 --- a/app/uploaders/event_image_uploader.rb +++ b/app/uploaders/event_image_uploader.rb @@ -16,10 +16,10 @@ def store_dir # Fill in the upload sizes once the image has been uploaded, so that # our model can use them for validation - def capture_size_before_cache(new_file) + def capture_size_before_cache(new_file) # Only do this once, to the original version if version_name.blank? - @upload_width, @upload_height = `identify -format "%wx %h" #{new_file.path}`.split(/x/).map { |dim| dim.to_i } + @upload_width, @upload_height = `identify -format "%wx %h" #{new_file.path}`.split(/x/).map(&:to_i) end end @@ -39,5 +39,4 @@ def capture_size_before_cache(new_file) def extension_whitelist %w(jpg jpeg gif png) end - end diff --git a/lib/pdf_generation/applications_pdf.rb b/lib/pdf_generation/applications_pdf.rb index aa252485..a16a279d 100644 --- a/lib/pdf_generation/applications_pdf.rb +++ b/lib/pdf_generation/applications_pdf.rb @@ -1,13 +1,13 @@ class ApplicationsPDF include Prawn::View - Prawn::Font::AFM.hide_m17n_warning = true #consider adding TTF font + Prawn::Font::AFM.hide_m17n_warning = true # consider adding TTF font # Generates a PDF file containing the details of every application for an event # # @param event [Event] the event whose applications are taken # @return [String] the generated PDF def self.generate(event) - self.new(event).create.render + new(event).create.render end def initialize(event) @@ -22,139 +22,140 @@ def initialize(event) # @return [ApplicationsPDF] self def create create_overview - @event.application_letters.each_with_index do |a,i| + @event.application_letters.each_with_index do |a, i| create_application_page(a, i) end self end private - def create_overview - text t("events.applicants_overview.title", title: @event.name), size: 20 - table description_table_data do + + def create_overview + text t('events.applicants_overview.title', title: @event.name), size: 20 + table description_table_data do + cells.borders = [] + cells.padding = 3 + column(0).font_style = :bold + column(0).align = :right + end + unless @event.application_letters.empty? + table overview_table_data, + header: 2, position: :center, row_colors: %w(F9F9F9 FFFFFF) do cells.borders = [] - cells.padding = 3 - column(0).font_style = :bold - column(0).align = :right - end - unless @event.application_letters.empty? - table overview_table_data, - header: 2, position: :center, row_colors: ["F9F9F9", "FFFFFF"] do - cells.borders = [] - row(1).borders = [:bottom] - row(1).font_style = :bold - row(0).font_style = :bold - row(0).padding = [5, 0, 5, 5] #minimize padding between first two rows - row(1).padding = [0, 5, 5, 5] - end + row(1).borders = [:bottom] + row(1).font_style = :bold + row(0).font_style = :bold + row(0).padding = [5, 0, 5, 5] # minimize padding between first two rows + row(1).padding = [0, 5, 5, 5] end end + end - def description_table_data - data = [[Event.human_attribute_name(:max_participants) + ":", @event.max_participants], - [Event.human_attribute_name(:date_ranges) + ":", @event.date_ranges[0].to_s]] - data += @event.date_ranges.drop(1).map { |d| ["", d.to_s] } - data += [[Event.human_attribute_name(:organizer) + ":", @event.organizer]] if @event.organizer - data += [[Event.human_attribute_name(:knowledge_level) + ":", @event.knowledge_level]] if @event.knowledge_level - data += [[t("events.applications_pdf.free_places") + ":", @event.compute_free_places], - [t("events.applications_pdf.occupied_places") + ":", @event.compute_occupied_places]] - end + def description_table_data + data = [[Event.human_attribute_name(:max_participants) + ':', @event.max_participants], + [Event.human_attribute_name(:date_ranges) + ':', @event.date_ranges[0].to_s]] + data += @event.date_ranges.drop(1).map { |d| ['', d.to_s] } + data += [[Event.human_attribute_name(:organizer) + ':', @event.organizer]] if @event.organizer + data += [[Event.human_attribute_name(:knowledge_level) + ':', @event.knowledge_level]] if @event.knowledge_level + data += [[t('events.applications_pdf.free_places') + ':', @event.compute_free_places], + [t('events.applications_pdf.occupied_places') + ':', @event.compute_occupied_places]] + end - def overview_table_data - #line breaks lead to weird table formatting, so we create 2 header rows to fit all the text - data = [["", "", "", t("events.applicants_overview.participations"), ""]] - data += [[Profile.human_attribute_name(:name), - Profile.human_attribute_name(:gender), - t('application_letters.show.age_when_event_starts'), - t("events.applicants_overview.accepted_rejected"), - ApplicationLetter.human_attribute_name(:status)]] - data += @event.application_letters.map do |a| - [a.user.profile.name, - t("profiles.genders.#{a.user.profile.gender}"), - a.applicant_age_when_event_starts, - "#{a.user.accepted_applications_count(@event)} / #{a.user.rejected_applications_count(@event)}", - t("application_status.#{a.status}")] - end + def overview_table_data + # line breaks lead to weird table formatting, so we create 2 header rows to fit all the text + data = [['', '', '', t('events.applicants_overview.participations'), '']] + data += [[Profile.human_attribute_name(:name), + Profile.human_attribute_name(:gender), + t('application_letters.show.age_when_event_starts'), + t('events.applicants_overview.accepted_rejected'), + ApplicationLetter.human_attribute_name(:status)]] + data += @event.application_letters.map do |a| + [a.user.profile.name, + t("profiles.genders.#{a.user.profile.gender}"), + a.applicant_age_when_event_starts, + "#{a.user.accepted_applications_count(@event)} / #{a.user.rejected_applications_count(@event)}", + t("application_status.#{a.status}")] end + end - def create_application_page(application_letter, index) - start_new_page - first_page = page_number - create_main_header(application_letter) - - bounding_box([bounds.left, bounds.top - 50], - width: bounds.width, - height: bounds.height - 100) do - create_application_page_content(application_letter) - end + def create_application_page(application_letter, index) + start_new_page + first_page = page_number + create_main_header(application_letter) - create_headers(application_letter, first_page, page_number) - create_footers(index, first_page, page_number) + bounding_box([bounds.left, bounds.top - 50], + width: bounds.width, + height: bounds.height - 100) do + create_application_page_content(application_letter) end - def create_application_page_content(application_letter) - table applicants_detail_data(application_letter) do - cells.borders = [] - cells.padding = 3 - column(0).font_style = :bold - column(0).align = :right - end - unless application_letter.annotation.nil? - pad_top(20) { text "#{t("application_letters.show.annotation_title")}", inline_format: true} - pad_top(5) { text application_letter.annotation } - end - pad_top(20) { text "#{ApplicationLetter.human_attribute_name(:motivation)}", inline_format: true} - pad_top(5) { text application_letter.motivation } - unless application_letter.application_notes.count == 0 - pad_top(15) do - text "#{ApplicationNote.model_name.human(count: application_letter.application_notes.count)}", inline_format: true - application_letter.application_notes.each do |note| - pad_top(5) { text note.note } - end + create_headers(application_letter, first_page, page_number) + create_footers(index, first_page, page_number) + end + + def create_application_page_content(application_letter) + table applicants_detail_data(application_letter) do + cells.borders = [] + cells.padding = 3 + column(0).font_style = :bold + column(0).align = :right + end + unless application_letter.annotation.nil? + pad_top(20) { text "#{t('application_letters.show.annotation_title')}", inline_format: true } + pad_top(5) { text application_letter.annotation } + end + pad_top(20) { text "#{ApplicationLetter.human_attribute_name(:motivation)}", inline_format: true } + pad_top(5) { text application_letter.motivation } + unless application_letter.application_notes.count == 0 + pad_top(15) do + text "#{ApplicationNote.model_name.human(count: application_letter.application_notes.count)}", inline_format: true + application_letter.application_notes.each do |note| + pad_top(5) { text note.note } end end end + end - def applicants_detail_data(application_letter) - [[Profile.human_attribute_name(:gender)+":", t("profiles.genders.#{application_letter.user.profile.gender}")], - [t('application_letters.show.age_when_event_starts')+":", application_letter.applicant_age_when_event_starts], - [User.human_attribute_name(:accepted_application_count)+":", application_letter.user.accepted_applications_count(@event)], - [User.human_attribute_name(:rejected_application_count)+":", application_letter.user.rejected_applications_count(@event)], - [Profile.human_attribute_name(:status)+":", t("application_status.#{application_letter.status}")]] - end + def applicants_detail_data(application_letter) + [[Profile.human_attribute_name(:gender) + ':', t("profiles.genders.#{application_letter.user.profile.gender}")], + [t('application_letters.show.age_when_event_starts') + ':', application_letter.applicant_age_when_event_starts], + [User.human_attribute_name(:accepted_application_count) + ':', application_letter.user.accepted_applications_count(@event)], + [User.human_attribute_name(:rejected_application_count) + ':', application_letter.user.rejected_applications_count(@event)], + [Profile.human_attribute_name(:status) + ':', t("application_status.#{application_letter.status}")]] + end - def create_main_header(application_letter) - text t("application_letters.application_page.title", name: application_letter.user.profile.name), size: 20 - text t("application_letters.application_page.for", event: @event.name), size: 14 - stroke_horizontal_rule - end + def create_main_header(application_letter) + text t('application_letters.application_page.title', name: application_letter.user.profile.name), size: 20 + text t('application_letters.application_page.for', event: @event.name), size: 14 + stroke_horizontal_rule + end - def create_headers(application_letter, first_page, last_page) - repeat(first_page + 1..last_page) do - bounding_box [bounds.left, bounds.top], width: bounds.width do - text @event.name - text_box application_letter.user.profile.name, align: :right - stroke_horizontal_rule - end + def create_headers(application_letter, first_page, last_page) + repeat(first_page + 1..last_page) do + bounding_box [bounds.left, bounds.top], width: bounds.width do + text @event.name + text_box application_letter.user.profile.name, align: :right + stroke_horizontal_rule end end + end - def create_footers(index, first_page, last_page) - repeat(first_page..last_page, dynamic: true) do - bounding_box [bounds.left, bounds.bottom + 25], width: bounds.width do - relative_page = page_number - first_page + 1 - relative_last_page = last_page - first_page + 1 - stroke_horizontal_rule - move_down(5) - text_box(t("events.applications_pdf.page") + " #{relative_page}/#{relative_last_page}", - at: [0, cursor], - align: :right) - text ApplicationLetter.model_name.human + " #{index + 1}/#{@application_letters_count}" - end + def create_footers(index, first_page, last_page) + repeat(first_page..last_page, dynamic: true) do + bounding_box [bounds.left, bounds.bottom + 25], width: bounds.width do + relative_page = page_number - first_page + 1 + relative_last_page = last_page - first_page + 1 + stroke_horizontal_rule + move_down(5) + text_box(t('events.applications_pdf.page') + " #{relative_page}/#{relative_last_page}", + at: [0, cursor], + align: :right) + text ApplicationLetter.model_name.human + " #{index + 1}/#{@application_letters_count}" end end + end - def t(string, options = {}) - I18n.t(string, options) - end + def t(string, options = {}) + I18n.t(string, options) + end end diff --git a/lib/pdf_generation/badges_pdf.rb b/lib/pdf_generation/badges_pdf.rb index 0b773063..63d2b349 100644 --- a/lib/pdf_generation/badges_pdf.rb +++ b/lib/pdf_generation/badges_pdf.rb @@ -7,7 +7,7 @@ class BadgesPDF Y_PADDING = 20 # top and bottom padding (if show_color, no bottom padding is used) LOGO_HEIGHT = 50 # height of the logo (only if logo) COLOR_HEIGHT = 20 # height of the colored rectangle (only if show_color) - ORGANISATION_HEIGHT_RATIO = 1.0/3 # fraction of the space for text that is occupied by the organisation (only if show_organisation). The remaining space is occupied by the name. + ORGANISATION_HEIGHT_RATIO = 1.0 / 3 # fraction of the space for text that is occupied by the organisation (only if show_organisation). The remaining space is occupied by the name. MAX_NAME_FONT_SIZE = 28 # font size for the name, is automatically lowered if the text does not fit MAX_ORGANISATION_FONT_SIZE = 16 # font size for the organisation, is automatically lowered if the text does not fit LINE_SPACING = 5 # empty space above text @@ -24,13 +24,13 @@ class BadgesPDF # @param logo [ActionDispatch::Http::UploadedFile, nil] the logo to add # @return [String] the generated PDF def self.generate(event, participants, name_format, show_color, show_organisation, logo) - self.new(event, participants, name_format, show_color, show_organisation, logo).create.render + new(event, participants, name_format, show_color, show_organisation, logo).create.render end def initialize(event, participants, name_format, show_color, show_organisation, logo) @event = event @participants = participants - @name_format = name_format || "full" + @name_format = name_format || 'full' @show_color = show_color || false @show_organisation = show_organisation || false @logo = logo @@ -45,112 +45,113 @@ def initialize(event, participants, name_format, show_color, show_organisation, # @return [BadgesPDF] self def create badges_per_page = COLUMN_NUMBER * ROW_NUMBER - @participants.each_slice(badges_per_page).each_with_index do | page_participants, page_number | + @participants.each_slice(badges_per_page).each_with_index do |page_participants, page_number| create_badge_page(page_participants, page_number) end self end private - def calculate_layout - @badge_width = bounds.width / COLUMN_NUMBER - @badge_height = bounds.height / ROW_NUMBER - v_space_left = @badge_height - v_space_left -= Y_PADDING - if @logo - v_space_left -= LOGO_HEIGHT - v_space_left -= LINE_SPACING - end - v_space_left -= @show_color ? COLOR_HEIGHT : Y_PADDING + def calculate_layout + @badge_width = bounds.width / COLUMN_NUMBER + @badge_height = bounds.height / ROW_NUMBER - if @show_organisation - @organisation_height = v_space_left * ORGANISATION_HEIGHT_RATIO - v_space_left -= @organisation_height - v_space_left -= LINE_SPACING - end - @name_height = v_space_left + v_space_left = @badge_height + v_space_left -= Y_PADDING + if @logo + v_space_left -= LOGO_HEIGHT + v_space_left -= LINE_SPACING end + v_space_left -= @show_color ? COLOR_HEIGHT : Y_PADDING - # Create a page with maximum 10 badges - def create_badge_page(participants, page_number) - # create no pagebreak for first page - start_new_page if page_number > 0 - - participants.each_slice(COLUMN_NUMBER).with_index do |(left, right), row| - create_badge(left, 0, bounds.height - row * @badge_height) - create_badge(right, @badge_width, bounds.height - row * @badge_height) unless right.nil? - end + if @show_organisation + @organisation_height = v_space_left * ORGANISATION_HEIGHT_RATIO + v_space_left -= @organisation_height + v_space_left -= LINE_SPACING end + @name_height = v_space_left + end - def create_badge(participant, x, y) - bounding_box [x, y], width: @badge_width, height: @badge_height do - cursor = bounds.top - cursor -= Y_PADDING - - if @logo - create_logo(cursor) - cursor -= LOGO_HEIGHT - cursor -= LINE_SPACING - end + # Create a page with maximum 10 badges + def create_badge_page(participants, page_number) + # create no pagebreak for first page + start_new_page if page_number > 0 - create_name(participant, cursor) - cursor -= @name_height + participants.each_slice(COLUMN_NUMBER).with_index do |(left, right), row| + create_badge(left, 0, bounds.height - row * @badge_height) + create_badge(right, @badge_width, bounds.height - row * @badge_height) unless right.nil? + end + end - if @show_organisation - cursor -= LINE_SPACING - create_organisation(participant, cursor) - cursor -= @organisation_height - end + def create_badge(participant, x, y) + bounding_box [x, y], width: @badge_width, height: @badge_height do + cursor = bounds.top + cursor -= Y_PADDING - create_color(participant, cursor) if @show_color - stroke_bounds + if @logo + create_logo(cursor) + cursor -= LOGO_HEIGHT + cursor -= LINE_SPACING end - end - def create_logo(y) - image @logo, - height: LOGO_HEIGHT, - position: :center, - vposition: bounds.top - y - end + create_name(participant, cursor) + cursor -= @name_height - def create_name(participant, y) - case @name_format - when "first" - name = participant.profile.first_name - when "last" - name = participant.profile.last_name - else - name = participant.profile.name + if @show_organisation + cursor -= LINE_SPACING + create_organisation(participant, cursor) + cursor -= @organisation_height end - text_box name, - at: [X_PADDING, y], - width: @badge_width - (X_PADDING * 2), - height: @name_height, - align: :center, - valign: :center, - size: MAX_NAME_FONT_SIZE, - overflow: :shrink_to_fit + create_color(participant, cursor) if @show_color + stroke_bounds end + end - def create_organisation(participant, y) - organisation = ApplicationLetter.where(event: @event, user: participant).first.organisation - text_box organisation, - at: [X_PADDING, y], - width: @badge_width - (X_PADDING * 2), - align: :center, - size: MAX_ORGANISATION_FONT_SIZE, - height: @organisation_height, - overflow: :shrink_to_fit - end + def create_logo(y) + image @logo, + height: LOGO_HEIGHT, + position: :center, + vposition: bounds.top - y + end - def create_color(participant, y) - color = ParticipantGroup::GROUPS[@event.participant_group_for(participant).group] - color = "FFFFFF" if color == "0" # in case the color is "none" - fill_color color - fill_rectangle [0, y], @badge_width, COLOR_HEIGHT - fill_color "0000" - end + def create_name(participant, y) + name = case @name_format + when 'first' + participant.profile.first_name + when 'last' + participant.profile.last_name + else + participant.profile.name + end + + text_box name, + at: [X_PADDING, y], + width: @badge_width - (X_PADDING * 2), + height: @name_height, + align: :center, + valign: :center, + size: MAX_NAME_FONT_SIZE, + overflow: :shrink_to_fit + end + + def create_organisation(participant, y) + organisation = ApplicationLetter.where(event: @event, user: participant).first.organisation + text_box organisation, + at: [X_PADDING, y], + width: @badge_width - (X_PADDING * 2), + align: :center, + size: MAX_ORGANISATION_FONT_SIZE, + height: @organisation_height, + overflow: :shrink_to_fit + end + + def create_color(participant, y) + color = ParticipantGroup::GROUPS[@event.participant_group_for(participant).group] + color = 'FFFFFF' if color == '0' # in case the color is "none" + fill_color color + fill_rectangle [0, y], @badge_width, COLOR_HEIGHT + fill_color '000000' + end end diff --git a/lib/pdf_generation/participants_pdf.rb b/lib/pdf_generation/participants_pdf.rb index 40a283c9..95f17fcc 100644 --- a/lib/pdf_generation/participants_pdf.rb +++ b/lib/pdf_generation/participants_pdf.rb @@ -1,9 +1,9 @@ class ParticipantsPDF include Prawn::View - Prawn::Font::AFM.hide_m17n_warning = true #consider adding TTF font - + Prawn::Font::AFM.hide_m17n_warning = true # consider adding TTF font + def self.generate(event) - self.new(event).create.render + new(event).create.render end def initialize(event) @@ -15,80 +15,78 @@ def initialize(event) @participants_vegetarian_application_letters = [] @participants_omnivorous_application_letters = [] @event.participants.each do |p| - letter = ApplicationLetter.find_by(user_id: p.id, event_id: @event.id) - if !letter.nil? - @participants_allergic_application_letters.push(letter) if letter.allergic - @participants_vegetarian_application_letters.push(letter) if letter.vegetarian - @participants_vegan_application_letters.push(letter) if letter.vegan - @participants_omnivorous_application_letters.push(letter) if !letter.vegan && !letter.vegetarian - end - + next if letter.nil? + @participants_allergic_application_letters.push(letter) if letter.allergic + @participants_vegetarian_application_letters.push(letter) if letter.vegetarian + @participants_vegan_application_letters.push(letter) if letter.vegan + @participants_omnivorous_application_letters.push(letter) if !letter.vegan && !letter.vegetarian end end - + # # param none # return [ApplicationsPDF] self - - def create + + def create create_summary self end private - def create_summary - text t("events.participants.print_title", title: @event.name), size: 20 - move_down 20 - text @event.date_ranges[0].to_s, size: 10 - move_down 20 - text t("events.participants.print_summary", count: @event.participants.count), size: 15 - - if @participants_omnivorous_application_letters.any? - text t("events.participants.print_summary_omnivorous", count: @participants_omnivorous_application_letters.count), size: 15 - @participants_omnivorous_application_letters.each do |p| - move_down 5 - text p.user.name, size: 10 - end - end - move_down 20 - - if @participants_vegan_application_letters.any? - text t("events.participants.print_summary_vegan", count: @participants_vegan_application_letters.count), size: 15 - @participants_vegan_application_letters.each do |p| - move_down 5 - text p.user.name, size: 10 - end + def create_summary + text t('events.participants.print_title', title: @event.name), size: 20 + move_down 20 + text @event.date_ranges[0].to_s, size: 10 + move_down 20 + text t('events.participants.print_summary', count: @event.participants.count), size: 15 + + if @participants_omnivorous_application_letters.any? + text t('events.participants.print_summary_omnivorous', count: @participants_omnivorous_application_letters.count), size: 15 + @participants_omnivorous_application_letters.each do |p| + move_down 5 + text p.user.name, size: 10 end + end - move_down 20 + move_down 20 - if @participants_vegetarian_application_letters.any? - text t("events.participants.print_summary_vegetarian", count: @participants_vegetarian_application_letters.count), size: 15 - @participants_vegetarian_application_letters.each do |p| - move_down 5 - text p.user.name, size: 10 - end + if @participants_vegan_application_letters.any? + text t('events.participants.print_summary_vegan', count: @participants_vegan_application_letters.count), size: 15 + @participants_vegan_application_letters.each do |p| + move_down 5 + text p.user.name, size: 10 end + end - move_down 20 + move_down 20 - if @participants_count > 0 - text t('events.participants.print_summary_allergic'), size: 15 - @participants_allergic_application_letters.each do |p| - output = p.user.name - output += " (" + t('activerecord.attributes.application_letter.vegan') + ")" if p.vegan - output += " (" + t('activerecord.attributes.application_letter.vegetarian') + ")" if p.vegetarian - output += " (" + t('activerecord.attributes.application_letter.omnivorous') + ")" if !p.vegetarian && !p.vegan - output += " : " + p.allergies.to_s - text output, size: 10 - move_down 5 - end + if @participants_vegetarian_application_letters.any? + text t('events.participants.print_summary_vegetarian', count: @participants_vegetarian_application_letters.count), size: 15 + @participants_vegetarian_application_letters.each do |p| + move_down 5 + text p.user.name, size: 10 end end - def t(string, options = {}) - I18n.t(string, options) + move_down 20 + + if @participants_count > 0 + text t('events.participants.print_summary_allergic'), size: 15 + @participants_allergic_application_letters.each do |p| + output = p.user.name + output += ' (' + t('activerecord.attributes.application_letter.vegan') + ')' if p.vegan + output += ' (' + t('activerecord.attributes.application_letter.vegetarian') + ')' if p.vegetarian + output += ' (' + t('activerecord.attributes.application_letter.omnivorous') + ')' if !p.vegetarian && !p.vegan + output += ' : ' + p.allergies.to_s + text output, size: 10 + move_down 5 + end end -end \ No newline at end of file + end + + def t(string, options = {}) + I18n.t(string, options) + end +end diff --git a/lib/tasks/sample_data.rake b/lib/tasks/sample_data.rake index 1421a6d3..4cc502ba 100644 --- a/lib/tasks/sample_data.rake +++ b/lib/tasks/sample_data.rake @@ -5,4 +5,4 @@ namespace :db do task populate_sample_data: :environment do add_sample_data end -end \ No newline at end of file +end diff --git a/spec/controllers/application_letters_controller_spec.rb b/spec/controllers/application_letters_controller_spec.rb index 6be1794f..c69f2413 100644 --- a/spec/controllers/application_letters_controller_spec.rb +++ b/spec/controllers/application_letters_controller_spec.rb @@ -30,7 +30,8 @@ let(:valid_session) { {} } before(:each) do - request.env['HTTP_REFERER'] = root_url + request.env['HTTP_REFERER'] = events_archive_url # Totally irrelevant, because the standard redirect url is already + # tested in another test branch end context "with an existing application letter" do @@ -168,6 +169,12 @@ put :update_status, id: @application.to_param, application_letter: new_status, session: valid_session expect(response).to redirect_to(request.env['HTTP_REFERER']) end + + it "redirects to root if no referer is set" do + request.env['HTTP_REFERER'] = nil + put :update_status, id: @application.to_param, application_letter: new_status, session: valid_session + expect(response).to redirect_to root_path + end end context "with invalid params" do