cst1/openstreetmap-website
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

not api endpoints, moved to button, fixed notifier message, fixed tests

Browse source
This commit is contained in:
Mikel Maron 2016-10-03 22:20:04 -04:00
parent 443080d7b0
commit 44b08cc35d
6 changed files with 72 additions and 86 deletions
Download patch file Download diff file Expand all files Collapse all files

10
app/controllers/diary_entry_controller.rb
View file

@ -81,21 +81,27 @@ class DiaryEntryController < ApplicationController
end
def subscribe
@entry = DiaryEntry.find(params[:id])
diary_entry = DiaryEntry.find(params[:id])
if ! diary_entry.subscribers.exists?(@user.id)
diary_entry.subscribers << @user
end
redirect_to :controller => "diary_entry", :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id
rescue ActiveRecord::RecordNotFound
render :action => "no_such_entry", :status => :not_found
end
def unsubscribe
@entry = DiaryEntry.find(params[:id])
diary_entry = DiaryEntry.find(params[:id])
if diary_entry.subscribers.exists?(@user.id)
diary_entry.subscribers.delete(@user)
end
redirect_to :controller => "diary_entry", :action => "view", :display_name => diary_entry.user.display_name, :id => diary_entry.id
rescue ActiveRecord::RecordNotFound
render :action => "no_such_entry", :status => :not_found
end
def list

1
app/models/notifier.rb
View file

@ -83,7 +83,6 @@ class Notifier < ActionMailer::Base
end
end
# FIXME mail should say your / their depending who's message it is
def diary_comment_notification(comment, recipient)
with_recipient_locale recipient do
@to_user = recipient.display_name

6
app/views/diary_entry/_diary_entry.html.erb
View file

@ -35,11 +35,5 @@
<%= link_to t('diary_entry.diary_entry.hide_link'), hide_diary_entry_path(:display_name => diary_entry.user.display_name, :id => diary_entry.id), :method => :post, :data => { :confirm => t('diary_entry.diary_entry.confirm') } %>
<% end %>
<% if @user and diary_entry.subscribers.exists?(@user.id) %>
<li><%= link_to t('javascripts.changesets.show.unsubscribe'), diary_entry_unsubscribe_url(diary_entry), :method => :post %></li>
<% elsif @user %>
<li><%= link_to t('javascripts.changesets.show.subscribe'), diary_entry_subscribe_url(diary_entry), :method => :post %></li>
<% end %>
</ul>
</div>

5
app/views/diary_entry/view.html.erb
View file

@ -21,6 +21,11 @@
<%= richtext_area :diary_comment, :body, :cols => 80, :rows => 15 %>
<%= submit_tag t('diary_entry.view.save_button') %>
<% end %>
<% if @user and @entry.subscribers.exists?(@user.id) %>
<div style='position:relative; top: -30px; left: 130px'><%= link_to t('javascripts.changesets.show.unsubscribe'), diary_entry_unsubscribe_path(:display_name => @entry.user.display_name, :id => @entry.id), :method => :post, :class => :button %></div>
<% elsif @user %>
<div style='position:relative; top: -30px; left: 130px'><%= link_to t('javascripts.changesets.show.subscribe'), diary_entry_subscribe_path(:display_name => @entry.user.display_name, :id => @entry.id), :method => :post, :class => :button %></div>
<% end %>
<% end %>
<%= if_not_logged_in(:div) do %>

24
config/locales/en.yml
View file

