fix MR comments

app/controllers/account_sessions_controller.rb

@@ -1,5 +1,4 @@
 class AccountSessionsController < ApplicationController
-  skip_before_action :redirect_locked_accounts
   def update
     authorize :account_session, :update?
     session[:active_account] = account_session_params[:account_id]

app/controllers/admin/account_locks_controller.rb

@@ -1,31 +0,0 @@
-class Admin::AccountLocksController < Admin::ApplicationController
-  before_action :set_account
-
-  def create
-    authorize :account_lock, :create?
-    @account.update(locked: true)
-    redirect_to admin_accounts_path, notice: 'Account locked'
-  end
-
-  def destroy
-    authorize :account_lock, :destroy?
-    @account.update(locked: false)
-    redirect_to admin_accounts_path, notice: 'Account unlocked'
-  end
-
-  private
-
-  def set_account
-    if params[:account_id].present?
-      @account = Account.find_by(slug: params[:account_id])
-    else
-      failure_redirect
-    end
-  rescue ActiveRecord::RecordNotFound
-    failure_redirect
-  end
-
-  def failure_redirect
-    redirect_to admin_accounts_path, alert: 'Failed to find the account'
-  end
-end

app/controllers/application_controller.rb

@@ -13,7 +13,6 @@ class ApplicationController < ActionController::Base
 
   include SetCurrentRequestDetails
   before_action :redirect_accountless
-  before_action :redirect_locked_accounts
 
   private
 
@@ -30,12 +29,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def redirect_locked_accounts
-    if Current.user && !Current.user.admin? && Current.account.present? && Current.account.locked?
-      redirect_to locked_account_path
-    end
-  end
-
   def signed_in_as_admin?
     signed_in? && current_user.admin?
   end

app/controllers/locked_accounts_controller.rb

@@ -1,10 +0,0 @@
-class LockedAccountsController < ApplicationController
-  skip_before_action :redirect_locked_accounts
-  skip_after_action :verify_policy_scoped
-
-  def index
-    unless Current.account.locked?
-      redirect_to projects_path
-    end
-  end
-end

app/policies/account_lock_policy.rb

@@ -1,9 +0,0 @@
-class AccountLockPolicy < ApplicationPolicy
-  def create?
-    user.admin?
-  end
-
-  def destroy?
-    user.admin?
-  end
-end

app/views/admin/accounts/_account.html.erb

@@ -30,11 +30,6 @@
         <%= link_to fa_icon("arrow-right", text: "Overview"), admin_account_path(account), class: "dropdown-item" %>
         <%= link_to fa_icon("pencil", text: "Edit"), edit_admin_account_path(account), class: "dropdown-item" %>
         <%= link_to fa_icon("arrow-right", text: "Account Managers"), account_auths_path({ account_id: account.id}), class: "dropdown-item" %>
-        <% if account.locked? %>
-          <%= link_to fa_icon("unlock", text: "Unlock Account"), [:admin, account, :lock], method: :delete, class: "dropdown-item" %>
-        <% else %>
-          <%= link_to fa_icon("lock", text: "Lock Account"), [:admin, account, :lock], method: :post, class: "dropdown-item" %>
-        <% end %>
       </div>
     </div>
   </td>

app/views/contracts/_logo_page.html.erb

@@ -0,0 +1,37 @@
+<% has_logo = local_assigns[:logo] %>
+<% has_user_photo = releasable.model_name == "AppearanceRelease" && releasable.person_photo.attached? %>
+
+<% if has_logo || has_user_photo %>
+  <table class="heading-table">
+    <tr>
+      <td>
+        <% if has_logo %>
+          <div class="logo">
+            <%= render "blank_contracts/logo", logo: logo %>
+          </div>
+        <% end %>
+      </td>
+      <td class="embed-person-photo">
+        <% if has_user_photo %>
+          <%= image_tag releasable.photos.first.variant(auto_orient: true, resize: "200x200"), id: "top-person-photo" %>
+        <% end %>
+      </td>
+    </tr>
+  </table>
+
+  <hr>
+<% end %>
+
+<% if preview %>
+  <h1>PREVIEW ONLY</h1>
+<% end %>
+
+<% if contract_template.body.present? %>
+  <%= contract_template.body %>
+  <br/>
+<% end %>
+
+<% if releasable.minor? && contract_template.guardian_clause.present? %>
+  <p class="text-left"><strong>Guardian Clause</strong></p>
+  <%= contract_template.guardian_clause %>
+<% end %>

app/views/contracts/pdf.html.erb

@@ -1,33 +1,12 @@
-<% if local_assigns[:logo] %>
+<% logo = local_assigns[:logo] ? local_assigns[:logo] : nil %>
-  <div class="logo">
-    <%= render "contracts/logo", logo: logo %>
-  </div>
-  <hr>
-<% end %>
 
+<% if logo || (releasable.model_name == 'AppearanceRelease' && releasable.person_photo.attached?) ||
+    contract_template.body.present? || (releasable.minor? && contract_template.guardian_clause.present?) %>
   <div class="page">
-  <% if preview %>
+    <%= render "contracts/logo_page", logo: logo, preview: preview, releasable: releasable, contract_template: contract_template %>
-    <h1>PREVIEW ONLY</h1>
-  <% end %>
-
-  <% if releasable.model_name == "AppearanceRelease" && releasable.person_photo.attached? %>
-    <div class="embed-person-photo">
-      <%= image_tag releasable.photos.first.variant(auto_orient: true, resize: "200x200"), id: "top-person-photo" %>
   </div>
-    <hr>
 <% end %>
 
