From 5a19f560dc87ed57044c408d451ab000e9514917 Mon Sep 17 00:00:00 2001 From: David Bluestein II Date: Fri, 31 Aug 2018 08:07:48 -0500 Subject: [PATCH 01/12] Updated README with our notes --- README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 61c53fb..d560099 100644 --- a/README.md +++ b/README.md @@ -42,11 +42,14 @@ We use open-uri with Nokogiri for URL fetching and parsing 3. search 2. Node walking -## Robustness +## Robustness Concerns -1. Validation on Expert URL, existence, formatting, etc. +1. Validation on Expert URL, existence, formatting, etc. Currently we just assume URL will be good, valid, and parsable. +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 would be good, count up number of hits and then sort by relevance, 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. -## Scalability +## Scalability Issues 1. Save Expert does an inline URL grab and parse 1. Save Expert does an inline URL Shortening From 935c1451d077fddffabe2ff388205b3574222689 Mon Sep 17 00:00:00 2001 From: David Bluestein II Date: Fri, 31 Aug 2018 08:37:28 -0500 Subject: [PATCH 02/12] Updated with better details --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index d560099..4a712bd 100644 --- a/README.md +++ b/README.md @@ -45,6 +45,7 @@ We use open-uri with Nokogiri for URL fetching and parsing ## Robustness Concerns 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 would be good, count up number of hits and then sort by relevance, 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. From 1b73439c789b7fd3250e79e93c8fc43eec37efdb Mon Sep 17 00:00:00 2001 From: David Bluestein II Date: Fri, 31 Aug 2018 08:38:41 -0500 Subject: [PATCH 03/12] Updated pages with some better navigation based on user feedback; updated results page with some headings and re-search again; minor topic filtering; better search algorithm --- app/controllers/experts_controller.rb | 1 + app/models/expert.rb | 17 ++++++++++++----- app/views/experts/search_experts.html.erb | 18 +++++++++++++++++- app/views/experts/show.html.erb | 2 +- app/views/pages/about.html.erb | 1 + 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/app/controllers/experts_controller.rb b/app/controllers/experts_controller.rb index 51311b5..80a3181 100644 --- a/app/controllers/experts_controller.rb +++ b/app/controllers/experts_controller.rb @@ -47,6 +47,7 @@ def remove_friend def search_experts @topic_experts = @expert.topic_experts(params[:q]) + @search_term = params[:q] end private diff --git a/app/models/expert.rb b/app/models/expert.rb index 5d6b771..eca37df 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -10,22 +10,24 @@ 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 @@ -41,8 +43,13 @@ def friends 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 } end end diff --git a/app/views/experts/search_experts.html.erb b/app/views/experts/search_experts.html.erb index 15bcc7d..6e85ee7 100644 --- a/app/views/experts/search_experts.html.erb +++ b/app/views/experts/search_experts.html.erb @@ -3,9 +3,25 @@

<%= @expert.name %>

+

+ <%= 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 %> +

+ +<% if @topic_experts.blank? %> + Sorry, no matches were found +<% else %> +

Experts matching "<%= @search_term %>"

<% @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' %> <% end %> - <%= te.name %>
+ <%= link_to te.name, te %>
+<% end %> <% end %> diff --git a/app/views/experts/show.html.erb b/app/views/experts/show.html.erb index 86da7ba..56529de 100644 --- a/app/views/experts/show.html.erb +++ b/app/views/experts/show.html.erb @@ -8,7 +8,7 @@ -

+

