mirror of
				https://github.com/superseriousbusiness/gotosocial.git
				synced 2025-10-31 07:42:26 -05:00 
			
		
		
		
	[bugfix] Fix duplicating fields on profile edit (#1788)
* [bugfix] Fix duplicating fields on profile edit * test non-duplicate fields
This commit is contained in:
		
					parent
					
						
							
								e1b7ab2603
							
						
					
				
			
			
				commit
				
					
						17b9a937b1
					
				
			
		
					 3 changed files with 198 additions and 136 deletions
				
			
		|  | @ -155,6 +155,7 @@ func (p *Processor) Update(ctx context.Context, account *gtsmodel.Account, form | |||
| 		} | ||||
| 
 | ||||
| 		// Process the raw fields we stored earlier. | ||||
| 		account.Fields = make([]*gtsmodel.Field, 0, len(account.FieldsRaw)) | ||||
| 		for _, fieldRaw := range account.FieldsRaw { | ||||
| 			field := >smodel.Field{} | ||||
| 
 | ||||
|  |  | |||
|  | @ -24,134 +24,153 @@ import ( | |||
| 	"github.com/stretchr/testify/suite" | ||||
| 	"github.com/superseriousbusiness/gotosocial/internal/ap" | ||||
| 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" | ||||
| 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel" | ||||
| ) | ||||
| 
 | ||||
| type AccountUpdateTestSuite struct { | ||||
| 	AccountStandardTestSuite | ||||
| } | ||||
| 
 | ||||
| func (suite *AccountStandardTestSuite) checkClientAPIChan(accountID string) { | ||||
| 	msg := <-suite.fromClientAPIChan | ||||
| 
 | ||||
| 	// Profile update. | ||||
| 	suite.Equal(ap.ActivityUpdate, msg.APActivityType) | ||||
| 	suite.Equal(ap.ObjectProfile, msg.APObjectType) | ||||
| 
 | ||||
| 	// Correct account updated. | ||||
| 	if msg.OriginAccount == nil { | ||||
| 		suite.FailNow("expected msg.OriginAccount not to be nil") | ||||
| 	} | ||||
| 	suite.Equal(accountID, msg.OriginAccount.ID) | ||||
| } | ||||
| 
 | ||||
| func (suite *AccountUpdateTestSuite) TestAccountUpdateSimple() { | ||||
| 	testAccount := suite.testAccounts["local_account_1"] | ||||
| 	testAccount := >smodel.Account{} | ||||
| 	*testAccount = *suite.testAccounts["local_account_1"] | ||||
| 
 | ||||
| 	locked := true | ||||
| 	displayName := "new display name" | ||||
| 	note := "#hello here i am!" | ||||
| 	var ( | ||||
| 		ctx          = context.Background() | ||||
| 		locked       = true | ||||
| 		displayName  = "new display name" | ||||
| 		note         = "#hello here i am!" | ||||
| 		noteExpected = `<p><a href="http://localhost:8080/tags/hello" class="mention hashtag" rel="tag nofollow noreferrer noopener" target="_blank">#<span>hello</span></a> here i am!</p>` | ||||
| 	) | ||||
| 
 | ||||
| 	form := &apimodel.UpdateCredentialsRequest{ | ||||
| 	// Call update function. | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(ctx, testAccount, &apimodel.UpdateCredentialsRequest{ | ||||
| 		DisplayName: &displayName, | ||||
| 		Locked:      &locked, | ||||
| 		Note:        ¬e, | ||||
| 	}) | ||||
| 	if errWithCode != nil { | ||||
| 		suite.FailNow(errWithCode.Error()) | ||||
| 	} | ||||
| 
 | ||||
| 	// should get no error from the update function, and an api model account returned | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(context.Background(), testAccount, form) | ||||
| 	suite.NoError(errWithCode) | ||||
| 	suite.NotNil(apiAccount) | ||||
| 
 | ||||
| 	// fields on the profile should be updated | ||||
| 	// Returned profile should be updated. | ||||
| 	suite.True(apiAccount.Locked) | ||||
| 	suite.Equal(displayName, apiAccount.DisplayName) | ||||
| 	suite.Equal(`<p><a href="http://localhost:8080/tags/hello" class="mention hashtag" rel="tag nofollow noreferrer noopener" target="_blank">#<span>hello</span></a> here i am!</p>`, apiAccount.Note) | ||||
| 	suite.Equal(noteExpected, apiAccount.Note) | ||||
| 
 | ||||
| 	// we should have an update in the client api channel | ||||
| 	msg := <-suite.fromClientAPIChan | ||||
| 	suite.Equal(ap.ActivityUpdate, msg.APActivityType) | ||||
| 	suite.Equal(ap.ObjectProfile, msg.APObjectType) | ||||
| 	suite.NotNil(msg.OriginAccount) | ||||
| 	suite.Equal(testAccount.ID, msg.OriginAccount.ID) | ||||
| 	suite.Nil(msg.TargetAccount) | ||||
| 	// We should have an update in the client api channel. | ||||
| 	suite.checkClientAPIChan(testAccount.ID) | ||||
| 
 | ||||
| 	// fields should be updated in the database as well | ||||
| 	dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) | ||||
| 	suite.NoError(err) | ||||
| 	// Check database model of account as well. | ||||
| 	dbAccount, err := suite.db.GetAccountByID(ctx, testAccount.ID) | ||||
| 	if err != nil { | ||||
| 		suite.FailNow(err.Error()) | ||||
| 	} | ||||
| 	suite.True(*dbAccount.Locked) | ||||
| 	suite.Equal(displayName, dbAccount.DisplayName) | ||||
| 	suite.Equal(`<p><a href="http://localhost:8080/tags/hello" class="mention hashtag" rel="tag nofollow noreferrer noopener" target="_blank">#<span>hello</span></a> here i am!</p>`, dbAccount.Note) | ||||
| 	suite.Equal(noteExpected, dbAccount.Note) | ||||
| } | ||||
| 
 | ||||
