minor refactoring to ease testing

YaroSpace · YaroSpace · commit a6377c2e046f · 2014-10-23T01:41:55.000+01:00
split rendering similar teams into a separate template

refactored #creare and added specs

refactored #exact_team_exists? and added specs

added shared notification_bar partial and linked to product_description layout

added showing notifications on team creation
diff --git a/app/controllers/teams_controller.rb b/app/controllers/teams_controller.rb
@@ -77,19 +77,34 @@ def new
   end
 
   def create
-    create_params = params.require(:team).permit(:selected, :slug, :name)
+    team_params = params.require(:team).permit(:selected, :slug, :name)
 
-    @team, confirmed = selected_or_new(create_params)
-    @teams = Team.any_of({ :name => /#{team_to_regex(@team)}.*/i }).limit(3).to_a unless confirmed
+    if team_params.fetch(:selected, nil) == 'true'
+      @team = Team.where(slug: team_params[:slug]).first
+      render :create and return
+    end
 
-    if @team.valid? and @teams.blank? and @team.new_record?
-      @team.add_user(current_user)
-      # @team.edited_by(current_user)
-      @team.save
+    @teams = Team.with_similar_names(team_params[:name])
+    unless @teams.empty?
+      @new_team_name = team_params[:name]
+      render 'similar_teams' and return
+    end
+
+    @team = Team.new(name: team_params[:name])
+    if @team.save
+    # TODO show flash on success or falure
       record_event('created team')
+      @team.add_user(current_user)
     end
   end
 
+  def get_similar_teams(name)
+    name.gsub!(/ \-\./, '.*')
+    #TODO move to Team scope
+    teams = Team.any_of({ :name => /#{name}.*/i }).limit(3).to_a
+    teams.empty? ? nil : teams
+  end
+
   def edit
     edit_params = params.permit(:slug, :id)
 
@@ -299,23 +314,6 @@ def previous_job(job)
     Opportunity.with_public_id(public_id) unless public_id.nil?
   end
 
-  def team_to_regex(team)
-    team.name.gsub(/ \-\./, '.*')
-  end
-
-  def selected_or_new(opts)
-    team = Team.new(name: opts[:name])
-    confirm = false
-
-    if opts[:selected]
-      if opts[:selected] == "true"
-        team = Team.where(:slug => opts[:slug]).first
-      end
-      confirm = true
-    end
-    [team, confirm]
-  end
-
   def ensure_analytics_access
     @team = Team.find(params[:id])
     return redirect_to team_path(@team) unless (@team.analytics? && @team.admin?(current_user)) || is_admin?
diff --git a/app/helpers/teams_helper.rb b/app/helpers/teams_helper.rb
@@ -163,8 +163,8 @@ def change_resume_path
     settings_path(anchor: 'jobs')
   end
 
-  def exact_team_exists?(teams, team)
-    teams.map { |team| Team.slugify(team.name) }.include? team.slug
+  def exact_team_exists?(teams, name)
+    teams.map { |team| Team.slugify(team.name) }.include? name
   end
 
   def team_connections_links(team)
@@ -194,4 +194,4 @@ def team_twitter_link(team)
   end
 
 
-end
+end
diff --git a/app/models/team.rb b/app/models/team.rb
@@ -174,6 +174,11 @@ def with_completed_section(section)
       empty = Team.new.send(section).is_a?(Array) ? [] : nil
       Team.where(section.to_sym.ne => empty)
     end
+
+    def with_similar_names(name)
+      name.gsub!(/ \-\./, '.*')
+      teams = Team.any_of({ :name => /#{name}.*/i }).limit(3).to_a
+    end
   end
 
   def relevancy
@@ -473,10 +478,11 @@ def sorted_team_members
   end
 
   def add_user(user)
-    user.update_attribute(:team_document_id, id.to_s)
     touch!
-    user.save!
-    user
+    user.tap do |u|
+      u.update_attribute(:team_document_id, id.to_s)
+      u.save!
+    end
   end
 
   def remove_user(user)
diff --git a/app/views/layouts/product_description.html.haml b/app/views/layouts/product_description.html.haml
@@ -24,7 +24,6 @@
     = yield :head
 
   %body#product-description
+    = render partial: 'shared/notification_bar'
     = yield
     = render partial: 'shared/footer'
-
-<!-- product_description.html.haml -->
diff --git a/app/views/shared/_notification_bar.html.haml b/app/views/shared/_notification_bar.html.haml
@@ -0,0 +1,7 @@
+#main-content
+  .notification-bar.hidden
+    .notification-bar-inside{ class: (flash[:error].blank? ? 'notice' : 'error') }
+      %p= flash[:notice] || flash[:error]
+      %a.close-notification{ onclick: "$(this).parent().parent().hide();" }
+        %span Close
+
diff --git a/app/views/teams/_similar_teams.html.haml b/app/views/teams/_similar_teams.html.haml
@@ -7,9 +7,9 @@
           .team-avatar
             =image_tag(team.avatar_url)
           %h4= link_to team.name, teamname_path(:slug => team.slug), :target => :new
-          = link_to 'Select', teams_path(Team.new, :team => { :slug=> team.slug, :selected => true }), :remote => true, :method => :post, :class => "select"
+          = link_to 'Select', teams_path(Team.new, :team => { :slug=> team.slug, selected: true }), :remote => true, :method => :post, :class => "select"
 
-- unless exact_team_exists?(@teams, @team)
+- unless exact_team_exists?(@teams, @new_team_name)
   .just-create-team
     %h3 None of the above are my team
-    = link_to "Create team #{@team.name}", teams_path(@team, :team => { :name => @team.name, :selected => false }), :remote => true, :method => :post, :class => "create-team"
+    = link_to "Create team #{@new_team_name}", teams_path(Team.new, :team => { :name => @new_team_name, selected: false }), :remote => true, :method => :post, :class => "create-team"
diff --git a/app/views/teams/create.js.erb b/app/views/teams/create.js.erb
@@ -1,9 +1,9 @@
-<% if @teams.blank? %>
+var notificationBar = $('#main-content').find('.notification-bar');
+notificationBar.find('.notification-bar-inside').find('p').html("<%= flash[:error] || flash[:notice] %>");
+notificationBar.show();
+
 $('section.feature:not(.payment)').hide();
 $('section.feature.payment').append('<%= escape_javascript(render :partial => "payment", :locals => {:account => @team.account || @team.build_account, :plan => @team.account.try(:current_plan)}) %>');
 $('section.feature.payment').show();
 setupPayment();
-<% else %>
-$('section.feature .intro .results').html('<%= escape_javascript(render :partial => 'similar_teams') %>');
-$('section.feature:not(.payment)').addClass('find-team');
-<% end %>
+
diff --git a/app/views/teams/similar_teams.js.erb b/app/views/teams/similar_teams.js.erb
@@ -0,0 +1,2 @@
+$('section.feature .intro .results').html('<%= escape_javascript(render :partial => 'similar_teams') %>');
+$('section.feature:not(.payment)').addClass('find-team');
diff --git a/spec/controllers/teams_controller_spec.rb b/spec/controllers/teams_controller_spec.rb
@@ -39,4 +39,63 @@
     end
   end
 
-end
+  describe "#create" , focus: true  do
+    let(:team) { Fabricate.build(:team) }
+
+    it 'renders an error message if action was unsuccessful' do
+
+    end
+
+    context 'a team is selected from a list of similar teams' do
+      it 'renders a template with a choice of tariff plans when user selects a team' do
+        post :create, :team => { selected: 'true', slug: 'team_name' }, format: :js
+        expect(response).to render_template('create')
+      end
+    end
+
+    context 'a team does not exist' do
+      let(:response) { post :create, :team => { name: 'team_name' }, format: :js }
+
+      before do
+        allow(controller).to receive(:get_similar_teams).and_return(nil)
+
+        allow(Team).to receive(:new).and_return(team)
+        allow(team).to receive(:save).and_return(true)
+        allow(team).to receive(:add_user).and_return(true)
+      end
+
+      it 'creates a new team' do
+        expect(team).to receive(:save)
+        response
+      end
+
+      it 'adds current user to the team' do
+        expect(team).to receive(:add_user).with(current_user)
+        response
+      end
+
+      it 'records an event "created team"' do
+        expect(controller).to receive(:record_event).with('created team')
+        response
+      end
+
+      it 'renders template with option to join' do
+        expect(response).to be_success
+        expect(response).to render_template('create')
+      end
+    end
+
+    context 'a team with similar name already exists' do
+      before do
+        allow(Team).to receive(:new).and_return(team)
+        allow(Team).to receive(:with_similar_names).and_return([ team ])
+      end
+
+      it 'renders a template with a list of similar teams' do
+        post :create, :team => { name: 'team_name' }, format: :js
+        expect(response).to render_template('similar_teams')
+      end
+    end
+  end
+
+end
diff --git a/spec/helpers/teams_helper_spec.rb b/spec/helpers/teams_helper_spec.rb
@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+RSpec.describe TeamsHelper, :type => :helper do
+  describe "#exact_team_exists?" do
+    let(:teams) { Fabricate.build_times(3, :team) }
+
+    it "returns true if there is a team with exact matching name" do
+      teams << Fabricate.build(:team, name: 'test_name', slug: 'test-name')
+      expect(helper.exact_team_exists?(teams, 'test-name')).to be_truthy
+    end
+
+    it "returns false if there is no team with exact mathcing name" do
+      expect(helper.exact_team_exists?(teams, 'non-existent team name')).to be_falsey
+    end
+
+
+  end
+end
diff --git a/spec/models/team_spec.rb b/spec/models/team_spec.rb
@@ -4,6 +4,22 @@
   let(:team) { Fabricate(:team) }
   let(:invitee) { Fabricate(:user) }
 
+  describe '#with_similar_names' do
+    let!(:team_1) { Fabricate(:team, name: 'dream_team') }
+    let!(:team_2) { Fabricate(:team, name: 'dream_group') }
+    let!(:team_3) { Fabricate(:team, name: 'test_team') }
+
+    it 'returns teams with similar names' do
+      result = Team.with_similar_names('team')
+      expect(result.count).to eq 2
+    end
+
+    it 'returns teams using wildcards' do
+      result = Team.with_similar_names('dr -.')
+      expect(result).to include(team_1, team_2)
+    end
+  end
+
   it 'adds the team id to the user when they are added to a team' do
     team.add_user(invitee)
     expect(invitee.reload.team).to eq(team)

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+$('section.feature .intro .results').html('<%= escape_javascript(render :partial => 'similar_teams') %>');`
	`2`	`+$('section.feature:not(.payment)').addClass('find-team');`