Merge pull request openSNP#19 from tsujigiri/known_phenotypes

Nice work, lots of unnecessary stuff is gone

app/controllers/phenotypes_controller.rb

@@ -39,10 +39,6 @@ def create
       redirect_to :action => "new"
     else
 
-      if @phenotype.known_phenotypes.include?(params[:user_phenotype][:variation]) == false
-        @phenotype.known_phenotypes << params[:user_phenotype][:variation]
-      end
-
       @phenotype.save
       @phenotype = Phenotype.find_by_characteristic(params[:phenotype][:characteristic])
       Resque.enqueue(Mailnewphenotype, @phenotype.id,current_user.id)

app/controllers/user_phenotypes_controller.rb

@@ -35,9 +35,6 @@ def create
 
   		@phenotype = Phenotype.find_by_id(params[:user_phenotype][:phenotype_id])
 
-  		if @phenotype.known_phenotypes.include?(params[:user_phenotype][:variation]) == false
-  		  @phenotype.known_phenotypes << params[:user_phenotype][:variation]
-  	  end
   		if @user_phenotype.save
 
   		    #check for new achievements

app/controllers/users_controller.rb

@@ -46,19 +46,19 @@ def index
     @title = "Listing all users"
 
     @result = []
-	  begin
-	    @users = User.find(:all)
-	    @users.each do |u|
-	      @user = {}
-	      @user["name"] = u.name
-	      @user["id"] = u.id
-	      @user["genotypes"] = []
-	      Genotype.find_all_by_user_id(u.id).each do |g|
-	        @genotype = {}
-	        @genotype["id"] = g.id
-	        @genotype["filetype"] = g.filetype
-	        @genotype["download_url"] = 'http://opensnp.org/data/' + g.fs_filename
-	        @user["genotypes"] << @genotype
+    begin
+      @users = User.find(:all)
+      @users.each do |u|
+        @user = {}
+        @user["name"] = u.name
+        @user["id"] = u.id
+        @user["genotypes"] = []
+        Genotype.find_all_by_user_id(u.id).each do |g|
+          @genotype = {}
+          @genotype["id"] = g.id
+          @genotype["filetype"] = g.filetype
+          @genotype["download_url"] = 'http://opensnp.org/data/' + g.fs_filename
+          @user["genotypes"] << @genotype
         end
       @result << @user
       end
@@ -149,22 +149,12 @@ def changepassword
   def update
     @user = User.find(params[:id])
 
-    # check whether the user has deleted phenotypes and change known_phenotypes
-    @pot_delete_phenotype_ids = []
-    if params[:user][:user_phenotypes_attributes] != nil
-
+    if params[:user][:user_phenotypes_attributes].present?
       params[:user][:user_phenotypes_attributes].each do |p|  
-        @phenotype = Phenotype.find(UserPhenotype.find(p[1]["id"]).phenotype_id)
-
+        @phenotype = UserPhenotype.find(p[1]["id"]).phenotype
         @old_variation = UserPhenotype.find_by_id(p[1]["id"]).variation
-        if UserPhenotype.find_all_by_phenotype_id_and_variation(@phenotype.id,@old_variation).length == 1
-          @phenotype.known_phenotypes.delete_if { |entry| entry == @old_variation }
-        end
-
-
-        @pot_delete_phenotype_ids << @phenotype.id
+        # TODO: known_phenotypes compare different now
         if @phenotype.known_phenotypes.include?(p[1]["variation"]) == false
-          @phenotype.known_phenotypes << p[1]["variation"]
           @phenotype.number_of_users = UserPhenotype.find_all_by_phenotype_id(@phenotype.id).length
           @phenotype.save
         end
@@ -175,16 +165,9 @@ def update
       @empty_websites = Homepage.find_all_by_user_id_and_url(current_user.id,"")
       @empty_websites.each do |ew| ew.delete end
 
-      if @pot_delete_phenotype_ids != []
-        @pot_delete_phenotype_ids.each do |pid|
-          if UserPhenotype.find_all_by_phenotype_id(pid).length == 0
-            Phenotype.delete(pid)
-          end
-        end
-      end
       Resque.enqueue(Recommendvariations)
-  	  Resque.enqueue(Recommendphenotypes)
-  	  
+      Resque.enqueue(Recommendphenotypes)
+      
       flash[:notice] =  "Successfully updated"
 
       if params[:user][:password] or params[:user][:avatar]
@@ -216,7 +199,7 @@ def check_to_create_phenotype(characteristic, variation, user_id)
     @phenotype = Phenotype.find_by_characteristic(characteristic)
     if @phenotype == nil
         # createphenotype if it doesn't exist
-        @phenotype = Phenotype.create(:characteristic => characteristic, :number_of_users => 1, :known_phenotypes => [variation])
+        @phenotype = Phenotype.create(:characteristic => characteristic, :number_of_users => 1)
     end
     @user_phenotype = UserPhenotype.find_by_phenotype_id(@phenotype.id)
     if @user_phenotype == nil
@@ -232,36 +215,13 @@ def destroy
     @user = User.find(params[:id])
     # delete the genotype(s)
     if @user.genotypes.length != 0