| func (suite *AccountUpdateTestSuite) TestAccountUpdateWithMention() { | ||||
| 	testAccount := suite.testAccounts["local_account_1"] | ||||
| 	testAccount := >smodel.Account{} | ||||
| 	*testAccount = *suite.testAccounts["local_account_1"] | ||||
| 
 | ||||
| 	var ( | ||||
| 		ctx          = context.Background() | ||||
| 		locked       = true | ||||
| 		displayName  = "new display name" | ||||
| 		note         = "#hello here i am!\n\ngo check out @1happyturtle, they have a cool account!" | ||||
| 		noteExpected = "<p><a href=\"http://localhost:8080/tags/hello\" class=\"mention hashtag\" rel=\"tag nofollow noreferrer noopener\" target=\"_blank\">#<span>hello</span></a> here i am!<br><br>go check out <span class=\"h-card\"><a href=\"http://localhost:8080/@1happyturtle\" class=\"u-url mention\" rel=\"nofollow noreferrer noopener\" target=\"_blank\">@<span>1happyturtle</span></a></span>, they have a cool account!</p>" | ||||
| 	) | ||||
| 
 | ||||
| 	form := &apimodel.UpdateCredentialsRequest{ | ||||
| 	// Call update function. | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(ctx, testAccount, &apimodel.UpdateCredentialsRequest{ | ||||
| 		DisplayName: &displayName, | ||||
| 		Locked:      &locked, | ||||
| 		Note:        ¬e, | ||||
| 	}) | ||||
| 	if errWithCode != nil { | ||||
| 		suite.FailNow(errWithCode.Error()) | ||||
| 	} | ||||
| 
 | ||||
| 	// should get no error from the update function, and an api model account returned | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(context.Background(), testAccount, form) | ||||
| 	suite.NoError(errWithCode) | ||||
| 	suite.NotNil(apiAccount) | ||||
| 
 | ||||
