jro_apps/redmine Commit - r10767:7775f86a69f1

Code cleanup in AuthSource controller and views....

Jean-Philippe Lang -

r10767:7775f86a69f1

parent child

Context file:

r10767:7775f86a69f1

Collapse all files

app/controllers/auth_sources_controller.rb +10 -4

                menu_item :ldap_authentication
                before_filter :require_admin
+               before_filter :find_auth_source, :only => [:edit, :update, :test_connection, :destroy]
                def index
                  @auth_source_pages, @auth_sources = paginate AuthSource, :per_page => 10
                def new
                  klass_name = params[:type] || 'AuthSourceLdap'
                  @auth_source = AuthSource.new_subclass_instance(klass_name, params[:auth_source])
+                 render_404 unless @auth_source
                end
                def create
                end
                def edit
-                 @auth_source = AuthSource.find(params[:id])
                end
                def update
-                 @auth_source = AuthSource.find(params[:id])
                  if @auth_source.update_attributes(params[:auth_source])
                    flash[:notice] = l(:notice_successful_update)
                    redirect_to auth_sources_path
                end
                def test_connection
-                 @auth_source = AuthSource.find(params[:id])
                  begin
                    @auth_source.test_connection
                    flash[:notice] = l(:notice_successful_connection)
                end
                def destroy
-                 @auth_source = AuthSource.find(params[:id])
                  unless @auth_source.users.exists?
                    @auth_source.destroy
                    flash[:notice] = l(:notice_successful_delete)
                  end
                  redirect_to auth_sources_path
                end
+               private
+               def find_auth_source
+                 @auth_source = AuthSource.find(params[:id])
+               rescue ActiveRecord::RecordNotFound
+                 render_404
+               end
              end

app/views/auth_sources/_form.html.erb +3 -10

		@@ -1,13 +1,6
1	1	<%= error_messages_for 'auth_source' %>
2	2
3		<div class="box">
4		<!--[form:auth_source]-->
5		<p><label for="auth_source_name"><%=l(:field_name)%> <span class="required">*</span></label>
6		<%= text_field 'auth_source', 'name' %></p>
7
8		<p><label for="auth_source_onthefly_register"><%=l(:field_onthefly)%></label>
9		<%= check_box 'auth_source', 'onthefly_register' %></p>
	3	<div class="box tabular">
	4	<p><%= f.text_field :name, :required => true %></p>
	5	<p><%= f.check_box :onthefly_register, :label => :field_onthefly %></p>
10	6	</div>
11
12		<!--[eoform:auth_source]-->
13

app/views/auth_sources/_form_auth_source_ldap.html.erb +19 -45

		@@ -1,50 +1,24
