Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

Commit

Permalink
Don’t output “Untitled Note” to note meta tags
Browse files Browse the repository at this point in the history
+ Add tests and update tests that need it
  • Loading branch information
reefdog committed Nov 11, 2016
1 parent 233cd8d commit 9e969ff
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 13 deletions.
7 changes: 7 additions & 0 deletions app/models/annotation.rb
Expand Up @@ -118,6 +118,13 @@ def self.public_note_counts_by_organization
.count
end

# The interface prefills the note title with "Untitled Note", so that gets
# saved to the database as the note's title. In certain interfaces (e.g.,
# Twitter cards) we only want to surface titles that a user actually provided.
def user_provided_title
(title.blank? || title == 'Untitled Note') ? '' : title
end

def page
document.pages.find_by_page_number(page_number)
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/annotations/show.html.erb
Expand Up @@ -2,7 +2,7 @@
(@stylesheets_header ||= []).push File.join(DC.asset_root(:agnostic => true), "/note_embed/note_embed.css")
@sharing_meta_tags = {
author: "#{@current_document.account_name} (#{@current_document.organization_name})",
title: truncate(@current_annotation.title || "Note on #{@current_document.title}", length: 70, separator: ' '),
title: truncate((@current_annotation.user_provided_title.present? && @current_annotation.user_provided_title) || "Note on #{@current_document.title}", length: 70, separator: ' '),
url: @current_annotation.contextual_url,
description: truncate((@current_annotation.content.present? && @current_annotation.content) || @current_document.description || "Source document contributed to #{DC::CONFIG['platform_name']} by #{@current_document.account_name} (#{@current_document.organization_name}).", length: 200, separator: ' '),
image: @current_annotation.page.image_url,
Expand Down
7 changes: 7 additions & 0 deletions test/controllers/annotations_controller_test.rb
Expand Up @@ -58,6 +58,13 @@ class AnnotationsControllerTest < ActionController::TestCase
end
end

it "should be able to recognize notes with default titles" do
note = doc.annotations[1]
assert_equal 'Untitled Note', note.title
assert_equal '', note.user_provided_title
end


# Tests that don't matter in read-only mode
unless Rails.application.config.read_only

Expand Down
2 changes: 1 addition & 1 deletion test/controllers/documents_controller_test.rb
Expand Up @@ -158,7 +158,7 @@ def test_per_page_note_counts
login_account!
get :per_page_note_counts, :id=>doc.id
assert_response :success
assert_equal( { "2"=>1, "1"=>1 }, json_body )
assert_equal( { "2"=>2, "1"=>1 }, json_body )
end

def test_queue_length
Expand Down
11 changes: 9 additions & 2 deletions test/fixtures/annotations.yml
@@ -1,4 +1,3 @@

public:
document: tv_manual
access: <%= DC::Access::PUBLIC %>
Expand All @@ -8,6 +7,15 @@ public:
organization: tribune
content: I have greatly enjoyed reading this document

default_title:
document: tv_manual
access: <%= DC::Access::PUBLIC %>
title: Untitled Note
page_number: 2
account: louis
organization: tribune
content:

private:
document: tv_manual
access: <%= DC::Access::PRIVATE %>
Expand All @@ -16,4 +24,3 @@ private:
account: louis
organization: tribune
content: the contents of this note are so unbelievable that I cannot in good conscience include them

2 changes: 1 addition & 1 deletion test/lib/statistics_test.rb
Expand Up @@ -87,7 +87,7 @@ def test_by_the_numbers
assert_equal( {:total=>0, :day=>0, :week=>0, :month=>0, :half_year=>0} , nums["Reviewers"] )
assert_equal( {:total=>2, :day=>0, :week=>0, :month=>1, :half_year=>2} , nums["Documents"] )
assert_equal( {:total=>136, :day=>0, :week=>0, :month=>68, :half_year=>136} , nums["Pages"] )
assert_equal( {:total=>2, :day=>2, :week=>2, :month=>2, :half_year=>2} , nums["Notes"] )
assert_equal( {:total=>3, :day=>3, :week=>3, :month=>3, :half_year=>3} , nums["Notes"] )
end

def test_pages_per_minute
Expand Down
6 changes: 3 additions & 3 deletions test/models/annotation_test.rb
Expand Up @@ -18,8 +18,8 @@ class AnnotationTest < ActiveSupport::TestCase
assert Annotation.accessible( louis ).include?( annotations(:private) )
end

it "returns document counts" do
assert_equal 2, Annotation.counts_for_documents( louis, [doc] )[ doc.id ]
it "returns note counts" do
assert_equal 3, Annotation.counts_for_documents( louis, [doc] )[ doc.id ]
end

it "populates author info" do
Expand All @@ -29,7 +29,7 @@ class AnnotationTest < ActiveSupport::TestCase
end

it "counts public notes by organization" do
assert_equal 1, Annotation.public_note_counts_by_organization[ tribune.id ]
assert_equal 2, Annotation.public_note_counts_by_organization[ tribune.id ]
end

it "finds it's page" do
Expand Down
10 changes: 5 additions & 5 deletions test/models/document_test.rb
Expand Up @@ -62,13 +62,13 @@ def test_publishes_documents_once_due
end

def test_sets_annotation_counts
assert_equal 2, doc.annotations.count
assert_equal 3, doc.annotations.count
Document.populate_annotation_counts( louis, [doc] )
assert_equal 2, doc.annotation_count
assert_equal 3, doc.annotation_count

assert_equal 102, doc.public_note_count # from fixture, is wildly inaccurate
doc.reset_public_note_count
assert_equal 1, doc.public_note_count # much better
assert_equal 2, doc.public_note_count # much better
end

def test_strips_whitespace_from_title
Expand All @@ -84,15 +84,15 @@ def test_combines_text_from_pages
end

def test_calculates_annotations_per_page
assert_equal( { 2=>1, 1=>1 }, doc.per_page_annotation_counts )
assert_equal( { 2=>2, 1=>1 }, doc.per_page_annotation_counts )
end

def test_calculates_ordered_sections
assert_equal [sections(:first)], doc.ordered_sections
end

def test_calculates_ordered_annotations
assert_equal [annotations(:private),annotations(:public)], doc.ordered_annotations( louis )
assert_equal [annotations(:private),annotations(:public),annotations(:default_title)], doc.ordered_annotations( louis )
end

def test_populates_author_information_on_annotations
Expand Down

0 comments on commit 9e969ff

Please sign in to comment.