From 53e98628f02f05cbd60140c4a0614611372c72e4 Mon Sep 17 00:00:00 2001 From: Sudheer Date: Wed, 9 Jan 2019 17:51:02 +0530 Subject: [PATCH] MM-13432 Remove getFilesForPost request state and use react state (#2496) * MM-13432 Remove getFilesForPost request state and use react state * Add test cases --- .../file_attachment_list.test.js.snap | 70 +++++++++++++++ .../file_attachment_list.js | 34 ++++--- .../file_attachment_list.test.js | 90 +++++++++++++++++++ app/components/file_attachment_list/index.js | 1 - 4 files changed, 182 insertions(+), 13 deletions(-) create mode 100644 app/components/file_attachment_list/__snapshots__/file_attachment_list.test.js.snap create mode 100644 app/components/file_attachment_list/file_attachment_list.test.js diff --git a/app/components/file_attachment_list/__snapshots__/file_attachment_list.test.js.snap b/app/components/file_attachment_list/__snapshots__/file_attachment_list.test.js.snap new file mode 100644 index 000000000..f943040a5 --- /dev/null +++ b/app/components/file_attachment_list/__snapshots__/file_attachment_list.test.js.snap @@ -0,0 +1,70 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`PostAttachmentOpenGraph should match snapshot with a single image file 1`] = ` + + + +`; diff --git a/app/components/file_attachment_list/file_attachment_list.js b/app/components/file_attachment_list/file_attachment_list.js index 5b6117a97..c8a129330 100644 --- a/app/components/file_attachment_list/file_attachment_list.js +++ b/app/components/file_attachment_list/file_attachment_list.js @@ -11,7 +11,6 @@ import { } from 'react-native'; import {Client4} from 'mattermost-redux/client'; -import {RequestStatus} from 'mattermost-redux/constants'; import {isDocument, isGif, isVideo} from 'app/utils/file'; import {getCacheFile} from 'app/utils/image_cache_manager'; @@ -33,7 +32,10 @@ export default class FileAttachmentList extends Component { onLongPress: PropTypes.func, postId: PropTypes.string.isRequired, theme: PropTypes.object.isRequired, - filesForPostRequest: PropTypes.object.isRequired, + }; + + static defaultProps = { + files: [], }; constructor(props) { @@ -42,15 +44,19 @@ export default class FileAttachmentList extends Component { this.items = []; this.previewItems = []; + this.state = { + loadingFiles: props.files.length === 0, + }; + this.buildGalleryFiles(props).then((results) => { this.galleryFiles = results; }); } componentDidMount() { - const {files, postId} = this.props; - if (!files || !files.length) { - this.props.actions.loadFilesForPostIfNecessary(postId); + const {files} = this.props; + if (files.length === 0) { + this.loadFilesForPost(); } } @@ -60,15 +66,19 @@ export default class FileAttachmentList extends Component { this.galleryFiles = results; }); } + if (!this.state.loadingFiles && nextProps.files.length === 0) { + this.setState({ + loadingFiles: true, + }); + this.loadFilesForPost(); + } } - componentDidUpdate() { - const {fileIds, files, filesForPostRequest, postId} = this.props; - - // Fixes an issue where files weren't loading with optimistic post - if (!files.length && fileIds.length > 0 && filesForPostRequest.status !== RequestStatus.STARTED) { - this.props.actions.loadFilesForPostIfNecessary(postId); - } + loadFilesForPost = async () => { + await this.props.actions.loadFilesForPostIfNecessary(this.props.postId); + this.setState({ + loadingFiles: false, + }); } buildGalleryFiles = async (props) => { diff --git a/app/components/file_attachment_list/file_attachment_list.test.js b/app/components/file_attachment_list/file_attachment_list.test.js new file mode 100644 index 000000000..245e0bac1 --- /dev/null +++ b/app/components/file_attachment_list/file_attachment_list.test.js @@ -0,0 +1,90 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. +import React from 'react'; +import {shallow} from 'enzyme'; + +import FileAttachment from './file_attachment_list.js'; +import Preferences from 'mattermost-redux/constants/preferences'; + +jest.mock('react-native-doc-viewer', () => ({ + openDoc: jest.fn(), +})); + +describe('PostAttachmentOpenGraph', () => { + const loadFilesForPostIfNecessary = jest.fn().mockImplementationOnce(() => Promise.resolve({data: {}})); + + const baseProps = { + actions: { + loadFilesForPostIfNecessary, + }, + canDownloadFiles: true, + deviceHeight: 680, + deviceWidth: 660, + fileIds: ['fileId'], + files: [{ + create_at: 1546893090093, + delete_at: 0, + extension: 'png', + has_preview_image: true, + height: 171, + id: 'fileId', + mime_type: 'image/png', + name: 'image.png', + post_id: 'postId', + size: 14894, + update_at: 1546893090093, + user_id: 'userId', + width: 425, + }], + postId: 'postId', + theme: Preferences.THEMES.default, + }; + + test('should match snapshot with a single image file', () => { + const wrapper = shallow( + + ); + + expect(wrapper.getElement()).toMatchSnapshot(); + }); + + test('should call loadFilesForPostIfNecessary when files does not exist', async () => { + const props = { + ...baseProps, + files: undefined, + }; + + const wrapper = shallow( + + ); + expect(wrapper.state('loadingFiles')).toBe(true); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); + await loadFilesForPostIfNecessary(); + wrapper.setProps({files: baseProps.files}); + expect(wrapper.state('loadingFiles')).toBe(false); + }); + + test('should call loadFilesForPostIfNecessary on componentUpdate and when files does not exist', async () => { + const loadFilesForPostIfNecessaryMock = jest.fn().mockImplementationOnce(() => Promise.resolve({data: {}})); + const props = { + ...baseProps, + files: [], + actions: { + loadFilesForPostIfNecessary: loadFilesForPostIfNecessaryMock, + }, + }; + + const wrapper = shallow( + + ); + expect(wrapper.state('loadingFiles')).toBe(true); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledTimes(1); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); + await loadFilesForPostIfNecessaryMock(); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledTimes(2); + expect(props.actions.loadFilesForPostIfNecessary).toHaveBeenCalledWith(props.postId); + await loadFilesForPostIfNecessaryMock(); + wrapper.setProps({files: baseProps.files}); + expect(wrapper.state('loadingFiles')).toBe(false); + }); +}); diff --git a/app/components/file_attachment_list/index.js b/app/components/file_attachment_list/index.js index 8e055bad9..2230cd5df 100644 --- a/app/components/file_attachment_list/index.js +++ b/app/components/file_attachment_list/index.js @@ -21,7 +21,6 @@ function makeMapStateToProps() { canDownloadFiles: canDownloadFilesOnMobile(state), files: getFilesForPost(state, ownProps.postId), theme: getTheme(state), - filesForPostRequest: state.requests.files.getFilesForPost, }; }; }