Commit b1ce8485 authored by Lyza Danger Gardner's avatar Lyza Danger Gardner Committed by Lyza Gardner

Refactor AnnotationUser per conventions

Refactor AnnotationUser to be more "dumb" like other leafy components
parent b328f7a8
import { SvgIcon } from '@hypothesis/frontend-shared'; import { SvgIcon } from '@hypothesis/frontend-shared';
import { useMemo } from 'preact/hooks'; import { useMemo } from 'preact/hooks';
import { withServices } from '../../service-context';
import { useStoreProxy } from '../../store/use-store'; import { useStoreProxy } from '../../store/use-store';
import { isThirdPartyUser, username } from '../../helpers/account-id';
import { import {
domainAndTitle, domainAndTitle,
isHighlight, isHighlight,
isReply, isReply,
hasBeenEdited, hasBeenEdited,
} from '../../helpers/annotation-metadata'; } from '../../helpers/annotation-metadata';
import { annotationDisplayName } from '../../helpers/annotation-user';
import { isPrivate } from '../../helpers/permissions'; import { isPrivate } from '../../helpers/permissions';
import Button from '../Button'; import Button from '../Button';
...@@ -19,6 +22,8 @@ import AnnotationUser from './AnnotationUser'; ...@@ -19,6 +22,8 @@ import AnnotationUser from './AnnotationUser';
/** /**
* @typedef {import("../../../types/api").Annotation} Annotation * @typedef {import("../../../types/api").Annotation} Annotation
* @typedef {import('../../services/service-url').ServiceUrlGetter} ServiceUrlGetter
* @typedef {import('../../../types/config').MergedConfig} MergedConfig
*/ */
/** /**
...@@ -27,6 +32,9 @@ import AnnotationUser from './AnnotationUser'; ...@@ -27,6 +32,9 @@ import AnnotationUser from './AnnotationUser';
* @prop {boolean} [isEditing] - Whether the annotation is actively being edited * @prop {boolean} [isEditing] - Whether the annotation is actively being edited
* @prop {number} replyCount - How many replies this annotation currently has * @prop {number} replyCount - How many replies this annotation currently has
* @prop {boolean} threadIsCollapsed - Is this thread currently collapsed? * @prop {boolean} threadIsCollapsed - Is this thread currently collapsed?
* @prop {ServiceUrlGetter} serviceUrl - Injected service
* @prop {MergedConfig} settings - Injected
*
*/ */
/** /**
...@@ -36,13 +44,37 @@ import AnnotationUser from './AnnotationUser'; ...@@ -36,13 +44,37 @@ import AnnotationUser from './AnnotationUser';
* *
* @param {AnnotationHeaderProps} props * @param {AnnotationHeaderProps} props
*/ */
export default function AnnotationHeader({ function AnnotationHeader({
annotation, annotation,
isEditing, isEditing,
replyCount, replyCount,
threadIsCollapsed, threadIsCollapsed,
serviceUrl,
settings,
}) { }) {
const store = useStoreProxy(); const store = useStoreProxy();
const authDomain = store.authDomain();
const displayNamesEnabled = store.isFeatureEnabled('client_display_names');
const isThirdParty = isThirdPartyUser(annotation.user, authDomain);
const authorDisplayName = annotationDisplayName(
annotation,
isThirdParty,
displayNamesEnabled
);
const authorLink = (() => {
if (!isThirdParty) {
return serviceUrl('user', { user: annotation.user });
} else {
return (
(settings.usernameUrl &&
`${settings.usernameUrl}${username(annotation.user)}`) ??
undefined
);
}
})();
const isCollapsedReply = isReply(annotation) && threadIsCollapsed; const isCollapsedReply = isReply(annotation) && threadIsCollapsed;
const annotationIsPrivate = isPrivate(annotation.permissions); const annotationIsPrivate = isPrivate(annotation.permissions);
...@@ -96,7 +128,10 @@ export default function AnnotationHeader({ ...@@ -96,7 +128,10 @@ export default function AnnotationHeader({
title="This annotation is visible only to you" title="This annotation is visible only to you"
/> />
)} )}
<AnnotationUser annotation={annotation} /> <AnnotationUser
authorLink={authorLink}
displayName={authorDisplayName}
/>
{showReplyButton && ( {showReplyButton && (
<Button <Button
className="AnnotationHeader__reply-toggle" className="AnnotationHeader__reply-toggle"
...@@ -143,3 +178,7 @@ export default function AnnotationHeader({ ...@@ -143,3 +178,7 @@ export default function AnnotationHeader({
</header> </header>
); );
} }
AnnotationHeader.injectedProps = ['serviceUrl', 'settings'];
export default withServices(AnnotationHeader);
import { isThirdPartyUser, username } from '../../helpers/account-id';
import { annotationDisplayName } from '../../helpers/annotation-user';
import { withServices } from '../../service-context';
/** /**
* @typedef {import("../../../types/api").Annotation} Annotation * @typedef {import("../../../types/api").Annotation} Annotation
* @typedef {import('../../../types/config').MergedConfig} MergedConfig
* @typedef {import('../../services/service-url').ServiceUrlGetter} ServiceUrlGetter
*/ */
/** /**
* @typedef AnnotationUserProps * @typedef AnnotationUserProps
* @prop {Annotation} annotation - The annotation whose user is relevant * @prop {string} [authorLink]
* @prop {Object} features - Injected service * @prop {string} displayName
* @prop {ServiceUrlGetter} serviceUrl - Injected service
* @prop {MergedConfig} settings - Injected service
*/ */
/** /**
...@@ -22,30 +14,13 @@ import { withServices } from '../../service-context'; ...@@ -22,30 +14,13 @@ import { withServices } from '../../service-context';
* *
* @param {AnnotationUserProps} props * @param {AnnotationUserProps} props
*/ */
function AnnotationUser({ annotation, features, serviceUrl, settings }) { function AnnotationUser({ authorLink, displayName }) {
const user = annotation.user; if (authorLink) {
const isFirstPartyUser = !isThirdPartyUser(user, settings.authDomain);
const username_ = username(user);
// How should the user's name be displayed?
const displayName = annotationDisplayName(
annotation,
!isFirstPartyUser,
features.flagEnabled('client_display_names')
);
const shouldLinkToActivity = isFirstPartyUser || settings.usernameUrl;
if (shouldLinkToActivity) {
return ( return (
<div className="AnnotationUser"> <div className="AnnotationUser">
<a <a
className="AnnotationUser__link" className="AnnotationUser__link"
href={ href={authorLink}
isFirstPartyUser
? serviceUrl('user', { user })
: `${settings.usernameUrl}${username_}`
}
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
> >
...@@ -62,6 +37,4 @@ function AnnotationUser({ annotation, features, serviceUrl, settings }) { ...@@ -62,6 +37,4 @@ function AnnotationUser({ annotation, features, serviceUrl, settings }) {
); );
} }
AnnotationUser.injectedProps = ['features', 'serviceUrl', 'settings']; export default AnnotationUser;
export default withServices(AnnotationUser);
...@@ -8,11 +8,15 @@ import mockImportedComponents from '../../../../test-util/mock-imported-componen ...@@ -8,11 +8,15 @@ import mockImportedComponents from '../../../../test-util/mock-imported-componen
import AnnotationHeader, { $imports } from '../AnnotationHeader'; import AnnotationHeader, { $imports } from '../AnnotationHeader';
describe('AnnotationHeader', () => { describe('AnnotationHeader', () => {
let fakeAccountId;
let fakeAnnotationDisplayName;
let fakeDomainAndTitle; let fakeDomainAndTitle;
let fakeIsHighlight; let fakeIsHighlight;
let fakeIsReply; let fakeIsReply;
let fakeHasBeenEdited; let fakeHasBeenEdited;
let fakeIsPrivate; let fakeIsPrivate;
let fakeServiceUrl;
let fakeSettings;
let fakeStore; let fakeStore;
const createAnnotationHeader = props => { const createAnnotationHeader = props => {
...@@ -22,6 +26,8 @@ describe('AnnotationHeader', () => { ...@@ -22,6 +26,8 @@ describe('AnnotationHeader', () => {
isEditing={false} isEditing={false}
replyCount={0} replyCount={0}
threadIsCollapsed={false} threadIsCollapsed={false}
serviceUrl={fakeServiceUrl}
settings={fakeSettings}
{...props} {...props}
/> />
); );
...@@ -34,7 +40,19 @@ describe('AnnotationHeader', () => { ...@@ -34,7 +40,19 @@ describe('AnnotationHeader', () => {
fakeHasBeenEdited = sinon.stub().returns(false); fakeHasBeenEdited = sinon.stub().returns(false);
fakeIsPrivate = sinon.stub(); fakeIsPrivate = sinon.stub();
fakeAccountId = {
isThirdPartyUser: sinon.stub().returns(false),
username: sinon.stub().returnsArg(0),
};
fakeAnnotationDisplayName = sinon.stub().returns('Robbie Burns');
fakeServiceUrl = sinon.stub().returns('http://example.com');
fakeSettings = { usernameUrl: 'http://foo.bar/' };
fakeStore = { fakeStore = {
authDomain: sinon.stub().returns('foo.com'),
isFeatureEnabled: sinon.stub().returns(false),
route: sinon.stub().returns('sidebar'), route: sinon.stub().returns('sidebar'),
setExpanded: sinon.stub(), setExpanded: sinon.stub(),
}; };
...@@ -42,12 +60,16 @@ describe('AnnotationHeader', () => { ...@@ -42,12 +60,16 @@ describe('AnnotationHeader', () => {
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({ $imports.$mock({
'../../store/use-store': { useStoreProxy: () => fakeStore }, '../../store/use-store': { useStoreProxy: () => fakeStore },
'../../helpers/account-id': fakeAccountId,
'../../helpers/annotation-metadata': { '../../helpers/annotation-metadata': {
domainAndTitle: fakeDomainAndTitle, domainAndTitle: fakeDomainAndTitle,
isHighlight: fakeIsHighlight, isHighlight: fakeIsHighlight,
isReply: fakeIsReply, isReply: fakeIsReply,
hasBeenEdited: fakeHasBeenEdited, hasBeenEdited: fakeHasBeenEdited,
}, },
'../../helpers/annotation-user': {
annotationDisplayName: fakeAnnotationDisplayName,
},
'../../helpers/permissions': { '../../helpers/permissions': {
isPrivate: fakeIsPrivate, isPrivate: fakeIsPrivate,
}, },
...@@ -84,6 +106,47 @@ describe('AnnotationHeader', () => { ...@@ -84,6 +106,47 @@ describe('AnnotationHeader', () => {
}); });
}); });
describe('annotation author (user) information', () => {
it('should link to author activity if first-party', () => {
fakeAccountId.isThirdPartyUser.returns(false);
const wrapper = createAnnotationHeader();
assert.equal(
wrapper.find('AnnotationUser').props().authorLink,
'http://example.com'
);
});
it('should link to author activity if third-party and has settings URL', () => {
fakeAccountId.isThirdPartyUser.returns(true);
const fakeAnnotation = fixtures.defaultAnnotation();
const wrapper = createAnnotationHeader({ annotation: fakeAnnotation });
assert.equal(
wrapper.find('AnnotationUser').props().authorLink,
`http://foo.bar/${fakeAnnotation.user}`
);
});
it('should not link to author if third-party and no settings URL', () => {
fakeAccountId.isThirdPartyUser.returns(true);
const wrapper = createAnnotationHeader({ settings: {} });
assert.isUndefined(wrapper.find('AnnotationUser').props().authorLink);
});
it('should pass the display name to AnnotationUser', () => {
const wrapper = createAnnotationHeader();
assert.equal(
wrapper.find('AnnotationUser').props().displayName,
'Robbie Burns'
);
});
});
describe('expand replies toggle button', () => { describe('expand replies toggle button', () => {
const findReplyButton = wrapper => const findReplyButton = wrapper =>
wrapper.find('Button').filter('.AnnotationHeader__reply-toggle'); wrapper.find('Button').filter('.AnnotationHeader__reply-toggle');
......
import { mount } from 'enzyme'; import { mount } from 'enzyme';
import { checkAccessibility } from '../../../../test-util/accessibility'; import { checkAccessibility } from '../../../../test-util/accessibility';
import mockImportedComponents from '../../../../test-util/mock-imported-components';
import AnnotationUser, { $imports } from '../AnnotationUser'; import AnnotationUser from '../AnnotationUser';
describe('AnnotationUser', () => { describe('AnnotationUser', () => {
let fakeAnnotation; const createAnnotationUser = props => {
let fakeAnnotationUser; return mount(<AnnotationUser displayName="Filbert Bronzo" {...props} />);
let fakeFeatures;
let fakeIsThirdPartyUser;
let fakeServiceUrl;
let fakeSettings;
let fakeUsername;
const createAnnotationUser = () => {
return mount(
<AnnotationUser
annotation={fakeAnnotation}
features={fakeFeatures}
serviceUrl={fakeServiceUrl}
settings={fakeSettings}
/>
);
};
beforeEach(() => {
fakeAnnotation = {
user: 'someone@hypothes.is',
};
fakeAnnotationUser = {
annotationDisplayName: sinon
.stub()
.callsFake(annotation => annotation.user),
}; };
fakeFeatures = { flagEnabled: sinon.stub() };
fakeIsThirdPartyUser = sinon.stub().returns(false);
fakeServiceUrl = sinon.stub();
fakeSettings = {};
fakeUsername = sinon.stub();
$imports.$mock(mockImportedComponents()); it('links to the author activity page if link provided', () => {
$imports.$mock({ const wrapper = createAnnotationUser({
'../../helpers/account-id': { authorLink: 'http://www.foo.bar/baz',
isThirdPartyUser: fakeIsThirdPartyUser,
username: fakeUsername,
},
'../../helpers/annotation-user': fakeAnnotationUser,
});
}); });
afterEach(() => { assert.isTrue(wrapper.find('a[href="http://www.foo.bar/baz"]').exists());
$imports.$restore();
}); });
describe('link to user activity', () => { it('does not link to the author activity page if no link provided', () => {
context('first-party user', () => {
it('should provide a link to the user profile', () => {
fakeIsThirdPartyUser.returns(false);
fakeServiceUrl.returns('link-to-user');
const wrapper = createAnnotationUser(); const wrapper = createAnnotationUser();
const linkEl = wrapper.find('a');
assert.isOk(linkEl.exists());
assert.calledWith(fakeServiceUrl, 'user', {
user: fakeAnnotation.user,
});
assert.equal(linkEl.prop('href'), 'link-to-user');
});
});
context('third-party user', () => { assert.isFalse(wrapper.find('a').exists());
beforeEach(() => {
fakeIsThirdPartyUser.returns(true);
}); });
it('should link to user if `settings.usernameUrl` is set', () => { it('renders the author name', () => {
fakeSettings.usernameUrl = 'http://example.com?user=';
fakeUsername.returns('elephant');
const wrapper = createAnnotationUser(); const wrapper = createAnnotationUser();
const linkEl = wrapper.find('a');
assert.isOk(linkEl.exists()); assert.equal(wrapper.text(), 'Filbert Bronzo');
assert.equal(linkEl.prop('href'), 'http://example.com?user=elephant');
});
it('should not link to user if `settings.usernameUrl` is not set', () => {
const wrapper = createAnnotationUser();
const linkEl = wrapper.find('a');
assert.isNotOk(linkEl.exists());
});
});
});
it('renders the annotation author name', () => {
fakeFeatures.flagEnabled.withArgs('client_display_names').returns(true);
createAnnotationUser();
assert.calledWith(
fakeAnnotationUser.annotationDisplayName,
fakeAnnotation,
false,
true
);
}); });
it( it(
......
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