| 	// fields on the profile should be updated | ||||
| 	// Returned profile should be updated. | ||||
| 	suite.True(apiAccount.Locked) | ||||
| 	suite.Equal(displayName, apiAccount.DisplayName) | ||||
| 	suite.Equal(noteExpected, apiAccount.Note) | ||||
| 
 | ||||
| 	// we should have an update in the client api channel | ||||
| 	msg := <-suite.fromClientAPIChan | ||||
| 	suite.Equal(ap.ActivityUpdate, msg.APActivityType) | ||||
| 	suite.Equal(ap.ObjectProfile, msg.APObjectType) | ||||
| 	suite.NotNil(msg.OriginAccount) | ||||
| 	suite.Equal(testAccount.ID, msg.OriginAccount.ID) | ||||
| 	suite.Nil(msg.TargetAccount) | ||||
| 	// We should have an update in the client api channel. | ||||
| 	suite.checkClientAPIChan(testAccount.ID) | ||||
| 
 | ||||
| 	// fields should be updated in the database as well | ||||
| 	dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) | ||||
| 	suite.NoError(err) | ||||
| 	// Check database model of account as well. | ||||
| 	dbAccount, err := suite.db.GetAccountByID(ctx, testAccount.ID) | ||||
| 	if err != nil { | ||||
| 		suite.FailNow(err.Error()) | ||||
| 	} | ||||
| 	suite.True(*dbAccount.Locked) | ||||
| 	suite.Equal(displayName, dbAccount.DisplayName) | ||||
| 	suite.Equal(noteExpected, dbAccount.Note) | ||||
| } | ||||
| 
 | ||||
| func (suite *AccountUpdateTestSuite) TestAccountUpdateWithMarkdownNote() { | ||||
| 	testAccount := suite.testAccounts["local_account_1"] | ||||
| 	testAccount := >smodel.Account{} | ||||
| 	*testAccount = *suite.testAccounts["local_account_1"] | ||||
| 
 | ||||
| 	note := "*hello* ~~here~~ i am!" | ||||
| 	expectedNote := `<p><em>hello</em> <del>here</del> i am!</p>` | ||||
| 	var ( | ||||
| 		ctx          = context.Background() | ||||
| 		note         = "*hello* ~~here~~ i am!" | ||||
| 		noteExpected = `<p><em>hello</em> <del>here</del> i am!</p>` | ||||
| 	) | ||||
| 
 | ||||
| 	form := &apimodel.UpdateCredentialsRequest{ | ||||
| 		Note: ¬e, | ||||
| 	// Set status content type of account 1 to markdown for this test. | ||||
| 	testAccount.StatusContentType = "text/markdown" | ||||
| 	if err := suite.db.UpdateAccount(ctx, testAccount, "status_content_type"); err != nil { | ||||
| 		suite.FailNow(err.Error()) | ||||
| 	} | ||||
| 
 | ||||
| 	// set default post content type of account 1 to markdown | ||||
| 	testAccount.StatusContentType = "text/markdown" | ||||
| 	// Call update function. | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(ctx, testAccount, &apimodel.UpdateCredentialsRequest{ | ||||
| 		Note: ¬e, | ||||
| 	}) | ||||
| 	if errWithCode != nil { | ||||
| 		suite.FailNow(errWithCode.Error()) | ||||
| 	} | ||||
| 
 | ||||
| 	// should get no error from the update function, and an api model account returned | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(context.Background(), testAccount, form) | ||||
| 	// reset test account to avoid breaking other tests | ||||
| 	testAccount.StatusContentType = "text/plain" | ||||
| 	suite.NoError(errWithCode) | ||||
| 	suite.NotNil(apiAccount) | ||||
| 	// Returned profile should be updated. | ||||
| 	suite.Equal(noteExpected, apiAccount.Note) | ||||
| 
 | ||||
| 	// fields on the profile should be updated | ||||
| 	suite.Equal(expectedNote, apiAccount.Note) | ||||
| 	// We should have an update in the client api channel. | ||||
| 	suite.checkClientAPIChan(testAccount.ID) | ||||
| 
 | ||||