@ -1118,8 +1118,8 @@ en:
paragraph_1_html: |
OpenStreetMap has few formal rules but we expect all participants to collaborate
with, and communicate with, the community. If you are considering
any activities other than editing by hand, please read and follow the guidelines on
<a href='http://wiki.openstreetmap.org/wiki/Import/Guidelines'>Imports</a> and
any activities other than editing by hand, please read and follow the guidelines on
<a href='http://wiki.openstreetmap.org/wiki/Import/Guidelines'>Imports</a> and
<a href='http://wiki.openstreetmap.org/wiki/Automated_Edits_code_of_conduct'>Automated Edits</a>.
questions:
title: Any questions?
@ -1145,7 +1145,7 @@ en:
title: Join the community
explanation_html: |
If you have noticed a problem with our map data, for example a road is missing or your address, the best way to
proceed is to join the OpenStreetMap community and add or repair the data yourself.
proceed is to join the OpenStreetMap community and add or repair the data yourself.
add_a_note:
instructions_html: |
Just click <a class='icon note'></a> or the same icon on the map display.
@ -1155,8 +1155,8 @@ en:
title: Other concerns
explanation_html: |
If you have concerns about how our data is being used or about the contents please consult our
<a href='/copyright'>copyright page</a> for more legal information, or contact the appropriate
<a href='http://wiki.osmfoundation.org/wiki/Working_Groups'>OSMF working group</a>.
<a href='/copyright'>copyright page</a> for more legal information, or contact the appropriate
<a href='http://wiki.osmfoundation.org/wiki/Working_Groups'>OSMF working group</a>.
help_page:
title: Getting Help
introduction: |
@ -1224,20 +1224,20 @@ en:
License page</a> for details.
legal_title: Legal
legal_html: |
This site and many other related services are formally operated by the
<a href='http://osmfoundation.org/'>OpenStreetMap Foundation</a> (OSMF)
on behalf of the community. Use of all OSMF operated services is subject
This site and many other related services are formally operated by the
<a href='http://osmfoundation.org/'>OpenStreetMap Foundation</a> (OSMF)
on behalf of the community. Use of all OSMF operated services is subject
to our <a href="http://wiki.openstreetmap.org/wiki/Acceptable_Use_Policy">
Acceptable Use Policies</a> and our <a href="http://wiki.osmfoundation.org/wiki/Privacy_Policy">Privacy Policy</a>
<br>
Please <a href='http://osmfoundation.org/Contact'>contact the OSMF</a>
<br>
Please <a href='http://osmfoundation.org/Contact'>contact the OSMF</a>
if you have licensing, copyright or other legal questions and issues.
partners_title: Partners
notifier:
diary_comment_notification:
subject: "[OpenStreetMap] %{user} commented on your diary entry"
subject: "[OpenStreetMap] %{user} commented on a diary entry"
hi: "Hi %{to_user},"
header: "%{from_user} has commented on your recent OpenStreetMap diary entry with the subject %{subject}:"
header: "%{from_user} has commented on the OpenStreetMap diary entry with the subject %{subject}:"
footer: "You can also read the comment at %{readurl} and you can comment at %{commenturl} or reply at %{replyurl}"
message_notification:
subject_header: "[OpenStreetMap] %{subject}"

112
test/controllers/diary_entry_controller_test.rb
View file

