Refactor `AnnotationPublishControl` to handle `revert` and `setPrivacy`

src/sidebar/components/annotation-omega.js

@@ -2,7 +2,7 @@ import { createElement } from 'preact';
 import propTypes from 'prop-types';
 
 import useStore from '../store/use-store';
-import { isNew, isReply, quote } from '../util/annotation-metadata';
+import { isNew, quote } from '../util/annotation-metadata';
 import { isShared } from '../util/permissions';
 
 import AnnotationActionBar from './annotation-action-bar';
@@ -24,7 +24,6 @@ function AnnotationOmega({
   showDocumentInfo,
 }) {
   const createDraft = useStore(store => store.createDraft);
-  const setDefault = useStore(store => store.setDefault);
 
   // An annotation will have a draft if it is being edited
   const draft = useStore(store => store.getDraft(annotation));
@@ -50,17 +49,8 @@ function AnnotationOmega({
     createDraft(annotation, { ...draft, text });
   };
 
-  const onSetPrivacy = ({ level }) => {
-    createDraft(annotation, { ...draft, isPrivate: level === 'private' });
-    // Persist this as privacy default for future annotations unless this is a reply
-    if (!isReply(annotation)) {
-      setDefault('annotationPrivacy', level);
-    }
-  };
-
   // TODO
   const fakeOnReply = () => alert('Reply: TBD');
-  const fakeOnRevert = () => alert('Revert changes: TBD');
   const fakeOnSave = () => alert('Save changes: TBD');
 
   return (
@@ -84,12 +74,9 @@ function AnnotationOmega({
       <footer className="annotation-footer">
         {isEditing && (
           <AnnotationPublishControl
-            group={group}
+            annotation={annotation}
             isDisabled={isEmpty}
-            isShared={!isPrivate}
-            onCancel={fakeOnRevert}
             onSave={fakeOnSave}
-            onSetPrivacy={onSetPrivacy}
           />
         )}
         {shouldShowLicense && <AnnotationLicense />}

src/sidebar/components/annotation-publish-control.js

 import { createElement } from 'preact';
 import propTypes from 'prop-types';
 
+import useStore from '../store/use-store';
+import { isNew, isReply } from '../util/annotation-metadata';
+import { isShared } from '../util/permissions';
 import { withServices } from '../util/service-context';
 import { applyTheme } from '../util/theme';
 
@@ -15,17 +18,40 @@ import MenuItem from './menu-item';
  *
  */
 function AnnotationPublishControl({
-  group,
+  annotation,
   isDisabled,
-  isShared,
-  onCancel,
   onSave,
-  onSetPrivacy,
   settings,
 }) {
-  const publishDestination = isShared ? group.name : 'Only Me';
+  const draft = useStore(store => store.getDraft(annotation));
+  const group = useStore(store => store.getGroup(annotation.group));
+
+  const createDraft = useStore(store => store.createDraft);
+  const removeDraft = useStore(store => store.removeDraft);
+  const setDefault = useStore(store => store.setDefault);
+  const removeAnnotations = useStore(store => store.removeAnnotations);
+
+  const isPrivate = draft ? draft.isPrivate : !isShared(annotation.permissions);
+
+  const publishDestination = isPrivate ? 'Only Me' : group.name;
   const themeProps = ['ctaTextColor', 'ctaBackgroundColor'];
 
+  // Revert changes to this annotation
+  const onCancel = () => {
+    removeDraft(annotation);
+    if (isNew(annotation)) {
+      removeAnnotations([annotation]);
+    }
+  };
+
+  const onSetPrivacy = level => {
+    createDraft(annotation, { ...draft, isPrivate: level === 'private' });
+    // Persist this as privacy default for future annotations unless this is a reply
+    if (!isReply(annotation)) {
+      setDefault('annotationPrivacy', level);
+    }
+  };
+
   const menuLabel = (
     <div className="annotation-publish-control__btn-dropdown-arrow">
       <div className="annotation-publish-control__btn-dropdown-arrow-separator" />
@@ -62,14 +88,14 @@ function AnnotationPublishControl({
           <MenuItem
             icon={group.type === 'open' ? 'public' : 'groups'}
             label={group.name}
-            isSelected={isShared}
-            onClick={() => onSetPrivacy({ level: 'shared' })}
+            isSelected={!isPrivate}
+            onClick={() => onSetPrivacy('shared')}
           />
           <MenuItem
             icon="lock"
             label="Only Me"
-            isSelected={!isShared}
-            onClick={() => onSetPrivacy({ level: 'private' })}
+            isSelected={isPrivate}
+            onClick={() => onSetPrivacy('private')}
           />
         </Menu>
       </div>
@@ -84,8 +110,7 @@ function AnnotationPublishControl({
 }
 
 AnnotationPublishControl.propTypes = {
-  /** The group the annotation is currently associated with */
-  group: propTypes.object.isRequired,
+  annotation: propTypes.object.isRequired,
 
   /**
    * Should the save button be disabled?
@@ -93,18 +118,9 @@ AnnotationPublishControl.propTypes = {
    */
   isDisabled: propTypes.bool,
 
-  /** The current privacy setting on the annotation. Is it shared to group? */
-  isShared: propTypes.bool,
-
-  /** Callback for cancel button click */
-  onCancel: propTypes.func.isRequired,
-
   /** Callback for save button click */
   onSave: propTypes.func.isRequired,
 
-  /** Callback when selecting a privacy option in the menu */
-  onSetPrivacy: propTypes.func.isRequired,
-
   /** services */
   settings: propTypes.object.isRequired,
 };

src/sidebar/components/annotation.js

@@ -261,18 +261,6 @@ function AnnotationController(
     });
   };
 
-  /**
-   * @ngdoc method
-   * @name annotation.AnnotationController#revert
-   * @description Reverts an edit in progress and returns to the viewer.
-   */
-  this.revert = function() {
-    store.removeDraft(self.annotation);
-    if (isNew(self.annotation)) {
-      $rootScope.$broadcast(events.ANNOTATION_DELETED, self.annotation);
-    }
-  };
-
   this.save = function() {
     if (!self.annotation.user) {
       flash.info('Please log in to save your annotations.');
@@ -313,32 +301,6 @@ function AnnotationController(
       });
   };
 
-  /**
-   * @ngdoc method
-   * @name annotation.AnnotationController#setPrivacy
-   *
-   * Set the privacy settings on the annotation to a predefined
-   * level. The supported levels are 'private' which makes the annotation
-   * visible only to its creator and 'shared' which makes the annotation
-   * visible to everyone in the group.
-   *
-   * The changes take effect when the annotation is saved
-   */
-  this.setPrivacy = function(privacy) {
-    // When the user changes the privacy level of an annotation they're
-    // creating or editing, we cache that and use the same privacy level the
-    // next time they create an annotation.
-    // But _don't_ cache it when they change the privacy level of a reply.
-    if (!isReply(self.annotation)) {
-      permissions.setDefault(privacy);
-    }
-    store.createDraft(self.annotation, {
-      tags: self.state().tags,
-      text: self.state().text,
-      isPrivate: privacy === 'private',
-    });
-  };
-
   this.user = function() {
     return self.annotation.user;
   };

src/sidebar/components/test/annotation-omega-test.js

@@ -45,7 +45,6 @@ describe('AnnotationOmega', () => {
 
     fakeMetadata = {
       isNew: sinon.stub(),
-      isReply: sinon.stub().returns(false),
       quote: sinon.stub(),
     };
 
@@ -59,7 +58,6 @@ describe('AnnotationOmega', () => {
       getGroup: sinon.stub().returns({
         type: 'private',
       }),
-      setDefault: sinon.stub(),
     };
 
     $imports.$mock(mockImportedComponents());
@@ -157,89 +155,29 @@ describe('AnnotationOmega', () => {
       assert.isFalse(wrapper.find('AnnotationPublishControl').exists());
     });
 
-    it('should set the publish control to disabled if annotation is empty', () => {
+    it('should enable the publish control if the annotation is not empty', () => {
       const draft = fixtures.defaultDraft();
-      draft.tags = [];
-      draft.text = '';
+      draft.text = 'bananas';
       fakeStore.getDraft.returns(draft);
 
       const wrapper = createComponent();
 
-      assert.isTrue(
+      assert.isFalse(
         wrapper.find('AnnotationPublishControl').props().isDisabled
       );
     });
 
-    it('should set `isShared` to `false` if annotation is private', () => {
-      const draft = fixtures.defaultDraft();
-      draft.isPrivate = true;
-
-      fakeStore.getDraft.returns(draft);
-
-      const wrapper = createComponent();
-
-      assert.isFalse(wrapper.find('AnnotationPublishControl').props().isShared);
-    });
-
-    it('should set `isShared` to `true` if annotation is shared', () => {
+    it('should set the publish control to disabled if annotation is empty', () => {
       const draft = fixtures.defaultDraft();
-      draft.isPrivate = false;
+      draft.tags = [];
+      draft.text = '';
       fakeStore.getDraft.returns(draft);
 
       const wrapper = createComponent();
 
-      assert.isTrue(wrapper.find('AnnotationPublishControl').props().isShared);
-    });
-
-    it('should update annotation privacy when changed by publish control', () => {
-      setEditingMode(true);
-
-      const wrapper = createComponent();
-
-      act(() => {
-        wrapper
-          .find('AnnotationPublishControl')
-          .props()
-          .onSetPrivacy({ level: 'private' });
-      });
-
-      const call = fakeStore.createDraft.getCall(0);
-
-      assert.calledOnce(fakeStore.createDraft);
-      assert.isTrue(call.args[1].isPrivate);
-    });
-
-    it('should update annotation privacy default on change', () => {
-      setEditingMode(true);
-
-      const wrapper = createComponent();
-
-      act(() => {
-        wrapper
-          .find('AnnotationPublishControl')
-          .props()
-          .onSetPrivacy({ level: 'private' });
-      });
-
-      assert.calledOnce(fakeStore.setDefault);
-      assert.calledWith(fakeStore.setDefault, 'annotationPrivacy', 'private');
-    });
-
-    it('should not update annotation privacy default on change if annotation is reply', () => {
-      fakeMetadata.isReply.returns(true);
-
-      setEditingMode(true);
-
-      const wrapper = createComponent();
-
-      act(() => {
-        wrapper
-          .find('AnnotationPublishControl')
-          .props()
-          .onSetPrivacy({ level: 'private' });
-      });
-
-      assert.equal(fakeStore.setDefault.callCount, 0);
+      assert.isTrue(
+        wrapper.find('AnnotationPublishControl').props().isDisabled
+      );
     });
   });
 

src/sidebar/components/test/annotation-publish-control-test.js

 import { mount } from 'enzyme';
 import { createElement } from 'preact';
 
+import * as fixtures from '../../test/annotation-fixtures';
 import AnnotationPublishControl from '../annotation-publish-control';
 import { $imports } from '../annotation-publish-control';
 
@@ -8,18 +9,17 @@ import mockImportedComponents from './mock-imported-components';
 
 describe('AnnotationPublishControl', () => {
   let fakeGroup;
+  let fakeMetadata;
   let fakeSettings;
+  let fakeStore;
   let fakeApplyTheme;
 
   const createAnnotationPublishControl = (props = {}) => {
     return mount(
       <AnnotationPublishControl
-        group={fakeGroup}
+        annotation={fixtures.defaultAnnotation()}
         isDisabled={false}
-        isShared={true}
-        onCancel={sinon.stub()}
         onSave={sinon.stub()}
-        onSetPrivacy={sinon.stub()}
         settings={fakeSettings}
         {...props}
       />
@@ -31,6 +31,12 @@ describe('AnnotationPublishControl', () => {
       name: 'Fake Group',
       type: 'private',
     };
+
+    fakeMetadata = {
+      isNew: sinon.stub(),
+      isReply: sinon.stub().returns(false),
+    };
+
     fakeSettings = {
       branding: {
         ctaTextColor: '#0f0',
@@ -38,10 +44,21 @@ describe('AnnotationPublishControl', () => {
       },
     };
 
+    fakeStore = {
+      createDraft: sinon.stub(),
+      getDraft: sinon.stub().returns(fixtures.defaultDraft()),
+      getGroup: sinon.stub().returns(fakeGroup),
+      setDefault: sinon.stub(),
+      removeAnnotations: sinon.stub(),
+      removeDraft: sinon.stub(),
+    };
+
     fakeApplyTheme = sinon.stub();
 
     $imports.$mock(mockImportedComponents());
     $imports.$mock({
+      '../store/use-store': callback => callback(fakeStore),
+      '../util/annotation-metadata': fakeMetadata,
       '../util/theme': {
         applyTheme: fakeApplyTheme,
       },
@@ -74,7 +91,7 @@ describe('AnnotationPublishControl', () => {
     const btnClass = '.annotation-publish-control__btn-primary';
     context('shared annotation', () => {
       it('should label the button with the group name', () => {
-        const wrapper = createAnnotationPublishControl({ isShared: true });
+        const wrapper = createAnnotationPublishControl();
 
         const btn = wrapper.find(btnClass);
         assert.equal(
@@ -87,7 +104,10 @@ describe('AnnotationPublishControl', () => {
 
     context('private annotation', () => {
       it('should label the button with "Only Me"', () => {
-        const wrapper = createAnnotationPublishControl({ isShared: false });
+        const draft = fixtures.defaultDraft();
+        draft.isPrivate = true;
+        fakeStore.getDraft.returns(draft);
+        const wrapper = createAnnotationPublishControl();
 
         const btn = wrapper.find(btnClass);
         assert.equal(btn.prop('title'), 'Publish this annotation to Only Me');
@@ -122,15 +142,35 @@ describe('AnnotationPublishControl', () => {
   describe('menu', () => {
     describe('share (to group) menu item', () => {
       it('should invoke privacy callback with shared privacy', () => {
-        const fakeOnSetPrivacy = sinon.stub();
-        const wrapper = createAnnotationPublishControl({
-          onSetPrivacy: fakeOnSetPrivacy,
-        });
+        const wrapper = createAnnotationPublishControl();
         const shareMenuItem = wrapper.find('MenuItem').first();
 
         shareMenuItem.prop('onClick')();
 
-        assert.calledWith(fakeOnSetPrivacy, { level: 'shared' });
+        const call = fakeStore.createDraft.getCall(0);
+
+        assert.calledOnce(fakeStore.createDraft);
+        assert.isFalse(call.args[1].isPrivate);
+      });
+
+      it('should update default privacy level to shared', () => {
+        const wrapper = createAnnotationPublishControl();
+        const privateMenuItem = wrapper.find('MenuItem').first();
+
+        privateMenuItem.prop('onClick')();
+
+        assert.calledOnce(fakeStore.setDefault);
+        assert.calledWith(fakeStore.setDefault, 'annotationPrivacy', 'shared');
+      });
+
+      it('should not update default privacy level if annotation is reply', () => {
+        fakeMetadata.isReply.returns(true);
+        const wrapper = createAnnotationPublishControl();
+        const privateMenuItem = wrapper.find('MenuItem').first();
+
+        privateMenuItem.prop('onClick')();
+
+        assert.equal(fakeStore.setDefault.callCount, 0);
       });
 
       it('should have a label that is the name of the group', () => {
@@ -164,16 +204,37 @@ describe('AnnotationPublishControl', () => {
 
     describe('private (only me) menu item', () => {
       it('should invoke callback with private privacy', () => {
-        const fakeOnSetPrivacy = sinon.stub();
-        const wrapper = createAnnotationPublishControl({
-          onSetPrivacy: fakeOnSetPrivacy,
-        });
+        const wrapper = createAnnotationPublishControl();
         const privateMenuItem = wrapper.find('MenuItem').at(1);
 
         privateMenuItem.prop('onClick')();
 
-        assert.calledWith(fakeOnSetPrivacy, { level: 'private' });
+        const call = fakeStore.createDraft.getCall(0);
+
+        assert.calledOnce(fakeStore.createDraft);
+        assert.isTrue(call.args[1].isPrivate);
+      });
+
+      it('should update default privacy level to private', () => {
+        const wrapper = createAnnotationPublishControl();
+        const privateMenuItem = wrapper.find('MenuItem').at(1);
+
+        privateMenuItem.prop('onClick')();
+
+        assert.calledOnce(fakeStore.setDefault);
+        assert.calledWith(fakeStore.setDefault, 'annotationPrivacy', 'private');
       });
+
+      it('should not update default privacy level if annotation is reply', () => {
+        fakeMetadata.isReply.returns(true);
+        const wrapper = createAnnotationPublishControl();
+        const privateMenuItem = wrapper.find('MenuItem').at(1);
+
+        privateMenuItem.prop('onClick')();
+
+        assert.equal(fakeStore.setDefault.callCount, 0);
+      });
+
       it('should use a private/lock icon', () => {
         const wrapper = createAnnotationPublishControl();
         const privateMenuItem = wrapper.find('MenuItem').at(1);
@@ -190,16 +251,25 @@ describe('AnnotationPublishControl', () => {
   });
 
   describe('cancel button', () => {
-    it('should have a cancel callback', () => {
-      const fakeOnCancel = sinon.stub();
-      const wrapper = createAnnotationPublishControl({
-        onCancel: fakeOnCancel,
-      });
+    it('should remove the current draft on cancel button click', () => {
+      const wrapper = createAnnotationPublishControl({});
+      const cancelBtn = wrapper.find('Button');
+
+      cancelBtn.props().onClick();
+
+      assert.calledOnce(fakeStore.removeDraft);
+      assert.calledWith(fakeStore.removeDraft, wrapper.props().annotation);
+      assert.equal(fakeStore.removeAnnotations.callCount, 0);
+    });
+
+    it('should remove the annotation from the store if it is new/unsaved', () => {
+      fakeMetadata.isNew.returns(true);
+      const wrapper = createAnnotationPublishControl({});
       const cancelBtn = wrapper.find('Button');
 
       cancelBtn.props().onClick();
 
-      assert.calledOnce(fakeOnCancel);
+      assert.calledOnce(fakeStore.removeAnnotations);
     });
   });
 });

src/sidebar/components/test/annotation-test.js

@@ -499,52 +499,6 @@ describe('annotation', function() {
       });
     });
 
-    describe('#setPrivacy', function() {
-      it('makes the annotation private when level is "private"', function() {
-        const parts = createDirective();
-        parts.controller.setPrivacy('private');
-        assert.calledWith(
-          fakeStore.createDraft,
-          parts.controller.annotation,
-          sinon.match({
-            isPrivate: true,
-          })
-        );
-      });
-
-      it('makes the annotation shared when level is "shared"', function() {
-        const parts = createDirective();
-        parts.controller.setPrivacy('shared');
-        assert.calledWith(
-          fakeStore.createDraft,
-          parts.controller.annotation,
-          sinon.match({
-            isPrivate: false,
-          })
-        );
-      });
-
-      it('sets the default visibility level if "shared"', function() {
-        const parts = createDirective();
-        parts.controller.edit();
-        parts.controller.setPrivacy('shared');
-        assert.calledWith(fakePermissions.setDefault, 'shared');
-      });
-
-      it('sets the default visibility if "private"', function() {
-        const parts = createDirective();
-        parts.controller.edit();
-        parts.controller.setPrivacy('private');
-        assert.calledWith(fakePermissions.setDefault, 'private');
-      });
-
-      it("doesn't save the visibility if the annotation is a reply", function() {
-        const parts = createDirective(fixtures.oldReply());
-        parts.controller.setPrivacy('private');
-        assert.notCalled(fakePermissions.setDefault);
-      });
-    });
-
     describe('#hasContent', function() {
       it('returns false if the annotation has no tags or text', function() {
         const controller = createDirective(fixtures.oldHighlight()).controller;
@@ -770,13 +724,6 @@ describe('annotation', function() {
         assert.equal(controller.state().text, 'unsaved-text');
       });
 
-      it('removes the draft when changes are discarded', function() {
-        const parts = createDirective();
-        parts.controller.edit();
-        parts.controller.revert();
-        assert.calledWith(fakeStore.removeDraft, parts.annotation);
-      });
-
       it('removes the draft when changes are saved', function() {
         const annotation = fixtures.defaultAnnotation();
         const controller = createDirective(annotation).controller;
@@ -786,22 +733,5 @@ describe('annotation', function() {
         });
       });
     });
-
-    describe('reverting edits', function() {
-      it('removes the current draft', function() {
-        const controller = createDirective(fixtures.defaultAnnotation())
-          .controller;
-        controller.edit();
-        controller.revert();
-        assert.calledWith(fakeStore.removeDraft, controller.annotation);
-      });
-
-      it('deletes the annotation if it was new', function() {
-        const controller = createDirective(fixtures.newAnnotation()).controller;
-        sandbox.spy($rootScope, '$broadcast');
-        controller.revert();
-        assert.calledWith($rootScope.$broadcast, events.ANNOTATION_DELETED);
-      });
-    });
   });
 });

src/sidebar/templates/annotation.html

@@ -41,12 +41,9 @@
 
     <div class="annotation-form-actions" ng-if="vm.editing()">
       <annotation-publish-control
-        group="vm.group()"
+        annotation="vm.annotation"
         is-disabled="!vm.hasContent()"
-        is-shared="vm.isShared()"
-        on-cancel="vm.revert()"
-        on-save="vm.save()"
-        on-set-privacy="vm.setPrivacy(level)"></annotation-publish-control>
+        on-save="vm.save()"></annotation-publish-control>
     </div>
 
     <annotation-license ng-if="vm.shouldShowLicense()"></annotation-license>