Commit 557e1fd164133fbf19568a92ea512a985766ec02
Committed by
Thiago Ribeiro
1 parent
126ccd93
Exists in
master
and in
53 other branches
Only admin can edit if software is public
- Fix software edit informations - Fix software communities: better error handling Signed-off-by: Daniela Soares Feitosa <danielafeitosa@colivre.coop.br> Signed-off-by: Marcos Ronaldo <marcos.rpj2@gmail.com> Signed-off-by: Simiao Carvalho <simiaosimis@gmail.com> Signed-off-by: Luciano Prestes Cavalcanti <lucianopcbr@gmail.com> (cherry picked from commit dc4912dfaf4d51d47ddc88967f5b6965b5221b5f)
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
| ... | ... | @@ -33,7 +33,8 @@ class SoftwareCommunitiesPluginMyprofileController < MyProfileController |
| 33 | 33 | software_info_insert_models.call(@list_operating_systems, 'operating_systems') |
| 34 | 34 | |
| 35 | 35 | begin |
| 36 | - @software_info.save! | |
| 36 | + raise NotAdminException unless can_change_public_software? | |
| 37 | + @software_info.update_attributes!(params[:software]) | |
| 37 | 38 | |
| 38 | 39 | @community = @software_info.community |
| 39 | 40 | @community.update_attributes!(params[:community]) |
| ... | ... | @@ -44,18 +45,29 @@ class SoftwareCommunitiesPluginMyprofileController < MyProfileController |
| 44 | 45 | redirect_to :controller => 'profile_editor', :action => 'index' |
| 45 | 46 | session[:notice] = _('Software updated successfully') |
| 46 | 47 | end |
| 47 | - rescue ActiveRecord::RecordInvalid => invalid | |
| 48 | + rescue NotAdminException, ActiveRecord::RecordInvalid => invalid | |
| 48 | 49 | update_new_software_errors |
| 49 | 50 | session[:notice] = _('Could not update software') |
| 50 | 51 | end |
| 51 | 52 | end |
| 52 | 53 | |
| 53 | - def disabled_public_software_field | |
| 54 | - !environment.admins.include?(current_user.person) | |
| 55 | - end | |
| 56 | - | |
| 57 | 54 | private |
| 58 | 55 | |
| 56 | + def can_change_public_software? | |
| 57 | + if !user.is_admin?(environment) | |
| 58 | + if params[:software][:public_software] | |
| 59 | + @software_info.errors.add(:public_software, _("You don't have permission to change public software status")) | |
| 60 | + return false | |
| 61 | + end | |
| 62 | + | |
| 63 | + if params[:software].keys.any?{|key| ["e_ping","e_mag","icp_brasil","e_arq","intern"].include?(key)} | |
| 64 | + @software_info.errors.add(:base, _("You don't have permission to change public software attributes")) | |
| 65 | + return false | |
| 66 | + end | |
| 67 | + end | |
| 68 | + return true | |
| 69 | + end | |
| 70 | + | |
| 59 | 71 | def add_software_erros |
| 60 | 72 | @errors = [] |
| 61 | 73 | @errors |= @community.errors.full_messages if @community |
| ... | ... | @@ -81,14 +93,12 @@ class SoftwareCommunitiesPluginMyprofileController < MyProfileController |
| 81 | 93 | |
| 82 | 94 | def constroy_software |
| 83 | 95 | @software_info = @profile.software_info |
| 84 | - params[:software][:public_software] ||= false unless @software_info.public_software? | |
| 85 | - @license = LicenseInfo.find(params[:license][:license_infos_id]) | |
| 86 | - @software_info.license_info = @license | |
| 87 | - @software_info.update_attributes(params[:software]) | |
| 88 | - | |
| 89 | 96 | another_license_version = nil |
| 90 | 97 | another_license_link = nil |
| 91 | 98 | if params[:license] |
| 99 | + @license = LicenseInfo.find(params[:license][:license_infos_id]) | |
| 100 | + @software_info.license_info = @license | |
| 101 | + | |
| 92 | 102 | another_license_version = params[:license][:version] |
| 93 | 103 | another_license_link = params[:license][:link] |
| 94 | 104 | end |
| ... | ... | @@ -143,7 +153,7 @@ class SoftwareCommunitiesPluginMyprofileController < MyProfileController |
| 143 | 153 | @list_databases = @software_info.software_databases |
| 144 | 154 | @list_languages = @software_info.software_languages |
| 145 | 155 | @list_operating_systems = @software_info.operating_systems |
| 146 | - @disabled_public_software_field = disabled_public_software_field | |
| 156 | + @non_admin_status = 'disabled' unless user.is_admin?(environment) | |
| 147 | 157 | |
| 148 | 158 | @license_version = @software_info.license_info.version |
| 149 | 159 | @license_id = @software_info.license_info.id |
| ... | ... | @@ -184,7 +194,6 @@ class SoftwareCommunitiesPluginMyprofileController < MyProfileController |
| 184 | 194 | add_software_erros |
| 185 | 195 | end |
| 186 | 196 | |
| 187 | - | |
| 188 | 197 | @error_community_name = @community.errors.include?(:name) ? "highlight-error" : "" if @community |
| 189 | 198 | @error_software_acronym = @software_info.errors.include?(:acronym) ? "highlight-error" : "" if @software_info |
| 190 | 199 | @error_software_domain = @community.errors.include?(:identifier) ? "highlight-error" : "" if @community |
| ... | ... | @@ -192,3 +201,5 @@ class SoftwareCommunitiesPluginMyprofileController < MyProfileController |
| 192 | 201 | @error_software_license = @license_info.errors.include?(:version) ? "highlight-error" : "" if @license_info |
| 193 | 202 | end |
| 194 | 203 | end |
| 204 | + | |
| 205 | +class NotAdminException < Exception; end | ... | ... |
src/noosfero-spb/software_communities/test/functional/software_communities_plugin_myprofile_controller_test.rb
| 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 | 7 | class SoftwareCommunitiesPluginMyprofileController; def rescue_action(e) raise e end; |
| ... | ... | @@ -93,7 +92,11 @@ class SoftwareCommunitiesPluginMyprofileControllerTest < ActionController::TestC |
| 93 | 92 | assert_equal SoftwareInfo.last.acronym, "test" |
| 94 | 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 | 100 | fields_software = software_fields |
| 98 | 101 | fields = software_edit_specific_fields |
| 99 | 102 | |
| ... | ... | @@ -103,15 +106,69 @@ class SoftwareCommunitiesPluginMyprofileControllerTest < ActionController::TestC |
| 103 | 106 | post( |
| 104 | 107 | :edit_software, |
| 105 | 108 | :profile => software.community.identifier, |
| 106 | - :library => fields[0], | |
| 107 | - :language => fields[1], | |
| 108 | - :database => fields[2], | |
| 109 | 109 | :operating_system => fields[3], |
| 110 | 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 | 172 | end |
| 116 | 173 | |
| 117 | 174 | should "create software_info with existing license_info" do | ... | ... |
src/noosfero-spb/software_communities/views/software_communities_plugin_myprofile/_public_software_info.html.erb
| 1 | 1 | <div id = "public_software"> |
| 2 | - <% if @disabled_public_software_field == true %> | |
| 3 | - <%= check_box_tag("software[public_software]", "true", @software_info.public_software?, :disabled => "disabled") %> | |
| 4 | - <%= label_tag _("Public Software"), _("Public software"), :class => "public_software_disabled" %> | |
| 5 | - <% else %> | |
| 6 | - <%= check_box_tag("software[public_software]", "true", @software_info.public_software?) %> | |
| 7 | - <%= label_tag _("Public Software"), _("Public software"), :class => "public_software_enabled" %> | |
| 8 | - <% end %> | |
| 2 | + <%= check_box_tag("software[public_software]", "true", @software_info.public_software?, :disabled => @non_admin_status) %> | |
| 3 | + <%= label_tag _("Public Software"), _("Public software") %> | |
| 9 | 4 | <div class="public-software-fields"> |
| 10 | 5 | <h4> <%= _("Public Software") %> </h4> |
| 11 | 6 | <div class="formfieldline"> |
| 12 | 7 | <%= label_tag _("Adherent to e-PING ?") %> |
| 13 | 8 | |
| 14 | 9 | <%= label_tag "e_ping_true", "Yes" %> |
| 15 | - <%= radio_button_tag("software[e_ping]", true, @software_info.e_ping)%> | |
| 10 | + <%= radio_button_tag("software[e_ping]", true, @software_info.e_ping, :disabled => @non_admin_status) %> | |
| 16 | 11 | <%= label_tag "e_ping_false", "No"%> |
| 17 | - <%= radio_button_tag("software[e_ping]", false, !@software_info.e_ping)%> | |
| 12 | + <%= radio_button_tag("software[e_ping]", false, !@software_info.e_ping, :disabled => @non_admin_status) %> | |
| 18 | 13 | </div> |
| 19 | 14 | |
| 20 | 15 | <div class="formfieldline"> |
| 21 | 16 | <%= label_tag _("Adherent to e-MAG ?") %> |
| 22 | 17 | |
| 23 | 18 | <%= label_tag "e_mag_true", "Yes"%> |
| 24 | - <%= radio_button_tag("software[e_mag]", true, @software_info.e_mag)%> | |
| 19 | + <%= radio_button_tag("software[e_mag]", true, @software_info.e_mag, :disabled => @non_admin_status) %> | |
| 25 | 20 | <%= label_tag "e_mag_false", "No"%> |
| 26 | - <%= radio_button_tag("software[e_mag]", false, !@software_info.e_mag)%> | |
| 21 | + <%= radio_button_tag("software[e_mag]", false, !@software_info.e_mag, :disabled => @non_admin_status) %> | |
| 27 | 22 | </div> |
| 28 | 23 | |
| 29 | 24 | <div class="formfieldline"> |
| 30 | 25 | <%= label_tag _("Adherent to ICP-Brasil ?") %> |
| 31 | 26 | |
| 32 | 27 | <%= label_tag "icp_brasil_true", "Yes"%> |
| 33 | - <%= radio_button_tag("software[icp_brasil]", true, @software_info.icp_brasil)%> | |
| 28 | + <%= radio_button_tag("software[icp_brasil]", true, @software_info.icp_brasil, :disabled => @non_admin_status) %> | |
| 34 | 29 | <%= label_tag "icp_brasil_false", "No"%> |
| 35 | - <%= radio_button_tag("software[icp_brasil]", false, !@software_info.icp_brasil)%> | |
| 30 | + <%= radio_button_tag("software[icp_brasil]", false, !@software_info.icp_brasil, :disabled => @non_admin_status) %> | |
| 36 | 31 | </div> |
| 37 | 32 | |
| 38 | 33 | <div class="formfieldline"> |
| 39 | 34 | <%= label_tag _("Adherent to e-ARQ ?") %> |
| 40 | 35 | |
| 41 | 36 | <%= label_tag "e_arq_true", "Yes"%> |
| 42 | - <%= radio_button_tag("software[e_arq]", true, @software_info.e_arq)%> | |
| 37 | + <%= radio_button_tag("software[e_arq]", true, @software_info.e_arq, :disabled => @non_admin_status) %> | |
| 43 | 38 | <%= label_tag "e_arq_false", "No"%> |
| 44 | - <%= radio_button_tag("software[e_arq]", false, !@software_info.e_arq)%> | |
| 39 | + <%= radio_button_tag("software[e_arq]", false, !@software_info.e_arq, :disabled => @non_admin_status) %> | |
| 45 | 40 | </div> |
| 46 | 41 | |
| 47 | 42 | <div class="formfieldline"> |
| 48 | 43 | <%= label_tag _("Internacionalizable ?") %> |
| 49 | 44 | |
| 50 | 45 | <%= label_tag "intern_true", "Yes" %> |
| 51 | - <%= radio_button_tag("software[intern]", true, @software_info.intern)%> | |
| 46 | + <%= radio_button_tag("software[intern]", true, @software_info.intern, :disabled => @non_admin_status) %> | |
| 52 | 47 | <%= label_tag "intern_false", "No"%> |
| 53 | - <%= radio_button_tag("software[intern]", false, !@software_info.intern)%> | |
| 48 | + <%= radio_button_tag("software[intern]", false, !@software_info.intern, :disabled => @non_admin_status) %> | |
| 54 | 49 | </div> |
| 55 | 50 | </div> |
| 56 | 51 | </div> | ... | ... |