Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tpi/not active companies #216

Merged
merged 14 commits into from
Jan 30, 2020
7 changes: 6 additions & 1 deletion app/admin/companies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

permit_params :name, :isin, :sector_id, :geography_id, :headquarters_geography_id,
:ca100, :market_cap_group, :visibility_status, :sedol,
:latest_information, :historical_comments
:latest_information, :historical_comments, :active

filter :isin_contains, label: 'ISIN'
filter :name_contains, label: 'Name'
Expand Down Expand Up @@ -49,6 +49,7 @@
row 'Management Quality Level', &:mq_level_tag
row :latest_information
row :historical_comments
row :active
row :created_at
row :updated_at
end
Expand Down Expand Up @@ -130,6 +131,7 @@
column :level, &:mq_level_tag
column :geography
column :headquarters_geography
column :active
tag_column :visibility_status

actions
Expand All @@ -148,6 +150,7 @@
column(:headquarters_geography) { |c| c.headquarters_geography.name }
column :latest_information
column :ca100
column :active
column :visibility_status
end

Expand Down Expand Up @@ -178,6 +181,8 @@
end
end

f.input :active

f.input :ca100

f.input :latest_information,
Expand Down
6 changes: 4 additions & 2 deletions app/controllers/tpi/companies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ def show
@company_presenter = ::Api::Presenters::Company.new(@company)

@sectors = TPISector.select(:id, :name, :slug).order(:name)
@companies = Company.published.joins(:sector).select(:id, :name, :slug, 'tpi_sectors.name as sector_name')
@companies =
Company.published.joins(:sector).select(:id, :name, :slug, 'tpi_sectors.name as sector_name', :active)
@companies = TPI::CompanyDecorator.decorate_collection(@companies)
end

def mq_assessment; end
Expand Down Expand Up @@ -49,7 +51,7 @@ def user_download
private

def fetch_company
@company = Company.published.friendly.find(params[:id])
@company = TPI::CompanyDecorator.decorate(Company.published.friendly.find(params[:id]))
end

def redirect_if_numeric_or_historic_slug
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/tpi/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module TPI
class SearchController < TPIController
def index
@sectors = TPISector.search(params[:query]).order(:name)
@companies = Company.published.search(params[:query]).order(:name)
@companies = TPI::CompanyDecorator.decorate_collection(Company.published.search(params[:query]).order(:name))

publications = Publication.search(params[:query]).order(:title)
news = NewsArticle.search(params[:query]).order(:title)
Expand Down
9 changes: 5 additions & 4 deletions app/controllers/tpi/sectors_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def cp_performance_chart_data
end

def user_download_all
companies_ids = Company.published.select(:id).where(sector_id: @sectors.pluck(:id))
companies_ids = Company.published.active.select(:id).where(sector_id: @sectors.pluck(:id))
mq_assessments = MQ::Assessment
.where(company_id: companies_ids)
.joins(:company)
Expand All @@ -72,7 +72,7 @@ def user_download_all
end

def user_download
companies_ids = @sector.companies.published.select(:id)
companies_ids = @sector.companies.published.active.select(:id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious - are we sure that scopes published and active are independent of each other? Or maybe inactive companies should be always unpublished?

If those are unrelated we could at least add another scope published_active as it looks those 2 go together at least in this controller.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Active flag is different from the publishable status. So you can have published && inactive companies, and they should still show up on the frontend, just not on all the charts, because an inactive company is a company that is not getting new assessments, but has historical ones. But we could add an extra scope for when we want both published and active if that makes it more clear.

mq_assessments = MQ::Assessment
.where(company_id: companies_ids)
.joins(:company)
Expand Down Expand Up @@ -108,15 +108,16 @@ def fetch_sectors
def fetch_companies
@companies = Company
.published
.active
.joins(:sector)
.select(:id, :name, :slug, :sector_id, 'tpi_sectors.name as sector_name')
end

def companies_scope(params)
if params[:id]
TPISector.friendly.find(params[:id]).companies.published
TPISector.friendly.find(params[:id]).companies.published.active
else
Company.published
Company.published.active
end
end
end
Expand Down
11 changes: 11 additions & 0 deletions app/decorators/tpi/company_decorator.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module TPI
class CompanyDecorator < Draper::Decorator
delegate_all

def name
return model.name if model.active

"#{model.name} (Not active)"
end
end
end
3 changes: 3 additions & 0 deletions app/models/company.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# sedol :string
# latest_information :text
# historical_comments :text
# active :boolean default(TRUE)
#

class Company < ApplicationRecord
Expand Down Expand Up @@ -51,6 +52,8 @@ class Company < ApplicationRecord
validates_presence_of :name, :slug, :isin, :market_cap_group
validates_uniqueness_of :slug, :name

scope :active, -> { where(active: true) }

def should_generate_new_friendly_id?
name_changed? || super
end
Expand Down
2 changes: 1 addition & 1 deletion app/services/api/charts/cp_performance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class CPPerformance
# }
# ]
def cp_performance_all_sectors_data
all_companies = Company.published.includes(:sector, :latest_cp_assessment)
all_companies = Company.published.active.includes(:sector, :latest_cp_assessment)

cp_alignment_data = {}
all_companies.each do |company|
Expand Down
3 changes: 2 additions & 1 deletion app/services/csv_import/companies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def company_attributes(row)
sedol: row[:sedol],
latest_information: row[:latest_information],
historical_comments: row[:historical_comments],
visibility_status: row[:visibility_status]
visibility_status: row[:visibility_status],
active: row[:active] || true
}
end
end
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20191221151801_add_active_to_company.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddActiveToCompany < ActiveRecord::Migration[6.0]
def change
add_column :companies, :active, :boolean, default: true
end
end
2 changes: 2 additions & 0 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
t.string "sedol"
t.text "latest_information"
t.text "historical_comments"
t.boolean "active", default: true
t.index ["discarded_at"], name: "index_companies_on_discarded_at"
t.index ["geography_id"], name: "index_companies_on_geography_id"
t.index ["headquarters_geography_id"], name: "index_companies_on_headquarters_geography_id"
Expand All @@ -115,6 +116,7 @@
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.string "content_type"
t.string "youtube_link"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some remnants from other branch ;)

t.index ["page_id"], name: "index_contents_on_page_id"
end

Expand Down
2 changes: 2 additions & 0 deletions spec/factories/companies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# sedol :string
# latest_information :text
# historical_comments :text
# active :boolean default(TRUE)
#

FactoryBot.define do
Expand All @@ -30,6 +31,7 @@
isin { SecureRandom.uuid }

ca100 { true }
active { true }
market_cap_group { Company::MARKET_CAP_GROUPS.sample }
visibility_status { Company::VISIBILITY.sample }

Expand Down
1 change: 1 addition & 0 deletions spec/models/company_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# sedol :string
# latest_information :text
# historical_comments :text
# active :boolean default(TRUE)
#

require 'rails_helper'
Expand Down