From a992caf74154abfe540cfdc7f5d15d2c0b63ca45 Mon Sep 17 00:00:00 2001
From: Dan Ungureanu <dan@ungureanu.me>
Date: Mon, 25 Nov 2019 14:32:19 +0200
Subject: [PATCH] DEV: Replace magic values (#8398)

Follow-up to 35942f7c7c9510161c42018543ac609254dafdbd.
---
 .../stylesheets/common/base/topic-post.scss      |  5 ++++-
 app/controllers/posts_controller.rb              |  4 ++--
 app/models/post.rb                               | 16 +++++++++-------
 app/serializers/post_serializer.rb               |  4 ++--
 lib/post_creator.rb                              |  6 +++---
 lib/topic_view.rb                                |  2 +-
 spec/components/post_creator_spec.rb             | 16 ++++++++--------
 spec/models/post_spec.rb                         |  4 ++--
 spec/requests/posts_controller_spec.rb           | 10 +++++-----
 spec/serializers/post_serializer_spec.rb         |  4 ++--
 10 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss
index b2fd2b5db14..42df697e4a0 100644
--- a/app/assets/stylesheets/common/base/topic-post.scss
+++ b/app/assets/stylesheets/common/base/topic-post.scss
@@ -910,7 +910,10 @@ a.mention-group {
   background-color: $tertiary-low;
   border-top: 1px solid $primary-low;
   display: flex;
-  max-width: calc(#{$topic-body-width} + #{$topic-avatar-width} - 0.1em);
+  max-width: calc(
+    #{$topic-body-width} + (#{$topic-body-width-padding} * 2) + #{$topic-avatar-width} -
+      (0.8em * 2)
+  );
   padding: 0.8em;
 
   &.old {
diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index b63d2539fe1..c1ba656186f 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -478,8 +478,8 @@ class PostsController < ApplicationController
     post = find_post_from_params
 
     if params[:notice].present?
-      post.custom_fields["notice_type"] = Post.notices[:custom]
-      post.custom_fields["notice_args"] = PrettyText.cook(params[:notice], features: { onebox: false })
+      post.custom_fields[Post::NOTICE_TYPE] = Post.notices[:custom]
+      post.custom_fields[Post::NOTICE_ARGS] = PrettyText.cook(params[:notice], features: { onebox: false })
       post.save_custom_fields
     else
       post.delete_post_notices
diff --git a/app/models/post.rb b/app/models/post.rb
index 2e0fd828020..a189e57a964 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -54,11 +54,13 @@ class Post < ActiveRecord::Base
   # We can pass several creating options to a post via attributes
   attr_accessor :image_sizes, :quoted_post_numbers, :no_bump, :invalidate_oneboxes, :cooking_options, :skip_unique_check, :skip_validation
 
-  LARGE_IMAGES      ||= "large_images".freeze
-  BROKEN_IMAGES     ||= "broken_images".freeze
-  DOWNLOADED_IMAGES ||= "downloaded_images".freeze
-  MISSING_UPLOADS ||= "missing uploads".freeze
-  MISSING_UPLOADS_IGNORED ||= "missing uploads ignored".freeze
+  LARGE_IMAGES            ||= "large_images"
+  BROKEN_IMAGES           ||= "broken_images"
+  DOWNLOADED_IMAGES       ||= "downloaded_images"
+  MISSING_UPLOADS         ||= "missing uploads"
+  MISSING_UPLOADS_IGNORED ||= "missing uploads ignored"
+  NOTICE_TYPE             ||= "notice_type"
+  NOTICE_ARGS             ||= "notice_args"
 
   SHORT_POST_CHARS ||= 1200
 
@@ -422,8 +424,8 @@ class Post < ActiveRecord::Base
   end
 
   def delete_post_notices
-    self.custom_fields.delete("notice_type")
-    self.custom_fields.delete("notice_args")
+    self.custom_fields.delete(Post::NOTICE_TYPE)
+    self.custom_fields.delete(Post::NOTICE_ARGS)
     self.save_custom_fields
   end
 
diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb
index 0e2a7b30728..8651252e845 100644
--- a/app/serializers/post_serializer.rb
+++ b/app/serializers/post_serializer.rb
@@ -368,7 +368,7 @@ class PostSerializer < BasicPostSerializer
   end
 
   def notice_type
-    post_custom_fields["notice_type"]
+    post_custom_fields[Post::NOTICE_TYPE]
   end
 
   def include_notice_type?
@@ -389,7 +389,7 @@ class PostSerializer < BasicPostSerializer
   end
 
   def notice_args
-    post_custom_fields["notice_args"]
+    post_custom_fields[Post::NOTICE_ARGS]
   end
 
   def include_notice_args?
diff --git a/lib/post_creator.rb b/lib/post_creator.rb
index 1deacf5c296..7359d8ec3ae 100644
--- a/lib/post_creator.rb
+++ b/lib/post_creator.rb
@@ -550,10 +550,10 @@ class PostCreator
       .first
 
     if !last_post_time
-      @post.custom_fields["notice_type"] = Post.notices[:new_user]
+      @post.custom_fields[Post::NOTICE_TYPE] = Post.notices[:new_user]
     elsif SiteSetting.returning_users_days > 0 && last_post_time < SiteSetting.returning_users_days.days.ago
-      @post.custom_fields["notice_type"] = Post.notices[:returning_user]
-      @post.custom_fields["notice_args"] = last_post_time.iso8601
+      @post.custom_fields[Post::NOTICE_TYPE] = Post.notices[:returning_user]
+      @post.custom_fields[Post::NOTICE_ARGS] = last_post_time.iso8601
     end
   end
 
diff --git a/lib/topic_view.rb b/lib/topic_view.rb
index aaa4ccc0868..0f26832202b 100644
--- a/lib/topic_view.rb
+++ b/lib/topic_view.rb
@@ -35,7 +35,7 @@ class TopicView
   end
 
   def self.default_post_custom_fields
-    @default_post_custom_fields ||= ["action_code_who", "notice_type", "notice_args", "requested_group_id"]
+    @default_post_custom_fields ||= [Post::NOTICE_TYPE, Post::NOTICE_ARGS, "action_code_who", "requested_group_id"]
   end
 
   def self.post_custom_fields_whitelisters
diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb
index c33d9b8cbb2..547b38736d0 100644
--- a/spec/components/post_creator_spec.rb
+++ b/spec/components/post_creator_spec.rb
@@ -1358,10 +1358,10 @@ describe PostCreator do
 
     it "generates post notices for new users" do
       post = PostCreator.create!(user, title: "one of my first topics", raw: "one of my first posts")
-      expect(post.custom_fields["notice_type"]).to eq("new_user")
+      expect(post.custom_fields[Post::NOTICE_TYPE]).to eq(Post.notices[:new_user])
 
       post = PostCreator.create!(user, title: "another one of my first topics", raw: "another one of my first posts")
-      expect(post.custom_fields["notice_type"]).to eq(nil)
+      expect(post.custom_fields[Post::NOTICE_TYPE]).to eq(nil)
     end
 
     it "generates post notices for returning users" do
@@ -1369,12 +1369,12 @@ describe PostCreator do
       old_post = Fabricate(:post, user: user, created_at: 31.days.ago)
 
       post = PostCreator.create!(user, title: "this is a returning topic", raw: "this is a post")
-      expect(post.custom_fields["notice_type"]).to eq(Post.notices[:returning_user])
-      expect(post.custom_fields["notice_args"]).to eq(old_post.created_at.iso8601)
+      expect(post.custom_fields[Post::NOTICE_TYPE]).to eq(Post.notices[:returning_user])
+      expect(post.custom_fields[Post::NOTICE_ARGS]).to eq(old_post.created_at.iso8601)
 
       post = PostCreator.create!(user, title: "this is another topic", raw: "this is my another post")
-      expect(post.custom_fields["notice_type"]).to eq(nil)
-      expect(post.custom_fields["notice_args"]).to eq(nil)
+      expect(post.custom_fields[Post::NOTICE_TYPE]).to eq(nil)
+      expect(post.custom_fields[Post::NOTICE_ARGS]).to eq(nil)
     end
 
     it "does not generate for non-human, staged or anonymous users" do
@@ -1383,8 +1383,8 @@ describe PostCreator do
       [anonymous, Discourse.system_user, staged].each do |user|
         expect(user.posts.size).to eq(0)
         post = PostCreator.create!(user, title: "#{user.username}'s first topic", raw: "#{user.name}'s first post")
-        expect(post.custom_fields["notice_type"]).to eq(nil)
-        expect(post.custom_fields["notice_args"]).to eq(nil)
+        expect(post.custom_fields[Post::NOTICE_TYPE]).to eq(nil)
+        expect(post.custom_fields[Post::NOTICE_ARGS]).to eq(nil)
       end
     end
   end
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index e37c3a5e874..72bf2cd0634 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -138,8 +138,8 @@ describe Post do
     context 'a post with notices' do
       let(:post) {
         post = Fabricate(:post, post_args)
-        post.custom_fields["notice_type"] = Post.notices[:returning_user]
-        post.custom_fields["notice_args"] = 1.day.ago
+        post.custom_fields[Post::NOTICE_TYPE] = Post.notices[:returning_user]
+        post.custom_fields[Post::NOTICE_ARGS] = 1.day.ago
         post.save_custom_fields
         post
       }
diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb
index 45e4327f3f8..32a581b16b2 100644
--- a/spec/requests/posts_controller_spec.rb
+++ b/spec/requests/posts_controller_spec.rb
@@ -1852,16 +1852,16 @@ describe PostsController do
 
       expect(response.status).to eq(200)
       public_post.reload
-      expect(public_post.custom_fields['notice_type']).to eq(Post.notices[:custom])
-      expect(public_post.custom_fields['notice_args']).to include('<p>Hello <em>world</em>!</p>')
-      expect(public_post.custom_fields['notice_args']).not_to include('onebox')
+      expect(public_post.custom_fields[Post::NOTICE_TYPE]).to eq(Post.notices[:custom])
+      expect(public_post.custom_fields[Post::NOTICE_ARGS]).to include('<p>Hello <em>world</em>!</p>')
+      expect(public_post.custom_fields[Post::NOTICE_ARGS]).not_to include('onebox')
 
       put "/posts/#{public_post.id}/notice.json", params: { notice: nil }
 
       expect(response.status).to eq(200)
       public_post.reload
-      expect(public_post.custom_fields['notice_type']).to eq(nil)
-      expect(public_post.custom_fields['notice_args']).to eq(nil)
+      expect(public_post.custom_fields[Post::NOTICE_TYPE]).to eq(nil)
+      expect(public_post.custom_fields[Post::NOTICE_ARGS]).to eq(nil)
     end
   end
 end
diff --git a/spec/serializers/post_serializer_spec.rb b/spec/serializers/post_serializer_spec.rb
index cd21e838aba..1032909aad3 100644
--- a/spec/serializers/post_serializer_spec.rb
+++ b/spec/serializers/post_serializer_spec.rb
@@ -207,8 +207,8 @@ describe PostSerializer do
 
     let(:post) {
       post = Fabricate(:post, user: user)
-      post.custom_fields["notice_type"] = Post.notices[:returning_user]
-      post.custom_fields["notice_args"] = 1.day.ago
+      post.custom_fields[Post::NOTICE_TYPE] = Post.notices[:returning_user]
+      post.custom_fields[Post::NOTICE_ARGS] = 1.day.ago
       post.save_custom_fields
       post
     }