Skip to content

Commit

Permalink
fix(media): prevent duplicate media downloads by checking message fet…
Browse files Browse the repository at this point in the history
…ch status (#2533)
  • Loading branch information
domw30 authored Dec 23, 2024
1 parent 9989202 commit 7d70300
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 5 deletions.
1 change: 1 addition & 0 deletions src/components/chat-view-container/chat-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ export class ChatView extends React.Component<Properties, State> {
loadAttachmentDetails={this.props.loadAttachmentDetails}
sendEmojiReaction={this.props.sendEmojiReaction}
reactions={message.reactions}
messagesFetchStatus={this.props.messagesFetchStatus}
{...message}
/>
</div>
Expand Down
29 changes: 26 additions & 3 deletions src/components/message/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ParentMessage } from './parent-message';
import { IconAlertCircle } from '@zero-tech/zui/icons';
import { Spinner } from '@zero-tech/zui/components/LoadingIndicator/Spinner';
import AttachmentCards from '../../platform-apps/channels/attachment-cards';
import { MessagesFetchState } from '../../store/channels';

describe('message', () => {
const sender = {
Expand Down Expand Up @@ -125,32 +126,52 @@ describe('message', () => {
expect(wrapper).toHaveElement(Spinner);
});

it('calls loadAttachmentDetails if no media url', () => {
it('calls loadAttachmentDetails if no media url and messagesFetchStatus is success', () => {
const loadAttachmentDetails = jest.fn();

subject({ messageId: 'test-id', loadAttachmentDetails, media: { url: null, type: MediaType.Image } });
subject({
messageId: 'test-id',
loadAttachmentDetails,
media: { url: null, type: MediaType.Image },
messagesFetchStatus: MessagesFetchState.SUCCESS,
});

expect(loadAttachmentDetails).toHaveBeenCalled();
});

it('calls loadAttachmentDetails if url is a matrix media url', () => {
it('calls loadAttachmentDetails if url is a matrix media url and messagesFetchStatus is success', () => {
const loadAttachmentDetails = jest.fn();
subject({
messageId: 'test-id',
loadAttachmentDetails,
media: { url: 'mxc://some-test-matrix-url', type: MediaType.Image },
messagesFetchStatus: MessagesFetchState.SUCCESS,
});

expect(loadAttachmentDetails).toHaveBeenCalled();
});

it('does not call loadAttachmentDetails if messagesFetchStatus is not success', () => {
const loadAttachmentDetails = jest.fn();

subject({
messageId: 'test-id',
loadAttachmentDetails,
media: { url: null, type: MediaType.Image },
messagesFetchStatus: MessagesFetchState.FAILED,
});

expect(loadAttachmentDetails).not.toHaveBeenCalled();
});

it('does not call loadAttachmentDetails if url is defined and not a matrix media url', () => {
const loadAttachmentDetails = jest.fn();

subject({
messageId: 'test-id',
loadAttachmentDetails,
media: { url: 'some-test-url', type: MediaType.Image, downloadStatus: MediaDownloadStatus.Failed },
messagesFetchStatus: MessagesFetchState.SUCCESS,
});

expect(loadAttachmentDetails).not.toHaveBeenCalled();
Expand All @@ -163,6 +184,7 @@ describe('message', () => {
messageId: 'test-id',
loadAttachmentDetails,
media: { url: null, type: MediaType.Image, downloadStatus: MediaDownloadStatus.Failed },
messagesFetchStatus: MessagesFetchState.SUCCESS,
});

expect(loadAttachmentDetails).not.toHaveBeenCalled();
Expand All @@ -175,6 +197,7 @@ describe('message', () => {
messageId: 'test-id',
loadAttachmentDetails,
media: { url: null, type: MediaType.Image, downloadStatus: MediaDownloadStatus.Loading },
messagesFetchStatus: MessagesFetchState.SUCCESS,
});

expect(loadAttachmentDetails).not.toHaveBeenCalled();
Expand Down
5 changes: 3 additions & 2 deletions src/components/message/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { download } from '../../lib/api/attachment';
import { LinkPreview } from '../link-preview';
import { getProvider } from '../../lib/cloudinary/provider';
import { MessageInput } from '../message-input/container';
import { User } from '../../store/channels';
import { MessagesFetchState, User } from '../../store/channels';
import { ParentMessage as ParentMessageType } from '../../lib/chat/types';
import { UserForMention } from '../message-input/utils';
import EditMessageActions from './edit-message-actions/edit-message-actions';
Expand Down Expand Up @@ -64,6 +64,7 @@ interface Properties extends MessageModel {
loadAttachmentDetails: (payload: { media: Media; messageId: string }) => void;
sendEmojiReaction: (messageId, key) => void;
onReportUser: (payload: { reportedUserId: string }) => void;
messagesFetchStatus: MessagesFetchState;
}

export interface State {
Expand Down Expand Up @@ -304,7 +305,7 @@ export class Message extends React.Component<Properties, State> {
const isLoading = downloadStatus === MediaDownloadStatus.Loading;
const hasFailed = downloadStatus === MediaDownloadStatus.Failed;

if (!hasFailed && !isLoading) {
if (!hasFailed && !isLoading && this.props.messagesFetchStatus === MessagesFetchState.SUCCESS) {
this.props.loadAttachmentDetails({ media, messageId: media.id ?? this.props.messageId.toString() });
}

Expand Down

0 comments on commit 7d70300

Please sign in to comment.