diff --git a/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx b/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx index 7ae218778..3be8eed73 100644 --- a/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx +++ b/webapp/src/components/sidebar_buttons/sidebar_buttons.jsx @@ -35,9 +35,13 @@ export default class SidebarButtons extends React.PureComponent { this.state = { refreshing: false, }; + this.mounted = false; + this.refreshing = false; } componentDidMount() { + this.mounted = true; + if (this.props.connected) { this.getData(); return; @@ -52,8 +56,16 @@ export default class SidebarButtons extends React.PureComponent { } } + componentWillUnmount() { + this.mounted = false; + } + getData = async (e) => { - if (this.state.refreshing) { + if (e) { + e.preventDefault(); + } + + if (this.refreshing) { return; } @@ -66,13 +78,16 @@ export default class SidebarButtons extends React.PureComponent { return; } - if (e) { - e.preventDefault(); - } - + this.refreshing = true; this.setState({refreshing: true}); - await this.props.actions.getSidebarContent(); - this.setState({refreshing: false}); + try { + await this.props.actions.getSidebarContent(); + } finally { + this.refreshing = false; + if (this.mounted) { + this.setState({refreshing: false}); + } + } }; openConnectWindow = (e) => { @@ -83,6 +98,7 @@ export default class SidebarButtons extends React.PureComponent { openRHS = (rhsState) => { this.props.actions.updateRhsState(rhsState); this.props.showRHSPlugin(); + this.getData(); }; render() { diff --git a/webapp/src/components/sidebar_buttons/sidebar_buttons.test.jsx b/webapp/src/components/sidebar_buttons/sidebar_buttons.test.jsx new file mode 100644 index 000000000..1ce0e746b --- /dev/null +++ b/webapp/src/components/sidebar_buttons/sidebar_buttons.test.jsx @@ -0,0 +1,134 @@ +import React from 'react'; +import {act, fireEvent, render} from '@testing-library/react'; + +import {RHSStates} from '../../constants'; + +import SidebarButtons from './sidebar_buttons'; + +// The component hides its controls during E2E testing. +// eslint-disable-next-line no-underscore-dangle +global.__E2E_TESTING__ = false; + +jest.mock('react-bootstrap', () => ({ + OverlayTrigger: ({children}) => children, + Tooltip: ({children}) => children, +})); + +jest.mock('mattermost-redux/utils/theme_utils', () => ({ + changeOpacity: (color) => color, + makeStyleFromTheme: (createStyle) => (theme) => createStyle(theme), +})); + +const baseProps = { + connected: true, + clientId: 'client-id', + enterpriseURL: '', + isTeamSidebar: false, + reviewTargetDays: 0, + reviews: [], + theme: { + centerChannelBg: '#ffffff', + centerChannelColor: '#333333', + }, + unreads: [], + yourAssignments: [], + yourPrs: [], +}; + +function renderSidebarButtons(overrides = {}) { + const actions = overrides.actions || { + getConnected: jest.fn(), + getSidebarContent: jest.fn().mockResolvedValue({}), + updateRhsState: jest.fn(), + }; + const showRHSPlugin = jest.fn(); + const result = render( + , + ); + + return {actions, showRHSPlugin, ...result}; +} + +beforeEach(() => { + jest.clearAllMocks(); +}); + +test('refreshes sidebar content when opening or switching the RHS view', async () => { + const {actions, showRHSPlugin, container} = renderSidebarButtons(); + await act(async () => Promise.resolve()); + actions.getSidebarContent.mockClear(); + + const links = container.querySelectorAll('a'); + await act(async () => { + links[1].click(); + }); + await act(async () => { + links[2].click(); + }); + + expect(actions.updateRhsState).toHaveBeenNthCalledWith(1, RHSStates.PRS); + expect(actions.updateRhsState).toHaveBeenNthCalledWith(2, RHSStates.REVIEWS); + expect(showRHSPlugin).toHaveBeenCalledTimes(2); + expect(actions.getSidebarContent).toHaveBeenCalledTimes(2); +}); + +test('does not start duplicate sidebar requests while an open refresh is pending', async () => { + let resolveRequest; + const pendingRequest = new Promise((resolve) => { + resolveRequest = resolve; + }); + const getSidebarContent = jest.fn().mockResolvedValueOnce({}).mockReturnValue(pendingRequest); + const {actions, container} = renderSidebarButtons({ + actions: { + getConnected: jest.fn(), + getSidebarContent, + updateRhsState: jest.fn(), + }, + }); + await act(async () => Promise.resolve()); + getSidebarContent.mockClear(); + + const links = container.querySelectorAll('a'); + act(() => { + links[1].click(); + links[2].click(); + }); + + expect(getSidebarContent).toHaveBeenCalledTimes(1); + expect(actions.updateRhsState).toHaveBeenCalledTimes(2); + await act(async () => { + resolveRequest({}); + await pendingRequest; + }); +}); + +test('prevents the refresh anchor default action while a refresh is pending', async () => { + let resolveRequest; + const pendingRequest = new Promise((resolve) => { + resolveRequest = resolve; + }); + const getSidebarContent = jest.fn().mockResolvedValueOnce({}).mockReturnValue(pendingRequest); + const {container, unmount} = renderSidebarButtons({ + actions: { + getConnected: jest.fn(), + getSidebarContent, + updateRhsState: jest.fn(), + }, + }); + await act(async () => Promise.resolve()); + + const refreshLink = container.querySelector('a[href="#"]'); + expect(fireEvent.click(refreshLink)).toBe(false); + expect(fireEvent.click(refreshLink)).toBe(false); + + unmount(); + await act(async () => { + resolveRequest({}); + await pendingRequest; + }); +}); diff --git a/webapp/src/components/sidebar_right/index.jsx b/webapp/src/components/sidebar_right/index.jsx index 3e62e1c69..7653aa5a7 100644 --- a/webapp/src/components/sidebar_right/index.jsx +++ b/webapp/src/components/sidebar_right/index.jsx @@ -4,7 +4,7 @@ import {connect} from 'react-redux'; import {bindActionCreators} from 'redux'; -import {getReviewsDetails, getYourPrsDetails} from '../../actions'; +import {getReviewsDetails, getYourPrsDetails, getSidebarContent} from '../../actions'; import {getSidebarData} from 'src/selectors'; @@ -30,6 +30,7 @@ function mapDispatchToProps(dispatch) { actions: bindActionCreators({ getYourPrsDetails, getReviewsDetails, + getSidebarContent, }, dispatch), }; } diff --git a/webapp/src/components/sidebar_right/sidebar_right.jsx b/webapp/src/components/sidebar_right/sidebar_right.jsx index 567527929..0b2c0e632 100644 --- a/webapp/src/components/sidebar_right/sidebar_right.jsx +++ b/webapp/src/components/sidebar_right/sidebar_right.jsx @@ -4,17 +4,39 @@ import React from 'react'; import PropTypes from 'prop-types'; import Scrollbars from 'react-custom-scrollbars-2'; +import {OverlayTrigger, Tooltip} from 'react-bootstrap'; + +import {makeStyleFromTheme, changeOpacity} from 'mattermost-redux/utils/theme_utils'; import {RHSStates} from '../../constants'; import GithubItems from './github_items'; +const getStyle = makeStyleFromTheme((theme) => { + return { + sectionHeader: { + padding: '15px', + display: 'flex', + justifyContent: 'space-between', + alignItems: 'center', + }, + refreshButton: { + color: changeOpacity(theme.centerChannelColor, 0.6), + cursor: 'pointer', + background: 'transparent', + border: 'none', + padding: 0, + }, + }; +}); + export function renderView(props) { return (
); + /> + ); } export function renderThumbHorizontal(props) { @@ -22,7 +44,8 @@ export function renderThumbHorizontal(props) {
); + /> + ); } export function renderThumbVertical(props) { @@ -30,7 +53,8 @@ export function renderThumbVertical(props) {
); + /> + ); } function mapGithubItemListToPrList(gilist) { @@ -78,10 +102,20 @@ export default class SidebarRight extends React.PureComponent { actions: PropTypes.shape({ getYourPrsDetails: PropTypes.func.isRequired, getReviewsDetails: PropTypes.func.isRequired, + getSidebarContent: PropTypes.func.isRequired, }).isRequired, }; + constructor(props) { + super(props); + this.state = {refreshing: false}; + this.mounted = false; + this.refreshing = false; + } + componentDidMount() { + this.mounted = true; + if (this.props.yourPrs && this.props.rhsState === RHSStates.PRS) { this.props.actions.getYourPrsDetails(mapGithubItemListToPrList(this.props.yourPrs)); } @@ -91,6 +125,29 @@ export default class SidebarRight extends React.PureComponent { } } + componentWillUnmount() { + this.mounted = false; + } + + handleRefresh = async (e) => { + if (e) { + e.preventDefault(); + } + if (this.refreshing) { + return; + } + this.refreshing = true; + this.setState({refreshing: true}); + try { + await this.props.actions.getSidebarContent(); + } finally { + this.refreshing = false; + if (this.mounted) { + this.setState({refreshing: false}); + } + } + }; + componentDidUpdate(prevProps) { if (shouldUpdateDetails(this.props.yourPrs, prevProps.yourPrs, RHSStates.PRS, this.props.rhsState, prevProps.rhsState)) { this.props.actions.getYourPrsDetails(mapGithubItemListToPrList(this.props.yourPrs)); @@ -102,6 +159,7 @@ export default class SidebarRight extends React.PureComponent { } render() { + const style = getStyle(this.props.theme); const baseURL = this.props.enterpriseURL ? this.props.enterpriseURL : 'https://github.com'; let orgQuery = ''; this.props.orgs.map((org) => { @@ -116,27 +174,23 @@ export default class SidebarRight extends React.PureComponent { switch (rhsState) { case RHSStates.PRS: - githubItems = yourPrs; title = 'Your Open Pull Requests'; listUrl = baseURL + '/pulls?q=is%3Aopen+is%3Apr+author%3A' + username + '+archived%3Afalse' + orgQuery; break; case RHSStates.REVIEWS: - githubItems = reviews; listUrl = baseURL + '/pulls?q=is%3Aopen+is%3Apr+review-requested%3A' + username + '+archived%3Afalse' + orgQuery; title = 'Pull Requests Needing Review'; break; case RHSStates.UNREADS: - githubItems = unreads; title = 'Unread Messages'; listUrl = baseURL + '/notifications'; break; case RHSStates.ASSIGNMENTS: - githubItems = yourAssignments; title = 'Your Assignments'; listUrl = baseURL + '/pulls?q=is%3Aopen+archived%3Afalse+assignee%3A' + username + orgQuery; @@ -163,6 +217,20 @@ export default class SidebarRight extends React.PureComponent { rel='noopener noreferrer' >{title} + {'Refresh'}} + > + +