Merge branch 'admin_public_software_fields_edit' into 'master'

Admin public software fields edit Only admin can edit if software is public - Fix software edit informations - Fix software communities: better error handling See merge request !76

Merge branch 'admin_public_software_fields_edit' into 'master'
Admin public software fields edit Only admin can edit if software is public - Fix software edit informations - Fix software communities: better error handling See merge request !76
Daniela Feitosa
2 parents f093099b dc4912df
Showing 3 changed files with 104 additions and 41 deletions Show diff stats
src/noosfero-spb/software_communities/controllers/software_communities_plugin_myprofile_controller.rb
src/noosfero-spb/software_communities/test/functional/software_communities_plugin_myprofile_controller_test.rb
src/noosfero-spb/software_communities/views/software_communities_plugin_myprofile/_public_software_info.html.erb
@@ -33,7 +33,8 @@ class SoftwareCommunitiesPluginMyprofileController &lt; MyProfileController
     software_info_insert_models.call(@list_operating_systems, 'operating_systems')
     begin
-      @software_info.save!
+      raise NotAdminException unless can_change_public_software?
+      @software_info.update_attributes!(params[:software])
       @community = @software_info.community
       @community.update_attributes!(params[:community])
@@ -44,18 +45,29 @@ class SoftwareCommunitiesPluginMyprofileController &lt; MyProfileController
         redirect_to :controller => 'profile_editor', :action => 'index'
         session[:notice] = _('Software updated successfully')
       end
-    rescue ActiveRecord::RecordInvalid => invalid
+    rescue NotAdminException, ActiveRecord::RecordInvalid => invalid
       update_new_software_errors
       session[:notice] = _('Could not update software')
     end
   end
-  def disabled_public_software_field
-    !environment.admins.include?(current_user.person)
-  end
-
   private
+  def can_change_public_software?
+    if !user.is_admin?(environment)
+      if params[:software][:public_software]
+        @software_info.errors.add(:public_software, _("You don't have permission to change public software status"))
+        return false
+      end
+
+      if params[:software].keys.any?{|key| ["e_ping","e_mag","icp_brasil","e_arq","intern"].include?(key)}
+        @software_info.errors.add(:base, _("You don't have permission to change public software attributes"))
+        return false
+      end
+    end
+    return true
+  end
+
   def add_software_erros
       @errors = []
       @errors |= @community.errors.full_messages if @community
@@ -81,14 +93,12 @@ class SoftwareCommunitiesPluginMyprofileController &lt; MyProfileController
   def constroy_software
     @software_info = @profile.software_info
-    params[:software][:public_software] ||= false unless @software_info.public_software?
-    @license = LicenseInfo.find(params[:license][:license_infos_id])
-    @software_info.license_info = @license
-    @software_info.update_attributes(params[:software])
-
     another_license_version = nil
     another_license_link = nil
     if params[:license]
+      @license = LicenseInfo.find(params[:license][:license_infos_id])
+      @software_info.license_info = @license
+
       another_license_version = params[:license][:version]
       another_license_link = params[:license][:link]
     end
@@ -143,7 +153,7 @@ class SoftwareCommunitiesPluginMyprofileController &lt; MyProfileController
     @list_databases = @software_info.software_databases
     @list_languages = @software_info.software_languages
     @list_operating_systems = @software_info.operating_systems
-    @disabled_public_software_field = disabled_public_software_field
+    @non_admin_status = 'disabled' unless user.is_admin?(environment)
     @license_version = @software_info.license_info.version
     @license_id = @software_info.license_info.id
@@ -184,7 +194,6 @@ class SoftwareCommunitiesPluginMyprofileController &lt; MyProfileController
       add_software_erros
     end
-
     @error_community_name = @community.errors.include?(:name) ? "highlight-error" : "" if @community
     @error_software_acronym = @software_info.errors.include?(:acronym) ? "highlight-error" : "" if @software_info
     @error_software_domain = @community.errors.include?(:identifier) ? "highlight-error" : "" if @community
