FIX: should not be able to rename a system badge

app/models/badge.rb

@@ -4,6 +4,11 @@ class Badge < ActiveRecord::Base
   # NOTE: These badge ids are not in order! They are grouped logically.
   #       When picking an id, *search* for it.
 
+  BasicUser = 1
+  Member = 2
+  Regular = 3
+  Leader = 4
+
   Welcome = 5
   NicePost = 6
   GoodPost = 7
@@ -100,7 +105,7 @@ class Badge < ActiveRecord::Base
   # fields that can not be edited on system badges
   def self.protected_system_fields
     [
-      :badge_type_id, :multiple_grant,
+      :name, :badge_type_id, :multiple_grant,
       :target_posts, :show_posts, :query,
       :trigger, :auto_revoke, :listable
     ]
@@ -121,6 +126,18 @@ class Badge < ActiveRecord::Base
     }
   end
 
+  def self.ensure_consistency!
+    exec_sql <<-SQL.squish
+      DELETE FROM user_badges
+            USING user_badges ub
+       LEFT JOIN users u ON u.id = ub.user_id
+           WHERE u.id IS NULL
+           AND user_badges.id = ub.id
+    SQL
+
+    Badge.find_each(&:reset_grant_count!)
+  end
+
   def awarded_for_trust_level?
     id <= 4
   end
@@ -137,14 +154,10 @@ class Badge < ActiveRecord::Base
   def default_icon=(val)
     unless self.image
       self.icon ||= val
-      self.icon = val if self.icon = "fa-certificate"
+      self.icon = val if self.icon == "fa-certificate"
     end
   end
 
-  def default_name=(val)
-    self.name ||= val
-  end
-
   def default_allow_title=(val)
     self.allow_title ||= val
   end
@@ -156,17 +169,6 @@ class Badge < ActiveRecord::Base
     end
   end
 
-  def self.ensure_consistency!
-    exec_sql <<SQL
-    DELETE FROM user_badges
-    USING user_badges ub
-    LEFT JOIN users u ON u.id = ub.user_id
-    WHERE u.id IS NULL AND user_badges.id = ub.id
-SQL
-
-    Badge.find_each(&:reset_grant_count!)
-  end
-
   def display_name
     key = "badges.#{i18n_name}.name"
     I18n.t(key, default: self.name)
@@ -178,10 +180,7 @@ SQL
   end
 
   def long_description=(val)
-    if val != long_description
+    self[:long_description] = val if val != long_description
-      self[:long_description] = val
-    end
-
     val
   end
 
@@ -191,10 +190,7 @@ SQL
   end
 
   def description=(val)
-    if val != description
+    self[:description] = val if val != description
-      self[:description] = val
-    end
-
     val
   end
 
@@ -206,9 +202,7 @@ SQL
   protected
 
     def ensure_not_system
-    unless id
+      self.id = [Badge.maximum(:id) + 1, 100].max unless id
-      self.id = [Badge.maximum(:id) + 1, 100].max
-    end
     end
 
     def i18n_name

db/fixtures/006_badges.rb

@@ -31,32 +31,31 @@ BadgeGrouping.seed do |g|
 end
 
 # BUGFIX
