Refactor complexity of code

Signed-off-by: Gabriela Navarro <navarro1703@gmail.com> Signed-off-by: Luciano Prestes <lucianopcbr@gmail.com>

Refactor complexity of code
Signed-off-by: Gabriela Navarro <navarro1703@gmail.com> Signed-off-by: Luciano Prestes <lucianopcbr@gmail.com>
Luciano Prestes · David Silva
1 parent 342ae8df
Showing 8 changed files with 72 additions and 70 deletions Show diff stats
controllers/mpog_software_plugin_controller.rb
controllers/mpog_software_plugin_myprofile_controller.rb
features/step_definitions/mpog_steps.rb
lib/ext/communities_block.rb
lib/ext/person.rb
lib/ext/user.rb
lib/juridical_nature.rb
lib/mpog_software_plugin.rb
@@ -113,17 +113,12 @@ class MpogSoftwarePluginController &lt; ApplicationController
     condition = !request.xhr? || params[:query].nil? || params[:field].nil?
     return render :json=>{} if condition
  
-    model = case params[:field]
-            when "software_language"
-              ProgrammingLanguage
-            else
-              DatabaseDescription
-            end
+    model = get_model_by_params_field
  
     data = model.where("name ILIKE ?", "%#{params[:query]}%").select("id, name")
-    data = data.collect { |db|
-      {:id=>db.id, :label=>db.name}
-    }
+                .collect { |db|
+                  {:id=>db.id, :label=>db.name}
+                }
  
     other = [model.select("id, name").last].collect { |db|
       {:id=>db.id, :label=>db.name}
@@ -236,4 +231,13 @@ class MpogSoftwarePluginController &lt; ApplicationController
       redirect_to :controller => "mpog_software_plugin", :action => "create_institution_admin"
     end
   end
+
+  def get_model_by_params_field
+    case params[:field]
+    when "software_language"
+      return ProgrammingLanguage
+    else
+      return DatabaseDescription
+    end
+  end
 end
@@ -15,7 +15,7 @@ class MpogSoftwarePluginMyprofileController &lt; MyProfileController
  
   def new_software
     set_software_as_template
-    
+
     @community = Community.new(params[:community])
     @community.environment = environment
     @software_info = SoftwareInfo.new(params[:software_info])
@@ -55,6 +55,7 @@ class MpogSoftwarePluginMyprofileController &lt; MyProfileController
         session[:notice] = _('Software updated sucessefuly')
       end
     rescue ActiveRecord::RecordInvalid => invalid
+      session[:notice] = _('Could not update software')
     end
   end
  
@@ -116,9 +116,9 @@ Given /^the following softwares$/ do |table|
     software_info.community = Community.create(:name=>item[:name])
  
     software_info.acronym = item[:acronym] if item[:acronym]
-    software_info.acronym = item[:operating_platform] if item[:operating_platform]
-    software_info.acronym = item[:objectives] if item[:objectives]
-    software_info.acronym = item[:features] if item[:features]
+    software_info.operating_platform = item[:operating_platform] if item[:operating_platform]
+    software_info.objectives = item[:objectives] if item[:objectives]
+    software_info.features = item[:features] if item[:features]
     software_info.public_software = item[:public_software] == "true" if item[:public_software]
  
     if item[:software_language]
@@ -3,21 +3,13 @@ require_dependency &#39;communities_block&#39;
 class CommunitiesBlock
  
   def profile_list
-    result = nil
-    visible_profiles = profiles.visible.includes([:image,:domains,:preferred_domain,:environment])
-    if !prioritize_profiles_with_image
-      result = visible_profiles.all(:limit => get_limit, :order => 'profiles.updated_at DESC').sort_by{ rand }
-    elsif profiles.visible.with_image.count >= get_limit
-      result = visible_profiles.with_image.all(:limit => get_limit * 5, :order => 'profiles.updated_at DESC').sort_by{ rand }
-    else
-      result = visible_profiles.with_image.sort_by{ rand } + visible_profiles.without_image.all(:limit => get_limit * 5, :order => 'profiles.updated_at DESC').sort_by{ rand }
-    end
+    result = get_visible_profiles
  
     list_without_software_and_institution = []
  
-    visible_profiles.each do |p|
-			if p.class == Community and !p.software? and !p.institution?
-				list_without_software_and_institution << p
+    result.each do |profile|
+			if profile.class == Community && !profile.software? && !profile.institution?
+				list_without_software_and_institution << profile
 			end
     end
  
@@ -30,4 +22,28 @@ class CommunitiesBlock
     profile_list.count
   end
  
+  private
+
+  def get_visible_profiles
+    visible_profiles = profiles.visible.includes(
+      [:image,:domains,:preferred_domain,:environment]
+    )
+    if !prioritize_profiles_with_image
+      return visible_profiles.all(
+        :limit => get_limit,
+        :order => 'profiles.updated_at DESC'
+      ).sort_by {rand}
+    elsif profiles.visible.with_image.count >= get_limit
+      return visible_profiles.with_image.all(
+        :limit => get_limit * 5,
+        :order => 'profiles.updated_at DESC'
+      ).sort_by {rand}
+    else
+      return visible_profiles.with_image.sort_by {rand} +
+        visible_profiles.without_image.all(
+          :limit => get_limit * 5, :order => 'profiles.updated_at DESC'
+        ).sort_by {rand}
+    end
+  end
+
 end
 \ No newline at end of file
@@ -10,37 +10,6 @@ class Person
  
   delegate :login, :to => :user, :prefix => true
  
-  scope :search, lambda { |name="", state="", city="", email=""|
-    like_sql = ""
-    values = []
-
-    unless name.blank?
-      like_sql << "name ILIKE ? OR identifier ILIKE ? AND "
-      values << "%#{name}%" << "%#{name}%"
-    end
-
-    unless state.blank?
-      like_sql << "data ILIKE ? AND "
-      values << "%:state: %#{state}%"
-    end
-
-    unless city.blank?
-      like_sql << "data ILIKE ? AND "
-      values << "%:city: %#{city}%"
-    end
-
-    unless email.blank?
-      like_sql << "email ILIKE ? AND "
-      values << "%#{email}%"
-    end
-    like_sql = like_sql[0..like_sql.length-5]
-
-    {
-      :joins => :user,
-      :conditions=>[like_sql, *values]
-    }
-  }
-
   def institutions
     institutions = []
     unless self.user.institutions.nil?
@@ -2,6 +2,8 @@ require_dependency &#39;user&#39;
  
 class User
  
+  GOV_SUFFIX = /^.*@[gov.br|jus.br|leg.br|mp.br]+$/
+
   has_and_belongs_to_many :institutions
  
   validate :email_different_secondary?, :email_has_already_been_used?,
@@ -51,28 +53,38 @@ class User
   end
  
   def email_suffix_is_gov?
-    test = /^.*@[gov.br|jus.br|leg.br|mp.br]+$/
-    primary_email_has_gov_suffix = false
-    secondary_email_has_gov_suffix = false
+    check_gov_suffix_in_secondary_email
+    check_gov_email_have_institution
+  end
  
-    if !self.email.nil? and self.email.length > 0
-      primary_email_has_gov_suffix = true  if test.match(self.email)
-    end
+  private
  
-    unless primary_email_has_gov_suffix
-      if !self.secondary_email.nil? and self.secondary_email.length > 0
-        secondary_email_has_gov_suffix = !!test.match(self.secondary_email)
-      end
+  def valid_format?(value, string_format)
+    !value.nil? && value.length > 0 && !string_format.match(value).nil?
+  end
+
+  def check_gov_suffix_in_secondary_email
+    unless primary_email_has_gov_suffix?
       self.errors.add(
         :base,
         _("The governamental email must be the primary one.")
-      ) if secondary_email_has_gov_suffix
+      ) if secondary_email_has_gov_suffix?
     end
