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
This commit is contained in:
Claire 2023-01-18 16:33:55 +01:00 committed by GitHub
parent d1387579b9
commit 4b92e59f4f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 83 additions and 11 deletions

View file

@ -79,6 +79,7 @@ class Api::V1::StatusesController < Api::BaseController
current_account.id, current_account.id,
text: status_params[:status], text: status_params[:status],
media_ids: status_params[:media_ids], media_ids: status_params[:media_ids],
media_attributes: status_params[:media_attributes],
sensitive: status_params[:sensitive], sensitive: status_params[:sensitive],
language: status_params[:language], language: status_params[:language],
spoiler_text: status_params[:spoiler_text], spoiler_text: status_params[:spoiler_text],
@ -128,6 +129,12 @@ class Api::V1::StatusesController < Api::BaseController
:language, :language,
:scheduled_at, :scheduled_at,
media_ids: [], media_ids: [],
media_attributes: [
:id,
:thumbnail,
:description,
:focus,
],
poll: [ poll: [
:multiple, :multiple,
:hide_totals, :hide_totals,

View file

@ -160,6 +160,18 @@ export function submitCompose(routerHistory) {
dispatch(submitComposeRequest()); 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({ api(getState).request({
url: statusId === null ? '/api/v1/statuses' : `/api/v1/statuses/${statusId}`, url: statusId === null ? '/api/v1/statuses' : `/api/v1/statuses/${statusId}`,
method: statusId === null ? 'post' : 'put', method: statusId === null ? 'post' : 'put',
@ -167,6 +179,7 @@ export function submitCompose(routerHistory) {
status, status,
in_reply_to_id: getState().getIn(['compose', 'in_reply_to'], null), in_reply_to_id: getState().getIn(['compose', 'in_reply_to'], null),
media_ids: media.map(item => item.get('id')), media_ids: media.map(item => item.get('id')),
media_attributes,
sensitive: getState().getIn(['compose', 'sensitive']), sensitive: getState().getIn(['compose', 'sensitive']),
spoiler_text: getState().getIn(['compose', 'spoiler']) ? getState().getIn(['compose', 'spoiler_text'], '') : '', spoiler_text: getState().getIn(['compose', 'spoiler']) ? getState().getIn(['compose', 'spoiler_text'], '') : '',
visibility: getState().getIn(['compose', 'privacy']), visibility: getState().getIn(['compose', 'privacy']),
@ -375,11 +388,31 @@ export function changeUploadCompose(id, params) {
return (dispatch, getState) => { return (dispatch, getState) => {
dispatch(changeUploadComposeRequest()); dispatch(changeUploadComposeRequest());
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 => { api(getState).put(`/api/v1/media/${id}`, params).then(response => {
dispatch(changeUploadComposeSuccess(response.data)); dispatch(changeUploadComposeSuccess(response.data, false));
}).catch(error => { }).catch(error => {
dispatch(changeUploadComposeFail(id, error)); dispatch(changeUploadComposeFail(id, error));
}); });
}
}; };
} }
@ -390,10 +423,11 @@ export function changeUploadComposeRequest() {
}; };
} }
export function changeUploadComposeSuccess(media) { export function changeUploadComposeSuccess(media, attached) {
return { return {
type: COMPOSE_UPLOAD_CHANGE_SUCCESS, type: COMPOSE_UPLOAD_CHANGE_SUCCESS,
media: media, media: media,
attached: attached,
skipLoading: true, skipLoading: true,
}; };
} }

View file

@ -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-thumbnail' style={{ transform: `scale(${scale})`, backgroundImage: `url(${media.get('preview_url')})`, backgroundPosition: `${x}% ${y}%` }}>
<div className='compose-form__upload__actions'> <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> <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> </div>
{(media.get('description') || '').length === 0 && !!media.get('unattached') && ( {(media.get('description') || '').length === 0 && (
<div className='compose-form__upload__warning'> <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> <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> </div>

View file

@ -320,7 +320,7 @@ class FocalPointModal extends ImmutablePureComponent {
<React.Fragment> <React.Fragment>
<label className='setting-text-label' htmlFor='upload-modal__thumbnail'><FormattedMessage id='upload_form.thumbnail' defaultMessage='Change thumbnail' /></label> <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> <label>
<span style={{ display: 'none' }}>{intl.formatMessage(messages.chooseImage)}</span> <span style={{ display: 'none' }}>{intl.formatMessage(messages.chooseImage)}</span>

View file

@ -444,7 +444,7 @@ export default function compose(state = initialState, action) {
.setIn(['media_modal', 'dirty'], false) .setIn(['media_modal', 'dirty'], false)
.update('media_attachments', list => list.map(item => { .update('media_attachments', list => list.map(item => {
if (item.get('id') === action.media.id) { if (item.get('id') === action.media.id) {
return fromJS(action.media).set('unattached', true); return fromJS(action.media).set('unattached', !action.attached);
} }
return item; return item;

View file

@ -10,6 +10,7 @@ class UpdateStatusService < BaseService
# @param [Integer] account_id # @param [Integer] account_id
# @param [Hash] options # @param [Hash] options
# @option options [Array<Integer>] :media_ids # @option options [Array<Integer>] :media_ids
# @option options [Array<Hash>] :media_attributes
# @option options [Hash] :poll # @option options [Hash] :poll
# @option options [String] :text # @option options [String] :text
# @option options [String] :spoiler_text # @option options [String] :spoiler_text
@ -50,10 +51,18 @@ class UpdateStatusService < BaseService
next_media_attachments = validate_media! next_media_attachments = validate_media!
added_media_attachments = next_media_attachments - previous_media_attachments 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) 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) @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 @status.media_attachments.reload
end end

View file

@ -87,6 +87,28 @@ RSpec.describe UpdateStatusService, type: :service do
end end
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 context 'when poll changes' do
let(:account) { Fabricate(:account) } 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 }) } 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 }) }