1	1	<%= error_messages_for 'auth_source' %>
2	2
3		<div class="box">
4		<!--[form:auth_source]-->
5		<p><label for="auth_source_name"><%=l(:field_name)%> <span class="required">*</span></label>
6		<%= text_field 'auth_source', 'name' %></p>
7
8		<p><label for="auth_source_host"><%=l(:field_host)%> <span class="required">*</span></label>
9		<%= text_field 'auth_source', 'host' %></p>
10
11		<p><label for="auth_source_port"><%=l(:field_port)%> <span class="required">*</span></label>
12		<%= text_field 'auth_source', 'port', :size => 6 %> <%= check_box 'auth_source', 'tls' %> LDAPS</p>
13
14		<p><label for="auth_source_account"><%=l(:field_account)%></label>
15		<%= text_field 'auth_source', 'account' %></p>
16
17		<p><label for="auth_source_account_password"><%=l(:field_password)%></label>
18		<%= password_field 'auth_source', 'account_password', :name => 'ignore',
19		:value => ((@auth_source.new_record? \|\| @auth_source.account_password.blank?) ? '' : ('x'*15)),
20		:onfocus => "this.value=''; this.name='auth_source[account_password]';",
21		:onchange => "this.name='auth_source[account_password]';" %></p>
22
23		<p><label for="auth_source_base_dn"><%=l(:field_base_dn)%> <span class="required">*</span></label>
24		<%= text_field 'auth_source', 'base_dn', :size => 60 %></p>
25
26		<p><label for="auth_source_custom_filter"><%=l(:field_auth_source_ldap_filter)%></label>
27		<%= text_field 'auth_source', 'filter', :size => 60 %></p>
28
29		<p><label for="auth_source_timeout"><%=l(:field_timeout)%></label>
30		<%= text_field 'auth_source', 'timeout', :size => 4 %></p>
31
32		<p><label for="auth_source_onthefly_register"><%=l(:field_onthefly)%></label>
33		<%= check_box 'auth_source', 'onthefly_register' %></p>
	3	<div class="box tabular">
	4	<p><%= f.text_field :name, :required => true %></p>
	5	<p><%= f.text_field :host, :required => true %></p>
	6	<p><%= f.text_field :port, :required => true, :size => 6 %> <%= f.check_box :tls, :no_label => true %> LDAPS</p>
	7	<p><%= f.text_field :account %></p>
	8	<p><%= f.password_field :account_password, :label => :field_password,
	9	:name => 'dummy_password',
	10	:value => ((@auth_source.new_record? \|\| @auth_source.account_password.blank?) ? '' : ('x'*15)),
	11	:onfocus => "this.value=''; this.name='auth_source[account_password]';",
	12	:onchange => "this.name='auth_source[account_password]';" %></p>
	13	<p><%= f.text_field :base_dn, :required => true, :size => 60 %></p>
	14	<p><%= f.text_field :filter, :size => 60, :label => :field_auth_source_ldap_filter %></p>
	15	<p><%= f.text_field :timeout, :size => 4 %></p>
	16	<p><%= f.check_box :onthefly_register, :label => :field_onthefly %></p>
34	17	</div>
35	18
36		<fieldset class="box"><legend><%=l(:label_attribute_plural)%></legend>
37		<p><label for="auth_source_attr_login"><%=l(:field_login)%> <span class="required">*</span></label>
38		<%= text_field ~~'auth_source', 'attr_login'~~, :size => 20 %></p>
39
40		<p><label for="auth_source_attr_firstname"><%=l(:field_firstname)%></label>
41		<%= text_field 'auth_source', 'attr_firstname', :size => 20 %></p>
42
43		<p><label for="auth_source_attr_lastname"><%=l(:field_lastname)%></label>
44		<%= text_field 'auth_source', 'attr_lastname', :size => 20 %></p>
45
46		<p><label for="auth_source_attr_mail"><%=l(:field_mail)%></label>
47		<%= text_field 'auth_source', 'attr_mail', :size => 20 %></p>
	19	<fieldset class="box tabular"><legend><%=l(:label_attribute_plural)%></legend>
	20	<p><%= f.text_field :attr_login, :required => true, :size => 20 %></p>
	21	<p><%= f.text_field :attr_firstname, :size => 20 %></p>
	22	<p><%= f.text_field :attr_lastname, :size => 20 %></p>
	23	<p><%= f.text_field :attr_mail, :size => 20 %></p>
48	24	</fieldset>
49		<!--[eoform:auth_source]-->
50

app/views/auth_sources/edit.html.erb +2 -2

              <h2><%=l(:label_auth_source)%> (<%= h(@auth_source.auth_method_name) %>)</h2>
-             <%= form_tag({:action => 'update', :id => @auth_source}, :method => :put, :class => "tabular") do %>
-               <%= render :partial => auth_source_partial_name(@auth_source) %>
+             <%= form_for @auth_source, :as => :auth_source, :url => auth_source_path(@auth_source), :html => {:id => 'auth_source_form'} do |f| %>
+               <%= render :partial => auth_source_partial_name(@auth_source), :locals => { :f => f } %>
                <%= submit_tag l(:button_save) %>
              <% end %>