+  end
  
+  def check_gov_email_have_institution
     self.errors.add(
       :base,
        _("Institution is obligatory if user has a government email.")
-    ) if primary_email_has_gov_suffix and self.institutions.blank?
+    ) if primary_email_has_gov_suffix? && self.institutions.blank?
+  end
+
+  def primary_email_has_gov_suffix?
+    valid_format?(self.email, GOV_SUFFIX)
+  end
+
+  def secondary_email_has_gov_suffix?
+    valid_format?(self.secondary_email, GOV_SUFFIX)
   end
  
 end
 \ No newline at end of file
@@ -9,7 +9,7 @@ class JuridicalNature &lt; ActiveRecord::Base
   def public_institutions
     Institution.where(
       :type=>"PublicInstitution",
-      :governmental_power_id=>self.id
+      :juridical_nature_id=>self.id
     )
   end
 end
@@ -42,7 +42,7 @@ class MpogSoftwarePlugin &lt; Noosfero::Plugin
       single_hash_transactions.each do |model,transaction|
         call_model_transaction(model,transaction)
       end
-      
+
   end
  
   def profile_editor_controller_filters
...	...	@@ -113,17 +113,12 @@ class MpogSoftwarePluginController < ApplicationController
113	113	condition = !request.xhr? \|\| params[:query].nil? \|\| params[:field].nil?
114	114	return render :json=>{} if condition
115	115
116		- model = case params[:field]
117		- when "software_language"
118		- ProgrammingLanguage
119		- else
120		- DatabaseDescription
121		- end
	116	+ model = get_model_by_params_field
