Skip to content

Commit

Permalink
[WIP] Convert image links to shortened URLs (#546)
Browse files Browse the repository at this point in the history
* tinyurl

* forgot to add in last

* shorten_url service, added columns

* fixed ShortenUrl, company and user #short...url methods

* fixes in models, extended error logs

* ShortenUrl and spec fixes

* cognitive complexity

* back to the helpers

* just a little change
  • Loading branch information
tomurb authored and patmbolger committed Jul 23, 2018
1 parent 95e8572 commit 70b41fe
Show file tree
Hide file tree
Showing 22 changed files with 264 additions and 11 deletions.
1 change: 0 additions & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,5 +131,4 @@ def allow_iframe_request
# cannot-display-my-rails-4-app-in-iframe-even-if-x-frame-options-is-allowall
end


end
5 changes: 5 additions & 0 deletions app/helpers/companies_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,9 @@ def company_number_entry_field(company_numbers)
text_field_tag :company_number, company_numbers,
id: 'shf_application_company_number'
end

def short_h_brand_url(company)
url = company_h_brand_url(0, company_id: company.id)
company.get_short_h_brand_url(url)
end
end
4 changes: 4 additions & 0 deletions app/helpers/users_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,8 @@ def user_has_open_application(user)
user.shf_application.state.to_sym.in?([:accepted, :rejected]) ? nil : t('yes')
end

def short_proof_of_membership_url(user)
url = proof_of_membership_url(user.id)
user.get_short_proof_of_membership_url(url)
end
end
12 changes: 12 additions & 0 deletions app/models/company.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,18 @@ def se_mailing_csv_str
AddressExporter.se_mailing_csv_str( main_address )
end

def get_short_h_brand_url(url)
found = self.short_h_brand_url
return found if found
short_url = ShortenUrl.short(url)
if short_url
self.update_attribute(:short_h_brand_url, short_url)
short_url
else
url
end
end


private

Expand Down
12 changes: 12 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ def grant_membership(send_email: true)
Arel.sql("lpad(membership_number, 20, '0')")
end

def get_short_proof_of_membership_url(url)
found = self.short_proof_of_membership_url
return found if found
short_url = ShortenUrl.short(url)
if short_url
self.update_attribute(:short_proof_of_membership_url, short_url)
short_url
else
url
end
end

private

def issue_membership_number
Expand Down
30 changes: 30 additions & 0 deletions app/services/shorten_url.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
class ShortenUrl

SHORTEN_URL_LOG = 'log/tinyurl.log'
SUCCESS_CODES = [200, 201, 202].freeze

def self.short(url)
response = HTTParty.get("http://tinyurl.com/api-create.php?url=#{url}")
raise HTTParty::Error if response.match?(/error/i) || !SUCCESS_CODES.include?(response.code)
response
rescue HTTParty::Error => error
log_error(error)
nil
end

private_class_method

def self.log_error(error)
ActivityLogger.open(SHORTEN_URL_LOG, 'TINYURL_API', 'shortening url', false) do |log|
log.record('error', "Exception: #{error.message}")
if response
log.record('error', "Attempted URL: #{url}")
log.record('error', "Response body: #{response.body}")
log.record('error', "HTTP code: #{response.code}")
else
log.record('error', "Exception raised by HTTParty")
end
end
end
end

2 changes: 1 addition & 1 deletion app/views/users/_company_h_brand.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
%br
%br
= t('users.show.use_this_image_link_html')
= company_h_brand_url(user, company_id: company.id)
= short_h_brand_url(company)

:javascript
window.onload = function(){
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/_proof_of_membership.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
%br
%br
= t('users.show.use_this_image_link_html')
= proof_of_membership_url(user)
= short_proof_of_membership_url(user)

:javascript
window.onload = function(){
Expand Down
1 change: 0 additions & 1 deletion config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
#
###


# 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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddShortHBrandUrlToCompanies < ActiveRecord::Migration[5.1]
def change
add_column :companies, :short_h_brand_url, :string
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddShortProofOfMembershipUrlToUsers < ActiveRecord::Migration[5.1]
def change
add_column :users, :short_proof_of_membership_url, :string
end
end
8 changes: 4 additions & 4 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 20180428103625) do
ActiveRecord::Schema.define(version: 20180719021503) do

# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
Expand Down Expand Up @@ -94,6 +94,8 @@
t.datetime "updated_at", null: false
t.text "description"
t.string "dinkurs_company_id"
t.boolean "show_dinkurs_events"
t.string "short_h_brand_url"
t.index ["company_number"], name: "index_companies_on_company_number", unique: true
end

Expand Down Expand Up @@ -229,6 +231,7 @@
t.string "member_photo_content_type"
t.integer "member_photo_file_size"
t.datetime "member_photo_updated_at"
t.string "short_proof_of_membership_url"
t.index ["email"], name: "index_users_on_email", unique: true
t.index ["membership_number"], name: "index_users_on_membership_number", unique: true
t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
Expand All @@ -240,9 +243,6 @@
add_foreign_key "company_applications", "companies"
add_foreign_key "company_applications", "shf_applications"
add_foreign_key "events", "companies"
add_foreign_key "company_applications", "companies"
add_foreign_key "company_applications", "shf_applications"
add_foreign_key "events", "companies"
add_foreign_key "payments", "companies"
add_foreign_key "payments", "users"
add_foreign_key "shf_applications", "member_app_waiting_reasons", column: "member_app_waiting_reasons_id"
Expand Down
10 changes: 7 additions & 3 deletions db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,8 @@ CREATE TABLE public.companies (
updated_at timestamp without time zone NOT NULL,
description text,
dinkurs_company_id character varying,
show_dinkurs_events boolean
show_dinkurs_events boolean,
short_h_brand_url character varying
);


Expand Down Expand Up @@ -700,7 +701,8 @@ CREATE TABLE public.users (
member_photo_file_name character varying,
member_photo_content_type character varying,
member_photo_file_size integer,
member_photo_updated_at timestamp without time zone
member_photo_updated_at timestamp without time zone,
short_proof_of_membership_url character varying
);


Expand Down Expand Up @@ -1318,6 +1320,8 @@ INSERT INTO "schema_migrations" (version) VALUES
('20180326103433'),
('20180328105100'),
('20180428103625'),
('20180624155644');
('20180624155644'),
('20180717043851'),
('20180719021503');


1 change: 1 addition & 0 deletions spec/factories/companies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
phone_number '123123123'
email 'thiscompany@example.com'
website 'http://www.example.com'
short_h_brand_url nil

transient do
num_addresses 1
Expand Down
2 changes: 2 additions & 0 deletions spec/factories/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
password 'my_password'
admin false
member false
short_proof_of_membership_url nil
member_photo do
File.new("#{Rails.root}/spec/fixtures/member_photos/photo_unavailable.png")
end
Expand Down Expand Up @@ -44,4 +45,5 @@
end
end


end
9 changes: 9 additions & 0 deletions spec/helpers/companies_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,4 +177,13 @@
.to match(/id="shf_application_company_number" value="0000000000"/)
end
end

describe '#short_h_brand_url' do
it 'returns value returned by Company#get_short_h_brand_url using generated url' do
url = company_h_brand_url(0, company_id: company.id)
allow(company).to receive(:get_short_h_brand_url).with(url).and_return(url)
expect(short_h_brand_url(company)).to eq(url)
end
end

end
8 changes: 8 additions & 0 deletions spec/helpers/users_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,12 @@
expect(user_has_open_application(user)).to be_nil
end
end

describe '#short_proof_of_membership_url' do
it 'returns value returned by User#get_short_proof_of_membership_url using generated url' do
url = proof_of_membership_url(user.id)
allow(user).to receive(:get_short_proof_of_membership_url).with(url).and_return(url)
expect(short_proof_of_membership_url(user)).to eq(url)
end
end
end
27 changes: 27 additions & 0 deletions spec/models/company_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@

RSpec.describe Company, type: :model do

let(:with_short_h_brand_url) do
create(:company, short_h_brand_url: 'http://www.tinyurl.com/hbrand')
end

let(:no_name) do
create(:company, name: '', company_number: '2120000142')
end
Expand Down Expand Up @@ -94,6 +98,7 @@
it { is_expected.to have_db_column :description }
it { is_expected.to have_db_column :dinkurs_company_id }
it { is_expected.to have_db_column :show_dinkurs_events }
it { is_expected.to have_db_column :short_h_brand_url }
end

describe 'Validations' do
Expand Down Expand Up @@ -697,4 +702,26 @@
end
end

describe '#get_short_h_brand_url' do
context 'there is already a shortened url in the table' do
it 'returns shortened url' do
url = 'http://localhost:3000/anvandare/0/company_h_brand?company_id=1'
expect(with_short_h_brand_url.get_short_h_brand_url(url)).to eq('http://www.tinyurl.com/hbrand')
end
end
context 'there is no shortened url in the table and ShortenUrl.short is called' do
it 'saves the result if the result is not nil and returns shortened url' do
url = 'http://localhost:3000/anvandare/0/company_h_brand?company_id=1'
allow(ShortenUrl).to receive(:short).with(url).and_return('http://tinyurl.com/hbrand2')
expect(complete_co.get_short_h_brand_url(url)).to eq(ShortenUrl.short(url))
expect(complete_co.short_h_brand_url).to eq(ShortenUrl.short(url))
end
it 'does not save anything if the result is nil and returns unshortened url' do
url = 'http://localhost:3000/anvandare/0/company_h_brand?company_id=1'
allow(ShortenUrl).to receive(:short).with(url).and_return(nil)
expect(complete_co.get_short_h_brand_url(url)).to eq(url)
expect(complete_co.short_h_brand_url).to eq(nil)
end
end
end
end
28 changes: 28 additions & 0 deletions spec/models/user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@
require 'email_spec/rspec'

RSpec.describe User, type: :model do

let(:user) { create(:user) }
let(:user_with_app) { create(:user_with_membership_app) }
let(:member) { create(:member_with_membership_app) }

let(:with_short_proof_of_membership_url) do
create(:user, short_proof_of_membership_url: 'http://www.tinyurl.com/proofofmembership')
end

let(:application) do
create(:shf_application, user: user, state: :accepted)
end
Expand Down Expand Up @@ -535,4 +540,27 @@
end
end
end

describe '#get_short_proof_of_membership_url' do
context 'there is already a shortened url in the table' do
it 'returns shortened url' do
url = 'http://localhost:3000/anvandare/0/company_h_brand?company_id=1'
expect(with_short_proof_of_membership_url.get_short_proof_of_membership_url(url)).to eq('http://www.tinyurl.com/proofofmembership')
end
end
context 'there is no shortened url in the table and ShortenUrl.short is called' do
it 'saves the result if the result is not nil and returns shortened url' do
url = 'http://localhost:3000/anvandare/0/company_h_brand?company_id=1'
allow(ShortenUrl).to receive(:short).with(url).and_return('http://tinyurl.com/proofofmembership2')
expect(user.get_short_proof_of_membership_url(url)).to eq(ShortenUrl.short(url))
expect(user.short_proof_of_membership_url).to eq(ShortenUrl.short(url))
end
it 'does not save anything if the result is nil and returns unshortened url' do
url = 'http://localhost:3000/anvandare/0/company_h_brand?company_id=1'
allow(ShortenUrl).to receive(:short).with(url).and_return(nil)
expect(user.get_short_proof_of_membership_url(url)).to eq(url)
expect(user.short_proof_of_membership_url).to eq(nil)
end
end
end
end
19 changes: 19 additions & 0 deletions spec/services/shorten_url_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
require 'rails_helper'

describe ShortenUrl do
describe '.short' do
it 'creates shortened link' do
VCR.use_cassette('shorten_url/short') do
shortened_url = ShortenUrl.short 'http://sverigeshundforetagare.se/anvandare/2/proof_of_membership'
expect(shortened_url).to match 'tinyurl.com/ya6sa84h'
end
end
it 'if the service raises an error, returns nil and writes to the log' do
VCR.use_cassette('shorten_url/error') do
expect(ActivityLogger).to receive(:open)
shortened_url = ShortenUrl.short '/'
expect(shortened_url).to eq nil
end
end
end
end
41 changes: 41 additions & 0 deletions spec/vcr_cassettes/shorten_url/error.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 70b41fe

Please sign in to comment.