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.

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

[Outreachy] Add All Posts Route /notes #8800

Merged
merged 20 commits into from
Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
4 changes: 2 additions & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ def set_sidebar(type = :generic, data = :all, args = {})
@maps = Tag.find_nodes_by_type(data, 'map', 20)
else # type is generic
# remove "classroom" postings; also switch to an EXCEPT operator in sql, see https://github.com/publiclab/plots2/issues/375
hidden_nids = Node.where(type: :note, status: 1).select { |n| n.has_a_tag('hidden:response') }.collect(&:nid)
hidden_nids = Node.hidden_response_node_ids
@notes = if params[:controller] == 'questions'
Node.questions
.joins(:revision)
else
Node.research_notes.joins(:revision).order('node.nid DESC').paginate(page: params[:page])
end
end
RuthNjeri marked this conversation as resolved.
Show resolved Hide resolved

@notes = @notes.where('node.nid != (?)', @node.nid) if @node
@notes = @notes.where('node_revisions.status = 1 AND node.nid NOT IN (?)', hidden_nids) unless hidden_nids.empty?
Expand Down
7 changes: 1 addition & 6 deletions app/controllers/home_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,7 @@ def research
def activity
blog = Tag.find_nodes_by_type('blog', 'note', 1).first
# remove "classroom" postings; also switch to an EXCEPT operator in sql, see https://github.com/publiclab/plots2/issues/375
hidden_nids = Node.joins(:node_tag)
.joins('LEFT OUTER JOIN term_data ON term_data.tid = community_tags.tid')
.select('node.*, term_data.*, community_tags.*')
.where(type: 'note', status: 1)
.where('term_data.name = (?)', 'hidden:response')
.collect(&:nid)
hidden_nids = Node.hidden_response_node_ids
notes = Node.where(type: 'note')
.where('node.nid NOT IN (?)', hidden_nids + [0]) # in case hidden_nids is empty
.order('nid DESC')
Expand Down
41 changes: 16 additions & 25 deletions app/controllers/notes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class NotesController < ApplicationController

def index
@title = I18n.t('notes_controller.research_notes')
set_sidebar
@pagy, @notes = pagy(published_notes.order('node.nid DESC'))
end

def tools
Expand Down Expand Up @@ -42,7 +42,7 @@ def shortlink
# display a revision, raw
def raw
response.headers['Content-Type'] = 'text/plain; charset=utf-8'
render plain: Node.find(params[:id]).latest.body
render plain: Node.find(params[:id]).latest&.body
end

def show
Expand Down Expand Up @@ -304,40 +304,23 @@ def author_topic
# notes with high # of likes
def liked
@title = I18n.t('notes_controller.highly_liked_research_notes')
@wikis = Node.limit(10)
.where(type: 'page', status: 1)
.order('nid DESC')

@notes = Node.research_notes
.where(status: 1)
.limit(20)
.order('cached_likes DESC')
@unpaginated = true
@pagy, @notes = pagy(published_notes.limit(100).order(cached_likes: :desc, nid: :desc))
render template: 'notes/index'
end

def recent
@title = I18n.t('notes_controller.recent_research_notes')
@wikis = Node.limit(10)
.where(type: 'page', status: 1)
.order('nid DESC')
@notes = Node.where(type: 'note', status: 1, created: Time.now.to_i - 1.weeks.to_i..Time.now.to_i)
.order('created DESC')
@unpaginated = true
@pagy, @notes = pagy(published_notes.where(created: Time.now.to_i - 1.weeks.to_i..Time.now.to_i)
.order('created DESC'))
render template: 'notes/index'
end

# notes with high # of views
def popular
@title = I18n.t('notes_controller.popular_research_notes')
@wikis = Node.limit(10)
.where(type: 'page', status: 1)
.order('nid DESC')
@notes = Node.research_notes
.limit(20)
.where(status: 1)
.order('views DESC')
@unpaginated = true
@pagy, @notes = pagy(published_notes
.limit(100)
.order(views: :desc, nid: :desc))
render template: 'notes/index'
end

