Merge pull request #3 from dbii/develop

Release 002

README.md

@@ -38,15 +38,15 @@ We use open-uri with Nokogiri for URL fetching and parsing
 
 ## Features
 
-3. Friend setup
-3. search
-2. Node walking
+## Robustness Concerns
 
-## Robustness
+1. Validation on Expert URL, existence, formatting, etc. Currently we just assume URL will be good, valid, and parsable.
+1. Expert Topics: There is minor cleanup of topics. Need to review common words and phrases and filter out high frequency non-relevant ones, or whatever handling needs to be done. Switch from word list to method for better handling.
+2. Expert Search: This is rudimentary by most standards. Postgres has built in full text search, but we thought that hid too much of the search algorithm (as per instructions). Ranked searching also is rudimentary, count up number of hits; but it could be enhanced maybe with a weight for the H1..H3 level in there. There are also engines like Elastisearch and Sphinx that handle the hard parts (and work with word stems, plurals, etc.) but those seemed to be prohibited by the instruction. All of which really says writing your own search from basic principals is "hard" to get it right, when you can leverage what the bigger players are doing.
+3. Expert Path: We wrote our own, but there is database recursion in postgres and other ways to do a better more complete node path. Depending on scale, a graph database for the relationships might be better.
+4. Friendship: We did a simple expert 1 to expert 2 model. In working on this we debated a two way model which has an expert and friend, and you would have to converse for your friend (a record I am an expert and you're my friend, and a record where you are the expert and I am the friend.). That solves some of the problem of searching, but introduces scaling issues of maintaining a consistent set of dual records for each edge between nodes, but it is worth considering.
 
-1. Validation on Expert URL, existence, formatting, etc.
-
-## Scalability
+## Scalability Issues
 
 1. Save Expert does an inline URL grab and parse
 1. Save Expert does an inline URL Shortening

app/controllers/experts_controller.rb

@@ -34,19 +34,20 @@ def friends
 
   def add_friend
     Friendship.create(expert_1_id: @expert.id, expert_2_id: params[:friend_id])
-    redirect_to @expert
+    redirect_to friends_expert_path(@expert)
   end
 
   def remove_friend
     friendship = Friendship.where(expert_1_id: @expert.id, expert_2_id: params[:friend_id]).or(Friendship.where(expert_2_id: @expert.id, expert_1_id: params[:friend_id])).first
     if friendship
       friendship.destroy
     end
-    redirect_to @expert
+    redirect_to friends_expert_path(@expert)
   end
 
   def search_experts
     @topic_experts = @expert.topic_experts(params[:q])
+    @search_term = params[:q]
   end
 
   private

app/helpers/experts_helper.rb

@@ -1,2 +1,9 @@
 module ExpertsHelper
+
+  def introduction_path(expert, target)
+    experts = expert.path_to_expert(target)
+    return "Sorry, no introduction path" unless experts
+    experts.map{|e| e.name}.join(" => ")
+  end
+
 end

app/models/expert.rb

@@ -10,39 +10,67 @@ class Expert < ApplicationRecord
   after_validation :generate_short_url
   after_create :get_topics
 
+  UNDESIRED_CONTENT = ['more', 'view', 'search', 'contents']
+
   def get_topics
     page = Nokogiri::HTML(open(url))
 
     page.css('h1').each do |element|
       content = element.content
-      next if content.blank?
+      next if content.blank? || UNDESIRED_CONTENT.include?(content.downcase)
       topics.create(tag: 'h1', content: content)
     end
     page.css('h2').each do |element|
       content = element.content
-      next if content.blank?
+      next if content.blank? || UNDESIRED_CONTENT.include?(content.downcase)
       topics.create(tag: 'h3', content: content)
     end
     page.css('h3').each do |element|
       content = element.content
-      next if content.blank?
+      next if content.blank? || UNDESIRED_CONTENT.include?(content.downcase)
       topics.create(tag: 'h3', content: content)
     end
   end
 
   def generate_short_url
-    bitly = Bitly.new(ENV["SHORTENER_USERNAME"],ENV["SHORTENER_API_KEY"])
-    response = bitly.shorten(url)
-    self.short_url = response.short_url
+    if url.blank?
+      errors.add :url, "can't be blank."
+    else
+      begin
+        bitly = Bitly.new(ENV["SHORTENER_USERNAME"],ENV["SHORTENER_API_KEY"])
+        response = bitly.shorten(url)
+        self.short_url = response.short_url
+      rescue BitlyError => e
+        errors.add :url, e.message
+      end
+    end
   end
 
   def friends
     Friendship.where(expert_1_id: id).map{|f| f.expert_2} + Friendship.where(expert_2_id: id).map{|f| f.expert_1}
   end
 
   def topic_experts(a_topic)
-    friend_ids = [11,12,13,1]
-    Topic.includes(:expert).where("content ilike ?", "%#{a_topic}%").where.not("experts.id": id).where.not("experts.id": friend_ids).map{|t| t.expert }
+    word_atoms = a_topic.split
+    friend_ids = Friendship.where(expert_1_id: id).map{|f| f.expert_2_id} + Friendship.where(expert_2_id: id).map{|f| f.expert_1_id}
+    matching_topics = Topic.includes(:expert).where.not("experts.id": id).where.not("experts.id": friend_ids)
+    word_atoms.each do |atom|
+      matching_topics = matching_topics.where("content ilike ?", "%#{atom}%")
+    end
+    matching_topics.map{|t| t.expert }.each_with_object(Hash.new(0)) {|expert, counts| counts[expert] += 1}
   end
 
+  # returns a path to the expert in the form of an array of experts, or false if the expert is unreachable
+  def path_to_expert(target, path=[])
+    path << self
+    return path if target == self
+    results = []
+    friends.each do |friend|
+      next if path.include?(friend)                     # been here before
+      new_path = friend.path_to_expert(target, path.clone)
+      results << new_path if new_path
+    end
+    return false if results.empty?                      # termination case
+    return results.sort_by(&:length).first              # winner winner chicken dinner
+  end
 end

app/models/friendship.rb

@@ -3,5 +3,9 @@ class Friendship < ApplicationRecord
   belongs_to :expert_2, :class_name => "Expert", :foreign_key => "expert_2_id"
 
   validates :expert_1_id, :expert_2_id, presence: true
+  validate  :no_identity_friendship
 
+  def no_identity_friendship
+    errors.add :expert_2, "can't be friends with themselves." if expert_1_id == expert_2_id
+  end
 end

app/views/experts/friends.html.erb

@@ -3,9 +3,9 @@
   <% @experts.each do |expert| %>
     <div>
       <% if @expert.friends.include?(expert) %>
-        <%= link_to "-", remove_friend_experts_path(@expert.id, expert.id), method: :delete %>
+        <%= link_to "-", remove_friend_experts_path(@expert.id, expert.id), method: :delete, class: 'btn btn-sm btn-primary' %>
       <% else %>
-        <%= link_to "+", add_friend_experts_path(@expert.id, expert.id), method: :post %>
+        <%= link_to "+", add_friend_experts_path(@expert.id, expert.id), method: :post, class: 'btn btn-sm btn-primary' %>
       <% end %>
       <%= expert.name %>
     </div>

app/views/experts/search_experts.html.erb

@@ -3,9 +3,26 @@
     <h1><%= @expert.name %></h1>
   </div>
 </div>
-<% @topic_experts.each do |te| %>
-  <%- if current_user %>
-    <%= link_to "+", add_friend_experts_path(@expert.id, te.id), method: :post, class: 'btn btn-sm btn-primary' %>
+<p>
+  <%= form_for(
+          @expert,
+          url: search_experts_expert_path,
+          method: :get,
+          ) do |f| %>
+    Search for another Skill: <%= text_field_tag :q %>
+    <%= f.submit 'Search', class: 'btn btn-sm btn-primary' %>
+  <% end %>
+</p>
+
+<% if @topic_experts.blank? %>
+  <p></p>Sorry, no experts were found for "<%= @search_term %>"</p>
+<% else %>
+  <p>Experts matching "<%= @search_term %>"</p>
+  <% @topic_experts.sort_by{|k,v| v }.reverse.each do |te, value| %>
+    <%- if current_user %>
+      <%= link_to "+", add_friend_experts_path(@expert.id, te.id), method: :post, class: 'btn btn-sm btn-primary' %>
+    <% end %>
+    <%= link_to te.name, te %> (Ranking: <%= value %>)<br/>
+    <%= introduction_path @expert, te %><br/>
   <% end %>
-  <%= te.name %><br/>
 <% end %>

app/views/experts/show.html.erb

@@ -8,7 +8,7 @@
   </div>
 </div>
 
-<p>
+<p>Find an expert with the skill you need:<br/>
   <%= form_for(
           @expert,
           url: search_experts_expert_path,
@@ -29,6 +29,9 @@
 
   <div class="col-md-6">
     <h2>Friends</h2>
+    <%- if current_user %>
+    <div><%= link_to "Manage Friends", friends_expert_path(@expert.id), class: 'btn btn-sm btn-primary' %></div>
+    <% end %>
 
     <% @expert.friends.each do |friend| %>
       <p>

app/views/pages/about.html.erb

@@ -1,2 +1,3 @@
 <h3>Experts</h3>
 <p>Welcome to the Experts Exchange</p>
+<p><%= link_to "Find experts", experts_path %> who will meet your needs!</p>

spec/factories/experts.rb

@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :expert do
-    name {"Jane Expert"}
+    sequence(:name)       { |n| "Jane Expert#{n}" }
     url {"https://en.wikipedia.org/wiki/Dog_training"}
   end
 end

spec/factories/friendships.rb

@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :friendship do
+    expert_1_id { 1 }
+    expert_2_id { 2 }
+  end
+end

spec/models/expert_spec.rb

@@ -18,5 +18,55 @@
       expect(expert).not_to be_valid
     end
 
+    it "should handle invalid urls" do
+      expert = build(:expert, url: 'wombat sox')
+      expect(expert).not_to be_valid
+    end
+  end
+
+  context "when finding a path to a friend" do
+
+    let!(:expert_1) { create :expert }
+    let!(:expert_2) { create :expert }
+    let!(:expert_3) { create :expert }
+    let!(:expert_4) { create :expert }
+
+    let!(:friendship_1_2) { create :friendship, expert_1_id: expert_1.id, expert_2_id: expert_2.id }
+    let!(:friendship_2_3) { create :friendship, expert_1_id: expert_2.id, expert_2_id: expert_3.id }
+    let!(:friendship_3_4) { create :friendship, expert_1_id: expert_3.id, expert_2_id: expert_4.id }
+
+    it "should find the identity path" do
+      expect(expert_1.path_to_expert(expert_1)).to eq([expert_1])
+    end
+
+    it "should find a one-step path" do
+      expect(expert_1.path_to_expert(expert_2)).to eq([expert_1, expert_2])
+    end
+
+    it "should find a two-step path" do
+      expect(expert_1.path_to_expert(expert_3)).to eq([expert_1, expert_2, expert_3])
+    end
+
+    it "should find a three-step path" do
+      expect(expert_1.path_to_expert(expert_4)).to eq([expert_1, expert_2, expert_3, expert_4])
+    end
+
+    context "with a cycle in friendships" do
+
+      let!(:friendship_3_1) { create :friendship, expert_1_id: expert_3.id, expert_2_id: expert_1.id }
+
+      it "should find the shortest path without getting lost in the circle" do
+        expect(expert_1.path_to_expert(expert_4)).to eq([expert_1, expert_3, expert_4])
+      end
+    end
+
+    context "with an unreachable expert" do
+
+      let!(:expert_5) { create :expert }
+
+      it "should not find a path" do
+        expect(expert_1.path_to_expert(expert_5)).to be_falsey
+      end
+    end
   end
 end

spec/models/friendship_spec.rb

@@ -0,0 +1,26 @@
+require 'rails_helper'
+
+RSpec.describe Friendship, type: :model do
+
+  let(:expert_1) { create :expert }
+  let(:expert_2) { create :expert }
+  let(:valid_attributes) { { expert_1_id: expert_1.id, expert_2_id: expert_2.id } }
+
+  context "when creating a friendship" do
+    it "is valid with expected valid attributes" do
+      expect(Friendship.new(valid_attributes)).to be_valid
+    end
+
+    it "requires expert 1" do
+      expect(Friendship.new(valid_attributes.merge(expert_1_id: nil))).not_to be_valid
+    end
+
+    it "requires expert 2" do
+      expect(Friendship.new(valid_attributes.merge(expert_2_id: nil))).not_to be_valid
+    end
+
+    it "doesn't allow experts to be friends with themselves" do
+      expect(Friendship.new(valid_attributes.merge(expert_2_id: expert_1.id))).not_to be_valid
+    end
+  end
+end