122	117
123	118	data = model.where("name ILIKE ?", "%#{params[:query]}%").select("id, name")
124		- data = data.collect { \|db\|
125		- {:id=>db.id, :label=>db.name}
126		- }
	119	+ .collect { \|db\|
	120	+ {:id=>db.id, :label=>db.name}
	121	+ }
127	122
128	123	other = [model.select("id, name").last].collect { \|db\|
129	124	{:id=>db.id, :label=>db.name}
...	...	@@ -236,4 +231,13 @@ class MpogSoftwarePluginController < ApplicationController
236	231	redirect_to :controller => "mpog_software_plugin", :action => "create_institution_admin"
237	232	end
238	233	end
	234	+
	235	+ def get_model_by_params_field
	236	+ case params[:field]
	237	+ when "software_language"
	238	+ return ProgrammingLanguage
	239	+ else
	240	+ return DatabaseDescription
	241	+ end
	242	+ end
239	243	end
...	...
...	...	@@ -15,7 +15,7 @@ class MpogSoftwarePluginMyprofileController < MyProfileController
15	15
16	16	def new_software
17	17	set_software_as_template
18		-
	18	+
19	19	@community = Community.new(params[:community])
20	20	@community.environment = environment
21	21	@software_info = SoftwareInfo.new(params[:software_info])
...	...	@@ -55,6 +55,7 @@ class MpogSoftwarePluginMyprofileController < MyProfileController
55	55	session[:notice] = _('Software updated sucessefuly')
56	56	end
57	57	rescue ActiveRecord::RecordInvalid => invalid
	58	+ session[:notice] = _('Could not update software')
58	59	end
59	60	end
60	61
...	...
...	...	@@ -116,9 +116,9 @@ Given /^the following softwares$/ do \|table\|
116	116	software_info.community = Community.create(:name=>item[:name])
117	117
118	118	software_info.acronym = item[:acronym] if item[:acronym]
119		- software_info.acronym = item[:operating_platform] if item[:operating_platform]
120		- software_info.acronym = item[:objectives] if item[:objectives]
121		- software_info.acronym = item[:features] if item[:features]
	119	+ software_info.operating_platform = item[:operating_platform] if item[:operating_platform]
	120	+ software_info.objectives = item[:objectives] if item[:objectives]
	121	+ software_info.features = item[:features] if item[:features]
122	122	software_info.public_software = item[:public_software] == "true" if item[:public_software]
123	123
124	124	if item[:software_language]
...	...
...	...	@@ -3,21 +3,13 @@ require_dependency 'communities_block'
3	3	class CommunitiesBlock
4	4
5	5	def profile_list
6		- result = nil
7		- visible_profiles = profiles.visible.includes([:image,:domains,:preferred_domain,:environment])
8		- if !prioritize_profiles_with_image
9		- result = visible_profiles.all(:limit => get_limit, :order => 'profiles.updated_at DESC').sort_by{ rand }
10		- elsif profiles.visible.with_image.count >= get_limit
11		- result = visible_profiles.with_image.all(:limit => get_limit * 5, :order => 'profiles.updated_at DESC').sort_by{ rand }
12		- else
13		- result = visible_profiles.with_image.sort_by{ rand } + visible_profiles.without_image.all(:limit => get_limit * 5, :order => 'profiles.updated_at DESC').sort_by{ rand }
14		- end
	6	+ result = get_visible_profiles
15	7
16	8	list_without_software_and_institution = []
17	9
18		- visible_profiles.each do \|p\|
19		- if p.class == Community and !p.software? and !p.institution?
20		- list_without_software_and_institution << p
	10	+ result.each do \|profile\|
	11	+ if profile.class == Community && !profile.software? && !profile.institution?
	12	+ list_without_software_and_institution << profile
21	13	end
22	14	end
23	15
...	...	@@ -30,4 +22,28 @@ class CommunitiesBlock
30	22	profile_list.count
31	23	end
32	24
	25	+ private
	26	+
	27	+ def get_visible_profiles
	28	+ visible_profiles = profiles.visible.includes(
	29	+ [:image,:domains,:preferred_domain,:environment]
	30	+ )
	31	+ if !prioritize_profiles_with_image
	32	+ return visible_profiles.all(
	33	+ :limit => get_limit,
	34	+ :order => 'profiles.updated_at DESC'
	35	+ ).sort_by {rand}
	36	+ elsif profiles.visible.with_image.count >= get_limit
	37	+ return visible_profiles.with_image.all(
	38	+ :limit => get_limit * 5,
	39	+ :order => 'profiles.updated_at DESC'
	40	+ ).sort_by {rand}
	41	+ else
	42	+ return visible_profiles.with_image.sort_by {rand} +
	43	+ visible_profiles.without_image.all(
	44	+ :limit => get_limit * 5, :order => 'profiles.updated_at DESC'
	45	+ ).sort_by {rand}
	46	+ end
	47	+ end
	48	+
33	49	end
34	50	\ No newline at end of file
...	...
...	...	@@ -10,37 +10,6 @@ class Person
10	10
11	11	delegate :login, :to => :user, :prefix => true
12	12
13		- scope :search, lambda { \|name="", state="", city="", email=""\|
14		- like_sql = ""
15		- values = []
16		-
17		- unless name.blank?
18		- like_sql << "name ILIKE ? OR identifier ILIKE ? AND "
19		- values << "%#{name}%" << "%#{name}%"
20		- end
21		-
22		- unless state.blank?
23		- like_sql << "data ILIKE ? AND "
24		- values << "%:state: %#{state}%"
25		- end
26		-
27		- unless city.blank?
28		- like_sql << "data ILIKE ? AND "
29		- values << "%:city: %#{city}%"
30		- end
31		-
32		- unless email.blank?
33		- like_sql << "email ILIKE ? AND "
34		- values << "%#{email}%"
35		- end
36		- like_sql = like_sql[0..like_sql.length-5]
37		-
38		- {
39		- :joins => :user,
40		- :conditions=>[like_sql, *values]
41		- }
42		- }
43		-
44	13	def institutions
45	14	institutions = []
46	15	unless self.user.institutions.nil?
...	...
...	...	@@ -2,6 +2,8 @@ require_dependency 'user'
2	2
3	3	class User
4	4
	5	+ GOV_SUFFIX = /^.*@[gov.br\|jus.br\|leg.br\|mp.br]+$/
	6	+
5	7	has_and_belongs_to_many :institutions
6	8
7	9	validate :email_different_secondary?, :email_has_already_been_used?,
...	...	@@ -51,28 +53,38 @@ class User
51	53	end
52	54
53	55	def email_suffix_is_gov?
54		- test = /^.*@[gov.br\|jus.br\|leg.br\|mp.br]+$/
55		- primary_email_has_gov_suffix = false
56		- secondary_email_has_gov_suffix = false
	56	+ check_gov_suffix_in_secondary_email
	57	+ check_gov_email_have_institution
	58	+ end
57	59
58		- if !self.email.nil? and self.email.length > 0
59		- primary_email_has_gov_suffix = true if test.match(self.email)
60		- end
	60	+ private
61	61
62		- unless primary_email_has_gov_suffix
63		- if !self.secondary_email.nil? and self.secondary_email.length > 0
64		- secondary_email_has_gov_suffix = !!test.match(self.secondary_email)
65		- end
	62	+ def valid_format?(value, string_format)
	63	+ !value.nil? && value.length > 0 && !string_format.match(value).nil?
	64	+ end
	65	+
	66	+ def check_gov_suffix_in_secondary_email
	67	+ unless primary_email_has_gov_suffix?
66	68	self.errors.add(
67	69	:base,
68	70	_("The governamental email must be the primary one.")
69		- ) if secondary_email_has_gov_suffix
	71	+ ) if secondary_email_has_gov_suffix?
