Merge pull request #19 from FireflyArtsCollective/cleanup-artists-con…

…troller

Cleanup artists controller

app/controllers/artists_controller.rb

@@ -1,86 +1,48 @@
 class ArtistsController < ApplicationController
-  load_and_authorize_resource only: [:index, :show]
+  load_and_authorize_resource
 
-  before_filter :initialize_user, except: [:show]
+  def show
+  end
 
-  def signup
-    @artist.artist_survey ||= ArtistSurvey.new
+  def new
+    @artist.artist_survey ||= @artist.build_artist_survey
   end
 
   def create
     if Artist.exists?(email: artist_params[:email].downcase)
       flash[:warning] = "The email address #{artist_params[:email.downcase]} already exists in our system"
-      render "signup_failure"
       return
     end
 
     @artist = Artist.new(artist_params)
     @artist.email = @artist.email.downcase
 
     if @artist.save
-      # save optional survey
-      artist_survey = ArtistSurvey.new(artist_survey_params)
-      artist_survey.artist_id = @artist.id
-      artist_survey.save
-
       # Send email!
       begin
-        UserMailer.account_activation("artists", @artist).deliver_now
+        UserMailer.account_activation('artists', @artist).deliver_now
         logger.info "email: artist account activation sent to #{@artist.email}"
       rescue
-        flash[:warning] = "Error sending email confirmation"
-        render "signup_failure"
-        return
+        flash[:warning] = 'Error sending email confirmation'
       end
-
-      render "signup_success"
-    else
-      @artist.artist_survey ||= ArtistSurvey.new(artist_survey_params)
-      render "signup"
     end
-  end
 
-  def delete_grant
-    if !artist_logged_in?
-      return
-    end
-
-    begin
-      @submission = GrantSubmission.find(params[:grant_id])
-    rescue
-      redirect_to action: "index"
-      return
-    end
-
-    # TODO: is this enough "security"?
-    if @submission.artist_id != current_artist.id
-      # Log more stuff
-      logger.info "SECURITY WARNING: Attempted to delete grant while not logged in as that artist"
-      redirect_to action: "index"
-      return
-    end
-    # Also should delete pdf from filesystem
-    @submission.destroy
-    redirect_to action: "index"
+    render 'new'
   end
 
   private
 
-  def initialize_user
-    @artist = Artist.new
-  end
-
   def artist_params
+    artist_survey_attribute_names = [
+      :has_attended_firefly, :has_attended_firefly_desc,
+      :has_attended_regional, :has_attended_regional_desc,
+      :has_attended_bm, :has_attended_bm_desc,
+      :can_use_as_example
+    ]
+
     params.require(:artist).permit(:name, :password_digest, :password,
         :password_confirmation, :email, :contact_name, :contact_phone,
         :contact_street, :contact_city, :contact_state, :contact_zipcode,
-        :contact_country)
-  end
-
-  def artist_survey_params
-    params.require(:artist).require(:artist_survey).permit(:has_attended_firefly,
-        :has_attended_firefly_desc, :has_attended_regional,
-        :has_attended_regional_desc, :has_attended_bm, :has_attended_bm_desc,
-        :can_use_as_example)
+        :contact_country, artist_survey_attributes: [artist_survey_attribute_names])
   end
 end

app/models/ability.rb

@@ -14,6 +14,10 @@ def initialize(user)
 
     # Allow an initial Admin to be crated by anyone
     can :manage, Admin unless Admin.exists?
+
+    can [:new, :create], Artist
+    can [:new, :create], Voter
+
     can :read, Grant, hidden: false
 
     if user.is_a?(Admin)

app/models/artist.rb

@@ -4,9 +4,11 @@ class Artist < ActiveRecord::Base
 
   has_secure_password
 
-  has_one :artist_survey
+  has_one :artist_survey, inverse_of: :artist
   has_many :grant_submissions
 
+  accepts_nested_attributes_for :artist_survey
+
   validates :name, presence: true, length: { minimum: 4 }
   validates :email, presence: true
   validates :password, length: { minimum: 4 }, on: :create

app/views/artists/signup.html.erb → app/views/artists/new.html.erb

@@ -32,7 +32,7 @@
     </div>
   </div>
 
-  <%= f.simple_fields_for @artist.artist_survey do |f_artist_survey| %>
+  <%= f.simple_fields_for :artist_survey do |f_artist_survey| %>
   <div class="panel panel-default">
     <div class="panel-heading">
       <h3 class="panel-title">Optional Survey</h3>