Expand Down Expand Up @@ -461,4 +444,12 @@ def location

[latitude, longitude]
end

def published_notes
hidden_nids = Node.hidden_response_node_ids

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.

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.

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.

Node.research_notes
.where('node.status = 1')
.where.not(nid: hidden_nids)
end
end
16 changes: 12 additions & 4 deletions app/models/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,13 @@ def slug_from_path
path.split('/').last
end

def has_a_tag(name)
return tags.where(name: name).size.positive?
def self.hidden_response_node_ids
Node.joins(:node_tag)
.joins('LEFT OUTER JOIN term_data ON term_data.tid = community_tags.tid')
.select('node.nid, term_data.tid, term_data.name, community_tags.tid')
.where(type: 'note', status: 1)
.where('term_data.name = (?)', 'hidden:response')
.collect(&:nid)
end

before_save :set_changed_and_created
Expand Down Expand Up @@ -920,8 +925,11 @@ def self.research_notes
.group('node.nid')
.collect(&:nid)

Node.where(type: 'note')
.where('node.nid NOT IN (?)', nids)
# The nids variable contains nodes with the Tag name 'question'
# that should be removed from the query
# if the nids are empty, the query in the else block
# will not return the valid notes
Node.where(type: 'note').where.not(nid: nids)
end

def body_preview(length = 100)
Expand Down
8 changes: 0 additions & 8 deletions app/views/notes/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,5 @@
<%= render :partial => "notes/notes" %>

<hr />

</div>

RuthNjeri marked this conversation as resolved.
Show resolved Hide resolved
<% if params[:controller] == "tag" || params[:controller] == "search" %>
<%= render :partial => "sidebar/related" %>
<% elsif params[:action] == "author" || params[:action] == "author_topic" %>
<%= render :partial => "sidebar/author" %>
<% else %>
<%= render :partial => "sidebar/user" %>
<% end %>
Comment on lines -45 to -51
Copy link
Collaborator

@cesswairimu cesswairimu Dec 17, 2020

Choose a reason for hiding this comment

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

The removal of this section is the one causing the tests to fail. Also I don't think we want to remove this sidebar...it gives access to recent wiki edits ... @RuthNjeri you might also want to add back the set_sidebar under index in the controller. what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @cesswairimu 😄

The removal of that section was part of this discussion with @jywarren where we replace it with the sort dropdown for the notes...

I think a user will still be able to access the recent wiki edits via the wiki section that will be linked on the notes page, I am working on this here, therefore, access to the recent edits won't be completely lost, just changed to another page....

I think this can also be open to further discussion before the PR is merged..

Copy link
Collaborator

@cesswairimu cesswairimu Dec 17, 2020

Choose a reason for hiding this comment

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

Oh okay, my bad missed that, then I think we might need to rewrite that test or remove it

2 changes: 1 addition & 1 deletion app/views/users/rss.rss.builder
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ xml.rss :version => "2.0" do
xml.author node.author.name
xml.pubDate node.created_at.to_s(:rfc822)
xml.link "https://" + request.host.to_s + node.path
xml.description auto_link(node.latest.render_body, sanitize: false)
xml.description auto_link(node.latest&.render_body, sanitize: false)
xml.guid url_for only_path: false, controller: 'notes', action: 'show', id: node.nid
end
end
Expand Down
8 changes: 7 additions & 1 deletion test/fixtures/node_tags.yml
Original file line number Diff line number Diff line change
Expand Up @@ -267,4 +267,10 @@ question4:
tid: 35
uid: 2
nid: 37
date: <%= DateTime.now.to_i %>
date: <%= DateTime.now.to_i %>

hidden_response_node_tag:
tid: 36
uid: 2
nid: 39
date: <%= DateTime.now.to_i %>
12 changes: 12 additions & 0 deletions test/fixtures/nodes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,15 @@ comment_note: # for testing comments on notes
type: "note"
cached_likes: 0
slug: "note-please-don-t-comment-here"

