From 7f4371f643f90c72ff70d4b3e2b93c874a966a08 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Tue, 30 Jan 2018 19:32:08 +0100 Subject: [PATCH 01/10] Add rbenv support for Capistrano --- Capfile | 1 + Gemfile | 1 + Gemfile.lock | 4 ++++ config/deploy.rb | 2 ++ 4 files changed, 8 insertions(+) diff --git a/Capfile b/Capfile index 6e790a9a0..0f81c1684 100644 --- a/Capfile +++ b/Capfile @@ -16,6 +16,7 @@ require 'capistrano/deploy' # https://github.com/capistrano/passenger # require 'capistrano/rails' +require 'capistrano/rbenv' # Load custom tasks from `lib/capistrano/tasks` if you have any defined Dir.glob('lib/capistrano/tasks/*.rake').each { |r| import r } diff --git a/Gemfile b/Gemfile index e35652d93..49996f05c 100644 --- a/Gemfile +++ b/Gemfile @@ -46,6 +46,7 @@ group :development do gem 'web-console', '2.1.3' gem 'capistrano', '~> 3.1' gem 'capistrano-rails', '~> 1.1' + gem 'capistrano-rbenv', '~> 2.1' gem 'airbrussh', require: false gem "quiet_assets" gem 'localeapp', '2.1.1', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 5812b2581..4e5301c44 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -93,6 +93,9 @@ GEM capistrano-rails (1.1.5) capistrano (~> 3.1) capistrano-bundler (~> 1.1) + capistrano-rbenv (2.1.3) + capistrano (~> 3.1) + sshkit (~> 1.3) capybara (2.4.4) mime-types (>= 1.16) nokogiri (>= 1.3.3) @@ -382,6 +385,7 @@ DEPENDENCIES byebug capistrano (~> 3.1) capistrano-rails (~> 1.1) + capistrano-rbenv (~> 2.1) capybara (~> 2.4.4) coffee-rails dalli diff --git a/config/deploy.rb b/config/deploy.rb index f7943b663..507e21b09 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -4,6 +4,8 @@ set :application, 'timeoverflow' set :repo_url, 'git@github.com:coopdevs/timeoverflow.git' +set :rbenv_type, :user + # Default branch is :master # ask :branch, `git rev-parse --abbrev-ref HEAD`.chomp From 43e8105782e3841d708e0570759a5cdb175b7c48 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Tue, 30 Jan 2018 19:32:57 +0100 Subject: [PATCH 02/10] Add staging environment --- config/database.yml | 4 ++ config/deploy/staging.rb | 1 + config/environments/staging.rb | 96 ++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+) create mode 100644 config/environments/staging.rb diff --git a/config/database.yml b/config/database.yml index 5d48942dd..5ff305bc2 100644 --- a/config/database.yml +++ b/config/database.yml @@ -25,6 +25,10 @@ test: <<: *defaults database: <%= ENV.fetch('DATABASE_NAME', 'timeoverflow_test') %> +staging: + <<: *defaults + database: <%= ENV.fetch('DATABASE_NAME') %> + production: # Set DATABASE_URL environment variable url: <%= ENV['DATABASE_URL'] %> diff --git a/config/deploy/staging.rb b/config/deploy/staging.rb index e69de29bb..59ea3607c 100644 --- a/config/deploy/staging.rb +++ b/config/deploy/staging.rb @@ -0,0 +1 @@ +server 'staging.timeoverflow.org', user: 'timeoverflow', roles: %w(app db web) diff --git a/config/environments/staging.rb b/config/environments/staging.rb new file mode 100644 index 000000000..fe4480f76 --- /dev/null +++ b/config/environments/staging.rb @@ -0,0 +1,96 @@ +Rails.application.configure do + # Settings specified here will take precedence over those in config/application.rb. + + # Code is not reloaded between requests. + config.cache_classes = true + + # Eager load code on boot. This eager loads most of Rails and + # your application in memory, allowing both threaded web servers + # and those relying on copy on write to perform better. + # Rake tasks automatically ignore this option for performance. + config.eager_load = true + + # Full error reports are disabled and caching is turned on. + config.consider_all_requests_local = false + config.action_controller.perform_caching = true + + # Enable Rack::Cache to put a simple HTTP cache in front of your application + # Add `rack-cache` to your Gemfile before enabling this. + # For large-scale production use, consider using a caching reverse proxy like + # NGINX, varnish or squid. + # config.action_dispatch.rack_cache = true + + # Disable serving static files from the `/public` folder by default since + # Apache or NGINX already handles this. + config.serve_static_files = ENV['RAILS_SERVE_STATIC_FILES'].present? + + # Compress JavaScripts and CSS. + config.assets.js_compressor = :uglifier + # config.assets.css_compressor = :sass + + # Do not fallback to assets pipeline if a precompiled asset is missed. + config.assets.compile = true # false + + # Asset digests allow you to set far-future HTTP expiration dates on all assets, + # yet still be able to expire them through the digest params. + config.assets.digest = true + + # `config.assets.precompile` and `config.assets.version` have moved to config/initializers/assets.rb + + # Specifies the header that your server uses for sending files. + # config.action_dispatch.x_sendfile_header = 'X-Sendfile' # for Apache + # config.action_dispatch.x_sendfile_header = 'X-Accel-Redirect' # for NGINX + + # Force all access to the app over SSL, use Strict-Transport-Security, and use secure cookies. + config.force_ssl = true + + # Use the lowest log level to ensure availability of diagnostic information + # when problems arise. + config.log_level = :debug + + # Prepend all log lines with the following tags. + # config.log_tags = [ :subdomain, :uuid ] + + # Use a different logger for distributed setups. + # config.logger = ActiveSupport::TaggedLogging.new(SyslogLogger.new) + + # Use a different cache store in production. + # config.cache_store = :mem_cache_store + config.cache_store = :dalli_store + + # Enable serving of images, stylesheets, and JavaScripts from an asset server. + # config.action_controller.asset_host = 'http://assets.example.com' + + # Ignore bad email addresses and do not raise email delivery errors. + # Set this to true and configure the email server for immediate delivery to raise delivery errors. + config.action_mailer.raise_delivery_errors = true + + config.action_mailer.delivery_method = :smtp + config.action_mailer.default_url_options = { + host: ENV["MAIL_LINK_HOST"], + protocol: (ENV["MAIL_LINK_PROTO"] || "https") + } + + smtp_env = Hash[ENV.map do |k,v| + if /^SMTP_(.*)$/ === k + [$1.downcase.to_sym, YAML.load(v)] + end + end.compact] + + if smtp_env.present? + config.action_mailer.smtp_settings = smtp_env + end + + # Enable locale fallbacks for I18n (makes lookups for any locale fall back to + # the I18n.default_locale when a translation cannot be found). + config.i18n.fallbacks = true + + # Send deprecation notices to registered listeners. + config.active_support.deprecation = :notify + + # Use default logging formatter so that PID and timestamp are not suppressed. + config.log_formatter = ::Logger::Formatter.new + + # Do not dump schema after migrations. + config.active_record.dump_schema_after_migration = false +end From f0d061fb5cba4b316d9e1f0ce922365be0c5a2fb Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Tue, 30 Jan 2018 20:14:01 +0100 Subject: [PATCH 03/10] Ask for branch to deploy --- config/deploy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/deploy.rb b/config/deploy.rb index 507e21b09..d842d9643 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -7,7 +7,7 @@ set :rbenv_type, :user # Default branch is :master -# ask :branch, `git rev-parse --abbrev-ref HEAD`.chomp +ask :branch, `git rev-parse --abbrev-ref HEAD`.chomp # Default deploy_to directory is /var/www/my_app_name # set :deploy_to, '/var/www/my_app_name' From 3b9d03b4ce590e16b3ce96054532464981aa3a94 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Wed, 31 Jan 2018 10:54:54 +0100 Subject: [PATCH 04/10] Add default DB name for staging --- config/database.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/database.yml b/config/database.yml index 5ff305bc2..5856d3b67 100644 --- a/config/database.yml +++ b/config/database.yml @@ -27,7 +27,7 @@ test: staging: <<: *defaults - database: <%= ENV.fetch('DATABASE_NAME') %> + database: <%= ENV.fetch('DATABASE_NAME', 'timeoverflow_staging') %> production: # Set DATABASE_URL environment variable From 325ce2101597dd8411b6807307f654ba29ef06aa Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Wed, 31 Jan 2018 16:20:26 +0100 Subject: [PATCH 05/10] Doc++ --- config/environments/staging.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/environments/staging.rb b/config/environments/staging.rb index fe4480f76..b451781fe 100644 --- a/config/environments/staging.rb +++ b/config/environments/staging.rb @@ -71,6 +71,8 @@ protocol: (ENV["MAIL_LINK_PROTO"] || "https") } + # Retrieve SMTP configuration from environment variables + # starting with `SMTP_` smtp_env = Hash[ENV.map do |k,v| if /^SMTP_(.*)$/ === k [$1.downcase.to_sym, YAML.load(v)] From 0d44fa937798861438c9bacf033aa1f27e68d119 Mon Sep 17 00:00:00 2001 From: Enrico Stano Date: Wed, 31 Jan 2018 16:22:00 +0100 Subject: [PATCH 06/10] Add secret token for staging environment --- config/secrets.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/secrets.yml b/config/secrets.yml index 296ee64bb..f915c5e4c 100644 --- a/config/secrets.yml +++ b/config/secrets.yml @@ -4,5 +4,8 @@ development: test: secret_key_base: fde628fa241d74a55d7b0fc8fe1b650091e296d6a4d1e51beba3bfa2ed5c143801c28aaadf318aaefc4423bca781f9715441298052b4f704a8c44f97968feb00 +staging: + secret_key_base: <%= ENV['SECRET_TOKEN'] %> + production: secret_key_base: <%= ENV['SECRET_TOKEN'] %> From c88e959ea9661837024d914b011d7a2d11c6a81d Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 1 Feb 2018 13:40:26 +0100 Subject: [PATCH 07/10] Provide id of destination account in link --- app/models/post.rb | 1 - app/views/offers/show.html.erb | 2 +- spec/fabricators/account_fabricator.rb | 2 ++ spec/views/offers/show.html.erb_spec.rb | 38 +++++++++++++++++++++++++ 4 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 spec/fabricators/account_fabricator.rb create mode 100644 spec/views/offers/show.html.erb_spec.rb diff --git a/app/models/post.rb b/app/models/post.rb index e8b2ee624..89b91ff25 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -43,7 +43,6 @@ def self.inherited(child) belongs_to :user belongs_to :organization belongs_to :publisher, class_name: "User", foreign_key: "publisher_id" - # belongs_to :member, class_name: "Member", foreign_key: "user_id" has_many :user_members, class_name: "Member", through: :user, source: :members has_many :transfers has_many :movements, through: :transfers diff --git a/app/views/offers/show.html.erb b/app/views/offers/show.html.erb index 4bdd093ce..a6135384a 100644 --- a/app/views/offers/show.html.erb +++ b/app/views/offers/show.html.erb @@ -12,7 +12,7 @@ <% end %> <% end %> <% if current_user and @offer.user != current_user %> - <%= link_to new_transfer_path(id: @offer.user.id, offer: @offer.id), + <%= link_to new_transfer_path(id: @offer.user.id, offer: @offer.id, destination_account_id: @destination_account.id), class: "btn btn-success" do %> <%= glyph :time %> <%= t ".give_time_for" %> diff --git a/spec/fabricators/account_fabricator.rb b/spec/fabricators/account_fabricator.rb new file mode 100644 index 000000000..7a31c15d1 --- /dev/null +++ b/spec/fabricators/account_fabricator.rb @@ -0,0 +1,2 @@ +Fabricator(:account) do +end diff --git a/spec/views/offers/show.html.erb_spec.rb b/spec/views/offers/show.html.erb_spec.rb new file mode 100644 index 000000000..8aba677fd --- /dev/null +++ b/spec/views/offers/show.html.erb_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe 'offers/show' do + let(:organization) { Fabricate(:organization) } + let(:logged_user) { Fabricate(:user) } + let(:member) { Fabricate(:member, organization: organization) } + let(:offer) { Fabricate(:offer, user: member.user) } + let(:destination_account) { Fabricate(:account) } + + before do + Fabricate( + :member, + organization: organization, + user: logged_user + ) + + allow(view).to receive(:admin?).and_return(false) + allow(view).to receive(:current_user).and_return(logged_user) + allow(view).to receive(:current_organization) { organization } + + allow(offer).to receive(:member).and_return(member) + end + + it 'renders a link to the transfer page' do + assign :offer, offer + assign :destination_account, destination_account + render template: 'offers/show' + + expect(rendered).to have_link( + t('offers.show.give_time_for'), + href: new_transfer_path( + id: offer.user.id, + offer: offer.id, + destination_account_id: destination_account.id + ) + ) + end +end From 49aaef086885bf445f714efcc097575f91af1d1d Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 1 Feb 2018 13:55:35 +0100 Subject: [PATCH 08/10] Provide destination_account to offers show view --- app/controllers/offers_controller.rb | 9 +++++++++ spec/controllers/offers_controller_spec.rb | 22 ++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/controllers/offers_controller.rb b/app/controllers/offers_controller.rb index 6e455ae02..88abbe395 100644 --- a/app/controllers/offers_controller.rb +++ b/app/controllers/offers_controller.rb @@ -18,4 +18,13 @@ def dashboard offers[category] = list if list.present? end end + + def show + super + @destination_account = @offer + .user + .members + .find_by(organization: current_organization) + .account + end end diff --git a/spec/controllers/offers_controller_spec.rb b/spec/controllers/offers_controller_spec.rb index 6bc90b741..d7ae17ebd 100644 --- a/spec/controllers/offers_controller_spec.rb +++ b/spec/controllers/offers_controller_spec.rb @@ -1,18 +1,20 @@ require "spec_helper" describe OffersController, type: :controller do - let (:test_organization) { Fabricate(:organization) } - let (:member) { Fabricate(:member, organization: test_organization) } - let (:another_member) { Fabricate(:member, organization: test_organization) } - let (:yet_another_member) { Fabricate(:member) } - let (:test_category) { Fabricate(:category) } - let! (:offer) do + let(:test_organization) { Fabricate(:organization) } + let(:member) { Fabricate(:member, organization: test_organization) } + let(:another_member) { Fabricate(:member, organization: test_organization) } + let(:yet_another_member) { Fabricate(:member) } + let(:test_category) { Fabricate(:category) } + let!(:offer) do Fabricate(:offer, user: member.user, organization: test_organization, category: test_category) end + include_context "stub browser locale" + before { set_browser_locale("ca") } describe "GET #index" do @@ -66,7 +68,15 @@ get "show", id: offer.id expect(assigns(:offer)).to eq(offer) end + + it 'assigns the account destination of the transfer' do + login(another_member.user) + + get :show, id: offer.id + expect(assigns(:destination_account)).to eq(member.account) + end end + context "without a logged in user" do it "assigns the requested offer to @offer" do get "show", id: offer.id From 56098b0bad05bc06235948a0b5ed46fcd1881e84 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 1 Feb 2018 14:17:50 +0100 Subject: [PATCH 09/10] Show link to login when guest user in offer page --- app/controllers/offers_controller.rb | 8 +-- spec/controllers/offers_controller_spec.rb | 10 ++-- spec/views/offers/show.html.erb_spec.rb | 67 +++++++++++++++++----- 3 files changed, 59 insertions(+), 26 deletions(-) diff --git a/app/controllers/offers_controller.rb b/app/controllers/offers_controller.rb index 88abbe395..5fe6d0a1f 100644 --- a/app/controllers/offers_controller.rb +++ b/app/controllers/offers_controller.rb @@ -21,10 +21,8 @@ def dashboard def show super - @destination_account = @offer - .user - .members - .find_by(organization: current_organization) - .account + + member = @offer.user.members.find_by(organization: current_organization) + @destination_account = member.account if member end end diff --git a/spec/controllers/offers_controller_spec.rb b/spec/controllers/offers_controller_spec.rb index d7ae17ebd..3c712c0c8 100644 --- a/spec/controllers/offers_controller_spec.rb +++ b/spec/controllers/offers_controller_spec.rb @@ -62,16 +62,14 @@ describe "GET #show" do context "with valid params" do context "with a logged user" do - it "assigns the requested offer to @offer" do - login(another_member.user) + before { login(another_member.user) } - get "show", id: offer.id + it "assigns the requested offer to @offer" do + get :show, id: offer.id expect(assigns(:offer)).to eq(offer) end it 'assigns the account destination of the transfer' do - login(another_member.user) - get :show, id: offer.id expect(assigns(:destination_account)).to eq(member.account) end @@ -79,7 +77,7 @@ context "without a logged in user" do it "assigns the requested offer to @offer" do - get "show", id: offer.id + get :show, id: offer.id expect(assigns(:offer)).to eq(offer) end end diff --git a/spec/views/offers/show.html.erb_spec.rb b/spec/views/offers/show.html.erb_spec.rb index 8aba677fd..65a6e21f3 100644 --- a/spec/views/offers/show.html.erb_spec.rb +++ b/spec/views/offers/show.html.erb_spec.rb @@ -2,37 +2,74 @@ describe 'offers/show' do let(:organization) { Fabricate(:organization) } - let(:logged_user) { Fabricate(:user) } let(:member) { Fabricate(:member, organization: organization) } - let(:offer) { Fabricate(:offer, user: member.user) } + let(:offer) { Fabricate(:offer, user: member.user, organization: organization) } let(:destination_account) { Fabricate(:account) } before do - Fabricate( - :member, - organization: organization, - user: logged_user - ) - allow(view).to receive(:admin?).and_return(false) - allow(view).to receive(:current_user).and_return(logged_user) allow(view).to receive(:current_organization) { organization } allow(offer).to receive(:member).and_return(member) end - it 'renders a link to the transfer page' do - assign :offer, offer - assign :destination_account, destination_account - render template: 'offers/show' + context 'when there is logged in' do + let(:logged_user) { Fabricate(:user) } + + before do + Fabricate( + :member, + organization: organization, + user: logged_user + ) + + allow(view).to receive(:current_user).and_return(logged_user) + end + + it 'renders a link to the transfer page' do + assign :offer, offer + assign :destination_account, destination_account + render template: 'offers/show' + + expect(rendered).to have_link( + t('offers.show.give_time_for'), + href: new_transfer_path( + id: offer.user.id, + offer: offer.id, + destination_account_id: destination_account.id + ) + ) + end + end + + context 'where is a guest user' do + before do + allow(view).to receive(:current_user).and_return(nil) + end + + it 'does not render a link to the transfer page' do + assign :offer, offer + assign :destination_account, destination_account + render template: 'offers/show' - expect(rendered).to have_link( + expect(rendered).not_to have_link( t('offers.show.give_time_for'), href: new_transfer_path( id: offer.user.id, offer: offer.id, destination_account_id: destination_account.id ) - ) + ) + end + + it 'renders a link to the login page' do + assign :offer, offer + render template: 'offers/show' + + expect(rendered).to have_link( + t('layouts.application.login'), + href: new_user_session_path + ) + end end end From 49359eced4499de2cdf30f13b227d7b75984be7f Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Mon, 5 Feb 2018 08:06:21 -0800 Subject: [PATCH 10/10] Add Skylight instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I discussed with @coopdevs about adding Skylight instrumentation to the TimeOverflow website under the new [Skylight for Open Source](https://www.skylight.io/oss) program. If you’re not already familiar with Skylight, it is a smart profiler for Rails apps. Skylight makes it easy to pinpoint performance issues in Rails applications. We work on a lot of open source projects ourselves, and in our experience it can be pretty hard to get contributors to work on application performance issues. Few contributors consider working on performance problems, and the ones that might be interested may not even know where to start. By making performance information more accessible, we hope to inspire potential contributors to tackle slow parts of your app, and have a good way to see if their contributions helped. Once this patch is merged and deloyed*, you will be able to view the performance data we collected at the [public Skylight dashboard](https://oss.skylight.io/app/applications/grDTNuzZRnyu). The dashboard will be accessible to anyone (no Skylight account required) to make it easy for contributors. (*Actually, I lied a little. We still need to set the `SKYLIGHT_AUTHENTICATION` environment variable to the appropiate API key on production, but I will work with @coopdevs on that off-thread.) --- Gemfile | 1 + Gemfile.lock | 3 +++ 2 files changed, 4 insertions(+) diff --git a/Gemfile b/Gemfile index 49996f05c..fd011be10 100644 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,7 @@ gem 'prawn-table' gem 'bundler', '>= 1.10.6' gem 'elasticsearch-model' gem 'elasticsearch-rails' +gem 'skylight' # Assets gem 'jquery-rails', '4.0.4' diff --git a/Gemfile.lock b/Gemfile.lock index 4e5301c44..de4b59982 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -330,6 +330,8 @@ GEM simple_form (3.1.0) actionpack (~> 4.0) activemodel (~> 4.0) + skylight (1.5.0) + activesupport (>= 3.0.0) sprockets (3.7.1) concurrent-ruby (~> 1.0) rack (> 1, < 3) @@ -425,6 +427,7 @@ DEPENDENCIES select2-rails shoulda (>= 3.5) simple_form (>= 3.0.0) + skylight thin uglifier (= 2.7.2) unicorn