Find an expert with the skill you need:
<%= form_for( @expert, url: search_experts_expert_path, diff --git a/app/views/pages/about.html.erb b/app/views/pages/about.html.erb index 6f2eb99..6fa036b 100644 --- a/app/views/pages/about.html.erb +++ b/app/views/pages/about.html.erb @@ -1,2 +1,3 @@

Experts

Welcome to the Experts Exchange

+

<%= link_to "Find experts", experts_path %> who will meet your needs!

From 13aa4b3e5c6a7c2fa6239ccaf321a73af1d15a2c Mon Sep 17 00:00:00 2001 From: David Bluestein II Date: Fri, 31 Aug 2018 08:43:59 -0500 Subject: [PATCH 04/12] Added Manage friends link, fixed redirect --- app/controllers/experts_controller.rb | 4 ++-- app/views/experts/friends.html.erb | 4 ++-- app/views/experts/show.html.erb | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/experts_controller.rb b/app/controllers/experts_controller.rb index 80a3181..ebaf054 100644 --- a/app/controllers/experts_controller.rb +++ b/app/controllers/experts_controller.rb @@ -34,7 +34,7 @@ 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 @@ -42,7 +42,7 @@ def remove_friend if friendship friendship.destroy end - redirect_to @expert + redirect_to friends_expert_path(@expert) end def search_experts diff --git a/app/views/experts/friends.html.erb b/app/views/experts/friends.html.erb index 0d0ab75..2a85fff 100644 --- a/app/views/experts/friends.html.erb +++ b/app/views/experts/friends.html.erb @@ -3,9 +3,9 @@ <% @experts.each do |expert| %>
<% 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 %>
diff --git a/app/views/experts/show.html.erb b/app/views/experts/show.html.erb index 56529de..371e5b0 100644 --- a/app/views/experts/show.html.erb +++ b/app/views/experts/show.html.erb @@ -29,6 +29,9 @@

Friends

+ <%- if current_user %> +
<%= link_to "Manage Friends", friends_expert_path(@expert.id), class: 'btn btn-sm btn-primary' %>
+ <% end %> <% @expert.friends.each do |friend| %>

From 860e7430e7f5a76721168c49003692dd2d1026b9 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 14:05:25 -0500 Subject: [PATCH 05/12] Add a Friendship spec; handle Bitly errors --- app/models/expert.rb | 14 +++++++++++--- spec/factories/experts.rb | 2 +- spec/factories/friendships.rb | 6 ++++++ spec/models/expert_spec.rb | 4 ++++ spec/models/friendship_spec.rb | 22 ++++++++++++++++++++++ 5 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 spec/factories/friendships.rb create mode 100644 spec/models/friendship_spec.rb diff --git a/app/models/expert.rb b/app/models/expert.rb index eca37df..042339b 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -33,9 +33,17 @@ def get_topics 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 diff --git a/spec/factories/experts.rb b/spec/factories/experts.rb index a4bc514..6e82a19 100644 --- a/spec/factories/experts.rb +++ b/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 diff --git a/spec/factories/friendships.rb b/spec/factories/friendships.rb new file mode 100644 index 0000000..8536f37 --- /dev/null +++ b/spec/factories/friendships.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :friendship do + expert_1_id { 1 } + expert_2_id { 2 } + end +end diff --git a/spec/models/expert_spec.rb b/spec/models/expert_spec.rb index 42bee18..b20608d 100644 --- a/spec/models/expert_spec.rb +++ b/spec/models/expert_spec.rb @@ -18,5 +18,9 @@ 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 end diff --git a/spec/models/friendship_spec.rb b/spec/models/friendship_spec.rb new file mode 100644 index 0000000..f7a6f6b --- /dev/null +++ b/spec/models/friendship_spec.rb @@ -0,0 +1,22 @@ +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 + end +end From 0b7a463d3f8075c554f19c34d2523d99a7449d52 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 18:27:24 -0500 Subject: [PATCH 06/12] Return only one search result per expert --- app/models/expert.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/expert.rb b/app/models/expert.rb index 042339b..199cde7 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -57,7 +57,7 @@ def topic_experts(a_topic) word_atoms.each do |atom| matching_topics = matching_topics.where("content ilike ?", "%#{atom}%") end - matching_topics.map{|t| t.expert } + matching_topics.map{|t| t.expert }.uniq end end From ea8eae8468e90bf2588979c66449659c0e8d72fe Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 18:28:14 -0500 Subject: [PATCH 07/12] Don't allow self-friendship --- app/models/friendship.rb | 4 ++++ spec/models/friendship_spec.rb | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/app/models/friendship.rb b/app/models/friendship.rb index 7f11a6e..74ff599 100644 --- a/app/models/friendship.rb +++ b/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 diff --git a/spec/models/friendship_spec.rb b/spec/models/friendship_spec.rb index f7a6f6b..95567f9 100644 --- a/spec/models/friendship_spec.rb +++ b/spec/models/friendship_spec.rb @@ -18,5 +18,9 @@ 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 From 6636bc53063bfac61fd3522ea1da7a9dac0c6814 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 18:28:51 -0500 Subject: [PATCH 08/12] Find experts and list in search results --- app/helpers/experts_helper.rb | 7 ++++ app/models/expert.rb | 13 +++++++ app/views/experts/search_experts.html.erb | 13 ++++--- spec/models/expert_spec.rb | 46 +++++++++++++++++++++++ 4 files changed, 73 insertions(+), 6 deletions(-) diff --git a/app/helpers/experts_helper.rb b/app/helpers/experts_helper.rb index 5b1d260..c6dad17 100644 --- a/app/helpers/experts_helper.rb +++ b/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 diff --git a/app/models/expert.rb b/app/models/expert.rb index 199cde7..50d349f 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -60,4 +60,17 @@ def topic_experts(a_topic) matching_topics.map{|t| t.expert }.uniq 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) + results << new_path if new_path + end + return false if results.empty? # termination case + return results.sort_by(&:length).reverse.first # winner winner chicken dinner + end end diff --git a/app/views/experts/search_experts.html.erb b/app/views/experts/search_experts.html.erb index 6e85ee7..09339c8 100644 --- a/app/views/experts/search_experts.html.erb +++ b/app/views/experts/search_experts.html.erb @@ -17,11 +17,12 @@ <% if @topic_experts.blank? %> Sorry, no matches were found <% else %> -

Experts matching "<%= @search_term %>"

-<% @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' %> +

Experts matching "<%= @search_term %>"

+ <% @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' %> + <% end %> + <%= link_to te.name, te %>
+ <%= introduction_path @expert, te %> <% end %> - <%= link_to te.name, te %>
-<% end %> <% end %> diff --git a/spec/models/expert_spec.rb b/spec/models/expert_spec.rb index b20608d..d42ebb0 100644 --- a/spec/models/expert_spec.rb +++ b/spec/models/expert_spec.rb @@ -23,4 +23,50 @@ 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 a three-step path without getting lost in the circle" do + expect(expert_1.path_to_expert(expert_4)).to eq([expert_1, expert_2, 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 From 562cfefe97603067a4380ce9889d8b529fecc93e Mon Sep 17 00:00:00 2001 From: David Bluestein II Date: Fri, 31 Aug 2018 20:22:20 -0500 Subject: [PATCH 09/12] Updated README, put in code to replace .uniq, better no match error --- README.md | 2 +- app/models/expert.rb | 2 +- app/views/experts/search_experts.html.erb | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 4a712bd..759a367 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ We use open-uri with Nokogiri for URL fetching and parsing 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 would be good, count up number of hits and then sort by relevance, 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. +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. diff --git a/app/models/expert.rb b/app/models/expert.rb index 50d349f..2cc8ff9 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -57,7 +57,7 @@ def topic_experts(a_topic) word_atoms.each do |atom| matching_topics = matching_topics.where("content ilike ?", "%#{atom}%") end - matching_topics.map{|t| t.expert }.uniq + 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 diff --git a/app/views/experts/search_experts.html.erb b/app/views/experts/search_experts.html.erb index 09339c8..e84152e 100644 --- a/app/views/experts/search_experts.html.erb +++ b/app/views/experts/search_experts.html.erb @@ -15,14 +15,14 @@

<% if @topic_experts.blank? %> - Sorry, no matches were found +

Sorry, no experts were found for "<%= @search_term %>"

<% else %>

Experts matching "<%= @search_term %>"

- <% @topic_experts.each do |te| %> + <% @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 %>
- <%= introduction_path @expert, te %> + <%= link_to te.name, te %> (Ranking: <%= value %>)
+ <%= introduction_path @expert, te %>
<% end %> <% end %> From 75113c999527604caf3afaa4013b77cf048ea26a Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 21:27:48 -0500 Subject: [PATCH 10/12] Fix a bug preventing tree walker from finding the shortest path --- app/models/expert.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/expert.rb b/app/models/expert.rb index 2cc8ff9..e59f047 100644 --- a/app/models/expert.rb +++ b/app/models/expert.rb @@ -67,10 +67,10 @@ def path_to_expert(target, path=[]) results = [] friends.each do |friend| next if path.include?(friend) # been here before - new_path = friend.path_to_expert(target, path) + 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).reverse.first # winner winner chicken dinner + return results.sort_by(&:length).first # winner winner chicken dinner end end From 00d6752e62ed3787f9812a8d77d47c0c9ddf2abc Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 21:28:04 -0500 Subject: [PATCH 11/12] Remove completed TODOs --- README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/README.md b/README.md index 759a367..2bb64e1 100644 --- a/README.md +++ b/README.md @@ -38,10 +38,6 @@ We use open-uri with Nokogiri for URL fetching and parsing ## Features -3. Friend setup -3. search -2. Node walking - ## Robustness Concerns 1. Validation on Expert URL, existence, formatting, etc. Currently we just assume URL will be good, valid, and parsable. From 71706456ef355a80c5d42260f2b8a51935f10b26 Mon Sep 17 00:00:00 2001 From: Steven Rogers Date: Fri, 31 Aug 2018 21:32:07 -0500 Subject: [PATCH 12/12] spec shortest path --- spec/models/expert_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/models/expert_spec.rb b/spec/models/expert_spec.rb index d42ebb0..1eff6d1 100644 --- a/spec/models/expert_spec.rb +++ b/spec/models/expert_spec.rb @@ -55,8 +55,8 @@ let!(:friendship_3_1) { create :friendship, expert_1_id: expert_3.id, expert_2_id: expert_1.id } - it "should find a three-step path without getting lost in the circle" do - expect(expert_1.path_to_expert(expert_4)).to eq([expert_1, expert_2, expert_3, expert_4]) + 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