hidden_response_note:
nid: 39
uid: 2
title: "Note tagged with hidden:response"
path: "/notes/admin/02-12-2020/note-tagged-with-hidden-response"
created: <%= DateTime.new(2020,2,12).to_i %>
changed: <%= DateTime.new(2020,2,12).to_i %>
status: 1
type: "note"
cached_likes: 0
slug: "note-tagged-with-hidden-response"
9 changes: 8 additions & 1 deletion test/fixtures/revisions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,11 @@ comment_note: # for testing comments on notes
body: "I beg you, please"
timestamp: <%= DateTime.new(2020,12,8).to_i %>
status: 1


hidden_response_note_revision: # for testing comments on notes
nid: 39
uid: 2
title: "Note tagged with hidden:response"
body: "This is a hidden note"
timestamp: <%= DateTime.new(2020,12,8).to_i %>
status: 1
8 changes: 6 additions & 2 deletions test/fixtures/tags.yml
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ sun_question:

# this tags node 37 as a question
# ultimately, this is for testing comments on that node.
question4:
question4:
tid: 35
name: question:general
name: question:general

hidden_response_tag:
tid: 36
name: hidden:response
11 changes: 4 additions & 7 deletions test/functional/notes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,9 @@ def teardown
assert_equal 4, node.status

get :index

assert_response :success
selector = css_select 'div.note'
assert_equal 28, selector.size
assert_select "div p", 'Pending approval by community moderators. Please be patient!'
assert_equal 16, selector.size
end

test 'first-timer moderated note (status=4) shown to moderator with notice and approval prompt in full view' do
Expand Down Expand Up @@ -397,8 +395,7 @@ def teardown

assert_response :success
selector = css_select 'div.note'
assert_equal 28, selector.size
assert_select 'a[data-test="spam"]','Spam'
assert_equal 16, selector.size
end

test 'post_note_error_no_title' do
Expand Down Expand Up @@ -692,7 +689,7 @@ def teardown
expected = [nodes(:one)]
questions = [nodes(:question)]
RuthNjeri marked this conversation as resolved.
Show resolved Hide resolved
assert (notes & expected).present?
assert (notes & questions).present?
assert !(notes & questions).present?
RuthNjeri marked this conversation as resolved.
Show resolved Hide resolved
end

test 'should list only research notes with status 1 in liked' do
Expand All @@ -705,7 +702,7 @@ def teardown
assert !(notes & questions).present?
end

test 'first note in /liked endpoint should be highest liked' do
test 'first note in /liked endpoint should be highest liked' do
get :liked
notes = assigns(:notes)
# gets highest liked note's number of likes
Expand Down
11 changes: 9 additions & 2 deletions test/unit/node_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ def setup
nodes(:draft), nodes(:post_test1), nodes(:post_test2),
nodes(:post_test3), nodes(:post_test4), nodes(:scraped_image), nodes(:search_trawling),
nodes(:purple_air_without_hyphen), nodes(:purple_air_with_hyphen),
nodes(:sun_note), nodes(:sunny_day_note), nodes(:comment_note)]
nodes(:sun_note), nodes(:sunny_day_note), nodes(:comment_note), nodes(:hidden_response_note)]
assert_equal expected, notes
end

Expand Down Expand Up @@ -508,7 +508,7 @@ def setup

# node.authors should be anyone who's written a revision for this node (a wiki, presumably)
test 'authors' do
authors = Node.last.authors
authors = Node.where(uid: 2, type: 'page').first.authors

assert authors
assert_equal 1, authors.length
Expand Down Expand Up @@ -586,4 +586,11 @@ def setup
assert nodes.include?(node1), "Should include note tagged with sun for sun*"
assert nodes.include?(node2), "Should include note tagged with sunny-day for sun*"
end

test 'for hidden_response_node_ids' do
node = nodes(:hidden_response_note)

hidden_nids = Node.hidden_response_node_ids
assert hidden_nids.include?(node.nid)
end
end