@ -85,11 +85,11 @@ class DiaryEntryControllerTest < ActionController::TestCase
)
assert_routing(
{ :path => "/user/username/diary/1/subscribe", :method => :post },
{ :controller => "diary_entry", :action => "subscribe", :id => "1" }
{ :controller => "diary_entry", :action => "subscribe", :display_name => "username", :id => "1" }
)
assert_routing(
{ :path => "/user/username/diary/1/unsubscribe", :method => :post },
{ :controller => "diary_entry", :action => "unsubscribe", :id => "1" }
{ :controller => "diary_entry", :action => "unsubscribe", :display_name => "username", :id => "1" }
)
end
@ -330,21 +330,23 @@ class DiaryEntryControllerTest < ActionController::TestCase
assert_select "h2", :text => "No entry with the id: 9999", :count => 1
end
# FIXME assert number of subscribers
# Now try an invalid comment with an empty body
assert_no_difference "ActionMailer::Base.deliveries.size" do
assert_no_difference "DiaryComment.count" do
post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "" } }, { :user => users(:public_user).id }
assert_no_difference "entry.subscribers.count" do
post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "" } }, { :user => users(:public_user).id }
end
end
end
assert_response :success
assert_template :view
# FIXME assert number of subscribers
# Now try again with the right id
assert_difference "ActionMailer::Base.deliveries.size", 1 do
assert_difference "ActionMailer::Base.deliveries.size", entry.subscribers.count do
assert_difference "DiaryComment.count", 1 do
post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "New comment" } }, { :user => users(:public_user).id }
assert_difference "entry.subscribers.count", 1 do
post :comment, { :display_name => entry.user.display_name, :id => entry.id, :diary_comment => { :body => "New comment" } }, { :user => users(:public_user).id }
end
end
end
assert_response :redirect
@ -360,8 +362,6 @@ class DiaryEntryControllerTest < ActionController::TestCase
assert_equal users(:public_user).id, comment.user_id
assert_equal "New comment", comment.body
# FIXME check number of subscribers
# Now view the diary entry, and check the new comment is present
get :view, :display_name => entry.user.display_name, :id => entry.id
assert_response :success
@ -380,7 +380,6 @@ class DiaryEntryControllerTest < ActionController::TestCase
# Generate some spammy content
spammy_text = 1.upto(50).map { |n| "http://example.com/spam#{n}" }.join(" ")
# FIXME assert number of subscribers
# Try creating a spammy comment
assert_difference "ActionMailer::Base.deliveries.size", 1 do
assert_difference "DiaryComment.count", 1 do
@ -663,91 +662,74 @@ class DiaryEntryControllerTest < ActionController::TestCase
##
# test subscribe success
def test_subscribe_success
basic_authorization(users(:public_user).email, "test")
changeset = changesets(:normal_user_closed_change)
diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
assert_difference "changeset.subscribers.count", 1 do
post :subscribe, :id => changeset.id
#basic_authorization(users(:public_user).email, "test")
assert_difference "diary_entry.subscribers.count", 1 do
post :subscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:public_user).id}
end
assert_response :success
assert_response :redirect
end
##
# test subscribe fail
def test_subscribe_fail
# unauthorized
changeset = changesets(:normal_user_closed_change)
assert_no_difference "changeset.subscribers.count" do
post :subscribe, :id => changeset.id
end
assert_response :unauthorized
diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
basic_authorization(users(:public_user).email, "test")
# bad changeset id
assert_no_difference "changeset.subscribers.count" do
post :subscribe, :id => 999111
# not signed in
assert_no_difference "diary_entry.subscribers.count", 1 do
post :subscribe, :id => diary_entry.id, :display_name => diary_entry.user.display_name
end
assert_response :forbidden
# bad diary id
post :subscribe, {:id => 999111, :display_name => "username"}, { :user => users(:public_user).id}
assert_response :not_found
# not closed changeset
changeset = changesets(:normal_user_first_change)
assert_no_difference "changeset.subscribers.count" do
post :subscribe, :id => changeset.id
end
assert_response :conflict
# trying to subscribe when already subscribed
changeset = changesets(:normal_user_subscribed_change)
assert_no_difference "changeset.subscribers.count" do
post :subscribe, :id => changeset.id
post :subscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:public_user).id}
assert_no_difference "diary_entry.subscribers.count" do
post :subscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:public_user).id}
end
assert_response :conflict
end
##
# test unsubscribe success
def test_unsubscribe_success
basic_authorization(users(:public_user).email, "test")
changeset = changesets(:normal_user_subscribed_change)
diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
assert_difference "changeset.subscribers.count", -1 do
post :unsubscribe, :id => changeset.id
assert_difference "diary_entry.subscribers.count", -1 do
post :unsubscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:normal_user).id}
end
assert_response :success
assert_response :redirect
post :subscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:public_user).id}
assert_difference "diary_entry.subscribers.count", -1 do
post :unsubscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:public_user).id}
end
assert_response :redirect
end
##
# test unsubscribe fail
def test_unsubscribe_fail
# unauthorized
changeset = changesets(:normal_user_closed_change)
assert_no_difference "changeset.subscribers.count" do
post :unsubscribe, :id => changeset.id
end
assert_response :unauthorized
diary_entry = create(:diary_entry, :user_id => users(:normal_user).id)
basic_authorization(users(:public_user).email, "test")
# bad changeset id
assert_no_difference "changeset.subscribers.count" do
post :unsubscribe, :id => 999111
# not signed in
assert_no_difference "diary_entry.subscribers.count" do
post :unsubscribe, :id => diary_entry.id, :display_name => diary_entry.user.display_name
end
assert_response :forbidden
# bad diary id
post :unsubscribe, {:id => 999111, :display_name => "username"}, { :user => users(:public_user).id}
assert_response :not_found
# not closed changeset
changeset = changesets(:normal_user_first_change)
assert_no_difference "changeset.subscribers.count" do
post :unsubscribe, :id => changeset.id
# trying to subscribe when already subscribed
assert_no_difference "diary_entry.subscribers.count" do
post :unsubscribe, {:id => diary_entry.id, :display_name => diary_entry.user.display_name}, { :user => users(:public_user).id}
end
assert_response :conflict
# trying to unsubscribe when not subscribed
changeset = changesets(:normal_user_closed_change)
assert_no_difference "changeset.subscribers.count" do
post :unsubscribe, :id => changeset.id
end
assert_response :not_found
end
private