Unverified Commit 8fe3253e authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub

Merge pull request #1648 from hypothesis/remove-isPrivate-passthru

Make Annotation sub-components compute their own privacy
parents cce385d5 96b734fc
...@@ -12,7 +12,6 @@ const Button = require('./button'); ...@@ -12,7 +12,6 @@ const Button = require('./button');
*/ */
function AnnotationActionBar({ function AnnotationActionBar({
annotation, annotation,
isPrivate,
onDelete, onDelete,
onEdit, onEdit,
onFlag, onFlag,
...@@ -49,8 +48,8 @@ function AnnotationActionBar({ ...@@ -49,8 +48,8 @@ function AnnotationActionBar({
<Button icon="reply" title="Reply" onClick={onReply} /> <Button icon="reply" title="Reply" onClick={onReply} />
{showShareAction && ( {showShareAction && (
<AnnotationShareControl <AnnotationShareControl
annotation={annotation}
group={annotationGroup} group={annotationGroup}
isPrivate={isPrivate}
shareUri={shareURI(annotation)} shareUri={shareURI(annotation)}
/> />
)} )}
...@@ -74,8 +73,6 @@ function AnnotationActionBar({ ...@@ -74,8 +73,6 @@ function AnnotationActionBar({
AnnotationActionBar.propTypes = { AnnotationActionBar.propTypes = {
annotation: propTypes.object.isRequired, annotation: propTypes.object.isRequired,
/** Is this annotation shared at the group level or marked as "only me"/private? */
isPrivate: propTypes.bool.isRequired,
/** Callbacks for when action buttons are clicked/tapped */ /** Callbacks for when action buttons are clicked/tapped */
onEdit: propTypes.func.isRequired, onEdit: propTypes.func.isRequired,
onDelete: propTypes.func.isRequired, onDelete: propTypes.func.isRequired,
......
...@@ -16,7 +16,6 @@ function AnnotationHeader({ ...@@ -16,7 +16,6 @@ function AnnotationHeader({
annotation, annotation,
isEditing, isEditing,
isHighlight, isHighlight,
isPrivate,
onReplyCountClick, onReplyCountClick,
replyCount, replyCount,
showDocumentInfo, showDocumentInfo,
...@@ -60,7 +59,7 @@ function AnnotationHeader({ ...@@ -60,7 +59,7 @@ function AnnotationHeader({
</div> </div>
<div className="annotation-header__row"> <div className="annotation-header__row">
<AnnotationShareInfo annotation={annotation} isPrivate={isPrivate} /> <AnnotationShareInfo annotation={annotation} />
{!isEditing && isHighlight && ( {!isEditing && isHighlight && (
<div className="annotation-header__highlight"> <div className="annotation-header__highlight">
<SvgIcon <SvgIcon
...@@ -84,8 +83,6 @@ AnnotationHeader.propTypes = { ...@@ -84,8 +83,6 @@ AnnotationHeader.propTypes = {
isEditing: propTypes.bool, isEditing: propTypes.bool,
/* Whether the annotation is a highlight */ /* Whether the annotation is a highlight */
isHighlight: propTypes.bool, isHighlight: propTypes.bool,
/* Whether the annotation is an "only me" (private) annotation */
isPrivate: propTypes.bool,
/* Callback for when the toggle-replies element is clicked */ /* Callback for when the toggle-replies element is clicked */
onReplyCountClick: propTypes.func.isRequired, onReplyCountClick: propTypes.func.isRequired,
/* How many replies this annotation currently has */ /* How many replies this annotation currently has */
......
...@@ -14,12 +14,17 @@ const SvgIcon = require('./svg-icon'); ...@@ -14,12 +14,17 @@ const SvgIcon = require('./svg-icon');
* "Popup"-style component for sharing a single annotation. * "Popup"-style component for sharing a single annotation.
*/ */
function AnnotationShareControl({ function AnnotationShareControl({
annotation,
analytics, analytics,
flash, flash,
group, group,
isPrivate, permissions,
shareUri, shareUri,
}) { }) {
const isPrivate = !permissions.isShared(
annotation.permissions,
annotation.user
);
const shareRef = useRef(); const shareRef = useRef();
const inputRef = useRef(); const inputRef = useRef();
...@@ -125,22 +130,23 @@ function AnnotationShareControl({ ...@@ -125,22 +130,23 @@ function AnnotationShareControl({
} }
AnnotationShareControl.propTypes = { AnnotationShareControl.propTypes = {
/* The annotation in question */
annotation: propTypes.object.isRequired,
/** group that the annotation is in /** group that the annotation is in
* If missing, this component will not render * If missing, this component will not render
* FIXME: Refactor after root cause is addressed * FIXME: Refactor after root cause is addressed
* See https://github.com/hypothesis/client/issues/1542 * See https://github.com/hypothesis/client/issues/1542
*/ */
group: propTypes.object, group: propTypes.object,
/** Is this annotation set to "only me"/private? */
isPrivate: propTypes.bool.isRequired,
/** The URI to view the annotation on its own */ /** The URI to view the annotation on its own */
shareUri: propTypes.string.isRequired, shareUri: propTypes.string.isRequired,
/* services */ /* services */
analytics: propTypes.object.isRequired, analytics: propTypes.object.isRequired,
flash: propTypes.object.isRequired, flash: propTypes.object.isRequired,
permissions: propTypes.object.isRequired,
}; };
AnnotationShareControl.injectedProps = ['analytics', 'flash']; AnnotationShareControl.injectedProps = ['analytics', 'flash', 'permissions'];
module.exports = withServices(AnnotationShareControl); module.exports = withServices(AnnotationShareControl);
const propTypes = require('prop-types'); const propTypes = require('prop-types');
const { createElement } = require('preact'); const { createElement } = require('preact');
const SvgIcon = require('./svg-icon'); const { withServices } = require('../util/service-context');
const useStore = require('../store/use-store'); const useStore = require('../store/use-store');
const SvgIcon = require('./svg-icon');
/** /**
* Render information about what group an annotation is in and * Render information about what group an annotation is in and
* whether it is private to the current user (only me) * whether it is private to the current user (only me)
*/ */
function AnnotationShareInfo({ annotation, isPrivate }) { function AnnotationShareInfo({ annotation, permissions }) {
const group = useStore(store => store.getGroup(annotation.group)); const group = useStore(store => store.getGroup(annotation.group));
// We may not have access to the group object beyond its ID // We may not have access to the group object beyond its ID
...@@ -19,6 +20,11 @@ function AnnotationShareInfo({ annotation, isPrivate }) { ...@@ -19,6 +20,11 @@ function AnnotationShareInfo({ annotation, isPrivate }) {
// URL (link) returned by the API for this group. Some groups do not have links // URL (link) returned by the API for this group. Some groups do not have links
const linkToGroup = hasGroup && group.links && group.links.html; const linkToGroup = hasGroup && group.links && group.links.html;
const isPrivate = !permissions.isShared(
annotation.permissions,
annotation.user
);
return ( return (
<div className="annotation-share-info"> <div className="annotation-share-info">
{linkToGroup && ( {linkToGroup && (
...@@ -59,8 +65,11 @@ function AnnotationShareInfo({ annotation, isPrivate }) { ...@@ -59,8 +65,11 @@ function AnnotationShareInfo({ annotation, isPrivate }) {
AnnotationShareInfo.propTypes = { AnnotationShareInfo.propTypes = {
/** The current annotation object for which sharing info will be rendered */ /** The current annotation object for which sharing info will be rendered */
annotation: propTypes.object.isRequired, annotation: propTypes.object.isRequired,
/** Is this an "only me" (private) annotation? */
isPrivate: propTypes.bool.isRequired, /** injected services */
permissions: propTypes.object.isRequired,
}; };
module.exports = AnnotationShareInfo; AnnotationShareInfo.injectedProps = ['permissions'];
module.exports = withServices(AnnotationShareInfo);
...@@ -13,7 +13,6 @@ describe('AnnotationHeader', () => { ...@@ -13,7 +13,6 @@ describe('AnnotationHeader', () => {
annotation={fixtures.defaultAnnotation()} annotation={fixtures.defaultAnnotation()}
isEditing={false} isEditing={false}
isHighlight={false} isHighlight={false}
isPrivate={false}
onReplyCountClick={sinon.stub()} onReplyCountClick={sinon.stub()}
replyCount={0} replyCount={0}
showDocumentInfo={false} showDocumentInfo={false}
......
...@@ -6,10 +6,12 @@ const AnnotationShareControl = require('../annotation-share-control'); ...@@ -6,10 +6,12 @@ const AnnotationShareControl = require('../annotation-share-control');
const mockImportedComponents = require('./mock-imported-components'); const mockImportedComponents = require('./mock-imported-components');
describe('AnnotationShareControl', () => { describe('AnnotationShareControl', () => {
let fakeAnnotation;
let fakeAnalytics; let fakeAnalytics;
let fakeCopyToClipboard; let fakeCopyToClipboard;
let fakeFlash; let fakeFlash;
let fakeGroup; let fakeGroup;
let fakePermissions;
let fakeShareUri; let fakeShareUri;
let container; let container;
...@@ -21,10 +23,11 @@ describe('AnnotationShareControl', () => { ...@@ -21,10 +23,11 @@ describe('AnnotationShareControl', () => {
function createComponent(props = {}) { function createComponent(props = {}) {
return mount( return mount(
<AnnotationShareControl <AnnotationShareControl
annotation={fakeAnnotation}
analytics={fakeAnalytics} analytics={fakeAnalytics}
flash={fakeFlash} flash={fakeFlash}
group={fakeGroup} group={fakeGroup}
isPrivate={false} permissions={fakePermissions}
shareUri={fakeShareUri} shareUri={fakeShareUri}
{...props} {...props}
/>, />,
...@@ -48,6 +51,12 @@ describe('AnnotationShareControl', () => { ...@@ -48,6 +51,12 @@ describe('AnnotationShareControl', () => {
container = document.createElement('div'); container = document.createElement('div');
document.body.appendChild(container); document.body.appendChild(container);
fakeAnnotation = {
group: 'fakegroup',
permissions: {},
user: 'acct:bar@foo.com',
};
fakeAnalytics = { fakeAnalytics = {
events: { events: {
ANNOTATION_SHARED: 'whatever', ANNOTATION_SHARED: 'whatever',
...@@ -64,6 +73,9 @@ describe('AnnotationShareControl', () => { ...@@ -64,6 +73,9 @@ describe('AnnotationShareControl', () => {
name: 'My Group', name: 'My Group',
type: 'private', type: 'private',
}; };
fakePermissions = {
isShared: sinon.stub().returns(true),
};
fakeShareUri = 'https://www.example.com'; fakeShareUri = 'https://www.example.com';
AnnotationShareControl.$imports.$mock(mockImportedComponents()); AnnotationShareControl.$imports.$mock(mockImportedComponents());
...@@ -152,26 +164,27 @@ describe('AnnotationShareControl', () => { ...@@ -152,26 +164,27 @@ describe('AnnotationShareControl', () => {
[ [
{ {
groupType: 'private', groupType: 'private',
isPrivate: false, isShared: true,
expected: 'Only members of the group My Group may view this annotation.', expected: 'Only members of the group My Group may view this annotation.',
}, },
{ {
groupType: 'open', groupType: 'open',
isPrivate: false, isShared: true,
expected: 'Anyone using this link may view this annotation.', expected: 'Anyone using this link may view this annotation.',
}, },
{ {
groupType: 'private', groupType: 'private',
isPrivate: true, isShared: false,
expected: 'Only you may view this annotation.', expected: 'Only you may view this annotation.',
}, },
{ {
groupType: 'open', groupType: 'open',
isPrivate: true, isShared: false,
expected: 'Only you may view this annotation.', expected: 'Only you may view this annotation.',
}, },
].forEach(testcase => { ].forEach(testcase => {
it(`renders the correct sharing information for a ${testcase.groupType} group when annotation privacy is ${testcase.isPrivate}`, () => { it(`renders the correct sharing information for a ${testcase.groupType} group when annotation sharing is ${testcase.isShared}`, () => {
fakePermissions.isShared.returns(testcase.isShared);
fakeGroup.type = testcase.groupType; fakeGroup.type = testcase.groupType;
const wrapper = createComponent({ isPrivate: testcase.isPrivate }); const wrapper = createComponent({ isPrivate: testcase.isPrivate });
openElement(wrapper); openElement(wrapper);
......
...@@ -10,12 +10,13 @@ describe('AnnotationShareInfo', () => { ...@@ -10,12 +10,13 @@ describe('AnnotationShareInfo', () => {
let fakeGroup; let fakeGroup;
let fakeStore; let fakeStore;
let fakeGetGroup; let fakeGetGroup;
let fakePermissions;
const createAnnotationShareInfo = props => { const createAnnotationShareInfo = props => {
return mount( return mount(
<AnnotationShareInfo <AnnotationShareInfo
annotation={fixtures.defaultAnnotation()} annotation={fixtures.defaultAnnotation()}
isPrivate={false} permissions={fakePermissions}
{...props} {...props}
/> />
); );
...@@ -31,6 +32,9 @@ describe('AnnotationShareInfo', () => { ...@@ -31,6 +32,9 @@ describe('AnnotationShareInfo', () => {
}; };
fakeGetGroup = sinon.stub().returns(fakeGroup); fakeGetGroup = sinon.stub().returns(fakeGroup);
fakeStore = { getGroup: fakeGetGroup }; fakeStore = { getGroup: fakeGetGroup };
fakePermissions = {
isShared: sinon.stub().returns(true),
};
AnnotationShareInfo.$imports.$mock(mockImportedComponents()); AnnotationShareInfo.$imports.$mock(mockImportedComponents());
AnnotationShareInfo.$imports.$mock({ AnnotationShareInfo.$imports.$mock({
...@@ -96,15 +100,19 @@ describe('AnnotationShareInfo', () => { ...@@ -96,15 +100,19 @@ describe('AnnotationShareInfo', () => {
describe('"only you" information', () => { describe('"only you" information', () => {
it('should not show privacy information if annotation is not private', () => { it('should not show privacy information if annotation is not private', () => {
const wrapper = createAnnotationShareInfo({ isPrivate: false }); const wrapper = createAnnotationShareInfo();
const privacy = wrapper.find('.annotation-share-info__private'); const privacy = wrapper.find('.annotation-share-info__private');
assert.notOk(privacy.exists()); assert.notOk(privacy.exists());
}); });
context('private annotation', () => { context('private annotation', () => {
beforeEach(() => {
fakePermissions.isShared.returns(false);
});
it('should show privacy icon', () => { it('should show privacy icon', () => {
const wrapper = createAnnotationShareInfo({ isPrivate: true }); const wrapper = createAnnotationShareInfo();
const privacyIcon = wrapper.find( const privacyIcon = wrapper.find(
'.annotation-share-info__private .annotation-share-info__icon' '.annotation-share-info__private .annotation-share-info__icon'
...@@ -114,7 +122,7 @@ describe('AnnotationShareInfo', () => { ...@@ -114,7 +122,7 @@ describe('AnnotationShareInfo', () => {
assert.equal(privacyIcon.prop('name'), 'lock'); assert.equal(privacyIcon.prop('name'), 'lock');
}); });
it('should not show "only me" text for first-party group', () => { it('should not show "only me" text for first-party group', () => {
const wrapper = createAnnotationShareInfo({ isPrivate: true }); const wrapper = createAnnotationShareInfo();
const privacyText = wrapper.find( const privacyText = wrapper.find(
'.annotation-share-info__private-info' '.annotation-share-info__private-info'
...@@ -124,7 +132,7 @@ describe('AnnotationShareInfo', () => { ...@@ -124,7 +132,7 @@ describe('AnnotationShareInfo', () => {
}); });
it('should show "only me" text for annotation in third-party group', () => { it('should show "only me" text for annotation in third-party group', () => {
fakeGetGroup.returns({ name: 'Some Name' }); fakeGetGroup.returns({ name: 'Some Name' });
const wrapper = createAnnotationShareInfo({ isPrivate: true }); const wrapper = createAnnotationShareInfo();
const privacyText = wrapper.find( const privacyText = wrapper.find(
'.annotation-share-info__private-info' '.annotation-share-info__private-info'
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
<annotation-header annotation="vm.annotation" <annotation-header annotation="vm.annotation"
is-editing="vm.editing()" is-editing="vm.editing()"
is-highlight="vm.isHighlight()" is-highlight="vm.isHighlight()"
is-private="vm.state().isPrivate"
on-reply-count-click="vm.onReplyCountClick()" on-reply-count-click="vm.onReplyCountClick()"
reply-count="vm.replyCount" reply-count="vm.replyCount"
show-document-info="vm.showDocumentInfo"> show-document-info="vm.showDocumentInfo">
...@@ -34,7 +33,7 @@ ...@@ -34,7 +33,7 @@
ng-click="vm.toggleCollapseBody($event)" ng-click="vm.toggleCollapseBody($event)"
ng-title="vm.collapseBody ? 'Show the full annotation text' : 'Show the first few lines only'" ng-title="vm.collapseBody ? 'Show the full annotation text' : 'Show the first few lines only'"
ng-bind="vm.collapseBody ? 'More' : 'Less'" ng-bind="vm.collapseBody ? 'More' : 'Less'"
h-branding="accentColor">mire</a> h-branding="accentColor">more</a>
</div> </div>
<!-- Tags --> <!-- Tags -->
<tag-editor <tag-editor
...@@ -42,7 +41,7 @@ ...@@ -42,7 +41,7 @@
annotation="vm.annotation" annotation="vm.annotation"
on-edit-tags="vm.setTags(tags)" on-edit-tags="vm.setTags(tags)"
tag-list="vm.state().tags" tag-list="vm.state().tags"
/> >
</tag-editor> </tag-editor>
<tag-list <tag-list
...@@ -81,7 +80,6 @@ ...@@ -81,7 +80,6 @@
<div class="annotation-actions" ng-if="!vm.isSaving && !vm.editing() && vm.id()"> <div class="annotation-actions" ng-if="!vm.isSaving && !vm.editing() && vm.id()">
<annotation-action-bar <annotation-action-bar
annotation="vm.annotation" annotation="vm.annotation"
is-private="vm.state().isPrivate"
on-delete="vm.delete()" on-delete="vm.delete()"
on-flag="vm.flag()" on-flag="vm.flag()"
on-edit="vm.edit()" on-edit="vm.edit()"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment