From 1fc17a8a43a87af89358953364872d565d38b8e8 Mon Sep 17 00:00:00 2001
From: Jack Davison <jack.davison@student.manchester.ac.uk>
Date: Thu, 23 Jun 2016 20:02:11 +0100
Subject: [PATCH] Switch to using to_sentence to construct tooltips

* Code in ruby now uses Array#to_sentence to construct award tooltips

* Coffeescript uses a combination of regexes for the same result
---
 app/assets/javascripts/awards_handler.js | 19 ++++++++++----
 app/helpers/issues_helper.rb             |  4 +--
 spec/helpers/issues_helper_spec.rb       |  2 +-
 spec/javascripts/awards_handler_spec.js  | 33 +++++++++++++++++++-----
 4 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js
index c753972d171..b06bad78305 100644
--- a/app/assets/javascripts/awards_handler.js
+++ b/app/assets/javascripts/awards_handler.js
@@ -1,5 +1,6 @@
 (function() {
   this.AwardsHandler = (function() {
+    const FROM_SENTENCE_REGEX = /(?:, and | and |, )/; //For separating lists produced by ruby's Array#toSentence
     function AwardsHandler() {
       this.aliases = gl.emojiAliases();
       $(document).off('click', '.js-add-award').on('click', '.js-add-award', (function(_this) {
@@ -204,14 +205,22 @@
       return $awardBlock.attr('data-original-title') || $awardBlock.attr('data-title') || '';
     };
 
+    AwardsHandler.prototype.toSentence = function(list) {
+      if(list.length <= 2){
+        return list.join(' and ');
+      }
+      else{
+        return list.slice(0, -1).join(', ') + ', and ' + list[list.length - 1];
+      }
+    };
+
     AwardsHandler.prototype.removeMeFromUserList = function($emojiButton, emoji) {
       var authors, awardBlock, newAuthors, originalTitle;
       awardBlock = $emojiButton;
       originalTitle = this.getAwardTooltip(awardBlock);
-      authors = originalTitle.split(', ');
+      authors = originalTitle.split(FROM_SENTENCE_REGEX);
       authors.splice(authors.indexOf('me'), 1);
-      newAuthors = authors.join(', ');
-      awardBlock.closest('.js-emoji-btn').removeData('original-title').attr('data-original-title', newAuthors);
+      awardBlock.closest('.js-emoji-btn').removeData('original-title').attr('data-original-title', this.toSentence(authors));
       return this.resetTooltip(awardBlock);
     };
 
@@ -221,10 +230,10 @@
       origTitle = this.getAwardTooltip(awardBlock);
       users = [];
       if (origTitle) {
-        users = origTitle.trim().split(', ');
+        users = origTitle.trim().split(FROM_SENTENCE_REGEX);
       }
       users.unshift('me');
-      awardBlock.attr('title', users.join(', '));
+      awardBlock.attr('title', this.toSentence(users));
       return this.resetTooltip(awardBlock);
     };
 
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index e8081d452c4..e5be8d2404a 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -122,9 +122,9 @@ module IssuesHelper
     current_user_name = names.delete('me')
     names = names.first(9).insert(0, current_user_name).compact
 
-    names << "and #{awards.size - names.size} more." if awards.size > names.size
+    names << "#{awards.size - names.size} more." if awards.size > names.size
 
-    names.join(', ')
+    names.to_sentence
   end
 
   def award_active_class(awards, current_user)
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index c4281f8f591..e0e3554819c 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -66,7 +66,7 @@ describe IssuesHelper do
     let!(:awards) { build_list(:award_emoji, 15) }
 
     it "returns a comma seperated list of 1-9 users" do
-      expect(award_user_list(awards.first(9), nil)).to eq(awards.first(9).map { |a| a.user.name }.join(', '))
+      expect(award_user_list(awards.first(9), nil)).to eq(awards.first(9).map { |a| a.user.name }.to_sentence)
     end
 
     it "displays the current user's name as 'me'" do
diff --git a/spec/javascripts/awards_handler_spec.js b/spec/javascripts/awards_handler_spec.js
index 70191026d0e..7d28a61558a 100644
--- a/spec/javascripts/awards_handler_spec.js
+++ b/spec/javascripts/awards_handler_spec.js
@@ -144,28 +144,49 @@
       });
     });
     describe('::addMeToUserList', function() {
-      return it('should prepend "me" to the award tooltip', function() {
+      it('should prepend "me" to the award tooltip', function() {
         var $thumbsUpEmoji, $votesBlock, awardUrl;
         awardUrl = awardsHandler.getAwardUrl();
         $votesBlock = $('.js-awards-block').eq(0);
         $thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
-        $thumbsUpEmoji.attr('data-title', 'sam, jerry, max, andy');
+        $thumbsUpEmoji.attr('data-title', 'sam, jerry, max, and andy');
         awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
         $thumbsUpEmoji.tooltip();
-        return expect($thumbsUpEmoji.data("original-title")).toBe('me, sam, jerry, max, andy');
+        return expect($thumbsUpEmoji.data("original-title")).toBe('me, sam, jerry, max, and andy');
+      });
+      return it('handles the special case where "me" is not cleanly comma seperated', function() {
+        var $thumbsUpEmoji, $votesBlock, awardUrl;
+        awardUrl = awardsHandler.getAwardUrl();
+        $votesBlock = $('.js-awards-block').eq(0);
+        $thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
+        $thumbsUpEmoji.attr('data-title', 'sam');
+        awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
+        $thumbsUpEmoji.tooltip();
+        return expect($thumbsUpEmoji.data("original-title")).toBe('me and sam');
       });
     });
     describe('::removeMeToUserList', function() {
-      return it('removes "me" from the front of the tooltip', function() {
+      it('removes "me" from the front of the tooltip', function() {
+        var $thumbsUpEmoji, $votesBlock, awardUrl;
+        awardUrl = awardsHandler.getAwardUrl();
+        $votesBlock = $('.js-awards-block').eq(0);
+        $thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
+        $thumbsUpEmoji.attr('data-title', 'me, sam, jerry, max, and andy');
+        $thumbsUpEmoji.addClass('active');
+        awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
+        $thumbsUpEmoji.tooltip();
+        return expect($thumbsUpEmoji.data("original-title")).toBe('sam, jerry, max, and andy');
+      });
+      return it('handles the special case where "me" is not cleanly comma seperated', function() {
         var $thumbsUpEmoji, $votesBlock, awardUrl;
         awardUrl = awardsHandler.getAwardUrl();
         $votesBlock = $('.js-awards-block').eq(0);
         $thumbsUpEmoji = $votesBlock.find('[data-emoji=thumbsup]').parent();
-        $thumbsUpEmoji.attr('data-title', 'me, sam, jerry, max, andy');
+        $thumbsUpEmoji.attr('data-title', 'me and sam');
         $thumbsUpEmoji.addClass('active');
         awardsHandler.addAward($votesBlock, awardUrl, 'thumbsup', false);
         $thumbsUpEmoji.tooltip();
-        return expect($thumbsUpEmoji.data("original-title")).toBe('sam, jerry, max, andy');
+        return expect($thumbsUpEmoji.data("original-title")).toBe('sam');
       });
     });
     describe('search', function() {
-- 
GitLab