From 64f13c05a5422d3a079a4b35efa56da8ddf5c2ee Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 28 May 2014 10:20:09 +0100 Subject: Minor tidying of PublicBodyController#list Spacing, parenthesis, etc --- app/controllers/public_body_controller.rb | 38 +++++++++++++++---------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'app/controllers/public_body_controller.rb') diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 862f4b318..51e2d5d22 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -109,17 +109,17 @@ class PublicBodyController < ApplicationController # Restrict the public bodies shown according to the tag # parameter supplied in the URL: - if @tag.nil? or @tag == "all" - @tag = "all" + if @tag.nil? || @tag == 'all' + @tag = 'all' elsif @tag == 'other' - category_list = PublicBodyCategories::get().tags().map{|c| "'"+c+"'"}.join(",") + category_list = PublicBodyCategories.get.tags.map{ |c| %Q('#{ c }') }.join(",") where_condition += base_tag_condition + " AND has_tag_string_tags.name in (#{category_list})) = 0" elsif @tag.scan(/./mu).size == 1 - @tag = Unicode.upcase @tag + @tag = Unicode.upcase(@tag) # The first letter queries have to be done on # translations, so just indicate to add that later: first_letter = true - elsif @tag.include?(":") + elsif @tag.include?(':') name, value = HasTagString::HasTagStringTag.split_tag_into_name_value(@tag) where_condition += base_tag_condition + " AND has_tag_string_tags.name = ? AND has_tag_string_tags.value = ?) > 0" where_parameters.concat [name, value] @@ -128,16 +128,16 @@ class PublicBodyController < ApplicationController where_parameters.concat [@tag] end - if @tag == "all" - @description = "" + if @tag == 'all' + @description = '' elsif @tag.size == 1 - @description = _("beginning with ‘{{first_letter}}’", :first_letter=>@tag) + @description = _("beginning with ‘{{first_letter}}’", :first_letter => @tag) else - category_name = PublicBodyCategories::get().by_tag()[@tag] + category_name = PublicBodyCategories.get.by_tag[@tag] if category_name.nil? - @description = _("matching the tag ‘{{tag_name}}’", :tag_name=>@tag) + @description = _("matching the tag ‘{{tag_name}}’", :tag_name => @tag) else - @description = _("in the category ‘{{category_name}}’", :category_name=>category_name) + @description = _("in the category ‘{{category_name}}’", :category_name => category_name) end end @@ -151,11 +151,11 @@ class PublicBodyController < ApplicationController FROM public_bodies LEFT OUTER JOIN public_body_translations as current_locale ON (public_bodies.id = current_locale.public_body_id - AND current_locale.locale = ? AND #{get_public_body_list_translated_condition 'current_locale', first_letter}) + AND current_locale.locale = ? AND #{ get_public_body_list_translated_condition('current_locale', first_letter) }) LEFT OUTER JOIN public_body_translations as default_locale ON (public_bodies.id = default_locale.public_body_id - AND default_locale.locale = ? AND #{get_public_body_list_translated_condition 'default_locale', first_letter}) - WHERE #{where_condition} AND COALESCE(current_locale.name, default_locale.name) IS NOT NULL + AND default_locale.locale = ? AND #{ get_public_body_list_translated_condition('default_locale', first_letter) }) + WHERE #{ where_condition } AND COALESCE(current_locale.name, default_locale.name) IS NOT NULL ORDER BY display_name} sql = [query, underscore_locale, like_query, like_query] sql.push @tag if first_letter @@ -173,14 +173,14 @@ class PublicBodyController < ApplicationController where_sql = [where_condition, like_query, like_query] where_sql.push @tag if first_letter where_sql += [underscore_locale] + where_parameters - @public_bodies = PublicBody.where(where_sql) \ - .joins(:translations) \ - .order("public_body_translations.name") \ - .paginate(:page => params[:page], :per_page => 100) + @public_bodies = PublicBody.where(where_sql). + joins(:translations). + order("public_body_translations.name"). + paginate(:page => params[:page], :per_page => 100) end respond_to do |format| - format.html { render :template => "public_body/list" } + format.html { render :template => 'public_body/list' } end end end -- cgit v1.2.3 From 96aed851f2d94f95591b9a8180da9d3aa1c57f0a Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 28 May 2014 10:39:27 +0100 Subject: Support simple searching of bodies by short_name --- app/controllers/public_body_controller.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'app/controllers/public_body_controller.rb') diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 51e2d5d22..96e69d333 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -157,9 +157,9 @@ class PublicBodyController < ApplicationController AND default_locale.locale = ? AND #{ get_public_body_list_translated_condition('default_locale', first_letter) }) WHERE #{ where_condition } AND COALESCE(current_locale.name, default_locale.name) IS NOT NULL ORDER BY display_name} - sql = [query, underscore_locale, like_query, like_query] + sql = [query, underscore_locale, like_query, like_query, like_query] sql.push @tag if first_letter - sql += [underscore_default_locale, like_query, like_query] + sql += [underscore_default_locale, like_query, like_query, like_query] sql.push @tag if first_letter sql += where_parameters @public_bodies = PublicBody.paginate_by_sql( @@ -170,7 +170,7 @@ class PublicBodyController < ApplicationController # The simpler case where we're just searching in the current locale: where_condition = get_public_body_list_translated_condition('public_body_translations', first_letter, true) + ' AND ' + where_condition - where_sql = [where_condition, like_query, like_query] + where_sql = [where_condition, like_query, like_query, like_query] where_sql.push @tag if first_letter where_sql += [underscore_locale] + where_parameters @public_bodies = PublicBody.where(where_sql). @@ -344,9 +344,11 @@ class PublicBodyController < ApplicationController end private + def get_public_body_list_translated_condition(table, first_letter=false, locale=nil) result = "(upper(#{table}.name) LIKE upper(?)" \ - " OR upper(#{table}.notes) LIKE upper (?))" + " OR upper(#{table}.notes) LIKE upper(?)" \ + " OR upper(#{table}.short_name) LIKE upper(?))" if first_letter result += " AND #{table}.first_letter = ?" end -- cgit v1.2.3 From b2219112ad185d4e6b07ad439da9eabd4907e32d Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 7 May 2014 17:14:51 +0100 Subject: Remove FasterCSV dependency The last remaining usage was removed in c1ee22fe --- app/controllers/public_body_controller.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'app/controllers/public_body_controller.rb') diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 96e69d333..9f2edd0df 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -5,7 +5,6 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: hello@mysociety.org; WWW: http://www.mysociety.org/ -require 'fastercsv' require 'confidence_intervals' require 'tempfile' -- cgit v1.2.3 From 468e9d5dfbbf2896b97553b8bf145acd0e8b95ca Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Thu, 8 May 2014 16:07:19 +0100 Subject: Extract public body CSV export to its own class - SRP: Do one thing. PublicBodyCSV converts a collection of bodies in to a CSV formatted String - Adds some parenthesis around parameters in PublicBodyController#list_all_csv - Let the controller handle what records to pull out for the CSV export Arguably this doesn't really need to be anything to do with PublicBody, but it allows us to set nice defaults. --- app/controllers/public_body_controller.rb | 33 ++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) (limited to 'app/controllers/public_body_controller.rb') diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 9f2edd0df..9e34396bc 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -190,6 +190,9 @@ class PublicBodyController < ApplicationController redirect_to list_public_bodies_url(:tag => @tag) end + # GET /body/all-authorities.csv + # + # Returns all public bodies (except for the internal admin authority) as CSV def list_all_csv # FIXME: this is just using the download directory for zip # archives, since we know that is allowed for X-Sendfile and @@ -197,21 +200,29 @@ class PublicBodyController < ApplicationController # used for the zips. However, really there should be a # generically named downloads directory that contains all # kinds of downloadable assets. - download_directory = File.join(InfoRequest.download_zip_dir(), - 'download') - FileUtils.mkdir_p download_directory + download_directory = File.join(InfoRequest.download_zip_dir, 'download') + FileUtils.mkdir_p(download_directory) output_leafname = 'all-authorities.csv' - output_filename = File.join download_directory, output_leafname + output_filename = File.join(download_directory, output_leafname) # Create a temporary file in the same directory, so we can # rename it atomically to the intended filename: - tmp = Tempfile.new output_leafname, download_directory + tmp = Tempfile.new(output_leafname, download_directory) tmp.close - # Export all the public bodies to that temporary path and make - # it readable: - PublicBody.export_csv tmp.path - FileUtils.chmod 0644, tmp.path - # Rename into place and send the file: - File.rename tmp.path, output_filename + + # Create the CSV + csv = PublicBodyCSV.new + PublicBody.visible.find_each(:include => [:translations, :tags]) do |public_body| + next if public_body.has_tag?('site_administration') + csv << public_body + end + + # Export all the public bodies to that temporary path, make it readable, + # and rename it + File.open(tmp.path, 'w') { |file| file.write(csv.generate) } + FileUtils.chmod(0644, tmp.path) + File.rename(tmp.path, output_filename) + + # Send the file send_file(output_filename, :type => 'text/csv; charset=utf-8; header=present', :filename => 'all-authorities.csv', -- cgit v1.2.3 From 249a1ef17747deadc773b6474df990d803294c44 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 4 Jun 2014 12:15:32 +0100 Subject: Move PublicBody domain logic from controller Moves the magic 'site_administration' tag logic to the PublicBody model. Easier to make the string passed to `PublicBody#has_tag?` configurable if we want to allow this to be set per install. --- app/controllers/public_body_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/controllers/public_body_controller.rb') diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 9e34396bc..bd019a613 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -212,8 +212,8 @@ class PublicBodyController < ApplicationController # Create the CSV csv = PublicBodyCSV.new PublicBody.visible.find_each(:include => [:translations, :tags]) do |public_body| - next if public_body.has_tag?('site_administration') - csv << public_body + next if public_body.site_administration? + csv << public_body end # Export all the public bodies to that temporary path, make it readable, -- cgit v1.2.3 From 42870985da06418461c847563fa860c11b4094a1 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 10 Jun 2014 10:04:29 +0100 Subject: Rename XXX comments with TODO: Picks these up in `rake notes` and adds semantic meaning --- app/controllers/public_body_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'app/controllers/public_body_controller.rb') diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 96e69d333..f909a5989 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -10,7 +10,7 @@ require 'confidence_intervals' require 'tempfile' class PublicBodyController < ApplicationController - # XXX tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL + # TODO: tidy this up with better error messages, and a more standard infrastructure for the redirect to canonical URL def show long_cache if MySociety::Format.simplify_url_part(params[:url_name], 'body') != params[:url_name] @@ -43,7 +43,7 @@ class PublicBodyController < ApplicationController query = InfoRequestEvent.make_query_from_params(params.merge(:latest_status => @view)) query += " requested_from:#{@public_body.url_name}" # Use search query for this so can collapse and paginate easily - # XXX really should just use SQL query here rather than Xapian. + # TODO: really should just use SQL query here rather than Xapian. sortby = "described" begin @xapian_requests = perform_search([InfoRequestEvent], query, sortby, 'request_collapse') @@ -86,7 +86,7 @@ class PublicBodyController < ApplicationController def list long_cache - # XXX move some of these tag SQL queries into has_tag_string.rb + # TODO: move some of these tag SQL queries into has_tag_string.rb like_query = params[:public_body_query] like_query = "" if like_query.nil? -- cgit v1.2.3