@@ -192,3 +201,5 @@ class SoftwareCommunitiesPluginMyprofileController &lt; MyProfileController
     @error_software_license = @license_info.errors.include?(:version) ? "highlight-error" : "" if @license_info
   end
 end
+
+class NotAdminException < Exception; end
-require File.dirname(__FILE__) + '/../../../../test/test_helper'
-require File.dirname(__FILE__) + '/../helpers/software_test_helper'
-require(
-  File.dirname(__FILE__) +
-  '/../../controllers/software_communities_plugin_myprofile_controller'
+require 'test_helper'
+require_relative '../helpers/software_test_helper'
+require_relative(
+  '../../controllers/software_communities_plugin_myprofile_controller'
 )
 class SoftwareCommunitiesPluginMyprofileController; def rescue_action(e) raise e end;
@@ -93,7 +92,11 @@ class SoftwareCommunitiesPluginMyprofileControllerTest &lt; ActionController::TestC
     assert_equal SoftwareInfo.last.acronym, "test"
   end
-  should 'upgrade a generic software to a public software' do
+  should 'only admin upgrade a generic software to a public software' do
+    admin_person = create_user('admin').person
+    @environment.add_admin(admin_person)
+
+    login_as(admin_person.user_login)
     fields_software = software_fields
     fields = software_edit_specific_fields
@@ -103,15 +106,69 @@ class SoftwareCommunitiesPluginMyprofileControllerTest &lt; ActionController::TestC
     post(
       :edit_software,
       :profile => software.community.identifier,
-      :library => fields[0],
-      :language => fields[1],
-      :database => fields[2],
       :operating_system => fields[3],
       :software => fields[4],
-      :license => fields[5]
     )
-    assert_equal true, SoftwareInfo.last.public_software?
+    assert SoftwareInfo.last.public_software?
+  end
+
+  should 'not upgrade a generic software to a public software if user is not an admin' do
+    fields_software = software_fields
+    fields = software_edit_specific_fields
+
+    fields[4]['public_software'] = true
+    software = create_software fields_software
+
+    post(
+      :edit_software,
+      :profile => software.community.identifier,
+      :software => fields[4]
+    )
+
+    refute SoftwareInfo.last.public_software?
+  end
+
+  ["e_ping","e_mag","icp_brasil","e_arq","intern"].map do |attr|
+    define_method "test_should_#{attr}_not_be_changed_by_not_admin" do
+      fields_software = software_fields
+      fields = software_edit_specific_fields
+
+      fields[4][attr]=true
+
+      software = create_software fields_software
+
+      post(
+        :edit_software,
+        :profile => software.community.identifier,
+        :software => fields[4]
+      )
+
+      refute SoftwareInfo.last.send(attr)
+    end
+  end
+
+  ["e_ping","e_mag","icp_brasil","e_arq","intern"].map do |attr|
+    define_method "test_should_#{attr}_be_changed_by_admin" do
+      admin_person = create_user('admin').person
+      @environment.add_admin(admin_person)
+      login_as(admin_person.user_login)
+
+      fields_software = software_fields
+      fields = software_edit_specific_fields
+
+      fields[4][attr]=true
+
+      software = create_software fields_software
+
+      post(
+        :edit_software,
+        :profile => software.community.identifier,
+        :software => fields[4]
+      )
+
+      assert SoftwareInfo.last.send(attr)
+    end
   end
   should "create software_info with existing license_info" do
 <div id = "public_software">
-  <% if @disabled_public_software_field == true %>
-    <%= check_box_tag("software[public_software]", "true", @software_info.public_software?, :disabled => "disabled") %>
-    <%= label_tag _("Public Software"), _("Public software"), :class => "public_software_disabled" %>
-  <% else %>
-    <%= check_box_tag("software[public_software]", "true", @software_info.public_software?) %>
-    <%= label_tag _("Public Software"), _("Public software"), :class => "public_software_enabled" %>
-  <% end %>
+  <%= check_box_tag("software[public_software]", "true", @software_info.public_software?, :disabled => @non_admin_status) %>
+  <%= label_tag _("Public Software"), _("Public software") %>
   <div class="public-software-fields">
     <h4> <%= _("Public Software") %> </h4>
     <div class="formfieldline">
       <%= label_tag _("Adherent to e-PING ?") %>
       <%= label_tag "e_ping_true", "Yes" %>
-      <%= radio_button_tag("software[e_ping]", true, @software_info.e_ping)%>
+      <%= radio_button_tag("software[e_ping]", true, @software_info.e_ping, :disabled => @non_admin_status) %>
       <%= label_tag "e_ping_false", "No"%>
-      <%= radio_button_tag("software[e_ping]", false, !@software_info.e_ping)%>
+      <%= radio_button_tag("software[e_ping]", false, !@software_info.e_ping, :disabled => @non_admin_status) %>
     </div>
     <div class="formfieldline">
       <%= label_tag _("Adherent to e-MAG ?") %>
       <%= label_tag "e_mag_true", "Yes"%>
-      <%= radio_button_tag("software[e_mag]", true, @software_info.e_mag)%>
+      <%= radio_button_tag("software[e_mag]", true, @software_info.e_mag, :disabled => @non_admin_status) %>
       <%= label_tag "e_mag_false", "No"%>
-      <%= radio_button_tag("software[e_mag]", false, !@software_info.e_mag)%>
+      <%= radio_button_tag("software[e_mag]", false, !@software_info.e_mag, :disabled => @non_admin_status) %>
     </div>
     <div class="formfieldline">
       <%= label_tag _("Adherent to ICP-Brasil ?") %>
       <%= label_tag "icp_brasil_true", "Yes"%>
-      <%= radio_button_tag("software[icp_brasil]", true, @software_info.icp_brasil)%>
+      <%= radio_button_tag("software[icp_brasil]", true, @software_info.icp_brasil, :disabled => @non_admin_status) %>
       <%= label_tag "icp_brasil_false", "No"%>
-      <%= radio_button_tag("software[icp_brasil]", false, !@software_info.icp_brasil)%>
+      <%= radio_button_tag("software[icp_brasil]", false, !@software_info.icp_brasil, :disabled => @non_admin_status) %>
     </div>
     <div class="formfieldline">
       <%= label_tag _("Adherent to e-ARQ ?") %>
       <%= label_tag "e_arq_true", "Yes"%>
-      <%= radio_button_tag("software[e_arq]", true, @software_info.e_arq)%>
+      <%= radio_button_tag("software[e_arq]", true, @software_info.e_arq, :disabled => @non_admin_status) %>
       <%= label_tag "e_arq_false", "No"%>
-      <%= radio_button_tag("software[e_arq]", false, !@software_info.e_arq)%>
+      <%= radio_button_tag("software[e_arq]", false, !@software_info.e_arq, :disabled => @non_admin_status) %>
     </div>
     <div class="formfieldline">
       <%= label_tag _("Internacionalizable ?") %>
       <%= label_tag "intern_true", "Yes" %>
-      <%= radio_button_tag("software[intern]", true, @software_info.intern)%>
+      <%= radio_button_tag("software[intern]", true, @software_info.intern, :disabled => @non_admin_status) %>
       <%= label_tag "intern_false", "No"%>
-      <%= radio_button_tag("software[intern]", false, !@software_info.intern)%>
+      <%= radio_button_tag("software[intern]", false, !@software_info.intern, :disabled => @non_admin_status) %>
     </div>
   </div>
 </div>
1	-require File.dirname(__FILE__) + '/../../../../test/test_helper'
2	-require File.dirname(__FILE__) + '/../helpers/software_test_helper'
3	-require(
4	- File.dirname(__FILE__) +
5	- '/../../controllers/software_communities_plugin_myprofile_controller'	1	+require 'test_helper'
		2	+require_relative '../helpers/software_test_helper'
		3	+require_relative(
		4	+ '../../controllers/software_communities_plugin_myprofile_controller'
6	)	5	)
7		6
8	class SoftwareCommunitiesPluginMyprofileController; def rescue_action(e) raise e end;	7	class SoftwareCommunitiesPluginMyprofileController; def rescue_action(e) raise e end;
	@@ -93,7 +92,11 @@ class SoftwareCommunitiesPluginMyprofileControllerTest < ActionController::TestC		@@ -93,7 +92,11 @@ class SoftwareCommunitiesPluginMyprofileControllerTest < ActionController::TestC
93	assert_equal SoftwareInfo.last.acronym, "test"	92	assert_equal SoftwareInfo.last.acronym, "test"
94	end	93	end
95		94
96	- should 'upgrade a generic software to a public software' do	95	+ should 'only admin upgrade a generic software to a public software' do
		96	+ admin_person = create_user('admin').person
		97	+ @environment.add_admin(admin_person)
		98	+
		99	+ login_as(admin_person.user_login)
97	fields_software = software_fields	100	fields_software = software_fields
98	fields = software_edit_specific_fields	101	fields = software_edit_specific_fields
99		102
	@@ -103,15 +106,69 @@ class SoftwareCommunitiesPluginMyprofileControllerTest < ActionController::TestC		@@ -103,15 +106,69 @@ class SoftwareCommunitiesPluginMyprofileControllerTest < ActionController::TestC
103	post(	106	post(
104	:edit_software,	107	:edit_software,
105	:profile => software.community.identifier,	108	:profile => software.community.identifier,
106	- :library => fields[0],
107	- :language => fields[1],
108	- :database => fields[2],
109	:operating_system => fields[3],	109	:operating_system => fields[3],
110	:software => fields[4],	110	:software => fields[4],
111	- :license => fields[5]
112	)	111	)
113		112
114	- assert_equal true, SoftwareInfo.last.public_software?	113	+ assert SoftwareInfo.last.public_software?
		114	+ end
		115	+
		116	+ should 'not upgrade a generic software to a public software if user is not an admin' do
		117	+ fields_software = software_fields
		118	+ fields = software_edit_specific_fields
		119	+
		120	+ fields[4]['public_software'] = true
		121	+ software = create_software fields_software
		122	+
		123	+ post(
		124	+ :edit_software,
		125	+ :profile => software.community.identifier,
		126	+ :software => fields[4]
		127	+ )
		128	+
		129	+ refute SoftwareInfo.last.public_software?
		130	+ end
		131	+
		132	+ ["e_ping","e_mag","icp_brasil","e_arq","intern"].map do \|attr\|
		133	+ define_method "test_should_#{attr}_not_be_changed_by_not_admin" do
		134	+ fields_software = software_fields
		135	+ fields = software_edit_specific_fields
		136	+
		137	+ fields[4][attr]=true
		138	+
		139	+ software = create_software fields_software
		140	+
		141	+ post(
		142	+ :edit_software,
		143	+ :profile => software.community.identifier,
		144	+ :software => fields[4]
		145	+ )
		146	+
		147	+ refute SoftwareInfo.last.send(attr)
		148	+ end
		149	+ end
		150	+
		151	+ ["e_ping","e_mag","icp_brasil","e_arq","intern"].map do \|attr\|
		152	+ define_method "test_should_#{attr}_be_changed_by_admin" do
		153	+ admin_person = create_user('admin').person
		154	+ @environment.add_admin(admin_person)
		155	+ login_as(admin_person.user_login)
		156	+
		157	+ fields_software = software_fields
		158	+ fields = software_edit_specific_fields
		159	+
		160	+ fields[4][attr]=true
		161	+
		162	+ software = create_software fields_software
		163	+
		164	+ post(
		165	+ :edit_software,
		166	+ :profile => software.community.identifier,
		167	+ :software => fields[4]
		168	+ )
		169	+
		170	+ assert SoftwareInfo.last.send(attr)
		171	+ end
115	end	172	end
116		173
117	should "create software_info with existing license_info" do	174	should "create software_info with existing license_info" do