diff options
| -rw-r--r-- | app/models/public_body.rb | 16 | ||||
| -rw-r--r-- | app/views/public_body/show.rhtml | 10 | ||||
| -rw-r--r-- | db/migrate/090_remove_tag_uniqueness.rb | 16 | ||||
| -rw-r--r-- | db/schema.rb | 4 | ||||
| -rw-r--r-- | spec/models/public_body_spec.rb | 16 |
5 files changed, 49 insertions, 13 deletions
diff --git a/app/models/public_body.rb b/app/models/public_body.rb index e0d02a5b6..ae1a38723 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -216,13 +216,23 @@ class PublicBody < ActiveRecord::Base end return false end - def get_tag_value(tag) + class TagNotFound < StandardError + end + def get_tag_values(tag) + found = false + results = [] for public_body_tag in self.public_body_tags if public_body_tag.name == tag - return public_body_tag.value + found = true + if !public_body_tag.value.nil? + results << public_body_tag.value + end end end - return false + if !found + raise TagNotFound + end + return results end def add_tag_if_not_already_present(tag) self.tag_string = self.tag_string + " " + tag diff --git a/app/views/public_body/show.rhtml b/app/views/public_body/show.rhtml index e2ea625cd..e6fe61a0a 100644 --- a/app/views/public_body/show.rhtml +++ b/app/views/public_body/show.rhtml @@ -11,10 +11,12 @@ <%= link_to "Publication scheme", @public_body.publication_scheme %><br> <% end %> <% if @public_body.has_tag?("charity") %> - <% if @public_body.get_tag_value("charity").match(/^SC/) %> - <%= link_to "Charity registration", "http://www.oscr.org.uk/CharityIndexDetails.aspx?id=" + @public_body.get_tag_value("charity") %><br> - <% else %> - <%= link_to "Charity registration", "http://www.charity-commission.gov.uk/SHOWCHARITY/RegisterOfCharities/CharityFramework.aspx?RegisteredCharityNumber=" + @public_body.get_tag_value("charity") %><br> + <% for tag_value in @public_body.get_tag_values("charity") %> + <% if tag_value.match(/^SC/) %> + <%= link_to "Charity registration", "http://www.oscr.org.uk/CharityIndexDetails.aspx?id=" + tag_value %><br> + <% else %> + <%= link_to "Charity registration", "http://www.charity-commission.gov.uk/SHOWCHARITY/RegisterOfCharities/CharityFramework.aspx?RegisteredCharityNumber=" + tag_value %><br> + <% end %> <% end %> <% end %> <%= link_to "View FOI email address", view_public_body_email_url(@public_body.url_name) %><br> diff --git a/db/migrate/090_remove_tag_uniqueness.rb b/db/migrate/090_remove_tag_uniqueness.rb new file mode 100644 index 000000000..1c06de439 --- /dev/null +++ b/db/migrate/090_remove_tag_uniqueness.rb @@ -0,0 +1,16 @@ +class RemoveTagUniqueness < ActiveRecord::Migration + def self.up + # MySQL cannot index text blobs like this + # XXX perhaps should change :name/:value to be a :string + if ActiveRecord::Base.connection.adapter_name != "MySQL" + remove_index :public_body_tags, [:public_body_id, :name] + # allow the key to repeat, but not the value also + add_index :public_body_tags, [:public_body_id, :name, :value], :unique => true + end + end + + def self.down + raise "No reverse migration" + end +end + diff --git a/db/schema.rb b/db/schema.rb index 7ac84dd30..c79ef31c9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -9,7 +9,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 89) do +ActiveRecord::Schema.define(:version => 90) do create_table "acts_as_xapian_jobs", :force => true do |t| t.string "model", :null => false @@ -175,7 +175,7 @@ ActiveRecord::Schema.define(:version => 89) do t.text "value" end - add_index "public_body_tags", ["name", "public_body_id"], :name => "index_public_body_tags_on_public_body_id_and_name", :unique => true + add_index "public_body_tags", ["name", "public_body_id", "value"], :name => "index_public_body_tags_on_public_body_id_and_name_and_value", :unique => true add_index "public_body_tags", ["name"], :name => "index_public_body_tags_on_name" create_table "public_body_versions", :force => true do |t| diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index d99fec40d..441a45b24 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -71,10 +71,12 @@ describe PublicBody, " using machine tags" do @public_body.has_tag?('cheese:green').should be_false @public_body.has_tag?('cheese').should be_true - @public_body.get_tag_value('cheese').should == 'green' + @public_body.get_tag_values('cheese').should == ['green'] - @public_body.get_tag_value('wondrous').should == nil - @public_body.get_tag_value('notthere').should == false + @public_body.get_tag_values('wondrous').should == [] + lambda { + @public_body.get_tag_values('notthere').should raise_error(PublicBody::TagNotFound) + } end it 'should cope with colons in value' do @@ -82,7 +84,13 @@ describe PublicBody, " using machine tags" do @public_body.tag_string.should == 'url:http://www.flourish.org' @public_body.has_tag?('url').should be_true - @public_body.get_tag_value('url').should == 'http://www.flourish.org' + @public_body.get_tag_values('url').should == ['http://www.flourish.org'] + end + + it 'should allow multiple tags of the same sort' do + @public_body.tag_string = 'url:http://www.theyworkforyou.com/ url:http://www.fixmystreet.com/' + @public_body.has_tag?('url').should be_true + @public_body.get_tag_values('url').should == ['http://www.theyworkforyou.com/', 'http://www.fixmystreet.com/'] end end |