Merge pull request #200 from acelaya-forks/feature/refactor-edit-tags-modal

Feature/refactor edit tags modal

CHANGELOG.md

@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 #### Changed
 
 * [#191](https://github.com/shlinkio/shlink-web-client/issues/191) Created `ForServerVersion` helper component which dynamically renders children if current server conditions are met.
+* [#189](https://github.com/shlinkio/shlink-web-client/issues/189) Simplified short url tags and short url deletion components and reducers, by removing redundant actions.
 
 #### Deprecated
 

src/short-urls/helpers/DeleteShortUrlModal.js

@@ -1,7 +1,7 @@
 import React from 'react';
 import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap';
 import PropTypes from 'prop-types';
-import { identity } from 'ramda';
+import { identity, pipe } from 'ramda';
 import { shortUrlType } from '../reducers/shortUrlsList';
 import { shortUrlDeletionType } from '../reducers/shortUrlDeletion';
 
@@ -15,21 +15,17 @@ export default class DeleteShortUrlModal extends React.Component {
     shortUrlDeletion: shortUrlDeletionType,
     deleteShortUrl: PropTypes.func,
     resetDeleteShortUrl: PropTypes.func,
-    shortUrlDeleted: PropTypes.func,
   };
 
   state = { inputValue: '' };
   handleDeleteUrl = (e) => {
     e.preventDefault();
 
-    const { deleteShortUrl, shortUrl, toggle, shortUrlDeleted } = this.props;
+    const { deleteShortUrl, shortUrl, toggle } = this.props;
     const { shortCode } = shortUrl;
 
     deleteShortUrl(shortCode)
-      .then(() => {
+      .then(toggle)
-        shortUrlDeleted(shortCode);
-        toggle();
-      })
       .catch(identity);
   };
 
@@ -40,16 +36,17 @@ export default class DeleteShortUrlModal extends React.Component {
   }
 
   render() {
-    const { shortUrl, toggle, isOpen, shortUrlDeletion } = this.props;
+    const { shortUrl, toggle, isOpen, shortUrlDeletion, resetDeleteShortUrl } = this.props;
     const { error, errorData } = shortUrlDeletion;
     const errorCode = error && (errorData.type || errorData.error);
     const hasThresholdError = errorCode === THRESHOLD_REACHED;
     const hasErrorOtherThanThreshold = error && errorCode !== THRESHOLD_REACHED;
+    const close = pipe(resetDeleteShortUrl, toggle);
 
     return (
-      <Modal isOpen={isOpen} toggle={toggle} centered>
+      <Modal isOpen={isOpen} toggle={close} centered>
         <form onSubmit={this.handleDeleteUrl}>
-          <ModalHeader toggle={toggle}>
+          <ModalHeader toggle={close}>
             <span className="text-danger">Delete short URL</span>
           </ModalHeader>
           <ModalBody>
@@ -77,7 +74,7 @@ export default class DeleteShortUrlModal extends React.Component {
             )}
           </ModalBody>
           <ModalFooter>
-            <button type="button" className="btn btn-link" onClick={toggle}>Cancel</button>
+            <button type="button" className="btn btn-link" onClick={close}>Cancel</button>
             <button
               type="submit"
               className="btn btn-danger"

src/short-urls/helpers/EditTagsModal.js

@@ -2,6 +2,7 @@ import React from 'react';
 import { Modal, ModalBody, ModalFooter, ModalHeader } from 'reactstrap';
 import PropTypes from 'prop-types';
 import { ExternalLink } from 'react-external-link';
+import { pipe } from 'ramda';
 import { shortUrlTagsType } from '../reducers/shortUrlTags';
 import { shortUrlType } from '../reducers/shortUrlsList';
 
@@ -12,7 +13,6 @@ const EditTagsModal = (TagsSelector) => class EditTagsModal extends React.Compon
     shortUrl: shortUrlType.isRequired,
     shortUrlTags: shortUrlTagsType,
     editShortUrlTags: PropTypes.func,
-    shortUrlTagsEdited: PropTypes.func,
     resetShortUrlsTags: PropTypes.func,
   };
 
@@ -20,28 +20,14 @@ const EditTagsModal = (TagsSelector) => class EditTagsModal extends React.Compon
     const { editShortUrlTags, shortUrl, toggle } = this.props;
 
     editShortUrlTags(shortUrl.shortCode, this.state.tags)
-      .then(() => {
+      .then(toggle)
-        this.tagsSaved = true;
-        toggle();
-      })
       .catch(() => {});
   };
-  refreshShortUrls = () => {
-    if (!this.tagsSaved) {
-      return;
-    }
-
-    const { shortUrlTagsEdited, shortUrl, shortUrlTags } = this.props;
-    const { tags } = shortUrlTags;
-
-    shortUrlTagsEdited(shortUrl.shortCode, tags);
-  };
 
   componentDidMount() {
     const { resetShortUrlsTags } = this.props;
 
     resetShortUrlsTags();
-    this.tagsSaved = false;
   }
 
   constructor(props) {
@@ -50,12 +36,13 @@ const EditTagsModal = (TagsSelector) => class EditTagsModal extends React.Compon
   }
 
   render() {
-    const { isOpen, toggle, shortUrl, shortUrlTags } = this.props;
+    const { isOpen, toggle, shortUrl, shortUrlTags, resetShortUrlsTags } = this.props;
     const url = shortUrl && (shortUrl.shortUrl || '');
+    const close = pipe(resetShortUrlsTags, toggle);
 
     return (
-      <Modal isOpen={isOpen} toggle={toggle} centered onClosed={() => this.refreshShortUrls()}>
+      <Modal isOpen={isOpen} toggle={close} centered>
-        <ModalHeader toggle={toggle}>
+        <ModalHeader toggle={close}>
           Edit tags for <ExternalLink href={url} />
         </ModalHeader>
         <ModalBody>
@@ -67,7 +54,7 @@ const EditTagsModal = (TagsSelector) => class EditTagsModal extends React.Compon
           )}
         </ModalBody>
         <ModalFooter>
-          <button className="btn btn-link" onClick={toggle}>Cancel</button>
+          <button className="btn btn-link" onClick={close}>Cancel</button>
           <button
             className="btn btn-primary"
             type="button"

src/short-urls/reducers/shortUrlDeletion.js

@@ -5,9 +5,8 @@ import { apiErrorType } from '../../utils/services/ShlinkApiClient';
 /* eslint-disable padding-line-between-statements */
 export const DELETE_SHORT_URL_START = 'shlink/deleteShortUrl/DELETE_SHORT_URL_START';
 export const DELETE_SHORT_URL_ERROR = 'shlink/deleteShortUrl/DELETE_SHORT_URL_ERROR';
-export const DELETE_SHORT_URL = 'shlink/deleteShortUrl/DELETE_SHORT_URL';
-export const RESET_DELETE_SHORT_URL = 'shlink/deleteShortUrl/RESET_DELETE_SHORT_URL';
 export const SHORT_URL_DELETED = 'shlink/deleteShortUrl/SHORT_URL_DELETED';
+export const RESET_DELETE_SHORT_URL = 'shlink/deleteShortUrl/RESET_DELETE_SHORT_URL';
 /* eslint-enable padding-line-between-statements */
 
 export const shortUrlDeletionType = PropTypes.shape({
@@ -27,7 +26,7 @@ const initialState = {
 export default handleActions({
   [DELETE_SHORT_URL_START]: (state) => ({ ...state, loading: true, error: false }),
   [DELETE_SHORT_URL_ERROR]: (state, { errorData }) => ({ ...state, errorData, loading: false, error: true }),
-  [DELETE_SHORT_URL]: (state, { shortCode }) => ({ ...state, shortCode, loading: false, error: false }),
+  [SHORT_URL_DELETED]: (state, { shortCode }) => ({ ...state, shortCode, loading: false, error: false }),
   [RESET_DELETE_SHORT_URL]: () => initialState,
 }, initialState);
 
@@ -38,7 +37,7 @@ export const deleteShortUrl = (buildShlinkApiClient) => (shortCode) => async (di
 
   try {
     await deleteShortUrl(shortCode);
-    dispatch({ type: DELETE_SHORT_URL, shortCode });
+    dispatch({ type: SHORT_URL_DELETED, shortCode });
   } catch (e) {
     dispatch({ type: DELETE_SHORT_URL_ERROR, errorData: e.response.data });
 
@@ -47,5 +46,3 @@ export const deleteShortUrl = (buildShlinkApiClient) => (shortCode) => async (di
 };
 
 export const resetDeleteShortUrl = createAction(RESET_DELETE_SHORT_URL);
-
-export const shortUrlDeleted = (shortCode) => ({ type: SHORT_URL_DELETED, shortCode });

src/short-urls/reducers/shortUrlTags.js

@@ -4,9 +4,8 @@ import PropTypes from 'prop-types';
 /* eslint-disable padding-line-between-statements */
 export const EDIT_SHORT_URL_TAGS_START = 'shlink/shortUrlTags/EDIT_SHORT_URL_TAGS_START';
 export const EDIT_SHORT_URL_TAGS_ERROR = 'shlink/shortUrlTags/EDIT_SHORT_URL_TAGS_ERROR';
-export const EDIT_SHORT_URL_TAGS = 'shlink/shortUrlTags/EDIT_SHORT_URL_TAGS';
-export const RESET_EDIT_SHORT_URL_TAGS = 'shlink/shortUrlTags/RESET_EDIT_SHORT_URL_TAGS';
 export const SHORT_URL_TAGS_EDITED = 'shlink/shortUrlTags/SHORT_URL_TAGS_EDITED';
+export const RESET_EDIT_SHORT_URL_TAGS = 'shlink/shortUrlTags/RESET_EDIT_SHORT_URL_TAGS';
 /* eslint-enable padding-line-between-statements */
 
 export const shortUrlTagsType = PropTypes.shape({
@@ -26,7 +25,7 @@ const initialState = {
 export default handleActions({
   [EDIT_SHORT_URL_TAGS_START]: (state) => ({ ...state, saving: true, error: false }),
   [EDIT_SHORT_URL_TAGS_ERROR]: (state) => ({ ...state, saving: false, error: true }),
-  [EDIT_SHORT_URL_TAGS]: (state, { shortCode, tags }) => ({ shortCode, tags, saving: false, error: false }),
+  [SHORT_URL_TAGS_EDITED]: (state, { shortCode, tags }) => ({ shortCode, tags, saving: false, error: false }),
   [RESET_EDIT_SHORT_URL_TAGS]: () => initialState,
 }, initialState);
 
@@ -37,7 +36,7 @@ export const editShortUrlTags = (buildShlinkApiClient) => (shortCode, tags) => a
   try {
     const normalizedTags = await updateShortUrlTags(shortCode, tags);
 
-    dispatch({ tags: normalizedTags, shortCode, type: EDIT_SHORT_URL_TAGS });
+    dispatch({ tags: normalizedTags, shortCode, type: SHORT_URL_TAGS_EDITED });
   } catch (e) {
     dispatch({ type: EDIT_SHORT_URL_TAGS_ERROR });
 
@@ -46,9 +45,3 @@ export const editShortUrlTags = (buildShlinkApiClient) => (shortCode, tags) => a
 };
 
 export const resetShortUrlsTags = createAction(RESET_EDIT_SHORT_URL_TAGS);
-
-export const shortUrlTagsEdited = (shortCode, tags) => ({
-  tags,
-  shortCode,
-  type: SHORT_URL_TAGS_EDITED,
-});

src/short-urls/services/provideServices.js

@@ -12,8 +12,8 @@ import EditMetaModal from '../helpers/EditMetaModal';
 import CreateShortUrlResult from '../helpers/CreateShortUrlResult';
 import { listShortUrls } from '../reducers/shortUrlsList';
 import { createShortUrl, resetCreateShortUrl } from '../reducers/shortUrlCreation';
-import { deleteShortUrl, resetDeleteShortUrl, shortUrlDeleted } from '../reducers/shortUrlDeletion';
+import { deleteShortUrl, resetDeleteShortUrl } from '../reducers/shortUrlDeletion';
-import { editShortUrlTags, resetShortUrlsTags, shortUrlTagsEdited } from '../reducers/shortUrlTags';
+import { editShortUrlTags, resetShortUrlsTags } from '../reducers/shortUrlTags';
 import { editShortUrlMeta, resetShortUrlMeta } from '../reducers/shortUrlMeta';
 import { resetShortUrlParams } from '../reducers/shortUrlsListParams';
 
@@ -52,16 +52,10 @@ const provideServices = (bottle, connect) => {
   );
 
   bottle.serviceFactory('DeleteShortUrlModal', () => DeleteShortUrlModal);
-  bottle.decorator('DeleteShortUrlModal', connect(
+  bottle.decorator('DeleteShortUrlModal', connect([ 'shortUrlDeletion' ], [ 'deleteShortUrl', 'resetDeleteShortUrl' ]));
-    [ 'shortUrlDeletion' ],
-    [ 'deleteShortUrl', 'resetDeleteShortUrl', 'shortUrlDeleted' ]
-  ));
 
   bottle.serviceFactory('EditTagsModal', EditTagsModal, 'TagsSelector');
-  bottle.decorator('EditTagsModal', connect(
+  bottle.decorator('EditTagsModal', connect([ 'shortUrlTags' ], [ 'editShortUrlTags', 'resetShortUrlsTags' ]));
-    [ 'shortUrlTags' ],
-    [ 'editShortUrlTags', 'resetShortUrlsTags', 'shortUrlTagsEdited' ]
-  ));
 
   bottle.serviceFactory('EditMetaModal', () => EditMetaModal);
   bottle.decorator('EditMetaModal', connect([ 'shortUrlMeta' ], [ 'editShortUrlMeta', 'resetShortUrlMeta' ]));
@@ -69,7 +63,6 @@ const provideServices = (bottle, connect) => {
   // Actions
   bottle.serviceFactory('editShortUrlTags', editShortUrlTags, 'buildShlinkApiClient');
   bottle.serviceFactory('resetShortUrlsTags', () => resetShortUrlsTags);
-  bottle.serviceFactory('shortUrlTagsEdited', () => shortUrlTagsEdited);
 
   bottle.serviceFactory('listShortUrls', listShortUrls, 'buildShlinkApiClient');
   bottle.serviceFactory('resetShortUrlParams', () => resetShortUrlParams);
@@ -79,7 +72,6 @@ const provideServices = (bottle, connect) => {
 
   bottle.serviceFactory('deleteShortUrl', deleteShortUrl, 'buildShlinkApiClient');
   bottle.serviceFactory('resetDeleteShortUrl', () => resetDeleteShortUrl);
-  bottle.serviceFactory('shortUrlDeleted', () => shortUrlDeleted);
 
   bottle.serviceFactory('editShortUrlMeta', editShortUrlMeta, 'buildShlinkApiClient');
   bottle.serviceFactory('resetShortUrlMeta', () => resetShortUrlMeta);

test/short-urls/CreateShortUrl.test.js

@@ -14,7 +14,7 @@ describe('<CreateShortUrl />', () => {
   const createShortUrl = jest.fn();
 
   beforeEach(() => {
-    const CreateShortUrl = createShortUrlsCreator(TagsSelector, () => '');
+    const CreateShortUrl = createShortUrlsCreator(TagsSelector, () => '', () => '');
 
     wrapper = shallow(
       <CreateShortUrl shortUrlCreationResult={shortUrlCreationResult} createShortUrl={createShortUrl} />

test/short-urls/SearchBar.test.js

@@ -9,7 +9,7 @@ import DateRangeRow from '../../src/utils/DateRangeRow';
 describe('<SearchBar />', () => {
   let wrapper;
   const listShortUrlsMock = jest.fn();
-  const SearchBar = searchBarCreator({});
+  const SearchBar = searchBarCreator({}, () => '');
 
   afterEach(() => {
     listShortUrlsMock.mockReset();

test/short-urls/helpers/DeleteShortUrlModal.test.js

@@ -21,7 +21,6 @@ describe('<DeleteShortUrlModal />', () => {
         toggle={identity}
         deleteShortUrl={deleteShortUrl}
         resetDeleteShortUrl={identity}
-        shortUrlDeleted={identity}
       />
     );
 

test/short-urls/helpers/EditTagsModal.test.js

@@ -8,7 +8,6 @@ describe('<EditTagsModal />', () => {
   const shortCode = 'abc123';
   const TagsSelector = () => '';
   const editShortUrlTags = jest.fn(() => Promise.resolve());
-  const shortUrlTagsEdited = jest.fn();
   const resetShortUrlsTags = jest.fn();
   const toggle = jest.fn();
   const createWrapper = (shortUrlTags) => {
@@ -25,7 +24,6 @@ describe('<EditTagsModal />', () => {
         shortUrlTags={shortUrlTags}
         toggle={toggle}
         editShortUrlTags={editShortUrlTags}
-        shortUrlTagsEdited={shortUrlTagsEdited}
         resetShortUrlsTags={resetShortUrlsTags}
       />
     );
@@ -107,28 +105,7 @@ describe('<EditTagsModal />', () => {
     const modal = wrapper.find(Modal);
 
     modal.simulate('closed');
-    expect(shortUrlTagsEdited).not.toHaveBeenCalled();
+    expect(editShortUrlTags).not.toHaveBeenCalled();
-  });
-
-  it('notifies tags have been edited when window is closed after saving', (done) => {
-    const wrapper = createWrapper({
-      shortCode,
-      tags: [],
-      saving: true,
-      error: false,
-    });
-    const saveBtn = wrapper.find('.btn-primary');
-    const modal = wrapper.find(Modal);
-
-    saveBtn.simulate('click');
-
-    // Wrap this expect in a setImmediate since it is called as a result of an inner promise
-    setImmediate(() => {
-      modal.simulate('closed');
-      expect(shortUrlTagsEdited).toHaveBeenCalledTimes(1);
-      expect(shortUrlTagsEdited).toHaveBeenCalledWith(shortCode, []);
-      done();
-    });
   });
 
   it('toggles modal when cancel button is clicked', () => {

test/short-urls/helpers/ShortUrlsRowMenu.test.js

@@ -17,7 +17,7 @@ describe('<ShortUrlsRowMenu />', () => {
     shortUrl: 'https://doma.in/abc123',
   };
   const createWrapper = () => {
-    const ShortUrlsRowMenu = createShortUrlsRowMenu(DeleteShortUrlModal, EditTagsModal, EditMetaModal);
+    const ShortUrlsRowMenu = createShortUrlsRowMenu(DeleteShortUrlModal, EditTagsModal, EditMetaModal, () => '');
 
     wrapper = shallow(
       <ShortUrlsRowMenu

test/short-urls/reducers/shortUrlDeletion.test.js

@@ -1,10 +1,9 @@
 import reducer, {
-  DELETE_SHORT_URL, DELETE_SHORT_URL_ERROR,
+  DELETE_SHORT_URL_ERROR,
   DELETE_SHORT_URL_START,
   RESET_DELETE_SHORT_URL,
   SHORT_URL_DELETED,
   resetDeleteShortUrl,
-  shortUrlDeleted,
   deleteShortUrl,
 } from '../../../src/short-urls/reducers/shortUrlDeletion';
 
@@ -26,8 +25,8 @@ describe('shortUrlDeletionReducer', () => {
         errorData: {},
       }));
 
-    it('returns shortCode on DELETE_SHORT_URL', () =>
+    it('returns shortCode on SHORT_URL_DELETED', () =>
-      expect(reducer(undefined, { type: DELETE_SHORT_URL, shortCode: 'foo' })).toEqual({
+      expect(reducer(undefined, { type: SHORT_URL_DELETED, shortCode: 'foo' })).toEqual({
         shortCode: 'foo',
         loading: false,
         error: false,
@@ -51,11 +50,6 @@ describe('shortUrlDeletionReducer', () => {
       expect(resetDeleteShortUrl()).toEqual({ type: RESET_DELETE_SHORT_URL }));
   });
 
-  describe('shortUrlDeleted', () => {
-    it('returns expected action', () =>
-      expect(shortUrlDeleted('abc123')).toEqual({ type: SHORT_URL_DELETED, shortCode: 'abc123' }));
-  });
-
   describe('deleteShortUrl', () => {
     const dispatch = jest.fn();
     const getState = jest.fn().mockReturnValue({ selectedServer: {} });
@@ -75,7 +69,7 @@ describe('shortUrlDeletionReducer', () => {
 
       expect(dispatch).toHaveBeenCalledTimes(2);
       expect(dispatch).toHaveBeenNthCalledWith(1, { type: DELETE_SHORT_URL_START });
-      expect(dispatch).toHaveBeenNthCalledWith(2, { type: DELETE_SHORT_URL, shortCode });
+      expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_DELETED, shortCode });
 
       expect(apiClientMock.deleteShortUrl).toHaveBeenCalledTimes(1);
       expect(apiClientMock.deleteShortUrl).toHaveBeenCalledWith(shortCode);

test/short-urls/reducers/shortUrlTags.test.js

@@ -1,12 +1,10 @@
 import reducer, {
-  EDIT_SHORT_URL_TAGS,
   EDIT_SHORT_URL_TAGS_ERROR,
   EDIT_SHORT_URL_TAGS_START,
   RESET_EDIT_SHORT_URL_TAGS,
   resetShortUrlsTags,
   SHORT_URL_TAGS_EDITED,
   editShortUrlTags,
-  shortUrlTagsEdited,
 } from '../../../src/short-urls/reducers/shortUrlTags';
 
 describe('shortUrlTagsReducer', () => {
@@ -28,8 +26,8 @@ describe('shortUrlTagsReducer', () => {
       });
     });
 
-    it('returns provided tags and shortCode on EDIT_SHORT_URL_TAGS', () => {
+    it('returns provided tags and shortCode on SHORT_URL_TAGS_EDITED', () => {
-      expect(reducer({}, { type: EDIT_SHORT_URL_TAGS, tags, shortCode })).toEqual({
+      expect(reducer({}, { type: SHORT_URL_TAGS_EDITED, tags, shortCode })).toEqual({
         tags,
         shortCode,
         saving: false,
@@ -51,14 +49,6 @@ describe('shortUrlTagsReducer', () => {
     it('creates expected action', () => expect(resetShortUrlsTags()).toEqual({ type: RESET_EDIT_SHORT_URL_TAGS }));
   });
 
-  describe('shortUrlTagsEdited', () => {
-    it('creates expected action', () => expect(shortUrlTagsEdited(shortCode, tags)).toEqual({
-      tags,
-      shortCode,
-      type: SHORT_URL_TAGS_EDITED,
-    }));
-  });
-
   describe('editShortUrlTags', () => {
     const updateShortUrlTags = jest.fn();
     const buildShlinkApiClient = jest.fn().mockResolvedValue({ updateShortUrlTags });
@@ -82,7 +72,7 @@ describe('shortUrlTagsReducer', () => {
       expect(updateShortUrlTags).toHaveBeenCalledWith(shortCode, tags);
       expect(dispatch).toHaveBeenCalledTimes(2);
       expect(dispatch).toHaveBeenNthCalledWith(1, { type: EDIT_SHORT_URL_TAGS_START });
-      expect(dispatch).toHaveBeenNthCalledWith(2, { type: EDIT_SHORT_URL_TAGS, tags: normalizedTags, shortCode });
+      expect(dispatch).toHaveBeenNthCalledWith(2, { type: SHORT_URL_TAGS_EDITED, tags: normalizedTags, shortCode });
     });
 
     it('dispatches error on failure', async () => {