diff --git a/app/controllers/manage_tags_controller.rb b/app/controllers/manage_tags_controller.rb
index cc65203..193c0ed 100644
--- a/app/controllers/manage_tags_controller.rb
+++ b/app/controllers/manage_tags_controller.rb
@@ -4,63 +4,65 @@ require 'extended_tag.rb'
 class ManageTagsController < ApplicationController
   # Index redirects to list action without modifing the url
   def index
-    list
-    render :action => 'list'
+    redirect_to :action => 'list'
   end
   
-  # Lists the tags strting with the top tags or with the chidren of @parent if its provided
+  # Lists the tags starting with the top tags or with the chidren of @parent if its provided
   def list
     @parent = Tag.find(params[:parent]) if params[:parent]
-    @tags = @parent ? @parent.children : Tag.find(:all).select{|t|!t.parent}
+    @tags = @parent ? @parent.children : Tag.roots
     @pending_tags = Tag.find_pendings
   end
 
-  # Prompt to data for a new tag
+  # Prompt for data to a new tag
   def new
-    @parent_tags = Tag.find_all
+    @parent_tags = Tag.find(:all)
     @tag = Tag.new
   end
 
   # Collects the data and creates a new tag with it
   def create
     @tag = Tag.new(params[:tag])
-    
     if @tag.save
       flash[:notice] = _('Tag was successfully created.')
       redirect_to :action => 'list'
     else
+      @parent_tags = Tag.find(:all)
       render :action => 'new'
     end
   end
 
   # Prompt for modifications on the attributes of a tag
   def edit
-    @tag = Tag.original_find(params[:id])
-    @parent_tags = Tag.find_all - @tag.descendents - [@tag]
+    @tag = Tag.find_with_pendings(params[:id])
+    @parent_tags = @tag.parents_candidates
   end
 
   # Do the modifications collected by edit
   def update
-    @tag = Tag.original_find(params[:id])
+    @tag = Tag.find_with_pendings(params[:id])
     if @tag.update_attributes(params[:tag])
       flash[:notice] = _('Tag was successfully updated.')
       redirect_to :action => 'list'
     else
+      @parent_tags = @tag.parents_candidates
       render :action => 'edit'
     end
   end
 
-  # Destroy a tag
+  # Destroy a tag and all its children
   def destroy
-    @tag = Tag.original_find(params[:id])
-    @tag.destroy
+    @tag = Tag.find_with_pendings(params[:id])
+    if @tag.destroy
+      flash[:notice] = _('Tag was successfuly destroyed')
+    end
     redirect_to :action => 'list'
   end
 
+  # Approve a pending tag so now ita can be used to tag things
   def approve
-    @tag = Tag.original_find(params[:id])
-    @tag.pending = false
-    if @tag.save
+    @tag = Tag.find_with_pendings(params[:id])
+    if @tag.update_attribute(:pending, false)
       flash[:notice] = _('Tag was successfuly approved')
       redirect_to :action => 'list'
     end
diff --git a/app/views/manage_tags/_form.rhtml b/app/views/manage_tags/_form.rhtml
index 44278db..3e36b02 100644
--- a/app/views/manage_tags/_form.rhtml
+++ b/app/views/manage_tags/_form.rhtml
@@ -1,3 +1,3 @@
 Name: <%= text_field 'tag', 'name' %> <br>
-Parent tag: <%= select_tag 'tag[parent_id]', ['<option value="0"></option>'] + @parent_tags.map {|pt|"<option value=\"#{pt.id}\" #{'selected="selected"' if @tag.parent == pt} >" + pt.name + '</option>'} %> <br>
+Parent tag: <%= select_tag 'tag[parent_id]', ['<option></option>'] + @parent_tags.map {|pt|"<option value=\"#{pt.id}\" #{'selected="selected"' if @tag.parent == pt} >" + pt.name + '</option>'} %> <br>
 Pending: <%= check_box 'tag', 'pending' %> <br>
diff --git a/app/views/manage_tags/edit.rhtml b/app/views/manage_tags/edit.rhtml
index 4cdb4ef..4bc8e04 100644
--- a/app/views/manage_tags/edit.rhtml
+++ b/app/views/manage_tags/edit.rhtml
@@ -1,7 +1,7 @@
 <h2> <%= _('Editing Tag') %> </h2>
 
-<%= start_form_tag :action => 'update', :id => @tag %>
+<% form_tag :action => 'update', :id => @tag do %>
   <%= render :partial => 'form' %>
   <%= submit_tag _('Update') %>
   <%= link_to _('Cancel'), {:action => 'list'} %>
-<%= end_form_tag %>
+<% end %>
diff --git a/app/views/manage_tags/new.rhtml b/app/views/manage_tags/new.rhtml
index 8444f4c..4271eff 100644
--- a/app/views/manage_tags/new.rhtml
+++ b/app/views/manage_tags/new.rhtml
@@ -1,7 +1,7 @@
 <h2> <%= _('New tag') %> </h2>
 
-<%= start_form_tag :action =>'create' %>
+<% form_tag :action =>'create' do%>
   <%= render :partial => 'form' %>
   <%= submit_tag _('Create') %>
   <%= link_to _('Cancel'), {:action => 'list'} %>