app/views/auth_sources/index.html.erb +1 -1

                  <td align="center"><%= h source.host %></td>
                  <td align="center"><%= h source.users.count %></td>
                  <td class="buttons">
-                   <%= link_to l(:button_test), {:action => 'test_connection', :id => source}, :class => 'icon icon-test' %>
+                   <%= link_to l(:button_test), try_connection_auth_source_path(source), :class => 'icon icon-test' %>
                    <%= delete_link auth_source_path(source) %>
                  </td>
                </tr>

app/views/auth_sources/new.html.erb +2 -2

              <h2><%=l(:label_auth_source_new)%> (<%= h(@auth_source.auth_method_name) %>)</h2>
-             <%= form_tag({:action => 'create'}, :class => "tabular") do %>
+             <%= labelled_form_for @auth_source, :as => :auth_source, :url => auth_sources_path, :html => {:id => 'auth_source_form'} do |f| %>
                <%= hidden_field_tag 'type', @auth_source.type %>
-               <%= render :partial => auth_source_partial_name(@auth_source) %>
+               <%= render :partial => auth_source_partial_name(@auth_source), :locals => { :f => f } %>
                <%= submit_tag l(:button_create) %>
              <% end %>

test/functional/auth_sources_controller_test.rb +29 -4

                  assert_equal AuthSourceLdap, source.class
                  assert source.new_record?
-                 assert_tag 'input', :attributes => {:name => 'type', :value => 'AuthSourceLdap'}
-                 assert_tag 'input', :attributes => {:name => 'auth_source[host]'}
+                 assert_select 'form#auth_source_form' do
+                   assert_select 'input[name=type][value=AuthSourceLdap]'
+                   assert_select 'input[name=?]', 'auth_source[host]'
+                 end
+               end
+               def test_new_with_invalid_type_should_respond_with_404
+                 get :new, :type => 'foo'
+                 assert_response 404
                end
                def test_create
                    assert_redirected_to '/auth_sources'
                  end
-                 source = AuthSourceLdap.first(:order => 'id DESC')
+                 source = AuthSourceLdap.order('id DESC').first
                  assert_equal 'Test', source.name
                  assert_equal '127.0.0.1', source.host
                  assert_equal 389, source.port
                  assert_response :success
                  assert_template 'edit'
-                 assert_tag 'input', :attributes => {:name => 'auth_source[host]'}
+                 assert_select 'form#auth_source_form' do
+                   assert_select 'input[name=?]', 'auth_source[host]'
+                 end
+               end
+               def test_edit_should_not_contain_password
+                 AuthSource.find(1).update_column :account_password, 'secret'
+                 get :edit, :id => 1
+                 assert_response :success
+                 assert_select 'input[value=secret]', 0
+                 assert_select 'input[name=dummy_password][value=?]', /x+/
+               end
+               def test_edit_invalid_should_respond_with_404
+                 get :edit, :id => 99
+                 assert_response 404
                end
                def test_update
                def test_destroy
                  assert_difference 'AuthSourceLdap.count', -1 do
                    delete :destroy, :id => 1
+                   assert_redirected_to '/auth_sources'
                  end
                end
                  assert_no_difference 'AuthSourceLdap.count' do
                    delete :destroy, :id => 1
+                   assert_redirected_to '/auth_sources'
                  end
                end

General Comments 0

Write
Preview

You need to be logged in to leave comments. Login now

No TODOs yet

	Site-wide shortcuts
/	Use quick search box
g h	Goto home page
g g	Goto my private gists page
g G	Goto my public gists page
g 0-9	Goto bookmarked items from 0-9
n r	New repository page
n g	New gist page

	Repositories
g s	Goto summary page
g c	Goto changelog page
g f	Goto files page
g F	Goto files page with file search activated
g p	Goto pull requests page
g o	Goto repository settings
g O	Goto repository access permissions settings
t s	Toggle sidebar on some pages