fix(thread): thread.sortLabels() now ignores 'Sent Mail' and other unnecessary labels. fixes T3454.

Summary: NOTE: this ticket and this diff do not address turning the 'Important' label into a more user-friendly chevron. I created T3477 for that.

Test Plan: added more tests. however, seems like somebody merged some tests which are failing.

Reviewers: bengotow, evan

Reviewed By: evan

Maniphest Tasks: T3454

Differential Revision: https://phab.nylas.com/D1963

internal_packages/message-list/lib/message-list.cjsx

@@ -253,7 +253,7 @@ class MessageList extends React.Component
     </div>
 
   _renderLabels: =>
-    labels = @state.currentThread.sortedLabels() ? []
+    labels = @state.currentThread.sortedLabels()
     labels.map (label) =>
       <MailLabel label={label} key={label.id} onRemove={ => @_onRemoveLabel(label) }/>
 

internal_packages/thread-list/lib/thread-list.cjsx

@@ -95,7 +95,7 @@ class ThreadList extends React.Component
         allCategoryId = CategoryStore.getStandardCategory('all')?.id
         ignoredIds = [currentCategoryId, allCategoryId]
 
-        for label in (thread.sortedLabels() ? [])
+        for label in (thread.sortedLabels())
           continue if label.id in ignoredIds
           if not c3LabelComponentCache[label.id]
             c3LabelComponentCache[label.id] = <MailLabel label={label} key={label.id} />

spec-nylas/models/thread-spec.coffee

@@ -1,11 +1,52 @@
 Message = require '../../src/flux/models/message'
 Thread = require '../../src/flux/models/thread'
+Label = require '../../src/flux/models/label'
 _ = require 'underscore'
 
-mockThread =
-  accountId: "abc"
-  participants: ["zip@example.com"]
-  subject: "blah"
-  id: "asdf"
-
 describe 'Thread', ->
+  describe '.sortLabels()', ->
+    getSortedLabels = (inputs) ->
+      labels = _.map inputs, (i) ->
+        new Label(name: i, displayName: i)
+      thread = new Thread(labels: labels)
+      return thread.sortedLabels()
+
+    it "puts 'important' label first, if it's present", ->
+      inputs = ['alphabetically before important', 'important']
+      actualOut = getSortedLabels inputs
+      expect(actualOut[0].displayName).toBe 'important'
+
+    it "ignores 'important' label if not present", ->
+      inputs = ['not important']
+      actualOut = getSortedLabels inputs
+      expect(actualOut.length).toBe 1
+      expect(actualOut[0].displayName).toBe 'not important'
+
+    it "doesn't display 'sent', 'all', 'archive', or 'drafts'", ->
+      inputs = ['sent', 'all', 'archive', 'drafts']
+      actualOut = getSortedLabels inputs
+      expect(actualOut.length).toBe 0
+
+    it "displays standard category names which aren't hidden next, if they're present", ->
+      inputs = ['inbox', 'important', 'social']
+      actualOut = _.pluck getSortedLabels(inputs), 'displayName'
+      expectedOut = ['important', 'inbox', 'social']
+      expect(actualOut).toEqual expectedOut
+
+    it "ignores standard category names if they aren't present", ->
+      inputs = ['social', 'work', 'important']
+      actualOut = _.pluck getSortedLabels(inputs), 'displayName'
+      expectedOut = ['important', 'social', 'work']
+      expect(actualOut).toEqual expectedOut
+
+    it "puts user-added categories at the end", ->
+      inputs = ['food', 'inbox']
+      actualOut = _.pluck getSortedLabels(inputs), 'displayName'
+      expectedOut = ['inbox', 'food']
+      expect(actualOut).toEqual expectedOut
+
+    it "sorts user-added categories by displayName", ->
+      inputs = ['work', 'social', 'receipts', 'important', 'inbox']
+      actualOut = _.pluck getSortedLabels(inputs), 'displayName'
+      expectedOut = ['important', 'inbox', 'receipts', 'social', 'work']
+      expect(actualOut).toEqual expectedOut

src/flux/models/thread.coffee

@@ -6,6 +6,7 @@ Model = require './model'
 Contact = require './contact'
 Actions = require '../actions'
 Attributes = require '../attributes'
+CategoryStore = require '../stores/category-store'
 
 Function::getter = (prop, get) ->
   Object.defineProperty @prototype, prop, {get, configurable: yes}
@@ -111,7 +112,26 @@ class Thread extends Model
   hasFolderName: (name) -> @hasCategoryName(name)
 
   sortedLabels: ->
-    return null unless @labels
-    _.sortBy @labels, (label) -> label.displayName
+    return [] unless @labels
+
+    out = []
+    isImportant = (l) -> l.name is 'important'
+    isStandardCategory = (l) -> l.name in CategoryStore.StandardCategoryNames
+    isUnhiddenStandardLabel = (l) ->
+      not isImportant(l) and \
+      isStandardCategory(l) and\
+      l.name not in CategoryStore.HiddenCategoryNames
+
+    importantLabel = _.find @labels, isImportant
+    out = out.concat importantLabel if importantLabel
+
+    standardLabels = _.filter @labels, isUnhiddenStandardLabel
+    out = out.concat standardLabels if standardLabels.length
+
+    userLabels = _.filter @labels, (l) ->
+      not isImportant(l) and not isStandardCategory(l)
+    out = out.concat _.sortBy(userLabels, 'displayName') if userLabels.length
+
+    out
 
 module.exports = Thread

src/flux/stores/category-store.coffee

@@ -31,6 +31,13 @@ class CategoryStore extends NylasStore
     "sent"
   ]
 
+  HiddenCategoryNames: [
+    "sent"
+    "drafts"
+    "all"
+    "archive"
+  ]
+
   AllMailName: "all"
 
   byId: (id) -> @_categoryCache[id]
@@ -81,6 +88,10 @@ class CategoryStore extends NylasStore
     _.compact @StandardCategoryNames.map (name) =>
       byStandardName[name]
 
+  getUnhiddenStandardCategories: ->
+    @getStandardCategories().filter (c) ->
+      not _.contains @HiddenCategoryNames, c.name
+
   # Public: Returns all of the categories that are not part of the standard
   # category set.
   #