diff options
| -rw-r--r-- | .travis.yml | 2 | ||||
| -rw-r--r-- | app/controllers/holiday_controller.rb | 4 | ||||
| -rw-r--r-- | app/models/holiday.rb | 56 | ||||
| -rw-r--r-- | app/models/info_request.rb | 8 | ||||
| -rw-r--r-- | app/views/request/_sidebar.rhtml | 8 | ||||
| -rw-r--r-- | app/views/track/_tracking_links.rhtml | 10 | ||||
| -rw-r--r-- | config/deploy.rb | 3 | ||||
| -rw-r--r-- | config/environment.rb | 2 | ||||
| -rw-r--r-- | config/general.yml-example | 3 | ||||
| -rw-r--r-- | db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb | 7 | ||||
| -rw-r--r-- | doc/CHANGES.md | 12 | ||||
| -rw-r--r-- | doc/INSTALL.md | 14 | ||||
| -rw-r--r-- | doc/THEMES.md | 2 | ||||
| -rw-r--r-- | spec/models/customstates.rb | 4 | ||||
| -rw-r--r-- | spec/models/holiday_spec.rb | 99 | ||||
| -rw-r--r-- | spec/views/request/_after_actions.rhtml_spec.rb | 71 |
16 files changed, 202 insertions, 103 deletions
diff --git a/.travis.yml b/.travis.yml index 4c8fbef6a..70f1e3ecf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,4 +22,4 @@ notifications: irc: "irc.freenode.org#alaveteli" email: recipients: - - seb.bacon@gmail.com + - cron-whatdotheyknow@mysociety.org diff --git a/app/controllers/holiday_controller.rb b/app/controllers/holiday_controller.rb index 7f62aa26d..9430c0756 100644 --- a/app/controllers/holiday_controller.rb +++ b/app/controllers/holiday_controller.rb @@ -14,7 +14,9 @@ class HolidayController < ApplicationController def due_date if params[:holiday] @request_date = Date.strptime(params[:holiday]) or raise "Invalid date" - @due_date = Holiday.due_date_from(@request_date, 20) + days_later = MySociety::Config.get('REPLY_LATE_AFTER_DAYS', 20) + working_or_calendar_days = MySociety::Config.get('WORKING_OR_CALENDAR_DAYS', 'working') + @due_date = Holiday.due_date_from(@request_date, days_later, working_or_calendar_days) @skipped = Holiday.all( :conditions => [ 'day >= ? AND day <= ?', @request_date.strftime("%F"), @due_date.strftime("%F") diff --git a/app/models/holiday.rb b/app/models/holiday.rb index debd88dec..1072f6a70 100644 --- a/app/models/holiday.rb +++ b/app/models/holiday.rb @@ -25,18 +25,34 @@ class Holiday < ActiveRecord::Base - # Calculate the date on which a request made on a given date falls due. + def Holiday.weekend_or_holiday?(date) + # TODO only fetch holidays after the start_date + holidays = self.all.collect { |h| h.day }.to_set + + date.wday == 0 || date.wday == 6 || holidays.include?(date) + end + + def Holiday.due_date_from(start_date, days, type_of_days) + case type_of_days + when "working" + Holiday.due_date_from_working_days(start_date, days) + when "calendar" + Holiday.due_date_from_calendar_days(start_date, days) + else + raise "Unexpected value for type_of_days: #{type_of_days}" + end + end + + # Calculate the date on which a request made on a given date falls due when + # days are given in working days # i.e. it is due by the end of that day. - def Holiday.due_date_from(start_date, working_days) + def Holiday.due_date_from_working_days(start_date, working_days) # convert date/times into dates start_date = start_date.to_date - # TODO only fetch holidays after the start_date - holidays = self.all.collect { |h| h.day }.to_set - - # Count forward (20) working days. We start with today as "day zero". The - # first of the twenty full working days is the next day. We return the - # date of the last of the twenty. + # Count forward the number of working days. We start with today as "day zero". The + # first of the full working days is the next day. We return the + # date of the last of the number of working days. # This response for example of a public authority complains that we had # it wrong. We didn't (even thought I changed the code for a while, @@ -46,15 +62,27 @@ class Holiday < ActiveRecord::Base days_passed = 0 response_required_by = start_date - # Now step forward into each of the 20 days. + # Now step forward into each of the working days. while days_passed < working_days - response_required_by += 1.day - next if response_required_by.wday == 0 || response_required_by.wday == 6 # weekend - next if holidays.include?(response_required_by) - days_passed += 1 + response_required_by += 1 + days_passed += 1 unless weekend_or_holiday?(response_required_by) end - return response_required_by + response_required_by end + # Calculate the date on which a request made on a given date falls due when + # the days are given in calendar days (rather than working days) + # If the due date falls on a weekend or a holiday then the due date is the next + # weekday that isn't a holiday. + def Holiday.due_date_from_calendar_days(start_date, days) + # convert date/times into dates + start_date = start_date.to_date + + response_required_by = start_date + days + while weekend_or_holiday?(response_required_by) + response_required_by += 1 + end + response_required_by + end end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index b62f67ee1..141440c6d 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -690,7 +690,8 @@ public # things, e.g. fees, not properly covered. def date_response_required_by days_later = MySociety::Config.get('REPLY_LATE_AFTER_DAYS', 20) - return Holiday.due_date_from(self.date_initial_request_last_sent_at, days_later) + working_or_calendar_days = MySociety::Config.get('WORKING_OR_CALENDAR_DAYS', 'working') + return Holiday.due_date_from(self.date_initial_request_last_sent_at, days_later, working_or_calendar_days) end # This is a long stop - even with UK public interest test extensions, 40 # days is a very long time. @@ -698,12 +699,13 @@ public last_sent = last_event_forming_initial_request very_late_days_later = MySociety::Config.get('REPLY_VERY_LATE_AFTER_DAYS', 40) school_very_late_days_later = MySociety::Config.get('SPECIAL_REPLY_VERY_LATE_AFTER_DAYS', 60) + working_or_calendar_days = MySociety::Config.get('WORKING_OR_CALENDAR_DAYS', 'working') if self.public_body.is_school? # schools have 60 working days maximum (even over a long holiday) - return Holiday.due_date_from(self.date_initial_request_last_sent_at, 60) + return Holiday.due_date_from(self.date_initial_request_last_sent_at, school_very_late_days_later, working_or_calendar_days) else # public interest test ICO guidance gives 40 working maximum - return Holiday.due_date_from(self.date_initial_request_last_sent_at, 40) + return Holiday.due_date_from(self.date_initial_request_last_sent_at, very_late_days_later, working_or_calendar_days) end end diff --git a/app/views/request/_sidebar.rhtml b/app/views/request/_sidebar.rhtml index 731bfb34e..dc0d2eb31 100644 --- a/app/views/request/_sidebar.rhtml +++ b/app/views/request/_sidebar.rhtml @@ -24,7 +24,7 @@ <% end %> <% else %> <p><%= _('Requests for personal information and vexatious requests are not considered valid for FOI purposes (<a href="/help/about">read more</a>).') %></p> - <p><%= ('If you believe this request is not suitable, you can report it for attention by the site administrators') %></p> + <p><%= _('If you believe this request is not suitable, you can report it for attention by the site administrators') %></p> <%= link_to _("Report this request"), report_path, :class => "link_button_green", :method => "POST" %> <% end %> <% end %> @@ -32,11 +32,11 @@ <div class="act_link"> <% tweet_link = "https://twitter.com/share?url=#{h(request.url)}&via=#{h(MySociety::Config.get('TWITTER_USERNAME', ''))}&text='#{h(@info_request.title)}'&related=#{_('alaveteli_foi:The software that runs {{site_name}}', :site_name => h(site_name))}" %> - <%= link_to '<img src="/images/twitter-16.png" alt="twitter icon">', tweet_link %> - <%= link_to _("Tweet this request"), tweet_link %> + <%= link_to '<img src="/images/twitter-16.png" alt="twitter icon">', tweet_link %> + <%= link_to _("Tweet this request"), tweet_link %> </div> <div class="act_link"> - <%= link_to '<img src="/images/wordpress.png" alt="" class="rss">', "http://wordpress.com/"%> + <%= link_to '<img src="/images/wordpress.png" alt="" class="rss">', "http://wordpress.com/"%> <%= link_to _("Start your own blog"), "http://wordpress.com/"%> </div> diff --git a/app/views/track/_tracking_links.rhtml b/app/views/track/_tracking_links.rhtml index 39f346eff..3ba9d15e2 100644 --- a/app/views/track/_tracking_links.rhtml +++ b/app/views/track/_tracking_links.rhtml @@ -4,12 +4,12 @@ end %> -<% if own_request %> +<% if own_request %> <p><%= _('This is your own request, so you will be automatically emailed when new responses arrive.')%></p> -<% elsif existing_track %> +<% elsif existing_track %> <p><%= track_thing.params[:verb_on_page_already] %></p> <div class="feed_link feed_link_<%=location%>"> - <%= link_to "Unsubscribe", {:controller => 'track', :action => 'update', :track_id => existing_track.id, :track_medium => "delete", :r => request.request_uri}, :class => "link_button_green" %> + <%= link_to _("Unsubscribe"), {:controller => 'track', :action => 'update', :track_id => existing_track.id, :track_medium => "delete", :r => request.request_uri}, :class => "link_button_green" %> </div> <% elsif track_thing %> <div class="feed_link feed_link_<%=location%>"> @@ -19,9 +19,9 @@ <%= link_to _("Follow"), do_track_url(track_thing), :class => "link_button_green" %> <% end %> </div> - + <div class="feed_link feed_link_<%=location%>"> - <%= link_to '<img src="/images/feed-16.png" alt="">', do_track_url(track_thing, 'feed') %> + <%= link_to '<img src="/images/feed-16.png" alt="">', do_track_url(track_thing, 'feed') %> <%= link_to (location == 'sidebar' ? _('RSS feed of updates') : _('RSS feed')), do_track_url(track_thing, 'feed') %> </div> <% end %> diff --git a/config/deploy.rb b/config/deploy.rb index 888710f83..f82379df0 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -48,6 +48,9 @@ namespace :deploy do links = { "#{release_path}/config/database.yml" => "#{shared_path}/database.yml", "#{release_path}/config/general.yml" => "#{shared_path}/general.yml", + "#{release_path}/config/rails_env.rb" => "#{shared_path}/rails_env.rb", + "#{release_path}/public/foi-live-creation.png" => "#{shared_path}/foi-live-creation.png", + "#{release_path}/public/foi-user-use.png" => "#{shared_path}/foi-user-use.png", "#{release_path}/files" => "#{shared_path}/files", "#{release_path}/cache" => "#{shared_path}/cache", "#{release_path}/vendor/plugins/acts_as_xapian/xapiandbs" => "#{shared_path}/xapiandbs", diff --git a/config/environment.rb b/config/environment.rb index 6234ae5c1..250d3eed0 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -32,7 +32,7 @@ require File.join(File.dirname(__FILE__), '../lib/old_rubygems_patch') # Application version -ALAVETELI_VERSION = '0.6.5.1' +ALAVETELI_VERSION = '0.6.6' Rails::Initializer.run do |config| # Load intial mySociety config diff --git a/config/general.yml-example b/config/general.yml-example index a6f657d96..fd27b151a 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -30,6 +30,9 @@ REPLY_LATE_AFTER_DAYS: 20 REPLY_VERY_LATE_AFTER_DAYS: 40 # We give some types of authority like schools a bit longer than everyone else SPECIAL_REPLY_VERY_LATE_AFTER_DAYS: 60 +# Whether the days above are given in working or calendar days. Value can be "working" or "calendar". +# Default is "working". +WORKING_OR_CALENDAR_DAYS: working # example public bodies for the home page, semicolon delimited - short_names FRONTPAGE_PUBLICBODY_EXAMPLES: 'tgq' diff --git a/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb b/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb index d77dbaa64..d187dcfa5 100644 --- a/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb +++ b/db/migrate/20120912170035_add_info_requests_count_to_public_bodies.rb @@ -2,11 +2,10 @@ class AddInfoRequestsCountToPublicBodies < ActiveRecord::Migration def self.up add_column :public_bodies, :info_requests_count, :integer, :null => false, :default => 0 - PublicBody.reset_column_information + PublicBody.connection.execute("UPDATE public_bodies + SET info_requests_count = (SELECT COUNT(*) FROM info_requests + WHERE public_body_id = public_bodies.id);") - PublicBody.find_each do |public_body| - public_body.update_attribute :info_requests_count, public_body.info_requests.length - end end diff --git a/doc/CHANGES.md b/doc/CHANGES.md index ae9418ef4..15df1dce5 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -1,3 +1,15 @@ +# Version 0.6.6 +## Highlighted features +* Adds deployment via Capistrano - see DEPLOY.md for details +* Speeds up several admin pages that were slow in large installs + +* [List of issues on github](https://github.com/mysociety/alaveteli/issues?milestone=22&state=closed) + +## Upgrade notes + +* Check out this version and run `rails-post-deploy` as usual. +* Run `rake temp:populate_request_classifications` to populate the new request_classifications table which is used in generating the request categorisation game league tables and progress widget. + # Version 0.6.5 * This is a minor release, to update all documentation and example files to reflect the move of the official repository to http://github.com/mysociety/alaveteli and the alavetelitheme and adminbootstraptheme themes to http://github.com/mysociety/alavetelitheme and http://github.com/mysociety/adminbootstraptheme respectively. * Some basic versioning has been added for themes. An ALAVETELI_VERSION constant has been added in config/environment.rb. When loading themes, `rails-post-deploy` now looks for a tag on the theme repository in the form 'use-with-alaveteli-0.6.5' that matches the ALAVETELI_VERSION being deployed - if it finds such a tag, the theme will be checked out from that tag, rather than from the HEAD of the theme repository. If no such tag is found, HEAD is used, as before [issue #573](https://github.com/mysociety/alaveteli/issues/573). diff --git a/doc/INSTALL.md b/doc/INSTALL.md index 0cf6b0fc7..b805ee0c5 100644 --- a/doc/INSTALL.md +++ b/doc/INSTALL.md @@ -25,6 +25,18 @@ master branch (which always contains the latest stable release): git checkout master +# Package pinning + +You need to configure [apt-pinning](http://wiki.debian.org/AptPreferences#Pinning-1) preferences in order to prevent packages being pulled from the debian testing distribution in preference to the stable distribution once you have added the testing repository as described below. + +In order to configure apt-pinning and to keep most packages coming from the Debian stable repository while installing the ones required from testing and the mySociety repository you need to run the following commands: + + echo "Package: *" >> /tmp/preferences + echo "Pin: release a=testing">> /tmp/preferences + echo "Pin-Priority: 50" >> /tmp/preferences + sudo cp /tmp/preferences /etc/apt/ + rm /tmp/preferences + # Install system dependencies These are packages that the software depends on: third-party software @@ -39,7 +51,7 @@ If you are running Debian, add the following repositories to deb http://ftp.debian.org/debian/ testing main non-free contrib The repositories above allow us to install the packages -`wkthmltopdf-static` and `bundler` using `apt`; so if you're running +`wkhtmltopdf-static` and `bundler` using `apt`; so if you're running Ubuntu, you won't be able to use the above repositories, and you will need to comment out those two lines in `config/packages` before following the next step (and install bundler manually). diff --git a/doc/THEMES.md b/doc/THEMES.md index 6c22764fc..c5e4a3eee 100644 --- a/doc/THEMES.md +++ b/doc/THEMES.md @@ -123,7 +123,7 @@ do this in the `alavetelitheme`. To do add states, create two modules in your theme, `InfoRequestCustomStates` and `RequestControllerCustomStates`. The -former must have these two methods: +former must have these methods: * `theme_calculate_status`: return a tag to identify the current state of the request * `theme_extra_states`: return a list of tags which identify the extra states you'd like to support diff --git a/spec/models/customstates.rb b/spec/models/customstates.rb index 3488e6730..bffbe86fb 100644 --- a/spec/models/customstates.rb +++ b/spec/models/customstates.rb @@ -13,7 +13,7 @@ module InfoRequestCustomStates return 'deadline_extended' if Time.now.strftime("%Y-%m-%d") < self.date_deadline_extended.strftime("%Y-%m-%d") return 'waiting_response_very_overdue' if - Time.now.strftime("%Y-%m-%d") > Holiday.due_date_from(self.date_deadline_extended, 15).strftime("%Y-%m-%d") + Time.now.strftime("%Y-%m-%d") > Holiday.due_date_from_working_days(self.date_deadline_extended, 15).strftime("%Y-%m-%d") return 'waiting_response_overdue' end return 'waiting_response_very_overdue' if @@ -27,7 +27,7 @@ module InfoRequestCustomStates # XXX shouldn't this be 15 days after the date the status was # changed to "deadline extended"? Or perhaps 15 days ater the # initial request due date? - return Holiday.due_date_from(self.date_response_required_by, 15) + return Holiday.due_date_from_working_days(self.date_response_required_by, 15) end module ClassMethods diff --git a/spec/models/holiday_spec.rb b/spec/models/holiday_spec.rb index 00ebc7279..5d3f76d24 100644 --- a/spec/models/holiday_spec.rb +++ b/spec/models/holiday_spec.rb @@ -3,47 +3,84 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe Holiday, " when calculating due date" do def due_date(ymd) - return Holiday.due_date_from(Date.strptime(ymd), 20).strftime("%F") + return Holiday.due_date_from_working_days(Date.strptime(ymd), 20).strftime("%F") end - it "handles no holidays" do - due_date('2008-10-01').should == '2008-10-29' - end + context "in working days" do + it "handles no holidays" do + due_date('2008-10-01').should == '2008-10-29' + end - it "handles non leap years" do - due_date('2007-02-01').should == '2007-03-01' - end + it "handles non leap years" do + due_date('2007-02-01').should == '2007-03-01' + end - it "handles leap years" do - due_date('2008-02-01').should == '2008-02-29' - end + it "handles leap years" do + due_date('2008-02-01').should == '2008-02-29' + end - it "handles Thursday start" do - due_date('2009-03-12').should == '2009-04-14' - end + it "handles Thursday start" do + due_date('2009-03-12').should == '2009-04-14' + end - it "handles Friday start" do - due_date('2009-03-13').should == '2009-04-15' - end + it "handles Friday start" do + due_date('2009-03-13').should == '2009-04-15' + end - # Delivery at the weekend ends up the same due day as if it had arrived on - # the Friday before. This is because the next working day (Monday) counts - # as day 1. - # See http://www.whatdotheyknow.com/help/officers#days - it "handles Saturday start" do - due_date('2009-03-14').should == '2009-04-15' - end - it "handles Sunday start" do - due_date('2009-03-15').should == '2009-04-15' - end + # Delivery at the weekend ends up the same due day as if it had arrived on + # the Friday before. This is because the next working day (Monday) counts + # as day 1. + # See http://www.whatdotheyknow.com/help/officers#days + it "handles Saturday start" do + due_date('2009-03-14').should == '2009-04-15' + end + it "handles Sunday start" do + due_date('2009-03-15').should == '2009-04-15' + end - it "handles Monday start" do - due_date('2009-03-16').should == '2009-04-16' - end + it "handles Monday start" do + due_date('2009-03-16').should == '2009-04-16' + end - it "handles Time objects" do - Holiday.due_date_from(Time.utc(2009, 03, 16, 12, 0, 0), 20).strftime('%F').should == '2009-04-16' + it "handles Time objects" do + Holiday.due_date_from_working_days(Time.utc(2009, 03, 16, 12, 0, 0), 20).strftime('%F').should == '2009-04-16' + end end + context "in calendar days" do + it "handles no holidays" do + Holiday.due_date_from_calendar_days(Date.new(2008, 10, 1), 20).should == Date.new(2008, 10, 21) + end + + it "handles the due date falling on a Friday" do + Holiday.due_date_from_calendar_days(Date.new(2008, 10, 4), 20).should == Date.new(2008, 10, 24) + end + + # If the due date would fall on a Saturday it should in fact fall on the next day that isn't a weekend + # or a holiday + it "handles the due date falling on a Saturday" do + Holiday.due_date_from_calendar_days(Date.new(2008, 10, 5), 20).should == Date.new(2008, 10, 27) + end + + it "handles the due date falling on a Sunday" do + Holiday.due_date_from_calendar_days(Date.new(2008, 10, 6), 20).should == Date.new(2008, 10, 27) + end + + it "handles the due date falling on a Monday" do + Holiday.due_date_from_calendar_days(Date.new(2008, 10, 7), 20).should == Date.new(2008, 10, 27) + end + + it "handles the due date falling on a day before a Holiday" do + Holiday.due_date_from_calendar_days(Date.new(2008, 12, 4), 20).should == Date.new(2008, 12, 24) + end + + it "handles the due date falling on a Holiday" do + Holiday.due_date_from_calendar_days(Date.new(2008, 12, 5), 20).should == Date.new(2008, 12, 29) + end + + it "handles Time objects" do + Holiday.due_date_from_calendar_days(Time.utc(2009, 03, 17, 12, 0, 0), 20).should == Date.new(2009, 4, 6) + end + end end diff --git a/spec/views/request/_after_actions.rhtml_spec.rb b/spec/views/request/_after_actions.rhtml_spec.rb index d04db3fc2..5b4734c52 100644 --- a/spec/views/request/_after_actions.rhtml_spec.rb +++ b/spec/views/request/_after_actions.rhtml_spec.rb @@ -1,85 +1,86 @@ require File.expand_path(File.join('..', '..', '..', 'spec_helper'), __FILE__) -describe 'when displaying actions that can be taken with regard to a request' do - - before do - @mock_body = mock_model(PublicBody, :name => 'test public body', +describe 'when displaying actions that can be taken with regard to a request' do + + before do + @mock_body = mock_model(PublicBody, :name => 'test public body', :url_name => 'test_public_body') - @mock_user = mock_model(User, :name => 'test user', + @mock_user = mock_model(User, :name => 'test user', :url_name => 'test_user') - @mock_request = mock_model(InfoRequest, :title => 'test request', - :user => @mock_user, - :user_name => @mock_user.name, + @mock_request = mock_model(InfoRequest, :title => 'test request', + :user => @mock_user, + :user_name => @mock_user.name, :is_external? => false, - :public_body => @mock_body, + :public_body => @mock_body, + :comments_allowed? => true, :url_title => 'test_request') assigns[:info_request] = @mock_request end - + def do_render render :partial => 'request/after_actions' end - + def expect_owner_div do_render response.should have_tag('div#owner_actions'){ yield } end - + def expect_anyone_div do_render response.should have_tag('div#anyone_actions'){ yield } end - + def expect_owner_link(text) expect_owner_div{ with_tag('a', :text => text) } end - + def expect_no_owner_link(text) expect_owner_div{ without_tag('a', :text => text) } end - + def expect_anyone_link(text) expect_anyone_div{ with_tag('a', :text => text) } end - + def expect_no_anyone_link(text) expect_anyone_div{ without_tag('a', :text => text) } end - - describe 'if the request is old and unclassified' do - - before do + + describe 'if the request is old and unclassified' do + + before do assigns[:old_unclassified] = true end - - it 'should not display a link for the request owner to update the status of the request' do + + it 'should not display a link for the request owner to update the status of the request' do expect_no_owner_link('Update the status of this request') end - - it 'should display a link for anyone to update the status of the request' do + + it 'should display a link for anyone to update the status of the request' do expect_anyone_link('Update the status of this request') end - + end - - describe 'if the request is not old and unclassified' do - - before do + + describe 'if the request is not old and unclassified' do + + before do assigns[:old_unclassified] = false end - - it 'should display a link for the request owner to update the status of the request' do + + it 'should display a link for the request owner to update the status of the request' do expect_owner_link('Update the status of this request') end - - it 'should not display a link for anyone to update the status of the request' do + + it 'should not display a link for anyone to update the status of the request' do expect_no_anyone_link('Update the status of this request') end - + end it 'should display a link for the request owner to request a review' do expect_owner_link('Request an internal review') end - + end |