| 	// we should have an update in the client api channel | ||||
| 	msg := <-suite.fromClientAPIChan | ||||
| 	suite.Equal(ap.ActivityUpdate, msg.APActivityType) | ||||
| 	suite.Equal(ap.ObjectProfile, msg.APObjectType) | ||||
| 	suite.NotNil(msg.OriginAccount) | ||||
| 	suite.Equal(testAccount.ID, msg.OriginAccount.ID) | ||||
| 	suite.Nil(msg.TargetAccount) | ||||
| 
 | ||||
| 	// fields should be updated in the database as well | ||||
| 	dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) | ||||
| 	// Check database model of account as well. | ||||
| 	dbAccount, err := suite.db.GetAccountByID(ctx, testAccount.ID) | ||||
| 	if err != nil { | ||||
| 		suite.FailNow(err.Error()) | ||||
| 	} | ||||
| 	suite.NoError(err) | ||||
| 	suite.Equal(expectedNote, dbAccount.Note) | ||||
| 	suite.Equal(noteExpected, dbAccount.Note) | ||||
| } | ||||
| 
 | ||||
| func (suite *AccountUpdateTestSuite) TestAccountUpdateWithFields() { | ||||
| 	testAccount := suite.testAccounts["local_account_1"] | ||||
| 	testAccount := >smodel.Account{} | ||||
| 	*testAccount = *suite.testAccounts["local_account_1"] | ||||
| 
 | ||||
| 	updateFields := []apimodel.UpdateField{ | ||||
| 	var ( | ||||
| 		ctx          = context.Background() | ||||
| 		updateFields = []apimodel.UpdateField{ | ||||
| 			{ | ||||
| 				Name:  func() *string { s := "favourite emoji"; return &s }(), | ||||
| 				Value: func() *string { s := ":rainbow:"; return &s }(), | ||||
|  | @ -161,31 +180,7 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateWithFields() { | |||
| 				Value: func() *string { s := "https://example.org"; return &s }(), | ||||
| 			}, | ||||
| 		} | ||||
| 
 | ||||
| 	form := &apimodel.UpdateCredentialsRequest{ | ||||
| 		FieldsAttributes: &updateFields, | ||||
| 	} | ||||
| 
 | ||||
| 	// should get no error from the update function, and an api model account returned | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(context.Background(), testAccount, form) | ||||
| 
 | ||||
| 	// reset test account to avoid breaking other tests | ||||
| 	testAccount.StatusContentType = "text/plain" | ||||
| 	suite.NoError(errWithCode) | ||||
| 	suite.NotNil(apiAccount) | ||||
| 	suite.EqualValues([]apimodel.Field{ | ||||
| 		{ | ||||
| 			Name:       "favourite emoji", | ||||
| 			Value:      ":rainbow:", | ||||
| 			VerifiedAt: (*string)(nil), | ||||
| 		}, | ||||
| 		{ | ||||
| 			Name:       "my website", | ||||
| 			Value:      "<a href=\"https://example.org\" rel=\"nofollow noreferrer noopener\" target=\"_blank\">https://example.org</a>", | ||||
| 			VerifiedAt: (*string)(nil), | ||||
| 		}, | ||||
| 	}, apiAccount.Fields) | ||||
| 	suite.EqualValues([]apimodel.Field{ | ||||
| 		fieldsExpectedRaw = []apimodel.Field{ | ||||
| 			{ | ||||
| 				Name:       "favourite emoji", | ||||
| 				Value:      ":rainbow:", | ||||
|  | @ -196,8 +191,20 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateWithFields() { | |||
| 				Value:      "https://example.org", | ||||
| 				VerifiedAt: (*string)(nil), | ||||
| 			}, | ||||
| 	}, apiAccount.Source.Fields) | ||||
| 	suite.EqualValues([]apimodel.Emoji{ | ||||
| 		} | ||||
| 		fieldsExpectedParsed = []apimodel.Field{ | ||||
| 			{ | ||||
| 				Name:       "favourite emoji", | ||||
| 				Value:      ":rainbow:", | ||||
| 				VerifiedAt: (*string)(nil), | ||||
| 			}, | ||||
| 			{ | ||||
| 				Name:       "my website", | ||||
| 				Value:      "<a href=\"https://example.org\" rel=\"nofollow noreferrer noopener\" target=\"_blank\">https://example.org</a>", | ||||
| 				VerifiedAt: (*string)(nil), | ||||
| 			}, | ||||
| 		} | ||||
| 		emojisExpected = []apimodel.Emoji{ | ||||
| 			{ | ||||
| 				Shortcode:       "rainbow", | ||||
| 				URL:             "http://localhost:8080/fileserver/01AY6P665V14JJR0AFVRT7311Y/emoji/original/01F8MH9H8E4VG3KDYJR9EGPXCQ.png", | ||||
|  | @ -205,27 +212,80 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateWithFields() { | |||
| 				VisibleInPicker: true, | ||||
| 				Category:        "reactions", | ||||
| 			}, | ||||
| 	}, apiAccount.Emojis) | ||||
| 		} | ||||
| 	) | ||||
| 
 | ||||
| 	// we should have an update in the client api channel | ||||
| 	msg := <-suite.fromClientAPIChan | ||||
| 	suite.Equal(ap.ActivityUpdate, msg.APActivityType) | ||||
| 	suite.Equal(ap.ObjectProfile, msg.APObjectType) | ||||
| 	suite.NotNil(msg.OriginAccount) | ||||
| 	suite.Equal(testAccount.ID, msg.OriginAccount.ID) | ||||
| 	suite.Nil(msg.TargetAccount) | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(ctx, testAccount, &apimodel.UpdateCredentialsRequest{ | ||||
| 		FieldsAttributes: &updateFields, | ||||
| 	}) | ||||
| 	if errWithCode != nil { | ||||
| 		suite.FailNow(errWithCode.Error()) | ||||
| 	} | ||||
| 
 | ||||
| 	// fields should be updated in the database as well | ||||
| 	dbAccount, err := suite.db.GetAccountByID(context.Background(), testAccount.ID) | ||||
| 	suite.NoError(err) | ||||
| 	suite.Equal("favourite emoji", dbAccount.Fields[0].Name) | ||||
| 	suite.Equal(":rainbow:", dbAccount.Fields[0].Value) | ||||
| 	suite.Equal("my website", dbAccount.Fields[1].Name) | ||||
| 	suite.Equal("<a href=\"https://example.org\" rel=\"nofollow noreferrer noopener\" target=\"_blank\">https://example.org</a>", dbAccount.Fields[1].Value) | ||||
| 	suite.Equal("favourite emoji", dbAccount.FieldsRaw[0].Name) | ||||
| 	suite.Equal(":rainbow:", dbAccount.FieldsRaw[0].Value) | ||||
| 	suite.Equal("my website", dbAccount.FieldsRaw[1].Name) | ||||
| 	suite.Equal("https://example.org", dbAccount.FieldsRaw[1].Value) | ||||
| 	// Returned profile should be updated. | ||||
| 	suite.EqualValues(fieldsExpectedRaw, apiAccount.Source.Fields) | ||||
| 	suite.EqualValues(fieldsExpectedParsed, apiAccount.Fields) | ||||
| 	suite.EqualValues(emojisExpected, apiAccount.Emojis) | ||||
| 
 | ||||
| 	// We should have an update in the client api channel. | ||||
| 	suite.checkClientAPIChan(testAccount.ID) | ||||
| 
 | ||||
| 	// Check database model of account as well. | ||||
| 	dbAccount, err := suite.db.GetAccountByID(ctx, testAccount.ID) | ||||
| 	if err != nil { | ||||
| 		suite.FailNow(err.Error()) | ||||
| 	} | ||||
| 	suite.Equal(fieldsExpectedParsed[0].Name, dbAccount.Fields[0].Name) | ||||
| 	suite.Equal(fieldsExpectedParsed[0].Value, dbAccount.Fields[0].Value) | ||||
| 	suite.Equal(fieldsExpectedParsed[1].Name, dbAccount.Fields[1].Name) | ||||
| 	suite.Equal(fieldsExpectedParsed[1].Value, dbAccount.Fields[1].Value) | ||||
| 	suite.Equal(fieldsExpectedRaw[0].Name, dbAccount.FieldsRaw[0].Name) | ||||
| 	suite.Equal(fieldsExpectedRaw[0].Value, dbAccount.FieldsRaw[0].Value) | ||||
| 	suite.Equal(fieldsExpectedRaw[1].Name, dbAccount.FieldsRaw[1].Name) | ||||
| 	suite.Equal(fieldsExpectedRaw[1].Value, dbAccount.FieldsRaw[1].Value) | ||||
| } | ||||
| 
 | ||||
| func (suite *AccountUpdateTestSuite) TestAccountUpdateNoteNotFields() { | ||||
| 	// local_account_2 already has some fields set. | ||||
| 	// We want to ensure that the fields don't change | ||||
| 	// even if the account note is updated. | ||||
| 	testAccount := >smodel.Account{} | ||||
| 	*testAccount = *suite.testAccounts["local_account_2"] | ||||
| 
 | ||||
| 	var ( | ||||
| 		ctx             = context.Background() | ||||
| 		fieldsRawBefore = len(testAccount.FieldsRaw) | ||||
| 		fieldsBefore    = len(testAccount.Fields) | ||||
| 		note            = "#hello here i am!" | ||||
| 		noteExpected    = `<p><a href="http://localhost:8080/tags/hello" class="mention hashtag" rel="tag nofollow noreferrer noopener" target="_blank">#<span>hello</span></a> here i am!</p>` | ||||
| 	) | ||||
| 
 | ||||
| 	// Call update function. | ||||
| 	apiAccount, errWithCode := suite.accountProcessor.Update(ctx, testAccount, &apimodel.UpdateCredentialsRequest{ | ||||
| 		Note: ¬e, | ||||
| 	}) | ||||
| 	if errWithCode != nil { | ||||
| 		suite.FailNow(errWithCode.Error()) | ||||
| 	} | ||||
| 
 | ||||
| 	// Returned profile should be updated. | ||||
| 	suite.True(apiAccount.Locked) | ||||
| 	suite.Equal(noteExpected, apiAccount.Note) | ||||
| 	suite.Equal(fieldsRawBefore, len(apiAccount.Source.Fields)) | ||||
| 	suite.Equal(fieldsBefore, len(apiAccount.Fields)) | ||||
| 
 | ||||
| 	// We should have an update in the client api channel. | ||||
| 	suite.checkClientAPIChan(testAccount.ID) | ||||
| 
 | ||||
| 	// Check database model of account as well. | ||||
| 	dbAccount, err := suite.db.GetAccountByID(ctx, testAccount.ID) | ||||
| 	if err != nil { | ||||
| 		suite.FailNow(err.Error()) | ||||
| 	} | ||||
| 	suite.True(*dbAccount.Locked) | ||||
| 	suite.Equal(noteExpected, dbAccount.Note) | ||||
| 	suite.Equal(fieldsRawBefore, len(dbAccount.FieldsRaw)) | ||||
| 	suite.Equal(fieldsBefore, len(dbAccount.Fields)) | ||||
| } | ||||
| 
 | ||||
| func TestAccountUpdateTestSuite(t *testing.T) { | ||||
|  |  | |||
|  | @ -526,6 +526,7 @@ func NewTestAccounts() map[string]*gtsmodel.Account { | |||
| 			SuspendedAt:           time.Time{}, | ||||
| 			HideCollections:       FalseBool(), | ||||
| 			SuspensionOrigin:      "", | ||||
| 			EnableRSS:             FalseBool(), | ||||
| 		}, | ||||
| 		"remote_account_1": { | ||||
| 			ID:                    "01F8MH5ZK5VRH73AKHQM6Y9VNX", | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue