[feature] Make instance thumbnail configurable via admin panel (#973)

* [feature] Make instance thumbnail configurable via admin panel

* log db errors in InstanceToAPIInstance

* only update instance in db if necessary

* start adding tests

* finish test

docs/api/swagger.yaml

@@ -969,6 +969,16 @@ definitions:
                 example: https://example.org/files/instance/thumbnail.jpeg
                 type: string
                 x-go-name: Thumbnail
+            thumbnail_description:
+                description: Description of the instance thumbnail.
+                example: picture of a cute lil' friendly sloth
+                type: string
+                x-go-name: ThumbnailDescription
+            thumbnail_type:
+                description: MIME type of the instance thumbnail.
+                example: image/png
+                type: string
+                x-go-name: ThumbnailType
             title:
                 description: The title of the instance.
                 example: GoToSocial Example Instance
@@ -3400,11 +3410,15 @@ paths:
                   maximum: 5000
                   name: terms
                   type: string
-                - description: Avatar of the instance.
+                - description: Thumbnail image to use for the instance.
                   in: formData
-                  name: avatar
+                  name: thumbnail
                   type: file
-                - description: Header of the instance.
+                - description: Image description of the submitted instance thumbnail.
+                  in: formData
+                  name: thumbnail_description
+                  type: string
+                - description: Header image to use for the instance.
                   in: formData
                   name: header
                   type: file

internal/api/client/instance/instancepatch.go

@@ -20,11 +20,13 @@
 
 import (
 	"errors"
+	"fmt"
 	"net/http"
 
 	"github.com/gin-gonic/gin"
 	"github.com/superseriousbusiness/gotosocial/internal/api"
 	"github.com/superseriousbusiness/gotosocial/internal/api/model"
+	"github.com/superseriousbusiness/gotosocial/internal/config"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/oauth"
 )
@@ -89,14 +91,19 @@
 //		maximum: 5000
 //		allowEmptyValue: true
 //	-
-//		name: avatar
+//		name: thumbnail
 //		in: formData
-//		description: Avatar of the instance.
+//		description: Thumbnail image to use for the instance.
 //		type: file
 //	-
+//		name: thumbnail_description
+//		in: formData
+//		description: Image description of the submitted instance thumbnail.
+//		type: string
+//	-
 //		name: header
 //		in: formData
-//		description: Header of the instance.
+//		description: Header image to use for the instance.
 //		type: file
 //
 //	security:
@@ -144,8 +151,7 @@ func (m *Module) InstanceUpdatePATCHHandler(c *gin.Context) {
 		return
 	}
 
-	if form.Title == nil && form.ContactUsername == nil && form.ContactEmail == nil && form.ShortDescription == nil && form.Description == nil && form.Terms == nil && form.Avatar == nil && form.Header == nil {
-		err := errors.New("empty form submitted")
+	if err := validateInstanceUpdate(form); err != nil {
 		api.ErrorHandler(c, gtserror.NewErrorBadRequest(err, err.Error()), m.processor.InstanceGet)
 		return
 	}
@@ -158,3 +164,35 @@ func (m *Module) InstanceUpdatePATCHHandler(c *gin.Context) {
 
 	c.JSON(http.StatusOK, i)
 }
+
+func validateInstanceUpdate(form *model.InstanceSettingsUpdateRequest) error {
+	if form.Title == nil &&
+		form.ContactUsername == nil &&
+		form.ContactEmail == nil &&
+		form.ShortDescription == nil &&
+		form.Description == nil &&
+		form.Terms == nil &&
+		form.Avatar == nil &&
+		form.AvatarDescription == nil &&
+		form.Header == nil {
+		return errors.New("empty form submitted")
+	}
+
+	maxImageSize := config.GetMediaImageMaxSize()
+	maxDescriptionChars := config.GetMediaDescriptionMaxChars()
+
+	// validate avatar if present
+	if form.Avatar != nil {
+		if size := form.Avatar.Size; size > int64(maxImageSize) {
+			return fmt.Errorf("file size limit exceeded: limit is %d bytes but desired instance avatar was %d bytes", maxImageSize, size)
+		}
+
+		if form.AvatarDescription != nil {
+			if length := len([]rune(*form.AvatarDescription)); length > maxDescriptionChars {
+				return fmt.Errorf("avatar description length must be less than %d characters (inclusive), but provided avatar description was %d chars", maxDescriptionChars, length)
+			}
+		}
+	}
+
+	return nil
+}

internal/api/client/instance/instancepatch_test.go

@@ -19,6 +19,8 @@
 package instance_test
 
 import (
+	"context"
+	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -246,6 +248,41 @@ func (suite *InstancePatchTestSuite) TestInstancePatch7() {
 	suite.Equal(`{"error":"Bad Request: mail: missing '@' or angle-addr"}`, string(b))
 }
 
+func (suite *InstancePatchTestSuite) TestInstancePatch8() {
+	requestBody, w, err := testrig.CreateMultipartFormData(
+		"thumbnail", "../../../../testrig/media/peglin.gif",
+		map[string]string{
+			"thumbnail_description": "A bouncing little green peglin.",
+		})
+	if err != nil {
+		panic(err)
+	}
+	bodyBytes := requestBody.Bytes()
+
+	// set up the request
+	recorder := httptest.NewRecorder()
+	ctx := suite.newContext(recorder, http.MethodPatch, instance.InstanceInformationPath, bodyBytes, w.FormDataContentType(), true)
+
+	// call the handler
+	suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
+	suite.Equal(http.StatusOK, recorder.Code)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+
+	b, err := io.ReadAll(result.Body)
+	suite.NoError(err)
+
+	instanceAccount, err := suite.db.GetInstanceAccount(context.Background(), "")
+	if err != nil {
+		suite.FailNow(err.Error())
+	}
+	suite.NotEmpty(instanceAccount.AvatarMediaAttachmentID)
+
+	expectedInstanceResponse := fmt.Sprintf(`{"uri":"http://localhost:8080","account_domain":"localhost:8080","title":"GoToSocial Testrig Instance","description":"\u003cp\u003eThis is the GoToSocial testrig. It doesn't federate or anything.\u003c/p\u003e\u003cp\u003eWhen the testrig is shut down, all data on it will be deleted.\u003c/p\u003e\u003cp\u003eDon't use this in production!\u003c/p\u003e","short_description":"\u003cp\u003eThis is the GoToSocial testrig. It doesn't federate or anything.\u003c/p\u003e\u003cp\u003eWhen the testrig is shut down, all data on it will be deleted.\u003c/p\u003e\u003cp\u003eDon't use this in production!\u003c/p\u003e","email":"admin@example.org","version":"0.0.0-testrig","registrations":true,"approval_required":true,"invites_enabled":false,"configuration":{"statuses":{"max_characters":5000,"max_media_attachments":6,"characters_reserved_per_url":25},"media_attachments":{"supported_mime_types":["image/jpeg","image/gif","image/png"],"image_size_limit":10485760,"image_matrix_limit":16777216,"video_size_limit":41943040,"video_frame_rate_limit":60,"video_matrix_limit":16777216},"polls":{"max_options":6,"max_characters_per_option":50,"min_expiration":300,"max_expiration":2629746},"accounts":{"allow_custom_css":true},"emojis":{"emoji_size_limit":51200}},"urls":{"streaming_api":"wss://localhost:8080"},"stats":{"domain_count":2,"status_count":16,"user_count":4},"thumbnail":"http://localhost:8080/fileserver/%s/attachment/original/%s.gif","thumbnail_type":"image/gif","thumbnail_description":"A bouncing little green peglin.","contact_account":{"id":"01F8MH17FWEB39HZJ76B6VXSKF","username":"admin","acct":"admin","display_name":"","locked":false,"bot":false,"created_at":"2022-05-17T13:10:59.000Z","note":"","url":"http://localhost:8080/@admin","avatar":"","avatar_static":"","header":"http://localhost:8080/assets/default_header.png","header_static":"http://localhost:8080/assets/default_header.png","followers_count":1,"following_count":1,"statuses_count":4,"last_status_at":"2021-10-20T10:41:37.000Z","emojis":[],"fields":[],"enable_rss":true},"max_toot_chars":5000}`, instanceAccount.ID, instanceAccount.AvatarMediaAttachmentID)
+	suite.Equal(expectedInstanceResponse, string(b))
+}
+
 func TestInstancePatchTestSuite(t *testing.T) {
 	suite.Run(t, &InstancePatchTestSuite{})
 }

internal/api/model/instance.go

@@ -77,6 +77,12 @@ type Instance struct {
 	// URL of the instance avatar/banner image.
 	// example: https://example.org/files/instance/thumbnail.jpeg
 	Thumbnail string `json:"thumbnail"`
+	// MIME type of the instance thumbnail.
+	// example: image/png
+	ThumbnailType string `json:"thumbnail_type,omitempty"`
+	// Description of the instance thumbnail.
+	// example: picture of a cute lil' friendly sloth
+	ThumbnailDescription string `json:"thumbnail_description,omitempty"`
 	// Contact account for the instance.
 	ContactAccount *Account `json:"contact_account,omitempty"`
 	// Maximum allowed length of a post on this instance, in characters.
@@ -221,7 +227,9 @@ type InstanceSettingsUpdateRequest struct {
 	// Terms and conditions of the instance, max 5,000 chars. HTML formatting accepted.
 	Terms *string `form:"terms" json:"terms" xml:"terms"`
 	// Image to use as the instance thumbnail.
-	Avatar *multipart.FileHeader `form:"avatar" json:"avatar" xml:"avatar"`
+	Avatar *multipart.FileHeader `form:"thumbnail" json:"thumbnail" xml:"thumbnail"`
+	// Image description for the instance avatar.
+	AvatarDescription *string `form:"thumbnail_description" json:"thumbnail_description" xml:"thumbnail_description"`
 	// Image to use as the instance header.
 	Header *multipart.FileHeader `form:"header" json:"header" xml:"header"`
 }

internal/processing/account/account.go

@@ -78,15 +78,14 @@ type Processor interface {
 	BlockCreate(ctx context.Context, requestingAccount *gtsmodel.Account, targetAccountID string) (*apimodel.Relationship, gtserror.WithCode)
 	// BlockRemove handles the removal of a block from requestingAccount to targetAccountID, either remote or local.
 	BlockRemove(ctx context.Context, requestingAccount *gtsmodel.Account, targetAccountID string) (*apimodel.Relationship, gtserror.WithCode)
-
-	// UpdateHeader does the dirty work of checking the header part of an account update form,
-	// parsing and checking the image, and doing the necessary updates in the database for this to become
-	// the account's new header image.
-	UpdateAvatar(ctx context.Context, avatar *multipart.FileHeader, accountID string) (*gtsmodel.MediaAttachment, error)
 	// UpdateAvatar does the dirty work of checking the avatar part of an account update form,
 	// parsing and checking the image, and doing the necessary updates in the database for this to become
 	// the account's new avatar image.
-	UpdateHeader(ctx context.Context, header *multipart.FileHeader, accountID string) (*gtsmodel.MediaAttachment, error)
+	UpdateAvatar(ctx context.Context, avatar *multipart.FileHeader, description *string, accountID string) (*gtsmodel.MediaAttachment, error)
+	// UpdateHeader does the dirty work of checking the header part of an account update form,
+	// parsing and checking the image, and doing the necessary updates in the database for this to become
+	// the account's new header image.
+	UpdateHeader(ctx context.Context, header *multipart.FileHeader, description *string, accountID string) (*gtsmodel.MediaAttachment, error)
 }
 
 type processor struct {

internal/processing/account/update.go

@@ -100,7 +100,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form
 	}
 
 	if form.Avatar != nil && form.Avatar.Size != 0 {
-		avatarInfo, err := p.UpdateAvatar(ctx, form.Avatar, account.ID)
+		avatarInfo, err := p.UpdateAvatar(ctx, form.Avatar, nil, account.ID)
 		if err != nil {
 			return nil, gtserror.NewErrorBadRequest(err)
 		}
@@ -110,7 +110,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form
 	}
 
 	if form.Header != nil && form.Header.Size != 0 {
-		headerInfo, err := p.UpdateHeader(ctx, form.Header, account.ID)
+		headerInfo, err := p.UpdateHeader(ctx, form.Header, nil, account.ID)
 		if err != nil {
 			return nil, gtserror.NewErrorBadRequest(err)
 		}
@@ -186,7 +186,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form
 // UpdateAvatar does the dirty work of checking the avatar part of an account update form,
 // parsing and checking the image, and doing the necessary updates in the database for this to become
 // the account's new avatar image.
-func (p *processor) UpdateAvatar(ctx context.Context, avatar *multipart.FileHeader, accountID string) (*gtsmodel.MediaAttachment, error) {
+func (p *processor) UpdateAvatar(ctx context.Context, avatar *multipart.FileHeader, description *string, accountID string) (*gtsmodel.MediaAttachment, error) {
 	maxImageSize := config.GetMediaImageMaxSize()
 	if avatar.Size > int64(maxImageSize) {
 		return nil, fmt.Errorf("UpdateAvatar: avatar with size %d exceeded max image size of %d bytes", avatar.Size, maxImageSize)
@@ -199,7 +199,8 @@ func (p *processor) UpdateAvatar(ctx context.Context, avatar *multipart.FileHead
 
 	isAvatar := true
 	ai := &media.AdditionalMediaInfo{
-		Avatar: &isAvatar,
+		Avatar:      &isAvatar,
+		Description: description,
 	}
 
 	processingMedia, err := p.mediaManager.ProcessMedia(ctx, dataFunc, nil, accountID, ai)
@@ -213,7 +214,7 @@ func (p *processor) UpdateAvatar(ctx context.Context, avatar *multipart.FileHead
 // UpdateHeader does the dirty work of checking the header part of an account update form,
 // parsing and checking the image, and doing the necessary updates in the database for this to become
 // the account's new header image.
-func (p *processor) UpdateHeader(ctx context.Context, header *multipart.FileHeader, accountID string) (*gtsmodel.MediaAttachment, error) {
+func (p *processor) UpdateHeader(ctx context.Context, header *multipart.FileHeader, description *string, accountID string) (*gtsmodel.MediaAttachment, error) {
 	maxImageSize := config.GetMediaImageMaxSize()
 	if header.Size > int64(maxImageSize) {
 		return nil, fmt.Errorf("UpdateHeader: header with size %d exceeded max image size of %d bytes", header.Size, maxImageSize)

internal/processing/instance.go

@@ -208,24 +208,42 @@ func (p *processor) InstancePatch(ctx context.Context, form *apimodel.InstanceSe
 		i.Terms = text.SanitizeHTML(*form.Terms) // html is OK in site terms, but we should sanitize it
 	}
 
-	// process avatar if provided
+	var updateInstanceAccount bool
+
+	// process instance avatar if provided
 	if form.Avatar != nil && form.Avatar.Size != 0 {
-		_, err := p.accountProcessor.UpdateAvatar(ctx, form.Avatar, ia.ID)
+		avatarInfo, err := p.accountProcessor.UpdateAvatar(ctx, form.Avatar, form.AvatarDescription, ia.ID)
 		if err != nil {
 			return nil, gtserror.NewErrorBadRequest(err, "error processing avatar")
 		}
+		ia.AvatarMediaAttachmentID = avatarInfo.ID
+		ia.AvatarMediaAttachment = avatarInfo
+		updateInstanceAccount = true
 	}
 
-	// process header if provided
+	// process instance header if provided
 	if form.Header != nil && form.Header.Size != 0 {
-		_, err := p.accountProcessor.UpdateHeader(ctx, form.Header, ia.ID)
+		headerInfo, err := p.accountProcessor.UpdateHeader(ctx, form.Header, nil, ia.ID)
 		if err != nil {
 			return nil, gtserror.NewErrorBadRequest(err, "error processing header")
 		}
+		ia.HeaderMediaAttachmentID = headerInfo.ID
+		ia.HeaderMediaAttachment = headerInfo
+		updateInstanceAccount = true
 	}
 
-	if err := p.db.UpdateByID(ctx, i, i.ID, updatingColumns...); err != nil {
-		return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error updating instance %s: %s", host, err))
+	if updateInstanceAccount {
+		// if either avatar or header is updated, we need
+		// to update the instance account that stores them
+		if _, err := p.db.UpdateAccount(ctx, ia); err != nil {
+			return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error updating instance account: %s", err))
+		}
+	}
+
+	if len(updatingColumns) != 0 {
+		if err := p.db.UpdateByID(ctx, i, i.ID, updatingColumns...); err != nil {
+			return nil, gtserror.NewErrorInternalError(fmt.Errorf("db error updating instance %s: %s", host, err))
+		}
 	}
 
 	ai, err := p.tc.InstanceToAPIInstance(ctx, i)

internal/typeutils/internaltofrontend.go

@@ -20,6 +20,7 @@
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 
@@ -665,12 +666,25 @@ func (c *converter) InstanceToAPIInstance(ctx context.Context, i *gtsmodel.Insta
 		mi.AccountDomain = config.GetAccountDomain()
 
 		if ia, err := c.db.GetInstanceAccount(ctx, ""); err == nil {
-			if ia.HeaderMediaAttachment != nil {
-				// take instance account header as instance thumbnail
-				mi.Thumbnail = ia.HeaderMediaAttachment.URL
-			} else {
-				// or just use a default
-				mi.Thumbnail = config.GetProtocol() + "://" + host + "/assets/logo.png"
+			// assume default logo
+			mi.Thumbnail = config.GetProtocol() + "://" + host + "/assets/logo.png"
+
+			// take instance account avatar as instance thumbnail if we can
+			if ia.AvatarMediaAttachmentID != "" {
+				if ia.AvatarMediaAttachment == nil {
+					avi, err := c.db.GetAttachmentByID(ctx, ia.AvatarMediaAttachmentID)
+					if err == nil {
+						ia.AvatarMediaAttachment = avi
+					} else if !errors.Is(err, db.ErrNoEntries) {
+						log.Errorf("InstanceToAPIInstance: error getting instance avatar attachment with id %s: %s", ia.AvatarMediaAttachmentID, err)
+					}
+				}
+
+				if ia.AvatarMediaAttachment != nil {
+					mi.Thumbnail = ia.AvatarMediaAttachment.URL
+					mi.ThumbnailType = ia.AvatarMediaAttachment.File.ContentType
+					mi.ThumbnailDescription = ia.AvatarMediaAttachment.Description
+				}
 			}
 		}
 

web/source/settings/admin/settings.js

@@ -36,6 +36,7 @@ const {
 
 module.exports = function AdminSettings() {
 	const dispatch = Redux.useDispatch();
+	const instance = Redux.useSelector(state => state.instances.adminSettings);
 
 	const [errorMsg, setError] = React.useState("");
 	const [statusMsg, setStatus] = React.useState("");
@@ -54,6 +55,23 @@ module.exports = function AdminSettings() {
 				placeHolder="My GoToSocial instance"
 			/>
 
+			<div className="file-upload">
+				<h3>Instance thumbnail</h3>
+				<div>
+					<img className="preview avatar" src={instance.thumbnail} alt={instance.thumbnail ? `Thumbnail image for the instance` : "No instance thumbnail image set"} />
+					<File 
+						id="thumbnail"
+						fileType="image/*"
+					/>
+				</div>
+			</div>
+
+			<TextInput
+				id="thumbnail_description"
+				name="Instance thumbnail description"
+				placeHolder="A cute little picture of a smiling sloth."
+			/>
+
 			<TextArea
 				id="short_description"
 				name="Short description"
@@ -82,27 +100,6 @@ module.exports = function AdminSettings() {
 				placeHolder=""
 			/>
 
-			{/* <div className="file-upload">
-				<h3>Instance avatar</h3>
-				<div>
-					<img className="preview avatar" src={instance.avatar} alt={instance.avatar ? `Avatar image for the instance` : "No instance avatar image set"} />
-					<File 
-						id="avatar"
-						fileType="image/*"
-					/>
-				</div>
-			</div>
-
-			<div className="file-upload">
-				<h3>Instance header</h3>
-				<div>
-					<img className="preview header" src={instance.header} alt={instance.header ? `Header image for the instance` : "No instance header image set"} />
-					<File 
-						id="header"
-						fileType="image/*"
-					/>
-				</div>
-			</div> */}
 			<Submit onClick={updateSettings} label="Save" errorMsg={errorMsg} statusMsg={statusMsg} />
 		</div>
 	);

web/source/settings/lib/api/admin.js

@@ -32,9 +32,9 @@ module.exports = function ({ apiCall, getChanges }) {
 					const state = getState().instances.adminSettings;
 
 					const update = getChanges(state, {
-						formKeys: ["title", "short_description", "description", "contact_account.username", "email", "terms"],
+						formKeys: ["title", "short_description", "description", "contact_account.username", "email", "terms", "thumbnail_description"],
 						renamedKeys: {"contact_account.username": "contact_username"},
-						// fileKeys: ["avatar", "header"]
+						fileKeys: ["thumbnail"]
 					});
 
 					return dispatch(apiCall("PATCH", "/api/v1/instance", update, "form"));

web/template/header.tmpl

@@ -20,7 +20,7 @@
 	{{ end }}<meta name="og:image" content="{{ .ogMeta.Image }}">
 	{{ if .ogMeta.ImageWidth }}<meta name="og:image:width" content="{{ .ogMeta.ImageWidth }}">
 	<meta name="og:image:height" content="{{ .ogMeta.ImageHeight }}">
-	{{ end }}{{ end }}<link rel="shortcut icon" href="/assets/logo.png" type="image/png">
+	{{ end }}{{ end }}<link rel="shortcut icon" href="{{ .instance.Thumbnail }}" type="{{ if .instance.ThumbnailType }}{{ .instance.ThumbnailType }}{{ else }}image/png{{ end }}">
 	<link rel="stylesheet" href="/assets/dist/_colors.css">
 	<link rel="stylesheet" href="/assets/dist/base.css">
 	{{range .stylesheets}}<link rel="stylesheet" href="{{.}}">
@@ -30,7 +30,7 @@
 	<div class="page">
 		<header>
 			<a aria-label="instance homepage" href="/" class="nounderline header">
-				<img src="/assets/logo.png" alt="Instance Logo"/>
+				<img src="{{ .instance.Thumbnail }}" alt="{{ if .instance.ThumbnailDescription }}{{ .instance.ThumbnailDescription }}{{ else }}Instance Logo{{ end }}"/>
 				<div>
 					<h1>
 						{{.instance.Title}}