Update / test remove MP letters task

There were a few edge cases which came up when I ran this task last night:

1) MySQL LIKE statements are case insensitive, so the query found things like "Francis Implementation Team" (as it's got a lower case mp in).
2) Even if it was case sensitive, %MP% would match people with MPhil degrees.
3) The `puts` statement being outside of the if statement caused some confusion - it would log things as updated that matched the query, but not the ruby gsub, even though they hadn't been updated.

I considered making the MySQL query stricter, but then we might actually miss things (like someone with MP in their letters, but separated with a comma, or lower case). Instead, I've just made it clear which ones are being skipped in the logs.

While writing tests for this, I also noticed an edge case where it would replace letters like "CBE MP VC" with "CBEVC" (i.e. it would remove both surrounding spaces from MP). Better to do it with split / join to avoid this I think.

lib/tasks/election.rake

@@ -3,12 +3,14 @@ namespace :election do
   task remove_mp_letters: :environment do
     puts "Removing MP from MP's letters:"
     Person.where('letters LIKE "%MP%"').find_each do |person|
-      puts "updating #{person.name}"
-      new_letters = person.letters.gsub(/(^|\s)MP(\s|$)/, "")
-      if person.letters != new_letters
+      new_letters = person.letters.split(" ").reject { _1 == "MP" }.join(" ")
+      if person.letters == new_letters
+        puts "skipped #{person.name} - includes MP (case insensitive), but doesn't match exactly"
+      else
+        old_name = person.name
         person.update!(letters: new_letters)
+        puts "updated #{old_name} to #{person.name}"
       end
-      puts "changed to #{person.name}"
     end
   end
 

test/unit/lib/tasks/election_remove_mp_letters_test.rb

@@ -0,0 +1,34 @@
+require "test_helper"
+require "rake"
+
+class RemoveMpLettersRake < ActiveSupport::TestCase
+  extend Minitest::Spec::DSL
+
+  teardown { task.reenable }
+
+  describe "#remove_mp_letters" do
+    let(:task) { Rake::Task["election:remove_mp_letters"] }
+
+    it "removes the letters MP from people names" do
+      member_a = create(:person, forename: "A", surname: "Member", letters: "MP")
+      member_b = create(:person, forename: "B", surname: "Member", letters: "CBE MP")
+      member_c = create(:person, forename: "C", surname: "Member", letters: "MP MEng")
+      member_d = create(:person, forename: "D", surname: "Member", letters: "CBE MP MPhil")
+      non_member_a = create(:person, forename: "A", surname: "Non-Member", letters: "CBE MPhil VC")
+
+      out, _err = capture_io { task.invoke }
+
+      assert_match /updated A Member MP to A Member/, out
+      assert_match /updated B Member CBE MP to B Member CBE/, out
+      assert_match /updated C Member MP MEng to C Member MEng/, out
+      assert_match /updated D Member CBE MP MPhil to D Member CBE MPhil/, out
+      assert_match /skipped A Non-Member/, out
+
+      assert_equal "A Member", member_a.reload.name
+      assert_equal "B Member CBE", member_b.reload.name
+      assert_equal "C Member MEng", member_c.reload.name
+      assert_equal "D Member CBE MPhil", member_d.reload.name
+      assert_equal "A Non-Member CBE MPhil VC", non_member_a.reload.name
+    end
+  end
+end