diff options
| -rw-r--r-- | app/controllers/admin_user_controller.rb | 3 | ||||
| -rw-r--r-- | app/controllers/application_controller.rb | 13 | ||||
| -rw-r--r-- | app/controllers/request_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/track_controller.rb | 8 | ||||
| -rw-r--r-- | app/controllers/user_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/info_request_event.rb | 42 | ||||
| -rw-r--r-- | app/models/post_redirect.rb | 2 | ||||
| -rw-r--r-- | app/models/request_mailer.rb | 2 | ||||
| -rw-r--r-- | app/models/track_thing.rb | 12 | ||||
| m--------- | commonlib | 0 | ||||
| -rw-r--r-- | config/general.yml-example | 5 | ||||
| -rw-r--r-- | config/routes.rb | 6 | ||||
| -rw-r--r-- | public/stylesheets/print.css | 7 | ||||
| -rw-r--r-- | spec/controllers/admin_user_controller_spec.rb | 10 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 2 | ||||
| -rw-r--r-- | spec/controllers/track_controller_spec.rb | 29 | ||||
| -rw-r--r-- | spec/integration/admin_spec.rb | 23 | ||||
| -rw-r--r-- | spec/integration/create_request_spec.rb | 44 |
18 files changed, 169 insertions, 49 deletions
diff --git a/app/controllers/admin_user_controller.rb b/app/controllers/admin_user_controller.rb index b2c084739..249030537 100644 --- a/app/controllers/admin_user_controller.rb +++ b/app/controllers/admin_user_controller.rb @@ -74,10 +74,9 @@ class AdminUserController < AdminController def login_as @admin_user = User.find(params[:id]) # check user does exist - post_redirect = PostRedirect.new( :uri => main_url(user_url(@admin_user)), :user_id => @admin_user.id) + post_redirect = PostRedirect.new( :uri => main_url(user_url(@admin_user)), :user_id => @admin_user.id, :circumstance => "login_as" ) post_redirect.save! url = main_url(confirm_url(:email_token => post_redirect.email_token, :only_path => true)) - session[:user_id] = nil # Log out current (usually admin) user, so we get logged in as the other user redirect_to url end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b681f455d..0508abe76 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -151,8 +151,8 @@ class ApplicationController < ActionController::Base false end - # Called from test code, is a mimic of User.confirm, for use in following email - # links when in controller tests (since we don't have full integration tests that + # Called from test code, is a mimic of UserController.confirm, for use in following email + # links when in controller tests (though we also have full integration tests that # can work over multiple controllers) def test_code_redirect_by_email_token(token, controller_example_group) post_redirect = PostRedirect.find_by_email_token(token) @@ -224,15 +224,15 @@ class ApplicationController < ActionController::Base post_redirect = PostRedirect.new(:uri => request.request_uri, :post_params => params, :reason_params => reason_params) post_redirect.save! - # 'modal' controls whether the sign-in form will be displayed in the typical full-blown - # page or on its own, useful for pop-ups + # 'modal' controls whether the sign-in form will be displayed in the typical full-blown + # page or on its own, useful for pop-ups redirect_to signin_url(:token => post_redirect.token, :modal => params[:modal]) return false end return true end - def authenticated_as_user?(user, reason_params) + def authenticated_as_user?(user, reason_params) reason_params[:user_name] = user.name reason_params[:user_url] = show_user_url(:url_name => user.url_name) if session[:user_id] @@ -274,6 +274,8 @@ class ApplicationController < ActionController::Base # XXX what is the built in Ruby URI munging function that can do this # choice of & vs. ? more elegantly than this dumb if statement? if uri.include?("?") + # XXX This looks odd. What would a fragment identifier be doing server-side? + # But it also looks harmless, so I’ll leave it just in case. if uri.include?("#") uri.sub!("#", "&post_redirect=1#") else @@ -294,6 +296,7 @@ class ApplicationController < ActionController::Base if params[:post_redirect] and session[:post_redirect_token] post_redirect = PostRedirect.find_by_token(session[:post_redirect_token]) params.update(post_redirect.post_params) + params[:post_redirect_user] = post_redirect.user end end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 313a57d7d..96c501755 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -344,7 +344,13 @@ class RequestController < ApplicationController return end - @info_request.user = authenticated_user + if params[:post_redirect_user] + # If an admin has clicked the confirmation link on a users behalf, + # we don’t want to reassign the request to the administrator. + @info_request.user = params[:post_redirect_user] + else + @info_request.user = authenticated_user + end # This automatically saves dependent objects, such as @outgoing_message, in the same transaction @info_request.save! # XXX send_message needs the database id, so we send after saving, which isn't ideal if the request broke here. diff --git a/app/controllers/track_controller.rb b/app/controllers/track_controller.rb index d858ab233..95b573cdc 100644 --- a/app/controllers/track_controller.rb +++ b/app/controllers/track_controller.rb @@ -50,11 +50,15 @@ class TrackController < ApplicationController raise ActiveRecord::RecordNotFound.new("None found") if @public_body.nil? # If found by historic name, or alternate locale name, redirect to new name if @public_body.url_name != params[:url_name] - redirect_to track_public_body_url(:url_name => @public_body.url_name, :feed => params[:feed]) + redirect_to track_public_body_url(:url_name => @public_body.url_name, :feed => params[:feed], :event_type => params[:event_type]) return end - @track_thing = TrackThing.create_track_for_public_body(@public_body) + if params[:event_type] + @track_thing = TrackThing.create_track_for_public_body(@public_body, params[:event_type]) + else + @track_thing = TrackThing.create_track_for_public_body(@public_body) + end return atom_feed_internal if params[:feed] == 'feed' diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 403cb9684..08726183e 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -182,7 +182,7 @@ class UserController < ApplicationController return end - if !User.stay_logged_in_on_redirect?(@user) + if !User.stay_logged_in_on_redirect?(@user) || post_redirect.circumstance == "login_as" @user = post_redirect.user @user.email_confirmed = true @user.save! diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 99f34cf9e..cb49596cb 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -36,25 +36,29 @@ class InfoRequestEvent < ActiveRecord::Base has_many :track_things_sent_emails validates_presence_of :event_type - validates_inclusion_of :event_type, :in => [ - 'sent', - 'resent', - 'followup_sent', - 'followup_resent', - - 'edit', # title etc. edited (in admin interface) - 'edit_outgoing', # outgoing message edited (in admin interface) - 'edit_comment', # comment edited (in admin interface) - 'destroy_incoming', # deleted an incoming message (in admin interface) - 'destroy_outgoing', # deleted an outgoing message (in admin interface) - 'redeliver_incoming', # redelivered an incoming message elsewhere (in admin interface) - 'move_request', # changed user or public body (in admin interface) - 'manual', # you did something in the db by hand - - 'response', - 'comment', - 'status_update' - ] + + def self.enumerate_event_types + [ + 'sent', + 'resent', + 'followup_sent', + 'followup_resent', + + 'edit', # title etc. edited (in admin interface) + 'edit_outgoing', # outgoing message edited (in admin interface) + 'edit_comment', # comment edited (in admin interface) + 'destroy_incoming', # deleted an incoming message (in admin interface) + 'destroy_outgoing', # deleted an outgoing message (in admin interface) + 'redeliver_incoming', # redelivered an incoming message elsewhere (in admin interface) + 'move_request', # changed user or public body (in admin interface) + 'manual', # you did something in the db by hand + + 'response', + 'comment', + 'status_update', + ] + end + validates_inclusion_of :event_type, :in => enumerate_event_types # user described state (also update in info_request) validate :must_be_valid_state diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb index 59cc86799..c9a6229a4 100644 --- a/app/models/post_redirect.rb +++ b/app/models/post_redirect.rb @@ -39,7 +39,7 @@ class PostRedirect < ActiveRecord::Base self.post_params_yaml = params.to_yaml end def post_params - if self.post_params_yaml.nil? + if self.post_params_yaml.nil? return {} end YAML.load(self.post_params_yaml) diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 83cce9045..177a39241 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -40,7 +40,7 @@ class RequestMailer < ApplicationMailer :filename => "original.eml", :transfer_encoding => '7bit', :content_disposition => 'inline' @body = { :info_request => info_request, - :contact_email => MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost') + :contact_email => MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost') } end diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb index 58d70ed86..b277e72b0 100644 --- a/app/models/track_thing.rb +++ b/app/models/track_thing.rb @@ -146,11 +146,15 @@ class TrackThing < ActiveRecord::Base return track_thing end - def TrackThing.create_track_for_public_body(public_body) + def TrackThing.create_track_for_public_body(public_body, event_type = nil) track_thing = TrackThing.new track_thing.track_type = 'public_body_updates' track_thing.public_body = public_body - track_thing.track_query = "requested_from:" + public_body.url_name + query = "requested_from:" + public_body.url_name + if InfoRequestEvent.enumerate_event_types.include?(event_type) + query += " variety:" + event_type + end + track_thing.track_query = query return track_thing end @@ -172,9 +176,9 @@ class TrackThing < ActiveRecord::Base when "users" query += " variety:user" when "authorities" - query += " variety:authority" + query += " variety:authority" end - end + end track_thing.track_query = query # XXX should extract requested_by:, request:, requested_from: # and stick their values into the respective relations. diff --git a/commonlib b/commonlib -Subproject c200fcbb73981113fcb2ccd132e1a2b386823c6 +Subproject 87bc80b7484740e1c21e335500feda4f2755d78 diff --git a/config/general.yml-example b/config/general.yml-example index ed04e0fd5..211161606 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -142,3 +142,8 @@ EXCEPTION_NOTIFICATIONS_TO: # This rate limiting can be turned off per-user via the admin interface MAX_REQUESTS_PER_USER_PER_DAY: 6 + +SURVEY_URL: http://survey.mysociety.org/ +# The shared secret needed to authenticate with the survey service +SURVEY_SECRET: 12345678910 + diff --git a/config/routes.rb b/config/routes.rb index fa387a106..747cc9b06 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -6,6 +6,9 @@ # # $Id: routes.rb,v 1.92 2009-10-14 22:01:27 francis Exp $ +# Allow easy extension from themes. Note these will have the highest priority. +load File.join('config', 'custom-routes.rb') + ActionController::Routing::Routes.draw do |map| # The priority is based upon order of creation: first created -> highest priority. @@ -14,9 +17,6 @@ ActionController::Routing::Routes.draw do |map| # map.connect 'products/:id', :controller => 'catalog', :action => 'view' # Keep in mind you can assign values other than :controller and :action - # Allow easy extension from themes. Note these will have the highest priority. - require File.join('config', 'custom-routes') - map.with_options :controller => 'general' do |general| general.frontpage '/', :action => 'frontpage' general.blog '/blog', :action => 'blog' diff --git a/public/stylesheets/print.css b/public/stylesheets/print.css index 43d7a1807..89be21019 100644 --- a/public/stylesheets/print.css +++ b/public/stylesheets/print.css @@ -39,4 +39,9 @@ div.correspondence { #other-country-notice { display: none; -}
\ No newline at end of file +} + +.not-for-print { + display: none !IMPORTANT; +} + diff --git a/spec/controllers/admin_user_controller_spec.rb b/spec/controllers/admin_user_controller_spec.rb index 60ac6969d..c2d645fd2 100644 --- a/spec/controllers/admin_user_controller_spec.rb +++ b/spec/controllers/admin_user_controller_spec.rb @@ -24,13 +24,7 @@ describe AdminUserController, "when administering users" do post_redirect = PostRedirect.get_last_post_redirect response.should redirect_to(:controller => 'user', :action => 'confirm', :email_token => post_redirect.email_token) end - - it "logs in as another user when already logged in as an admin" do - session[:user_id] = users(:admin_user).id - get :login_as, :id => users(:bob_smith_user).id - post_redirect = PostRedirect.get_last_post_redirect - response.should redirect_to(:controller => 'user', :action => 'confirm', :email_token => post_redirect.email_token) - session[:user_id].should be_nil - end + + # See also "allows an admin to log in as another user" in spec/integration/admin_spec.rb end diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 9018f76fe..9add32f1e 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -631,7 +631,7 @@ describe RequestController, "when creating a new request" do it "should accept a public body parameter" do get :new, :public_body_id => @body.id - assigns[:info_request].public_body.should == @body + assigns[:info_request].public_body.should == @body response.should render_template('new') end diff --git a/spec/controllers/track_controller_spec.rb b/spec/controllers/track_controller_spec.rb index 0dc5db607..5d299caa5 100644 --- a/spec/controllers/track_controller_spec.rb +++ b/spec/controllers/track_controller_spec.rb @@ -183,6 +183,35 @@ describe TrackController, "when viewing JSON version of a track feed" do end +describe TrackController, "when tracking a public body" do + integrate_views + + before(:each) do + load_raw_emails_data + rebuild_xapian_index + end + + it "should work" do + geraldine = public_bodies(:geraldine_public_body) + get :track_public_body, :feed => 'feed', :url_name => geraldine.url_name + response.should be_success + response.should render_template('track/atom_feed') + tt = assigns[:track_thing] + tt.public_body.should == geraldine + tt.track_type.should == 'public_body_updates' + tt.track_query.should == "requested_from:" + geraldine.url_name + end + it "should filter by event type" do + geraldine = public_bodies(:geraldine_public_body) + get :track_public_body, :feed => 'feed', :url_name => geraldine.url_name, :event_type => 'sent' + response.should be_success + response.should render_template('track/atom_feed') + tt = assigns[:track_thing] + tt.public_body.should == geraldine + tt.track_type.should == 'public_body_updates' + tt.track_query.should == "requested_from:" + geraldine.url_name + " variety:sent" + end +end diff --git a/spec/integration/admin_spec.rb b/spec/integration/admin_spec.rb new file mode 100644 index 000000000..caf741749 --- /dev/null +++ b/spec/integration/admin_spec.rb @@ -0,0 +1,23 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +require "base64" + +describe "When administering the site" do + it "allows an admin to log in as another user" do + # First log in as Joe Admin + admin_user = users(:admin_user) + admin_user.email_confirmed = true + admin_user.save! + post_via_redirect "/profile/sign_in", :user_signin => {:email => admin_user.email, :password => "jonespassword"} + response.should be_success + + # Now fetch the "log in as" link to log in as Bob + admin_username = MySociety::Config.get('ADMIN_USERNAME') + admin_password = MySociety::Config.get('ADMIN_PASSWORD') + get_via_redirect "/admin/user/login_as/#{users(:bob_smith_user).id}", nil, { + "Authorization" => "Basic " + Base64.encode64("#{admin_username}:#{admin_password}").strip + } + response.should be_success + session[:user_id].should == users(:bob_smith_user).id + end +end diff --git a/spec/integration/create_request_spec.rb b/spec/integration/create_request_spec.rb new file mode 100644 index 000000000..6f336d406 --- /dev/null +++ b/spec/integration/create_request_spec.rb @@ -0,0 +1,44 @@ +require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') + +describe "When creating requests" do + it "should associate the request with the requestor, even if it is approved by an admin" do + # This is a test for https://github.com/sebbacon/alaveteli/issues/446 + + params = { :info_request => { :public_body_id => public_bodies(:geraldine_public_body).id, + :title => "Why is your quango called Geraldine?", :tag_string => "" }, + :outgoing_message => { :body => "This is a silly letter. It is too short to be interesting." }, + :submitted_new_request => 1, :preview => 0 + } + + # Initially we are not logged in. Try to create a new request. + post "/new", params + # We expect to be redirected to the login page + post_redirect = PostRedirect.get_last_post_redirect + response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) + follow_redirect! + response.should render_template("user/sign") + + # Now log in as an unconfirmed user. + post "/profile/sign_in", :user_signin => {:email => users(:unconfirmed_user).email, :password => "jonespassword"}, :token => post_redirect.token + # This will trigger a confirmation mail. Get the PostRedirect for later. + response.should render_template("user/confirm") + post_redirect = PostRedirect.get_last_post_redirect + + # Now log in as an admin user, then follow the confirmation link in the email that was sent to the unconfirmed user + admin_user = users(:admin_user) + admin_user.email_confirmed = true + admin_user.save! + post_via_redirect "/profile/sign_in", :user_signin => {:email => admin_user.email, :password => "jonespassword"} + response.should be_success + get "/c/" + post_redirect.email_token + follow_redirect! + response.location.should =~ %r(/request/(.+)/new) + response.location =~ %r(/request/(.+)/new) + url_title = $1 + info_request = InfoRequest.find_by_url_title(url_title) + info_request.should_not be_nil + + # Make sure the request is still owned by the user who made it, not the admin who confirmed it + info_request.user_id.should == users(:unconfirmed_user).id + end +end |