Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #11331: Created random generator to fetch nodes randomly as cards #11332

Closed
wants to merge 3 commits into from
Closed

Fixes #11331: Created random generator to fetch nodes randomly as cards #11332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ae260f
Fixes #11331: Created random generator to fetch nodes randomly as cards
KarishmaVanwari Aug 5, 2022
73ab354
fixes to sidebar html and get_recommendations unit test
jywarren Sep 5, 2022
1850bf3
Update test/unit/tag_test.rb
jywarren Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/controllers/wiki_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ def show
if [email protected]? # it's a place page!
@tags = @node.tags
@tags += [Tag.find_by(name: params[:id])] if Tag.find_by(name: params[:id])

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace detected.

tag1, tag2 = @node.normal_tags(:followers).includes(:tag).pluck(:name).first(2)
Copy link
Member

@jywarren jywarren Aug 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
tag1, tag2 = @node.normal_tags(:followers).includes(:tag).pluck(:name).first(2)
puts ">>>>>>>>>>>>>>>>>>>>>>>>>>>"
puts @node.inspect
puts ">>>>>>>>>>>>>>>>>>>>>>>>>>>"
tag1, tag2 = @node.normal_tags(:followers).includes(:tag).pluck(:name).first(2)


Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace detected.

# get recommendations
@recommendations = Tag.get_recommendations(tag1, tag2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @KarishmaVanwari, since you want the random cards to be displayed in the dashboard's sidebar shouldn't this be in home_controller?

plots2/app/controllers/home_controller.rb

Lines 39 to 59 in ca319fc

def dashboard_v2
@title = I18n.t('dashboard._header.dashboard')
# The new dashboard displays the blog and topics list
if current_user
@blog = Tag.find_nodes_by_type('blog', 'note', 1).limit(1).first
# Tags without the blog tag and everything tag to avoid double display
exclude_tids = Tag.where(name: %w(blog everything)).pluck(:tid)
@pagy, @tag_subscriptions = pagy(
TagSelection
.select('tag_selections.tid, term_data.name')
.where(user_id: current_user.id, following: true)
.where.not(tid: exclude_tids)
.joins("INNER JOIN term_data ON tag_selections.tid = term_data.tid")
.order("term_data.activity_timestamp DESC")
)
@trending_tags = trending
render template: 'dashboard_v2/dashboard'
else
redirect_to '/research'
end
end

Please let me know if I'm wrong or missing something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we put it in a reusable named methodin application_controller so it can be used in multiple places?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried placing this piece of code on line 54 in home_conroller but it gives the following error -
NoMethodError in HomeController#dashboard_v2
undefined method normal_tags for nil:NilClass .

@TildaDares @jywarren

else # it's a new wiki page!
@title = I18n.t('wiki_controller.new_wiki_page')
if current_user
Expand Down
30 changes: 30 additions & 0 deletions app/models/tag.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,4 +478,34 @@ def span(start, fin)
def range(fin, week)
(fin.to_i - week.weeks.to_i).to_s..(fin.to_i - (week - 1).weeks.to_i).to_s
end

def self.get_recommendations(tag1, tag2)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private (on line 464) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.


tag1_content_nids = find_recommended_nodes(tag1)
tag2_content_nids = find_recommended_nodes(tag2)

random_content_nids = tag1_content_nids.sample(3) + tag2_content_nids.sample(3)

Node.where("nid IN (?)", random_content_nids)
end

def self.find_recommended_nodes(tagnames, limit = 10)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private (on line 464) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.


date_ranges = [1.years.ago..3.years.ago, 4.years.ago..6.years.ago, 7.years.ago..9.years.ago]

selected_date_range = date_ranges.sample(1)

nodes = Node.where("cached_likes > 20 AND views > 100", status: 1)
.where(created: selected_date_range)
.includes(:tag)
.references(:term_data)
.where('term_data.name IN (?)', tagnames)

Node.where('node.nid IN (?)', nodes.collect(&:nid))
.includes(:revision, :tag)
.references(:node_revisions)
.where(status: 1)
.limit(limit)
.pluck(:nid)
end
end
14 changes: 13 additions & 1 deletion app/views/dashboard_v2/_sidebar.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,19 @@
</div>
</div>
</div>

<div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we move this into its own partial - to load with <%= render partial: "nodes/random" %> and the partial template would be _random.html.erb.

<% @recommendations ||= Tag.get_recommendations("blog", "three") %>
<% @recommendations.each do |node| %>
<div class="card recommendations" style="width: 18rem;">
<img class="card-img-top" src="#" alt="Card image cap">
<div class="card-body">
<h5 class="card-title"><%= node.nid %></h5>
<p class="card-text">Post content here</p>
<a href="#" class="btn btn-primary">Read more button here</a>
</div>
</div>
<% end %>
</div>

<style>

Expand Down
6 changes: 6 additions & 0 deletions test/unit/tag_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,4 +294,10 @@ def setup
assert lat.location_tag?
assert lon.location_tag?
end

test 'Tag.get_recommendations' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, here's the unit test. Let's ensure it returns something! We could even test how many nodes it returns, or something else. But most importantly that it returns nodes at all!

Copy link
Member

@jywarren jywarren Sep 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, see below where I made a further suggestion about the unit test: #11332 (comment) (it got collapsed because I accepted the change)

nodes = Tag.get_recommendations("blog", "test")
assert_not_nil nodes
jywarren marked this conversation as resolved.
Show resolved Hide resolved
assert nodes.length > 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! This assertion failed! So, looks like we have been getting an empty array back:

https://github.com/publiclab/plots2/runs/8217990567?check_suite_focus=true#step:5:85

TagTest#test_Tag.get_recommendations (23.30s)
        Expected false to be truthy.
        test/unit/tag_test.rb:301:in `block in <class:TagTest>'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, so now, we can try to fix that. Once we push a fix up as a commit to this PR, the test will start to pass again -- that's actually "Test Driven Development" -- we set up a specific enough test condition (or "assertion") that our test will pass once it's working properly. The test actually precedes the final solution!

end
end