70	72	end
	73	+ end
71	74
	75	+ def check_gov_email_have_institution
72	76	self.errors.add(
73	77	:base,
74	78	_("Institution is obligatory if user has a government email.")
75		- ) if primary_email_has_gov_suffix and self.institutions.blank?
	79	+ ) if primary_email_has_gov_suffix? && self.institutions.blank?
	80	+ end
	81	+
	82	+ def primary_email_has_gov_suffix?
	83	+ valid_format?(self.email, GOV_SUFFIX)
	84	+ end
	85	+
	86	+ def secondary_email_has_gov_suffix?
	87	+ valid_format?(self.secondary_email, GOV_SUFFIX)
76	88	end
77	89
78	90	end
79	91	\ No newline at end of file
...	...
...	...	@@ -9,7 +9,7 @@ class JuridicalNature < ActiveRecord::Base
9	9	def public_institutions
10	10	Institution.where(
11	11	:type=>"PublicInstitution",
12		- :governmental_power_id=>self.id
	12	+ :juridical_nature_id=>self.id
13	13	)
14	14	end
15	15	end
...	...
...	...	@@ -42,7 +42,7 @@ class MpogSoftwarePlugin < Noosfero::Plugin
42	42	single_hash_transactions.each do \|model,transaction\|
43	43	call_model_transaction(model,transaction)
44	44	end
45		-
	45	+
46	46	end
47	47
48	48	def profile_editor_controller_filters
...	...