From 4b92e59f4fea4486ee6e5af7421e7945d5f7f998 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Wed, 18 Jan 2023 16:33:55 +0100
Subject: [PATCH] Add support for editing media description and focus point of
 already-posted statuses (#20878)

* Add backend support for editing media attachments of existing posts

* Allow editing media attachments of already-posted toots

* Add tests
---
 app/controllers/api/v1/statuses_controller.rb |  7 +++
 app/javascript/mastodon/actions/compose.js    | 46 ++++++++++++++++---
 .../features/compose/components/upload.js     |  4 +-
 .../ui/components/focal_point_modal.js        |  2 +-
 app/javascript/mastodon/reducers/compose.js   |  2 +-
 app/services/update_status_service.rb         | 11 ++++-
 spec/services/update_status_service_spec.rb   | 22 +++++++++
 7 files changed, 83 insertions(+), 11 deletions(-)

diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb
index 6290a1746..9a8c0c161 100644
--- a/app/controllers/api/v1/statuses_controller.rb
+++ b/app/controllers/api/v1/statuses_controller.rb
@@ -79,6 +79,7 @@ class Api::V1::StatusesController < Api::BaseController
       current_account.id,
       text: status_params[:status],
       media_ids: status_params[:media_ids],
+      media_attributes: status_params[:media_attributes],
       sensitive: status_params[:sensitive],
       language: status_params[:language],
       spoiler_text: status_params[:spoiler_text],
@@ -128,6 +129,12 @@ class Api::V1::StatusesController < Api::BaseController
       :language,
       :scheduled_at,
       media_ids: [],
+      media_attributes: [
+        :id,
+        :thumbnail,
+        :description,
+        :focus,
+      ],
       poll: [
         :multiple,
         :hide_totals,
diff --git a/app/javascript/mastodon/actions/compose.js b/app/javascript/mastodon/actions/compose.js
index 531a5eb2b..72e592935 100644
--- a/app/javascript/mastodon/actions/compose.js
+++ b/app/javascript/mastodon/actions/compose.js
@@ -160,6 +160,18 @@ export function submitCompose(routerHistory) {
 
     dispatch(submitComposeRequest());
 
+    // If we're editing a post with media attachments, those have not
+    // necessarily been changed on the server. Do it now in the same
+    // API call.
+    let media_attributes;
+    if (statusId !== null) {
+      media_attributes = media.map(item => ({
+        id: item.get('id'),
+        description: item.get('description'),
+        focus: item.get('focus'),
+      }));
+    }
+
     api(getState).request({
       url: statusId === null ? '/api/v1/statuses' : `/api/v1/statuses/${statusId}`,
       method: statusId === null ? 'post' : 'put',
@@ -167,6 +179,7 @@ export function submitCompose(routerHistory) {
         status,
         in_reply_to_id: getState().getIn(['compose', 'in_reply_to'], null),
         media_ids: media.map(item => item.get('id')),
+        media_attributes,
         sensitive: getState().getIn(['compose', 'sensitive']),
         spoiler_text: getState().getIn(['compose', 'spoiler']) ? getState().getIn(['compose', 'spoiler_text'], '') : '',
         visibility: getState().getIn(['compose', 'privacy']),
@@ -375,11 +388,31 @@ export function changeUploadCompose(id, params) {
   return (dispatch, getState) => {
     dispatch(changeUploadComposeRequest());
 
-    api(getState).put(`/api/v1/media/${id}`, params).then(response => {
-      dispatch(changeUploadComposeSuccess(response.data));
-    }).catch(error => {
-      dispatch(changeUploadComposeFail(id, error));
-    });
+    let media = getState().getIn(['compose', 'media_attachments']).find((item) => item.get('id') === id);
+
+    // Editing already-attached media is deferred to editing the post itself.
+    // For simplicity's sake, fake an API reply.
+    if (media && !media.get('unattached')) {
+      let { description, focus } = params;
+      const data = media.toJS();
+
+      if (description) {
+        data.description = description;
+      }
+
+      if (focus) {
+        focus = focus.split(',');
+        data.meta = { focus: { x: parseFloat(focus[0]), y: parseFloat(focus[1]) } };
+      }
+
+      dispatch(changeUploadComposeSuccess(data, true));
+    } else {
+      api(getState).put(`/api/v1/media/${id}`, params).then(response => {
+        dispatch(changeUploadComposeSuccess(response.data, false));
+      }).catch(error => {
+        dispatch(changeUploadComposeFail(id, error));
+      });
+    }
   };
 }
 
@@ -390,10 +423,11 @@ export function changeUploadComposeRequest() {
   };
 }
 
-export function changeUploadComposeSuccess(media) {
+export function changeUploadComposeSuccess(media, attached) {
   return {
     type: COMPOSE_UPLOAD_CHANGE_SUCCESS,
     media: media,
+    attached: attached,
     skipLoading: true,
   };
 }
diff --git a/app/javascript/mastodon/features/compose/components/upload.js b/app/javascript/mastodon/features/compose/components/upload.js
index b08307ade..af06ce1bf 100644
--- a/app/javascript/mastodon/features/compose/components/upload.js
+++ b/app/javascript/mastodon/features/compose/components/upload.js
@@ -43,10 +43,10 @@ export default class Upload extends ImmutablePureComponent {
             <div className='compose-form__upload-thumbnail' style={{ transform: `scale(${scale})`, backgroundImage: `url(${media.get('preview_url')})`, backgroundPosition: `${x}% ${y}%` }}>
               <div className='compose-form__upload__actions'>
                 <button type='button' className='icon-button' onClick={this.handleUndoClick}><Icon id='times' /> <FormattedMessage id='upload_form.undo' defaultMessage='Delete' /></button>
-                {!!media.get('unattached') && (<button type='button' className='icon-button' onClick={this.handleFocalPointClick}><Icon id='pencil' /> <FormattedMessage id='upload_form.edit' defaultMessage='Edit' /></button>)}
+                <button type='button' className='icon-button' onClick={this.handleFocalPointClick}><Icon id='pencil' /> <FormattedMessage id='upload_form.edit' defaultMessage='Edit' /></button>
               </div>
 
-              {(media.get('description') || '').length === 0 && !!media.get('unattached') && (
+              {(media.get('description') || '').length === 0 && (
                 <div className='compose-form__upload__warning'>
                   <button type='button' className='icon-button' onClick={this.handleFocalPointClick}><Icon id='info-circle' /> <FormattedMessage id='upload_form.description_missing' defaultMessage='No description added' /></button>
                 </div>
diff --git a/app/javascript/mastodon/features/ui/components/focal_point_modal.js b/app/javascript/mastodon/features/ui/components/focal_point_modal.js
index 479f4abd2..b9dbd9390 100644
--- a/app/javascript/mastodon/features/ui/components/focal_point_modal.js
+++ b/app/javascript/mastodon/features/ui/components/focal_point_modal.js
@@ -320,7 +320,7 @@ class FocalPointModal extends ImmutablePureComponent {
               <React.Fragment>
                 <label className='setting-text-label' htmlFor='upload-modal__thumbnail'><FormattedMessage id='upload_form.thumbnail' defaultMessage='Change thumbnail' /></label>
 
-                <Button disabled={isUploadingThumbnail} text={intl.formatMessage(messages.chooseImage)} onClick={this.handleFileInputClick} />
+                <Button disabled={isUploadingThumbnail || !media.get('unattached')} text={intl.formatMessage(messages.chooseImage)} onClick={this.handleFileInputClick} />
 
                 <label>
                   <span style={{ display: 'none' }}>{intl.formatMessage(messages.chooseImage)}</span>
diff --git a/app/javascript/mastodon/reducers/compose.js b/app/javascript/mastodon/reducers/compose.js
index 9ce7e97ed..1760c7c89 100644
--- a/app/javascript/mastodon/reducers/compose.js
+++ b/app/javascript/mastodon/reducers/compose.js
@@ -444,7 +444,7 @@ export default function compose(state = initialState, action) {
       .setIn(['media_modal', 'dirty'], false)
       .update('media_attachments', list => list.map(item => {
         if (item.get('id') === action.media.id) {
-          return fromJS(action.media).set('unattached', true);
+          return fromJS(action.media).set('unattached', !action.attached);
         }
 
         return item;
diff --git a/app/services/update_status_service.rb b/app/services/update_status_service.rb
index c4c934976..f75fdf55d 100644
--- a/app/services/update_status_service.rb
+++ b/app/services/update_status_service.rb
@@ -10,6 +10,7 @@ class UpdateStatusService < BaseService
   # @param [Integer] account_id
   # @param [Hash] options
   # @option options [Array<Integer>] :media_ids
+  # @option options [Array<Hash>] :media_attributes
   # @option options [Hash] :poll
   # @option options [String] :text
   # @option options [String] :spoiler_text
@@ -50,10 +51,18 @@ class UpdateStatusService < BaseService
     next_media_attachments     = validate_media!
     added_media_attachments    = next_media_attachments - previous_media_attachments
 
+    (@options[:media_attributes] || []).each do |attributes|
+      media = next_media_attachments.find { |attachment| attachment.id == attributes[:id].to_i }
+      next if media.nil?
+
+      media.update!(attributes.slice(:thumbnail, :description, :focus))
+      @media_attachments_changed ||= media.significantly_changed?
+    end
+
     MediaAttachment.where(id: added_media_attachments.map(&:id)).update_all(status_id: @status.id)
 
     @status.ordered_media_attachment_ids = (@options[:media_ids] || []).map(&:to_i) & next_media_attachments.map(&:id)
-    @media_attachments_changed = previous_media_attachments.map(&:id) != @status.ordered_media_attachment_ids
+    @media_attachments_changed ||= previous_media_attachments.map(&:id) != @status.ordered_media_attachment_ids
     @status.media_attachments.reload
   end
 
diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb
index 71a73be5b..16e981d2b 100644
--- a/spec/services/update_status_service_spec.rb
+++ b/spec/services/update_status_service_spec.rb
@@ -87,6 +87,28 @@ RSpec.describe UpdateStatusService, type: :service do
     end
   end
 
+  context 'when already-attached media changes' do
+    let!(:status) { Fabricate(:status, text: 'Foo') }
+    let!(:media_attachment) { Fabricate(:media_attachment, account: status.account, description: 'Old description') }
+
+    before do
+      status.media_attachments << media_attachment
+      subject.call(status, status.account_id, text: 'Foo', media_ids: [media_attachment.id], media_attributes: [{ id: media_attachment.id, description: 'New description' }])
+    end
+
+    it 'does not detach media attachment' do
+      expect(media_attachment.reload.status_id).to eq status.id
+    end
+
+    it 'updates the media attachment description' do
+      expect(media_attachment.reload.description).to eq 'New description'
+    end
+
+    it 'saves edit history' do
+      expect(status.edits.map { |edit| edit.ordered_media_attachments.map(&:description) }).to eq [['Old description'], ['New description']]
+    end
+  end
+
   context 'when poll changes' do
     let(:account) { Fabricate(:account) }
     let!(:status) { Fabricate(:status, text: 'Foo', account: account, poll_attributes: {options: %w(Foo Bar), account: account, multiple: false, hide_totals: false, expires_at: 7.days.from_now }) }