mysociety / alaveteli

Provide a Freedom of Information request system for your jurisdiction
https://alaveteli.org
Other
389 stars 196 forks source link

`remove_tag` method is broken for `key:value` tags #8239

Open laurentS opened 5 months ago

laurentS commented 5 months ago

The remove_tag method works fine for "name only" tags, but if a value is associated, it does not do what I would expect. See the sample console session below, ir is an InfoRequest but this does not matter:

irb(main):035:0> ir.add_tag_if_not_already_present("somename")
=> " somename"
irb(main):036:0> ir.tag_string
=> "somename"
irb(main):037:0> ir.add_tag_if_not_already_present("othername:othervalue")
=> "somename othername:othervalue"
irb(main):038:0> ir.tag_string
=> "somename othername:othervalue"
irb(main):039:0> ir.remove_tag("othername")
=> "somename :othervalue"  # expected "somename"
irb(main):040:0> ir.add_tag_if_not_already_present("morename:morevalue")
=> "somename :othervalue morename:morevalue"
irb(main):041:0> ir.remove_tag("morename:morevalue")
=> "somename :othervalue morename:morevalue"
irb(main):042:0> ir.tag_array
=> [["somename", nil], ["", "othervalue"], ["morename", "morevalue"]]

I would expect ir.remove_tag("othername") to either remove the tag entirely (with the value), or do nothing. Likewise, ir.remove_tag("morename:morevalue") should remove the key:value pair.

If multiple values exist for a given tag, say "key1:v1 key1:v2" I would expect:

I don't think empty strings should be allowed as tag names by the way, it looks like asking for trouble at some point :)

Let me know if you agree with the above, and I can send a PR.