@@ -49,7 +49,7 @@
       <%= f_artist_survey.input :has_attended_bm, as: :radio_buttons, label: 'I have attended Burning Man' %>
       <%= f_artist_survey.text_area :has_attended_bm_desc, placeholder: 'Please elaborate. How have you participated at Burning Man?', class: 'form-control', rows: '3'  %>
 
-      <%= f_artist_survey.input :can_use_as_example, label: 'If I am awarded a grant, Firefly has permission to use my grant application as an example of a successful application in future years' %>
+      <%= f_artist_survey.input :can_use_as_example, label: 'If I am awarded a grant, Firefly has permission to use my grant application as an example of a successful application in future years', as: :boolean %>
     </div>
   </div>
   <% end %>

app/views/sessions/artists/new.html.erb

@@ -1,4 +1,4 @@
-<%= link_to "Register as an artist", artists_signup_path %> to access submit grant applications.
+<%= link_to "Register as an artist", new_artist_path %> to access submit grant applications.
 
 <h2>Have an account?</h2>
 

config/routes.rb

@@ -7,9 +7,6 @@
   get 'password_resets/new'
   get 'password_resets/edit'
 
-  get 'artists/signup' => 'artists#signup'
-  post 'artists/signup' => 'artists#create'
-
   get 'voters/signup' => 'voters#signup'
   post 'voters/signup' => 'voters#create'
 

spec/controllers/artists_controller_spec.rb

@@ -0,0 +1,72 @@
+describe ArtistsController do
+  subject { response }
+
+  describe '#index' do
+    def go!
+      get :index
+    end
+
+    it { go!; is_expected.to be_forbidden }
+
+    context 'when admin signed in' do
+      let!(:admin) { FactoryGirl.create(:admin) }
+
+      before do
+        sign_in admin
+      end
+
+      it 'is ok' do
+        go!
+        expect(response).to render_template('index')
+        expect(response).to be_ok
+      end
+    end
+  end
+
+  describe '#new' do
+    def go!
+      get :new
+    end
+
+    before { go! }
+
+    it { is_expected.to render_template('new') }
+    it { is_expected.to be_ok }
+  end
+
+  describe '#create' do
+    def go!
+      post :create, artist_params
+    end
+
+    let(:artist_attributes) { FactoryGirl.attributes_for(:artist) }
+    let(:artist_survey_attributes) { FactoryGirl.attributes_for(:artist_survey) }
+    let(:artist_params) do
+      {
+        artist: artist_attributes.merge(artist_survey_attributes: artist_survey_attributes)
+      }
+    end
+
+    it 'creates Artist' do
+      expect { go! }.to change { Artist.count }.by(1)
+    end
+
+    it 'creates ArtistSurvey' do
+      expect { go! }.to change { ArtistSurvey.count }.by(1)
+    end
+
+    it 'sends email' do
+      expect(UserMailer).to receive(:account_activation)
+      go!
+    end
+
+    context 'with invalid params' do
+      let(:artist_attributes) { FactoryGirl.attributes_for(:artist, email: '') }
+
+      it 'displays form' do
+        expect { go! }.not_to change { Admin.count }
+        expect(response).to render_template('new')
+      end
+    end
+  end
+end

spec/models/ability_spec.rb

@@ -16,6 +16,14 @@
     it { is_expected.not_to be_able_to(:read, FactoryGirl.build(:grant, hidden: true)) }
   end
 
+  shared_examples 'signup Voter and Artist' do
+    it { is_expected.to be_able_to(:new, Voter) }
+    it { is_expected.to be_able_to(:create, Voter) }
+
+    it { is_expected.to be_able_to(:new, Artist) }
+    it { is_expected.to be_able_to(:create, Artist) }
+  end
+
   context 'with nil' do
     let(:user) { nil }
 
@@ -47,6 +55,7 @@
 
     it_behaves_like 'can manage Admin unless Admin.exists?'
     it_behaves_like 'can read non-hidden Grants'
+    it_behaves_like 'signup Voter and Artist'
 
     it { is_expected.to be_able_to(:manage, artist_survey) }
     it { is_expected.to be_able_to(:manage, grant_submission) }
@@ -84,6 +93,7 @@
 
     it_behaves_like 'can manage Admin unless Admin.exists?'
     it_behaves_like 'can read non-hidden Grants'
+    it_behaves_like 'signup Voter and Artist'
 
     it { is_expected.to be_able_to(:vote, GrantSubmission.new) }