From 017f07d45d93e5d514e4756a0c57965aa0c9c146 Mon Sep 17 00:00:00 2001 From: Nicolas Perriault Date: Sat, 29 Apr 2017 11:57:26 +0200 Subject: [PATCH] Improve reply prefix computation. (#103) --- src/Mastodon/Helper.elm | 49 +++++--- src/Model.elm | 35 ++---- src/View.elm | 20 ++-- tests/Fixtures.elm | 56 +++++++++- tests/MastodonTest/HelperTest.elm | 178 +++++++++++++++++------------- 5 files changed, 210 insertions(+), 128 deletions(-) diff --git a/src/Mastodon/Helper.elm b/src/Mastodon/Helper.elm index f5f444b..d9d8289 100644 --- a/src/Mastodon/Helper.elm +++ b/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 - exposing - ( Notification - , NotificationAggregate - , Reblog(..) - , Status - ) +import Mastodon.Model exposing (..) 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 = - acct == account.acct && username == account.username +sameAccount { id, acct, username } account = + -- Note: different instances can share the same id for different accounts. + id == account.id && acct == account.acct && username == account.username diff --git a/src/Model.elm b/src/Model.elm index 8d55d43..51f62e1 100644 --- a/src/Model.elm +++ b/src/Model.elm @@ -400,31 +400,18 @@ 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 + { draft + | in_reply_to = Just status + , status = Mastodon.Helper.getReplyPrefix currentUser status + , sensitive = Maybe.withDefault False status.sensitive + , spoiler_text = + if status.spoiler_text == "" then + Nothing else - "@" ++ status.account.acct ++ " " ++ mentions - in - { draft - | in_reply_to = Just status - , status = (String.trim newStatus) ++ " " - , sensitive = Maybe.withDefault False status.sensitive - , spoiler_text = - if status.spoiler_text == "" then - Nothing - else - Just status.spoiler_text - , visibility = status.visibility - } - ! [ Dom.focus "status" |> Task.attempt (always NoOp) ] + Just status.spoiler_text + , visibility = status.visibility + } + ! [ Dom.focus "status" |> Task.attempt (always NoOp) ] updateViewer : ViewerMsg -> Maybe Viewer -> ( Maybe Viewer, Cmd Msg ) diff --git a/src/View.elm b/src/View.elm index 0122ee0..96a8e68 100644 --- a/src/View.elm +++ b/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) ] - , if Mastodon.Helper.sameAccount status.account currentUser then + [ icon "star", text (toString sourceStatus.favourites_count) ] + , 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 diff --git a/tests/Fixtures.elm b/tests/Fixtures.elm index 40010d0..5aeebe8 100644 --- a/tests/Fixtures.elm +++ b/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 = "

hey @vjousse

" + , 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 = "

fake post

" + , 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 diff --git a/tests/MastodonTest/HelperTest.elm b/tests/MastodonTest/HelperTest.elm index d927e99..575a96a 100644 --- a/tests/MastodonTest/HelperTest.elm +++ b/tests/MastodonTest/HelperTest.elm @@ -2,91 +2,115 @@ 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 "Aggegate test" - [ test "Aggregate Notifications" <| + describe "Mastodon.Helper tests" + [ describe "Reply tests" + [ test "Simple reply" <| \() -> - Fixtures.notifications - |> Mastodon.Helper.aggregateNotifications - |> Expect.equal - [ { type_ = "mention" - , status = Just Fixtures.statusNicoToVjousse - , accounts = [ Fixtures.accountNico ] - , created_at = "2017-04-24T20:16:20.973Z" - } - , { type_ = "follow" - , status = Nothing - , accounts = [ Fixtures.accountNico, Fixtures.accountSkro ] - , created_at = "2017-04-24T20:13:47.431Z" - } - ] - , test "Dedupes aggregated accounts" <| + Fixtures.statusNicoToVjousse + |> getReplyPrefix Fixtures.accountVjousse + |> Expect.equal "@n1k0 " + , test "Keeping replying to a previous post mentioning a user" <| \() -> - Fixtures.duplicateAccountNotifications - |> Mastodon.Helper.aggregateNotifications - |> List.map (.accounts >> List.length) - |> Expect.equal [ 1 ] - , test "Add follows notification to aggregate" <| + Fixtures.statusNicoToVjousse + |> getReplyPrefix Fixtures.accountNico + |> Expect.equal "@vjousse " + , test "Replying to original poster and reblogger" <| \() -> - Fixtures.notifications - |> Mastodon.Helper.aggregateNotifications - |> (Mastodon.Helper.addNotificationToAggregates Fixtures.notificationPloumFollowsVjousse) - |> Expect.equal - [ { type_ = "mention" - , status = Just Fixtures.statusNicoToVjousse - , accounts = [ Fixtures.accountNico ] - , created_at = "2017-04-24T20:16:20.973Z" - } - , { type_ = "follow" - , status = Nothing - , accounts = [ Fixtures.accountPloum, Fixtures.accountNico, Fixtures.accountSkro ] - , created_at = "2017-04-24T20:13:47.431Z" - } - ] - , test "Add mention notification to aggregate" <| + Fixtures.statusReblogged + |> getReplyPrefix Fixtures.accountNico + |> Expect.equal "@ploum @vjousse " + , test "Exclude replier from generated reply prefix" <| \() -> - Fixtures.notifications - |> Mastodon.Helper.aggregateNotifications - |> (Mastodon.Helper.addNotificationToAggregates Fixtures.notificationNicoMentionVjousse) - |> Expect.equal - [ { type_ = "mention" - , status = Just Fixtures.statusNicoToVjousse - , accounts = [ Fixtures.accountNico, Fixtures.accountNico ] - , created_at = "2017-04-24T20:16:20.973Z" - } - , { type_ = "follow" - , status = Nothing - , accounts = [ Fixtures.accountNico, Fixtures.accountSkro ] - , created_at = "2017-04-24T20:13:47.431Z" - } - ] - , test "Add new mention notification to aggregate" <| - \() -> - Fixtures.notifications - |> Mastodon.Helper.aggregateNotifications - |> (Mastodon.Helper.addNotificationToAggregates Fixtures.notificationNicoMentionVjousseAgain) - |> Expect.equal - [ { type_ = "mention" - , status = Just Fixtures.statusNicoToVjousseAgain - , accounts = [ Fixtures.accountNico ] - , created_at = "2017-04-25T07:41:23.546Z" - } - , { type_ = "mention" - , status = Just Fixtures.statusNicoToVjousse - , accounts = [ Fixtures.accountNico ] - , created_at = "2017-04-24T20:16:20.973Z" - } - , { type_ = "follow" - , status = Nothing - , accounts = [ Fixtures.accountNico, Fixtures.accountSkro ] - , created_at = "2017-04-24T20:13:47.431Z" - } - ] + Fixtures.statusNicoToVjousse + |> getReplyPrefix Fixtures.accountNico + |> Expect.equal "@vjousse " + ] + , describe "Notification test suite" + [ describe "Aggegate test" + [ test "Aggregate Notifications" <| + \() -> + Fixtures.notifications + |> aggregateNotifications + |> Expect.equal + [ { type_ = "mention" + , status = Just Fixtures.statusNicoToVjousse + , accounts = [ Fixtures.accountNico ] + , created_at = "2017-04-24T20:16:20.973Z" + } + , { type_ = "follow" + , status = Nothing + , accounts = [ Fixtures.accountNico, Fixtures.accountSkro ] + , created_at = "2017-04-24T20:13:47.431Z" + } + ] + , test "Dedupes aggregated accounts" <| + \() -> + Fixtures.duplicateAccountNotifications + |> aggregateNotifications + |> List.map (.accounts >> List.length) + |> Expect.equal [ 1 ] + , test "Add follows notification to aggregate" <| + \() -> + Fixtures.notifications + |> aggregateNotifications + |> (addNotificationToAggregates Fixtures.notificationPloumFollowsVjousse) + |> Expect.equal + [ { type_ = "mention" + , status = Just Fixtures.statusNicoToVjousse + , accounts = [ Fixtures.accountNico ] + , created_at = "2017-04-24T20:16:20.973Z" + } + , { type_ = "follow" + , status = Nothing + , accounts = [ Fixtures.accountPloum, Fixtures.accountNico, Fixtures.accountSkro ] + , created_at = "2017-04-24T20:13:47.431Z" + } + ] + , test "Add mention notification to aggregate" <| + \() -> + Fixtures.notifications + |> aggregateNotifications + |> (addNotificationToAggregates Fixtures.notificationNicoMentionVjousse) + |> Expect.equal + [ { type_ = "mention" + , status = Just Fixtures.statusNicoToVjousse + , accounts = [ Fixtures.accountNico, Fixtures.accountNico ] + , created_at = "2017-04-24T20:16:20.973Z" + } + , { type_ = "follow" + , status = Nothing + , accounts = [ Fixtures.accountNico, Fixtures.accountSkro ] + , created_at = "2017-04-24T20:13:47.431Z" + } + ] + , test "Add new mention notification to aggregate" <| + \() -> + Fixtures.notifications + |> aggregateNotifications + |> (addNotificationToAggregates Fixtures.notificationNicoMentionVjousseAgain) + |> Expect.equal + [ { type_ = "mention" + , status = Just Fixtures.statusNicoToVjousseAgain + , accounts = [ Fixtures.accountNico ] + , created_at = "2017-04-25T07:41:23.546Z" + } + , { type_ = "mention" + , status = Just Fixtures.statusNicoToVjousse + , accounts = [ Fixtures.accountNico ] + , created_at = "2017-04-24T20:16:20.973Z" + } + , { type_ = "follow" + , status = Nothing + , accounts = [ Fixtures.accountNico, Fixtures.accountSkro ] + , created_at = "2017-04-24T20:13:47.431Z" + } + ] + ] ] ]