-  <% if contract_template.body.present? %>
-    <%= contract_template.body %>
-    <br/>
-  <% end %>
-
-  <% if releasable.minor? && contract_template.guardian_clause.present? %>
-    <p class="text-left"><strong>Guardian Clause</strong></p>
-    <%= contract_template.guardian_clause %>
-  <% end %>
-</div>
-
 <% if contract_template.present? && contract_template.has_questionnaire? %>
   <div class="page">
     <%= render "contracts/questionnaire", releasable: releasable, contract_template: contract_template, preview: preview %>

app/views/locked_accounts/index.html.erb

@@ -1 +0,0 @@
-<p><%= t '.account_locked_message' %></p>

config/locales/en.yml

@@ -1651,6 +1651,3 @@ en:
         edit: Edit
         report: Report
       generating: Generating...
-  locked_accounts:
-    index:
-      account_locked_message: This account is locked. Please contact a BIG admin.

config/locales/es.yml

@@ -705,6 +705,3 @@ es:
         production_elements_logs: Production Elements Logs, and more (ES)
         reduces_labor_cost: Reduces labor costs (ES)
         simplifies_cue_sheets: Simplifies Music Cue Sheets, Graphic Cue Sheets (ES)
-  locked_accounts:
-    index:
-      account_locked_message: This account is locked. Please contact a BIG admin. (ES)

config/routes.rb

@@ -30,9 +30,7 @@ Rails.application.routes.draw do
     namespace :admin do
       mount Sidekiq::Web => '/background_queue', as: :background_queue
 
-      resources :accounts, only: [:index, :new, :create, :edit, :update, :show] do
+      resources :accounts, only: [:index, :new, :create, :edit, :update, :show]
-        resource :account_lock, path: :lock, as: :lock, only: [:create, :destroy]
-      end
       resources :users, only: [:index, :new, :create, :edit, :update, :destroy] do
         resource :masquerade, only: :create
       end
@@ -50,9 +48,7 @@ Rails.application.routes.draw do
   scope "(:locale)", locale: AVAILABLE_LOCALES_REGEX do
     resource :account_session, only: [:update]
     resource :session, only: [:destroy]
-    resource :account, only: [:new, :create, :update] do
+    resource :account, only: [:new, :create, :update]
-      get 'locked' => 'locked_accounts#index'
-    end
     resources :account_auths, only: [:index, :create, :update, :destroy]
     resources :projects, shallow: true do
       resources :acquired_media_releases, except: [:show], concerns: [:contractable, :notable, :file_uploadable]

db/migrate/20200908085319_add_locked_to_accounts.rb

@@ -1,5 +0,0 @@
-class AddLockedToAccounts < ActiveRecord::Migration[6.0]
-  def change
-    add_column :accounts, :locked, :boolean, default: false
-  end
-end

db/structure.sql

@@ -95,8 +95,7 @@ CREATE TABLE public.accounts (
     slug character varying,
     plan_uid character varying,
     created_at timestamp without time zone NOT NULL,
-    updated_at timestamp without time zone NOT NULL,
+    updated_at timestamp without time zone NOT NULL
-    locked boolean DEFAULT false
 );
 
 
@@ -4028,7 +4027,6 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20200812060406'),
 ('20200819070738'),
 ('20200820082501'),
-('20200824171649'),
+('20200824171649');
-('20200908085319');
 
 

spec/features/admin_managing_accounts_spec.rb

@@ -30,24 +30,6 @@ feature "Admin managing accounts" do
     expect(page).to have_content "Created at less than a minute ago"
   end
 
-  scenario "locks and unlocks account" do
-    sign_in current_user
-    visit admin_signed_in_root_path
-    expect(Account.last.locked?).to eq false
-
-    click_button "Manage"
-    expect(page).not_to have_content "Unlock Account"
-    click_link "Lock Account"
-
-    expect(Account.last.locked?).to eq true
-
-    click_button "Manage"
-    expect(page).not_to have_content "Lock Account"
-    click_link "Unlock Account"
-
-    expect(Account.last.locked?).to eq false
-  end
-
   scenario "sees videos for an account in the system" do
     visit_account_overview_page
 

spec/features/user_managing_appearance_releases_spec.rb

@@ -684,6 +684,15 @@ feature 'User managing appearance releases' do
       expect(page).to have_selector("#top-person-photo[src^='#{person_photo_url}']")
     end
 
+    scenario "viewing the contract PDF - it shows person photo on first page if person photo is attached" do
+      appearance_release = create(:appearance_release_with_contract_template, :native, :minor_with_guardian_photo, project: project)
+
+      visit view_release_pdf_html_preview_link_for(appearance_release)
+
+      person_photo_url = url_for(appearance_release.person_photo.variant(auto_orient: true, resize: "200x200")).to_s
+      expect(page).to have_selector("#top-person-photo[src^='#{person_photo_url}']")
+    end
+
     scenario 'deleting a release', js: true do
       appearance_release = create(:appearance_release, project: project)
 

spec/features/user_managing_locked_account_spec.rb

@@ -1,34 +0,0 @@
-require "rails_helper"
-
-feature "User managing locked account" do
-  let(:user) { create(:user, :account_manager) }
-  let(:project) { create(:project) }
-
-  before do
-    sign_in(user)
-    user.accounts.first.update(locked: true)
-  end
-
-  scenario "user is redirected to custom landing page when opens projects index page" do
-    paths = [
-      projects_path,
-      project_path(project),
-      project_task_requests_path(project),
-      project_contract_templates_path(project),
-      project_broadcasts_path(project),
-      project_videos_path(project),
-    ]
-
-    paths.each do |path|
-      visit path
-
-      expect(page).to have_content locked_account_warning
-    end
-  end
-
-  private
-
-  def locked_account_warning
-    t 'locked_accounts.index.account_locked_message'
-  end
-end