-Badge.exec_sql "UPDATE badges
+Badge.exec_sql <<-SQL.squish
+  UPDATE badges
      SET badge_grouping_id = -1
    WHERE NOT EXISTS (
-                  SELECT 1 FROM badge_groupings g
+            SELECT 1
+              FROM badge_groupings g
              WHERE g.id = badge_grouping_id
-                ) OR (id < 100 AND badge_grouping_id = #{BadgeGrouping::Other} )"
+         ) OR (id < 100 AND badge_grouping_id = #{BadgeGrouping::Other})
+SQL
 
-# Trust level system badges.
+[
-trust_level_badges = [
+  [Badge::BasicUser, "Basic User", BadgeType::Bronze],
-  {id: 1, name: "Basic User", type: BadgeType::Bronze},
+  [Badge::Member,    "Member",     BadgeType::Bronze],
-  {id: 2, name: "Member", type: BadgeType::Bronze},
+  [Badge::Regular,   "Regular",    BadgeType::Silver],
-  {id: 3, name: "Regular", type: BadgeType::Silver},
+  [Badge::Leader,    "Leader",     BadgeType::Gold],
-  {id: 4, name: "Leader", type: BadgeType::Gold}
+].each do |id, name, type|
-]
-
-trust_level_badges.each do |spec|
   Badge.seed do |b|
-    b.id = spec[:id]
+    b.id = id
-    b.default_name = spec[:name]
+    b.name = name
-    b.badge_type_id = spec[:type]
+    b.badge_type_id = type
-    b.query = BadgeQueries.trust_level(spec[:id])
+    b.query = BadgeQueries.trust_level(id)
     b.default_badge_grouping_id = BadgeGrouping::TrustLevel
     b.trigger = Badge::Trigger::TrustLevelChange
-
     # allow title for tl3 and above
-    b.default_allow_title = spec[:id] > 2
+    b.default_allow_title = id > 2
     b.default_icon = "fa-user"
     b.system = true
   end
@@ -64,7 +63,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::Reader
-  b.default_name = "Reader"
+  b.name = "Reader"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = false
@@ -77,7 +76,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::ReadGuidelines
-  b.default_name = "Read Guidelines"
+  b.name = "Read Guidelines"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = false
@@ -90,7 +89,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstLink
-  b.default_name = "First Link"
+  b.name = "First Link"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -103,7 +102,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstQuote
-  b.default_name = "First Quote"
+  b.name = "First Quote"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -116,7 +115,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstLike
-  b.default_name = "First Like"
+  b.name = "First Like"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -129,7 +128,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstFlag
-  b.default_name = "First Flag"
+  b.name = "First Flag"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -149,7 +148,7 @@ end
 ].each do |id, name, type, count, trust_level|
   Badge.seed do |b|
     b.id = id
-    b.default_name = name
+    b.name = name
     b.default_icon = "fa-user-plus"
     b.badge_type_id = type
     b.multiple_grant = false
@@ -166,7 +165,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstShare
-  b.default_name = "First Share"
+  b.name = "First Share"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -182,12 +181,10 @@ end
  [Badge::NiceShare, "Nice Share", BadgeType::Bronze, 25],
  [Badge::GoodShare, "Good Share", BadgeType::Silver, 300],
  [Badge::GreatShare, "Great Share", BadgeType::Gold, 1000],
-].each do |spec|
+].each do |id, name, level, count|
-
-  id, name, level, count = spec
   Badge.seed do |b|
     b.id = id
-    b.default_name = name
+    b.name = name
     b.badge_type_id = level
     b.multiple_grant = true
     b.target_posts = true
@@ -202,7 +199,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::Welcome
-  b.default_name = "Welcome"
+  b.name = "Welcome"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -215,7 +212,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::Autobiographer
-  b.default_name = "Autobiographer"
+  b.name = "Autobiographer"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.query = BadgeQueries::Autobiographer
@@ -226,7 +223,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::Editor
-  b.default_name = "Editor"
+  b.name = "Editor"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.query = BadgeQueries::Editor
@@ -236,26 +233,22 @@ Badge.seed do |b|
   b.system = true
 end
 
-#
+[
-# Like system badges.
+  [Badge::NicePost,   "Nice Post",   BadgeType::Bronze, false],
-like_badges = [
+  [Badge::GoodPost,   "Good Post",   BadgeType::Silver, false],
-  {id: Badge::NicePost, name: "Nice Post", type: BadgeType::Bronze},
+  [Badge::GreatPost,  "Great Post",  BadgeType::Gold,   false],
-  {id: Badge::GoodPost, name: "Good Post", type: BadgeType::Silver},
+  [Badge::NiceTopic,  "Nice Topic",  BadgeType::Bronze, true],
-  {id: Badge::GreatPost, name: "Great Post", type: BadgeType::Gold},
+  [Badge::GoodTopic,  "Good Topic",  BadgeType::Silver, true],
-  {id: Badge::NiceTopic, name: "Nice Topic", type: BadgeType::Bronze, topic: true},
+  [Badge::GreatTopic, "Great Topic", BadgeType::Gold,   true],
-  {id: Badge::GoodTopic, name: "Good Topic", type: BadgeType::Silver, topic: true},
+].each do |id, name, type, topic|
-  {id: Badge::GreatTopic, name: "Great Topic", type: BadgeType::Gold, topic: true}
-]
-
-like_badges.each do |spec|
   Badge.seed do |b|
-    b.id = spec[:id]
+    b.id = id
-    b.default_name = spec[:name]
+    b.name = name
-    b.badge_type_id = spec[:type]
+    b.badge_type_id = type
     b.multiple_grant = true
     b.target_posts = true
     b.show_posts = true
