Improve reply prefix computation. (#103)

src/Mastodon/Helper.elm

@@ -1,21 +1,15 @@
 module Mastodon.Helper
     exposing
-        ( accountMentioned
+        ( extractReblog
-        , extractReblog
         , aggregateNotifications
         , addNotificationToAggregates
+        , getReplyPrefix
         , notificationToAggregate
         , sameAccount
         )
 
 import List.Extra exposing (groupWhile, uniqueBy)
-import Mastodon.Model
+import Mastodon.Model exposing (..)
-    exposing
-        ( Notification
-        , NotificationAggregate
-        , Reblog(..)
-        , Status
-        )
 
 
 extractReblog : Status -> Status
@@ -28,6 +22,33 @@ extractReblog status =
             status
 
 
+getReplyPrefix : Account -> Status -> String
+getReplyPrefix replier status =
+    -- Note: the Mastodon API doesn't consistently return mentions in the order
+    --       they appear in the status text, we do nothing about that.
+    let
+        posters =
+            case status.reblog of
+                Just (Mastodon.Model.Reblog reblog) ->
+                    [ reblog.account, status.account ] |> List.map toMention
+
+                Nothing ->
+                    (toMention status.account) :: status.mentions
+
+        finalPosters =
+            posters
+                |> uniqueBy .acct
+                |> List.filter (\m -> m /= (toMention replier))
+                |> List.map (\m -> "@" ++ m.acct)
+    in
+        (String.join " " finalPosters) ++ " "
+
+
+toMention : Account -> Mention
+toMention { id, url, username, acct } =
+    Mention id url username acct
+
+
 notificationToAggregate : Notification -> NotificationAggregate
 notificationToAggregate notification =
     NotificationAggregate
@@ -143,11 +164,7 @@ aggregateNotifications notifications =
             |> List.reverse
 
 
-accountMentioned : Mastodon.Model.Account -> Mastodon.Model.Mention -> Bool
-accountMentioned { acct, username } mention =
-    acct == mention.acct && username == mention.username
-
-
 sameAccount : Mastodon.Model.Account -> Mastodon.Model.Account -> Bool
-sameAccount { acct, username } account =
+sameAccount { id, acct, username } account =
-    acct == account.acct && username == account.username
+    -- Note: different instances can share the same id for different accounts.
+    id == account.id && acct == account.acct && username == account.username

src/Model.elm

@@ -400,22 +400,9 @@ updateDraft draftMsg currentUser draft =
             { draft | visibility = visibility } ! []
 
         UpdateReplyTo status ->
-            let
-                mentions =
-                    status.mentions
-                        |> List.filter (\m -> not (Mastodon.Helper.accountMentioned currentUser m))
-                        |> List.map (\m -> "@" ++ m.acct)
-                        |> String.join " "
-
-                newStatus =
-                    if Mastodon.Helper.sameAccount status.account currentUser then
-                        mentions
-                    else
-                        "@" ++ status.account.acct ++ " " ++ mentions
-            in
             { draft
                 | in_reply_to = Just status
-                    , status = (String.trim newStatus) ++ " "
+                , status = Mastodon.Helper.getReplyPrefix currentUser status
                 , sensitive = Maybe.withDefault False status.sensitive
                 , spoiler_text =
                     if status.spoiler_text == "" then

src/View.elm

@@ -261,7 +261,7 @@ accountTimelineView account statuses label iconName =
 statusActionsView : Mastodon.Model.Status -> Mastodon.Model.Account -> Html Msg
 statusActionsView status currentUser =
     let
-        targetStatus =
+        sourceStatus =
             Mastodon.Helper.extractReblog status
 
         baseBtnClasses =
@@ -270,18 +270,18 @@ statusActionsView status currentUser =
         ( reblogClasses, reblogEvent ) =
             case status.reblogged of
                 Just True ->
-                    ( baseBtnClasses ++ " reblogged", Unreblog targetStatus.id )
+                    ( baseBtnClasses ++ " reblogged", Unreblog sourceStatus.id )
 
                 _ ->
-                    ( baseBtnClasses, Reblog targetStatus.id )
+                    ( baseBtnClasses, Reblog sourceStatus.id )
 
         ( favClasses, favEvent ) =
             case status.favourited of
                 Just True ->
-                    ( baseBtnClasses ++ " favourited", RemoveFavorite targetStatus.id )
+                    ( baseBtnClasses ++ " favourited", RemoveFavorite sourceStatus.id )
 
                 _ ->
-                    ( baseBtnClasses, AddFavorite targetStatus.id )
+                    ( baseBtnClasses, AddFavorite sourceStatus.id )
 
         statusDate =
             Date.fromString status.created_at
@@ -294,24 +294,24 @@ statusActionsView status currentUser =
             [ a
                 [ class baseBtnClasses
                 , onClickWithPreventAndStop <|
-                    DraftEvent (UpdateReplyTo targetStatus)
+                    DraftEvent (UpdateReplyTo status)
                 ]
                 [ icon "share-alt" ]
             , a
                 [ class reblogClasses
                 , onClickWithPreventAndStop reblogEvent
                 ]
-                [ icon "fire", text (toString status.reblogs_count) ]
+                [ icon "fire", text (toString sourceStatus.reblogs_count) ]
             , a
                 [ class favClasses
                 , onClickWithPreventAndStop favEvent
                 ]
-                [ icon "star", text (toString status.favourites_count) ]
+                [ icon "star", text (toString sourceStatus.favourites_count) ]
-            , if Mastodon.Helper.sameAccount status.account currentUser then
+            , if Mastodon.Helper.sameAccount sourceStatus.account currentUser then
                 a
                     [ class <| baseBtnClasses ++ " btn-delete"
                     , href ""
-                    , onClickWithPreventAndStop <| DeleteStatus status.id
+                    , onClickWithPreventAndStop <| DeleteStatus sourceStatus.id
                     ]
                     [ icon "trash" ]
               else

tests/Fixtures.elm

@@ -1,6 +1,6 @@
 module Fixtures exposing (..)
 
-import Mastodon.Model exposing (Account, Notification, NotificationAggregate, Status)
+import Mastodon.Model exposing (..)
 
 
 accountSkro : Account
@@ -135,6 +135,60 @@ statusNicoToVjousseAgain =
     }
 
 
+statusPloumToVjousse : Status
+statusPloumToVjousse =
+    { account = accountPloum
+    , content = "<p>hey @vjousse</p>"
+    , created_at = "2017-04-25T07:41:23.492Z"
+    , favourited = Nothing
+    , favourites_count = 0
+    , id = 752169
+    , in_reply_to_account_id = Nothing
+    , in_reply_to_id = Nothing
+    , media_attachments = []
+    , mentions =
+        [ { id = 26303
+          , url = "https://mamot.fr/@vjousse"
+          , username = "vjousse"
+          , acct = "vjousse"
+          }
+        ]
+    , reblog = Nothing
+    , reblogged = Nothing
+    , reblogs_count = 0
+    , sensitive = Just False
+    , spoiler_text = ""
+    , tags = []
+    , uri = "tag:mamot.fr,2017-04-25:objectId=752169:objectType=Status"
+    , url = "https://mamot.fr/@n1k0/752169"
+    , visibility = "public"
+    }
+
+
+statusReblogged : Status
+statusReblogged =
+    { account = accountVjousse
+    , content = "<p>fake post</p>"
+    , created_at = "2017-04-24T20:16:20.922Z"
+    , favourited = Nothing
+    , favourites_count = 0
+    , id = 737932
+    , in_reply_to_account_id = Nothing
+    , in_reply_to_id = Nothing
+    , media_attachments = []
+    , mentions = []
+    , reblog = Just (Reblog statusPloumToVjousse)
+    , reblogged = Nothing
+    , reblogs_count = 0
+    , sensitive = Just False
+    , spoiler_text = ""
+    , tags = []
+    , uri = "tag:mamot.fr,2017-04-24:objectId=737932:objectType=Status"
+    , url = "https://mamot.fr/@n1k0/737932"
+    , visibility = "public"
+    }
+
+
 notificationNicoMentionVjousse : Notification
 notificationNicoMentionVjousse =
     { id = 224284

tests/MastodonTest/HelperTest.elm

@@ -2,18 +2,41 @@ module MastodonTest.HelperTest exposing (..)
 
 import Test exposing (..)
 import Expect
-import Mastodon.Helper
+import Mastodon.Helper exposing (..)
 import Fixtures
 
 
 all : Test
 all =
-    describe "Notification test suite"
+    describe "Mastodon.Helper tests"
+        [ describe "Reply tests"
+            [ test "Simple reply" <|
+                \() ->
+                    Fixtures.statusNicoToVjousse
+                        |> getReplyPrefix Fixtures.accountVjousse
+                        |> Expect.equal "@n1k0 "
+            , test "Keeping replying to a previous post mentioning a user" <|
+                \() ->
+                    Fixtures.statusNicoToVjousse
+                        |> getReplyPrefix Fixtures.accountNico
+                        |> Expect.equal "@vjousse "
+            , test "Replying to original poster and reblogger" <|
+                \() ->
+                    Fixtures.statusReblogged
+                        |> getReplyPrefix Fixtures.accountNico
+                        |> Expect.equal "@ploum @vjousse "
+            , test "Exclude replier from generated reply prefix" <|
+                \() ->
+                    Fixtures.statusNicoToVjousse
+                        |> getReplyPrefix Fixtures.accountNico
+                        |> Expect.equal "@vjousse "
+            ]
+        , describe "Notification test suite"
             [ describe "Aggegate test"
                 [ test "Aggregate Notifications" <|
                     \() ->
                         Fixtures.notifications
-                        |> Mastodon.Helper.aggregateNotifications
+                            |> aggregateNotifications
                             |> Expect.equal
                                 [ { type_ = "mention"
                                   , status = Just Fixtures.statusNicoToVjousse
@@ -29,14 +52,14 @@ all =
                 , test "Dedupes aggregated accounts" <|
                     \() ->
                         Fixtures.duplicateAccountNotifications
-                        |> Mastodon.Helper.aggregateNotifications
+                            |> aggregateNotifications
                             |> List.map (.accounts >> List.length)
                             |> Expect.equal [ 1 ]
                 , test "Add follows notification to aggregate" <|
                     \() ->
                         Fixtures.notifications
-                        |> Mastodon.Helper.aggregateNotifications
+                            |> aggregateNotifications
-                        |> (Mastodon.Helper.addNotificationToAggregates Fixtures.notificationPloumFollowsVjousse)
+                            |> (addNotificationToAggregates Fixtures.notificationPloumFollowsVjousse)
                             |> Expect.equal
                                 [ { type_ = "mention"
                                   , status = Just Fixtures.statusNicoToVjousse
@@ -52,8 +75,8 @@ all =
                 , test "Add mention notification to aggregate" <|
                     \() ->
                         Fixtures.notifications
-                        |> Mastodon.Helper.aggregateNotifications
+                            |> aggregateNotifications
-                        |> (Mastodon.Helper.addNotificationToAggregates Fixtures.notificationNicoMentionVjousse)
+                            |> (addNotificationToAggregates Fixtures.notificationNicoMentionVjousse)
                             |> Expect.equal
                                 [ { type_ = "mention"
                                   , status = Just Fixtures.statusNicoToVjousse
@@ -69,8 +92,8 @@ all =
                 , test "Add new mention notification to aggregate" <|
                     \() ->
                         Fixtures.notifications
-                        |> Mastodon.Helper.aggregateNotifications
+                            |> aggregateNotifications
-                        |> (Mastodon.Helper.addNotificationToAggregates Fixtures.notificationNicoMentionVjousseAgain)
+                            |> (addNotificationToAggregates Fixtures.notificationNicoMentionVjousseAgain)
                             |> Expect.equal
                                 [ { type_ = "mention"
                                   , status = Just Fixtures.statusNicoToVjousseAgain
@@ -90,3 +113,4 @@ all =
                                 ]
                 ]
             ]
+        ]