-<%= end_form_tag %>
+<% end %>
diff --git a/lib/extended_tag.rb b/lib/extended_tag.rb
index b0e57c4..626cd40 100644
--- a/lib/extended_tag.rb
+++ b/lib/extended_tag.rb
@@ -1,23 +1,32 @@
 class Tag
-  @@original_find = self.method(:find)
-  def self.original_find(*args)
+  @@original_find = self.method(:find) 
+  # Rename the find method to find_with_pendings that includes all tags in the search regardless if its pending or not 
+  def self.find_with_pendings(*args)
       @@original_find.call(*args)
   end
-
+  # Redefine the find method to exclude the pending tags from the search not allowing to tag something with an unapproved tag
   def self.find(*args)
     self.with_scope(:find => { :conditions => ['pending = ?', false] }) do
-       return self.original_find(*args)
+      self.find_with_pendings(*args)
     end
   end
 
+  # Return all the tags that were suggested but not yet approved
   def self.find_pendings
-    self.original_find(:all, :conditions => ['pending = ?', true])
+    self.find_with_pendings(:all, :conditions => ['pending = ?', true])
   end
 
+  # All the tags that can be a new parent for this tag, that is all but itself and its descendents to avoid loops
+  def parents_candidates
+    Tag.find(:all) - descendents - [self]
+  end
+  
+  # All tags that have this tag as its one of its ancestors
   def descendents
     children.to_a.sum([], &:descendents) + children 
   end
 
+  # Test if this tag has been approved already
   def aproved?
     not pending?
   end
diff --git a/test/functional/manage_tags_controller_test.rb b/test/functional/manage_tags_controller_test.rb
new file mode 100644
index 0000000..64a07b5
--- /dev/null
+++ b/test/functional/manage_tags_controller_test.rb
@@ -0,0 +1,102 @@
+require File.dirname(__FILE__) + '/../test_helper'
+require 'manage_tags_controller'
+
+# Re-raise errors caught by the controller.
+class ManageTagsController; def rescue_action(e) raise e end; end
+
+class ManageTagsControllerTest < Test::Unit::TestCase
+  fixtures :tags, :users, :blocks, :profiles, :virtual_communities, :boxes, :domains
+  def setup
+    @controller = ManageTagsController.new
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+  end
+
+  # Replace this with your real tests.
+  def test_get_index
+    get :index
+    assert_response :redirect
+    assert_redirected_to :action => 'list'
+  end
+
+  def test_list
+    get :list
+    assert_response :success
+    assert_template 'list'
+    assert_not_nil assigns(:tags)
+    assert_not_nil assigns(:pending_tags)
+    assert_nil assigns(:parent), 'the list should not scoped'
+  end
+
+  def test_scoped_list
+    assert_nothing_raised { Tag.find(1) }
+    get :list, :parent => Tag.find(1)
+    assert_response :success
+    assert_template 'list'
+    assert_not_nil assigns(:parent), 'the list should be scoped'
+    assert_not_nil assigns(:tags)
+    assert_not_nil assigns(:pending_tags)
+  end
+
+  def test_new
+    get :new
+    assert_response :success
+    assert_template 'new'
+    assert_not_nil assigns(:parent_tags)
+    assert_not_nil assigns(:tag)
+  end
+
+  def test_create
+    post :create, :tag => {:name => 'test_tag'}
+    assert_response :redirect
+    assert_redirected_to :action => 'list'
+    assert_not_nil assigns(:tag)
+  end
+
+  def test_create_wrong
+    post :create, :tag => {:name => ''}
+    assert_response :success
+    assert_template 'new'
+  end
+
+  def test_edit
+    assert_nothing_raised { Tag.find(1) }
+    get :edit, :id => 1
+    assert assigns(:tag)
+    assert assigns(:parent_tags)
+  end
+
+  def test_update
+    assert_nothing_raised { Tag.find(1) }
+    post :update, :id => 1, :tag => {:name => 'altered_tag'}
+    assert_response :redirect
+    assert_redirected_to :action => 'list'
+    assert assigns(:tag)
+  end
+
+  def test_update_wrong
+    assert_nothing_raised { Tag.find(1) }
+    post :update, :id => 1, :tag => {:name => ''}
+    assert_response :success
+    assert_template 'edit'
+    assert assigns(:parent_tags)
+  end
+
+  def test_destroy
+    assert_nothing_raised { Tag.find(1) }
+    post :destroy, :id => 1
+    assert_response :redirect
+    assert_redirected_to :action => 'list'
+    assert_not_nil flash[:notice]
+    assert_raise(ActiveRecord::RecordNotFound) { Tag.find(1) }
+  end
+
+  def test_approve
+    assert_nothing_raised { Tag.find_with_pendings(4) }
+    assert Tag.find_with_pendings(4).pending?
+    post :approve, :id => 4
+    assert_response :redirect
+    assert_redirected_to :action => 'list'
+    assert ( not Tag.find_with_pendings(4).pending? )
+  end
+end
--
libgit2 0.21.2