-        @user.genotypes.each do |ug|
-            Resque.enqueue(Deletegenotype, ug)
-            File.delete(::Rails.root.to_s+"/public/data/"+ ug.fs_filename)
-            ug.delete
-        end
-    end
-
-    @user.user_achievements.each do |ua|
-      UserAchievement.delete(ua)
-    end
-
-    @messages = Message.find_all_by_user_id(@user_id)
-
-    @messages.each do |mt|
-      Message.delete(mt)
-    end
-
-    @user.user_phenotypes.each do |up|
-      @phenotype = Phenotype.find_by_id(up.phenotype_id)
-
-      if UserPhenotype.find_all_by_phenotype_id_and_variation(@phenotype.id,up.variation).length == 1
-        @phenotype.known_phenotypes.delete_if { |entry| entry == up.variation }
-        @phenotype.save
-      end
-
-      if @phenotype.user_phenotypes.length == 1
-        Phenotype.delete(@phenotype)
+      @user.genotypes.each do |ug|
+        Resque.enqueue(Deletegenotype, ug)
+        File.delete(::Rails.root.to_s+"/public/data/"+ ug.fs_filename)
+        ug.delete
       end
-      UserPhenotype.delete(up)
     end
+
     flash[:notice] = "Thank you for using openSNP. Goodbye!"
     User.delete(@user)
     redirect_to root_url

app/models/phenotype.rb

@@ -1,18 +1,19 @@
 class Phenotype < ActiveRecord::Base
-   has_many :user_phenotypes
-   has_many :phenotype_comments
-   serialize :known_phenotypes
+  has_many :user_phenotypes, dependent: :destroy
+  has_many :phenotype_comments, dependent: :destroy
 
-   validates_presence_of :characteristic
+  validates_presence_of :characteristic
 
-   searchable do
-	   text :characteristic
-   end
-
-   after_create :default_array
-
-   def default_array
-     self.known_phenotypes ||= []
-   end
-
+  searchable do
+    text :characteristic
+  end
+
+  def known_phenotypes
+    @known_phenotypes ||=
+      user_phenotypes.map(&:variation).
+      map(&:downcase).
+      uniq.
+      map(&:camelize)
+  end
+
 end

app/models/user.rb

@@ -11,16 +11,16 @@ class User < ActiveRecord::Base
   #after_create :make_standard_phenotypes
 
   # dependent so stuff gets destroyed on delete
-  has_many :user_phenotypes, :dependent => :destroy
-  has_many :phenotypes, :through => :user_phenotypes
-  has_many :genotypes, :dependent => :destroy
+  has_many :user_phenotypes, dependent: :destroy
+  has_many :phenotypes, through: :user_phenotypes
+  has_many :genotypes, dependent: :destroy
   # user_snps needs some extra-logic to decrease the counters
-  has_many :user_snps, :dependent => :destroy
-  has_many :snps, :through => :user_snps
-  has_many :homepages, :dependent => :destroy
-  has_many :messages
-  has_many :user_achievements, :dependent => :destroy
-  has_many :achievements, :through => :user_achievements
+  has_many :user_snps, dependent: :destroy
+  has_many :snps, through: :user_snps
+  has_many :homepages, dependent: :destroy
+  has_many :messages, dependent: :destroy
+  has_many :user_achievements, dependent: :destroy
+  has_many :achievements, through: :user_achievements
   has_many :snp_comments
   has_many :phenotype_comments
 
@@ -53,7 +53,7 @@ def check_and_make_standard_phenotypes(charact)
        @phen_id = Phenotype.find_by_characteristic(charact).id
        UserPhenotype.create(:phenotype_id => @phen_id, :variation => '', :user_id => id)
      else
-       @phen_id = Phenotype.create(:characteristic => charact,:known_phenotypes => []).id
+       @phen_id = Phenotype.create(:characteristic => charact).id
           UserPhenotype.create(:phenotype_id => @phen_id, :variation => '', :user_id => id)
      end
    end

app/models/user_phenotype.rb

@@ -1,5 +1,5 @@
 class UserPhenotype < ActiveRecord::Base
-   belongs_to :phenotype
+   belongs_to :phenotype, dependent: :destroy
    belongs_to :user
    validates_presence_of :variation
 

test/factories.rb

@@ -28,7 +28,6 @@
 
 Factory.define :phenotype do |p|
   p.characteristic "Penis length"
-  p.known_phenotypes [ "foo", "bar" ]
 end
 
 Factory.define :user_phenotype do |up|

test/unit/phenotype_test.rb

@@ -0,0 +1,17 @@
+require_relative '../test_helper'
+
+class PhenotypeTest < ActiveSupport::TestCase
+  context "Phenotype" do
+    setup do
+      Sunspot.stubs(:index)
+      @phenotype = Factory(:phenotype)
+      @phenotype.instance_variable_set(:@known_phenotypes, nil)
+    end
+
+    should "know some phenotypes" do
+      @phenotype.user_phenotypes.create(variation: "Ping pong")
+      @phenotype.user_phenotypes.create(variation: "ping pong")
+      assert_equal ["Ping pong"], @phenotype.known_phenotypes
+    end
+  end
+end