From 8bd8c6fb455d90c1ad0fe84430b7ea59bd1075f3 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 4 Oct 2024 19:22:52 +0200 Subject: [PATCH] [bugfix] Include own account in conversation when no other accounts involved (#3387) --- docs/api/swagger.yaml | 7 +- internal/api/model/conversation.go | 4 + internal/typeutils/internaltofrontend.go | 91 +++-- internal/typeutils/internaltofrontend_test.go | 315 ++++++++++++++++++ 4 files changed, 386 insertions(+), 31 deletions(-) diff --git a/docs/api/swagger.yaml b/docs/api/swagger.yaml index 54d318111..c0dc6de89 100644 --- a/docs/api/swagger.yaml +++ b/docs/api/swagger.yaml @@ -950,7 +950,12 @@ definitions: with "direct message" visibility. properties: accounts: - description: Participants in the conversation. + description: |- + Participants in the conversation. + + If this is a conversation between no accounts (ie., a self-directed DM), + this will include only the requesting account itself. Otherwise, it will + include every other account in the conversation *except* the requester. items: $ref: '#/definitions/account' type: array diff --git a/internal/api/model/conversation.go b/internal/api/model/conversation.go index ba961cecb..41ac7d6ba 100644 --- a/internal/api/model/conversation.go +++ b/internal/api/model/conversation.go @@ -27,6 +27,10 @@ type Conversation struct { // Is the conversation currently marked as unread? Unread bool `json:"unread"` // Participants in the conversation. + // + // If this is a conversation between no accounts (ie., a self-directed DM), + // this will include only the requesting account itself. Otherwise, it will + // include every other account in the conversation *except* the requester. Accounts []Account `json:"accounts"` // The last status in the conversation. May be `null`. LastStatus *Status `json:"last_status"` diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index 4e76837cd..76d8ec81b 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -1832,46 +1832,23 @@ func (c *Converter) NotificationToAPINotification( func (c *Converter) ConversationToAPIConversation( ctx context.Context, conversation *gtsmodel.Conversation, - requestingAccount *gtsmodel.Account, + requester *gtsmodel.Account, filters []*gtsmodel.Filter, mutes *usermute.CompiledUserMuteList, ) (*apimodel.Conversation, error) { apiConversation := &apimodel.Conversation{ - ID: conversation.ID, - Unread: !*conversation.Read, - Accounts: []apimodel.Account{}, - } - for _, account := range conversation.OtherAccounts { - var apiAccount *apimodel.Account - blocked, err := c.state.DB.IsEitherBlocked(ctx, requestingAccount.ID, account.ID) - if err != nil { - return nil, gtserror.Newf( - "DB error checking blocks between accounts %s and %s: %w", - requestingAccount.ID, - account.ID, - err, - ) - } - if blocked || account.IsSuspended() { - apiAccount, err = c.AccountToAPIAccountBlocked(ctx, account) - } else { - apiAccount, err = c.AccountToAPIAccountPublic(ctx, account) - } - if err != nil { - return nil, gtserror.Newf( - "error converting account %s to API representation: %w", - account.ID, - err, - ) - } - apiConversation.Accounts = append(apiConversation.Accounts, *apiAccount) + ID: conversation.ID, + Unread: !*conversation.Read, } + + // Populate most recent status in convo; + // can be nil if this status is filtered. if conversation.LastStatus != nil { var err error apiConversation.LastStatus, err = c.StatusToAPIStatus( ctx, conversation.LastStatus, - requestingAccount, + requester, statusfilter.FilterContextNotifications, filters, mutes, @@ -1885,6 +1862,60 @@ func (c *Converter) ConversationToAPIConversation( } } + // If no other accounts are involved in this convo, + // just include the requesting account and return. + // + // See: https://github.com/superseriousbusiness/gotosocial/issues/3385#issuecomment-2394033477 + otherAcctsLen := len(conversation.OtherAccounts) + if otherAcctsLen == 0 { + apiAcct, err := c.AccountToAPIAccountPublic(ctx, requester) + if err != nil { + err := gtserror.Newf( + "error converting account %s to API representation: %w", + requester.ID, err, + ) + return nil, err + } + + apiConversation.Accounts = []apimodel.Account{*apiAcct} + return apiConversation, nil + } + + // Other accounts are involved in the + // convo. Convert each to API model. + apiConversation.Accounts = make([]apimodel.Account, otherAcctsLen) + for i, account := range conversation.OtherAccounts { + blocked, err := c.state.DB.IsEitherBlocked(ctx, + requester.ID, account.ID, + ) + if err != nil { + err := gtserror.Newf( + "db error checking blocks between accounts %s and %s: %w", + requester.ID, account.ID, err, + ) + return nil, err + } + + // API account model varies depending + // on status of conversation participant. + var apiAcct *apimodel.Account + if blocked || account.IsSuspended() { + apiAcct, err = c.AccountToAPIAccountBlocked(ctx, account) + } else { + apiAcct, err = c.AccountToAPIAccountPublic(ctx, account) + } + + if err != nil { + err := gtserror.Newf( + "error converting account %s to API representation: %w", + account.ID, err, + ) + return nil, err + } + + apiConversation.Accounts[i] = *apiAcct + } + return apiConversation, nil } diff --git a/internal/typeutils/internaltofrontend_test.go b/internal/typeutils/internaltofrontend_test.go index 57c401e6f..139d8913a 100644 --- a/internal/typeutils/internaltofrontend_test.go +++ b/internal/typeutils/internaltofrontend_test.go @@ -3358,6 +3358,321 @@ func (suite *InternalToFrontendTestSuite) TestIntReqToAPI() { }`, string(b)) } +func (suite *InternalToFrontendTestSuite) TestConversationToAPISelfConvo() { + var ( + ctx = context.Background() + requester = suite.testAccounts["local_account_1"] + lastStatus = suite.testStatuses["local_account_1_status_1"] + filters []*gtsmodel.Filter = nil + mutes *usermute.CompiledUserMuteList = nil + ) + + convo := >smodel.Conversation{ + ID: "01J9C6K86PKZ5GY5WXV94DGH6R", + CreatedAt: testrig.TimeMustParse("2022-06-10T15:22:08Z"), + UpdatedAt: testrig.TimeMustParse("2022-06-10T15:22:08Z"), + AccountID: requester.ID, + Account: requester, + OtherAccounts: nil, + LastStatus: lastStatus, + Read: util.Ptr(true), + } + + apiConvo, err := suite.typeconverter.ConversationToAPIConversation( + ctx, + convo, + requester, + filters, + mutes, + ) + if err != nil { + suite.FailNow(err.Error()) + } + + b, err := json.MarshalIndent(apiConvo, "", " ") + if err != nil { + suite.FailNow(err.Error()) + } + + // No other accounts involved, so we should only + // have our own account in the "accounts" field. + suite.Equal(`{ + "id": "01J9C6K86PKZ5GY5WXV94DGH6R", + "unread": false, + "accounts": [ + { + "id": "01F8MH1H7YV1Z7D2C8K2730QBF", + "username": "the_mighty_zork", + "acct": "the_mighty_zork", + "display_name": "original zork (he/they)", + "locked": false, + "discoverable": true, + "bot": false, + "created_at": "2022-05-20T11:09:18.000Z", + "note": "\u003cp\u003ehey yo this is my profile!\u003c/p\u003e", + "url": "http://localhost:8080/@the_mighty_zork", + "avatar": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/original/01F8MH58A357CV5K7R7TJMSH6S.jpg", + "avatar_static": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/small/01F8MH58A357CV5K7R7TJMSH6S.webp", + "avatar_description": "a green goblin looking nasty", + "header": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/original/01PFPMWK2FF0D9WMHEJHR07C3Q.jpg", + "header_static": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.webp", + "header_description": "A very old-school screenshot of the original team fortress mod for quake", + "followers_count": 2, + "following_count": 2, + "statuses_count": 8, + "last_status_at": "2024-01-10T09:24:00.000Z", + "emojis": [], + "fields": [], + "enable_rss": true + } + ], + "last_status": { + "id": "01F8MHAMCHF6Y650WCRSCP4WMY", + "created_at": "2021-10-20T10:40:37.000Z", + "in_reply_to_id": null, + "in_reply_to_account_id": null, + "sensitive": true, + "spoiler_text": "introduction post", + "visibility": "public", + "language": "en", + "uri": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY", + "url": "http://localhost:8080/@the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY", + "replies_count": 2, + "reblogs_count": 1, + "favourites_count": 1, + "favourited": false, + "reblogged": false, + "muted": false, + "bookmarked": false, + "pinned": false, + "content": "hello everyone!", + "reblog": null, + "application": { + "name": "really cool gts application", + "website": "https://reallycool.app" + }, + "account": { + "id": "01F8MH1H7YV1Z7D2C8K2730QBF", + "username": "the_mighty_zork", + "acct": "the_mighty_zork", + "display_name": "original zork (he/they)", + "locked": false, + "discoverable": true, + "bot": false, + "created_at": "2022-05-20T11:09:18.000Z", + "note": "\u003cp\u003ehey yo this is my profile!\u003c/p\u003e", + "url": "http://localhost:8080/@the_mighty_zork", + "avatar": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/original/01F8MH58A357CV5K7R7TJMSH6S.jpg", + "avatar_static": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/small/01F8MH58A357CV5K7R7TJMSH6S.webp", + "avatar_description": "a green goblin looking nasty", + "header": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/original/01PFPMWK2FF0D9WMHEJHR07C3Q.jpg", + "header_static": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.webp", + "header_description": "A very old-school screenshot of the original team fortress mod for quake", + "followers_count": 2, + "following_count": 2, + "statuses_count": 8, + "last_status_at": "2024-01-10T09:24:00.000Z", + "emojis": [], + "fields": [], + "enable_rss": true + }, + "media_attachments": [], + "mentions": [], + "tags": [], + "emojis": [], + "card": null, + "poll": null, + "text": "hello everyone!", + "interaction_policy": { + "can_favourite": { + "always": [ + "public", + "me" + ], + "with_approval": [] + }, + "can_reply": { + "always": [ + "public", + "me" + ], + "with_approval": [] + }, + "can_reblog": { + "always": [ + "public", + "me" + ], + "with_approval": [] + } + } + } +}`, string(b)) +} + +func (suite *InternalToFrontendTestSuite) TestConversationToAPI() { + var ( + ctx = context.Background() + requester = suite.testAccounts["local_account_1"] + lastStatus = suite.testStatuses["local_account_1_status_1"] + filters []*gtsmodel.Filter = nil + mutes *usermute.CompiledUserMuteList = nil + ) + + convo := >smodel.Conversation{ + ID: "01J9C6K86PKZ5GY5WXV94DGH6R", + CreatedAt: testrig.TimeMustParse("2022-06-10T15:22:08Z"), + UpdatedAt: testrig.TimeMustParse("2022-06-10T15:22:08Z"), + AccountID: requester.ID, + Account: requester, + OtherAccounts: []*gtsmodel.Account{ + suite.testAccounts["local_account_2"], + }, + LastStatus: lastStatus, + Read: util.Ptr(false), + } + + apiConvo, err := suite.typeconverter.ConversationToAPIConversation( + ctx, + convo, + requester, + filters, + mutes, + ) + if err != nil { + suite.FailNow(err.Error()) + } + + b, err := json.MarshalIndent(apiConvo, "", " ") + if err != nil { + suite.FailNow(err.Error()) + } + + // One other account is involved, so they + // should in the "accounts" field and not us. + suite.Equal(`{ + "id": "01J9C6K86PKZ5GY5WXV94DGH6R", + "unread": true, + "accounts": [ + { + "id": "01F8MH5NBDF2MV7CTC4Q5128HF", + "username": "1happyturtle", + "acct": "1happyturtle", + "display_name": "happy little turtle :3", + "locked": true, + "discoverable": false, + "bot": false, + "created_at": "2022-06-04T13:12:00.000Z", + "note": "\u003cp\u003ei post about things that concern me\u003c/p\u003e", + "url": "http://localhost:8080/@1happyturtle", + "avatar": "", + "avatar_static": "", + "header": "http://localhost:8080/assets/default_header.webp", + "header_static": "http://localhost:8080/assets/default_header.webp", + "followers_count": 1, + "following_count": 1, + "statuses_count": 8, + "last_status_at": "2021-07-28T08:40:37.000Z", + "emojis": [], + "fields": [ + { + "name": "should you follow me?", + "value": "maybe!", + "verified_at": null + }, + { + "name": "age", + "value": "120", + "verified_at": null + } + ], + "hide_collections": true + } + ], + "last_status": { + "id": "01F8MHAMCHF6Y650WCRSCP4WMY", + "created_at": "2021-10-20T10:40:37.000Z", + "in_reply_to_id": null, + "in_reply_to_account_id": null, + "sensitive": true, + "spoiler_text": "introduction post", + "visibility": "public", + "language": "en", + "uri": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY", + "url": "http://localhost:8080/@the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY", + "replies_count": 2, + "reblogs_count": 1, + "favourites_count": 1, + "favourited": false, + "reblogged": false, + "muted": false, + "bookmarked": false, + "pinned": false, + "content": "hello everyone!", + "reblog": null, + "application": { + "name": "really cool gts application", + "website": "https://reallycool.app" + }, + "account": { + "id": "01F8MH1H7YV1Z7D2C8K2730QBF", + "username": "the_mighty_zork", + "acct": "the_mighty_zork", + "display_name": "original zork (he/they)", + "locked": false, + "discoverable": true, + "bot": false, + "created_at": "2022-05-20T11:09:18.000Z", + "note": "\u003cp\u003ehey yo this is my profile!\u003c/p\u003e", + "url": "http://localhost:8080/@the_mighty_zork", + "avatar": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/original/01F8MH58A357CV5K7R7TJMSH6S.jpg", + "avatar_static": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/avatar/small/01F8MH58A357CV5K7R7TJMSH6S.webp", + "avatar_description": "a green goblin looking nasty", + "header": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/original/01PFPMWK2FF0D9WMHEJHR07C3Q.jpg", + "header_static": "http://localhost:8080/fileserver/01F8MH1H7YV1Z7D2C8K2730QBF/header/small/01PFPMWK2FF0D9WMHEJHR07C3Q.webp", + "header_description": "A very old-school screenshot of the original team fortress mod for quake", + "followers_count": 2, + "following_count": 2, + "statuses_count": 8, + "last_status_at": "2024-01-10T09:24:00.000Z", + "emojis": [], + "fields": [], + "enable_rss": true + }, + "media_attachments": [], + "mentions": [], + "tags": [], + "emojis": [], + "card": null, + "poll": null, + "text": "hello everyone!", + "interaction_policy": { + "can_favourite": { + "always": [ + "public", + "me" + ], + "with_approval": [] + }, + "can_reply": { + "always": [ + "public", + "me" + ], + "with_approval": [] + }, + "can_reblog": { + "always": [ + "public", + "me" + ], + "with_approval": [] + } + } + } +}`, string(b)) +} + func TestInternalToFrontendTestSuite(t *testing.T) { suite.Run(t, new(InternalToFrontendTestSuite)) }