-    b.query = BadgeQueries.like_badge(Badge.like_badge_counts[spec[:id]], spec[:topic])
+    b.query = BadgeQueries.like_badge(Badge.like_badge_counts[id], topic)
     b.default_badge_grouping_id = BadgeGrouping::Posting
     b.trigger = Badge::Trigger::PostAction
     b.system = true
@@ -264,7 +257,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::OneYearAnniversary
-  b.default_name = "Anniversary"
+  b.name = "Anniversary"
   b.default_icon = "fa-clock-o"
   b.badge_type_id = BadgeType::Silver
   b.query = BadgeQueries::OneYearAnniversary
@@ -278,11 +271,10 @@ end
  [Badge::PopularLink, "Popular Link", BadgeType::Bronze, 50],
  [Badge::HotLink,     "Hot Link",     BadgeType::Silver, 300],
  [Badge::FamousLink,  "Famous Link",  BadgeType::Gold,   1000],
-].each do |spec|
+].each do |id, name, level, count|
-  id, name, level, count = spec
   Badge.seed do |b|
     b.id = id
-    b.default_name = name
+    b.name = name
     b.badge_type_id = level
     b.multiple_grant = true
     b.target_posts = true
@@ -300,12 +292,10 @@ end
   [Badge::Appreciated, "Appreciated", BadgeType::Bronze, 1, 20],
   [Badge::Respected,   "Respected",   BadgeType::Silver, 2, 100],
   [Badge::Admired,     "Admired",     BadgeType::Gold,   5, 300],
-].each do |spec|
+].each do |id, name, level, like_count, post_count|
-  id, name, level, like_count, post_count = spec
   Badge.seed do |b|
     b.id = id
     b.name = name
-    b.default_name = name
     b.default_icon = "fa-heart"
     b.badge_type_id = level
     b.query = BadgeQueries.liked_posts(post_count, like_count)
@@ -321,11 +311,10 @@ end
   [Badge::ThankYou,   "Thank You",  BadgeType::Bronze, 20, 10],
   [Badge::GivesBack,  "Gives Back", BadgeType::Silver, 100, 100],
   [Badge::Empathetic, "Empathetic", BadgeType::Gold,   500, 1000]
-].each do |spec|
+].each do |id, name, level, count, ratio|
-  id, name, level, count, ratio = spec
   Badge.seed do |b|
     b.id = id
-    b.default_name = name
+    b.name = name
     b.default_icon = "fa-heart"
     b.badge_type_id = level
     b.query = BadgeQueries.liked_back(count, ratio)
@@ -341,12 +330,10 @@ end
   [Badge::OutOfLove,   "Out of Love",   BadgeType::Bronze, 1],
   [Badge::HigherLove,  "Higher Love",   BadgeType::Silver, 5],
   [Badge::CrazyInLove, "Crazy in Love", BadgeType::Gold,   20],
-].each do |spec|
+].each do |id, name, level, count|
-  id, name, level, count = spec
   Badge.seed do |b|
     b.id = id
     b.name = name
-    b.default_name = name
     b.default_icon = "fa-heart"
     b.badge_type_id = level
     b.query = BadgeQueries.like_rate_limit(count)
@@ -360,7 +347,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstMention
-  b.default_name = "First Mention"
+  b.name = "First Mention"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -374,7 +361,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstEmoji
-  b.default_name = "First Emoji"
+  b.name = "First Emoji"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -388,7 +375,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstOnebox
-  b.default_name = "First Onebox"
+  b.name = "First Onebox"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true
@@ -402,7 +389,7 @@ end
 
 Badge.seed do |b|
   b.id = Badge::FirstReplyByEmail
-  b.default_name = "First Reply By Email"
+  b.name = "First Reply By Email"
   b.badge_type_id = BadgeType::Bronze
   b.multiple_grant = false
   b.target_posts = true

spec/controllers/admin/badges_controller_spec.rb

@@ -76,6 +76,19 @@ describe Admin::BadgesController do
 
     context '.update' do
 
+      it 'does not update the name of system badges' do
+        editor_badge = Badge.find(Badge::Editor)
+        editor_badge_name = editor_badge.name
+
+        xhr :put, :update,
+            id: editor_badge.id,
+            name: "123456"
+
+        expect(response).to be_success
+        editor_badge.reload
+        expect(editor_badge.name).to eq(editor_badge_name)
+      end
+
       it 'does not allow query updates if badge_sql is disabled' do
         badge.query = "select 123"
         badge.save