Merge pull request #2078 from hypothesis/convert-stream-content

Convert stream content to Preact

src/sidebar/components/stream-content.js

-import { watch } from '../util/watch';
+import { createElement } from 'preact';
+import { useCallback, useEffect } from 'preact/hooks';
+import propTypes from 'prop-types';
 
-// @ngInject
-function StreamContentController($scope, store, api, rootThread, searchFilter) {
-  /** `offset` parameter for the next search API call. */
-  let offset = 0;
+import { withServices } from '../util/service-context';
+import useStore from '../store/use-store';
 
-  /** Load annotations fetched from the API into the app. */
-  const load = function (result) {
-    offset += result.rows.length;
-    const annots = [...result.rows, ...result.replies];
-    store.addAnnotations(annots);
-  };
+import ThreadList from './thread-list';
 
-  const currentQuery = () => store.routeParams().q;
+/**
+ * The main content of the "stream" route (https://hypothes.is/stream)
+ */
+function StreamContent({
+  api,
+  rootThread: rootThreadService,
+  searchFilter,
+  toastMessenger,
+}) {
+  const addAnnotations = useStore(store => store.addAnnotations);
+  const clearAnnotations = useStore(store => store.clearAnnotations);
+  const currentQuery = useStore(store => store.routeParams().q);
+  const setSortKey = useStore(store => store.setSortKey);
 
   /**
-   * Fetch the next `limit` annotations starting from `offset` from the API.
+   * Fetch annotations from the API and display them in the stream.
+   *
+   * @param {string} query - The user-supplied search query
    */
-  const fetch = function (limit) {
-    const query = Object.assign(
-      {
+  const loadAnnotations = useCallback(
+    async query => {
+      const queryParams = {
         _separate_replies: true,
-        offset: offset,
-        limit: limit,
-      },
-      searchFilter.toObject(currentQuery())
-    );
 
-    api
-      .search(query)
-      .then(load)
-      .catch(function (err) {
-        console.error(err);
-      });
-  };
+        // nb. There is currently no way to load anything except the first
+        // 20 matching annotations in the UI.
+        offset: 0,
+        limit: 20,
 
-  function clearAndFetch() {
-    // In case this route loaded after a client-side route change (eg. from
-    // '/a/:id'), clear any existing annotations.
-    store.clearAnnotations();
-
-    // Fetch initial batch of annotations.
-    offset = 0;
-    fetch(20);
-  }
+        ...searchFilter.toObject(query),
+      };
+      const results = await api.search(queryParams);
+      addAnnotations([...results.rows, ...results.replies]);
+    },
+    [addAnnotations, api, searchFilter]
+  );
 
-  const unsubscribe = watch(store.subscribe, currentQuery, () => {
-    clearAndFetch();
+  // Update the stream when this route is initially displayed and whenever
+  // the search query is updated.
+  useEffect(() => {
+    // Sort the stream so that the newest annotations are at the top
+    setSortKey('Newest');
+    clearAnnotations();
+    loadAnnotations(currentQuery).catch(err => {
+      toastMessenger.error(`Unable to fetch annotations: ${err.message}`);
     });
-  $scope.$on('$destroy', unsubscribe);
-
-  clearAndFetch();
+  }, [
+    clearAnnotations,
+    currentQuery,
+    loadAnnotations,
+    setSortKey,
+    toastMessenger,
+  ]);
 
-  this.setCollapsed = store.setCollapsed;
-  this.rootThread = () => rootThread.thread(store.getState());
+  const rootThread = useStore(store =>
+    rootThreadService.thread(store.getState())
+  );
 
-  // Sort the stream so that the newest annotations are at the top
-  store.setSortKey('Newest');
+  return <ThreadList thread={rootThread} />;
 }
 
-export default {
-  controller: StreamContentController,
-  controllerAs: 'vm',
-  bindings: {},
-  template: require('../templates/stream-content.html'),
+StreamContent.propTypes = {
+  // Injected services.
+  api: propTypes.object,
+  rootThread: propTypes.object,
+  searchFilter: propTypes.object,
+  toastMessenger: propTypes.object,
 };
+
+StreamContent.injectedProps = [
+  'api',
+  'rootThread',
+  'searchFilter',
+  'toastMessenger',
+];
+
+export default withServices(StreamContent);

src/sidebar/components/test/stream-content-test.js

-import angular from 'angular';
-import EventEmitter from 'tiny-emitter';
+import { mount } from 'enzyme';
+import { createElement } from 'preact';
 
-import streamContent from '../stream-content';
+import mockImportedComponents from '../../../test-util/mock-imported-components';
+import { waitFor } from '../../../test-util/wait';
 
-class FakeRootThread extends EventEmitter {
-  constructor() {
-    super();
-    this.thread = sinon.stub();
-  }
-}
+import StreamContent, { $imports } from '../stream-content';
 
-describe('StreamContentController', function () {
-  let $componentController;
-  let fakeStore;
+describe('StreamContent', () => {
+  let fakeApi;
   let fakeRootThread;
   let fakeSearchFilter;
-  let fakeApi;
-  let fakeStreamer;
-  let fakeStreamFilter;
+  let fakeStore;
+  let fakeToastMessenger;
 
-  before(function () {
-    angular.module('h', []).component('streamContent', streamContent);
-  });
+  beforeEach(() => {
+    fakeApi = {
+      search: sinon.stub().resolves({ rows: [], replies: [], total: 0 }),
+    };
 
-  beforeEach(function () {
-    fakeStore = {
-      addAnnotations: sinon.stub(),
-      clearAnnotations: sinon.spy(),
-      routeParams: sinon.stub().returns({ id: 'test' }),
-      setCollapsed: sinon.spy(),
-      setForceVisible: sinon.spy(),
-      setSortKey: sinon.spy(),
-      subscribe: sinon.spy(),
+    fakeRootThread = {
+      thread: sinon.stub().returns({}),
     };
 
     fakeSearchFilter = {
-      generateFacetedFilter: sinon.stub(),
       toObject: sinon.stub().returns({}),
     };
 
-    fakeApi = {
-      search: sinon.stub().resolves({ rows: [], replies: [], total: 0 }),
-    };
-
-    fakeStreamer = {
-      open: sinon.spy(),
-      close: sinon.spy(),
-      setConfig: sinon.spy(),
-      connect: sinon.spy(),
+    fakeStore = {
+      addAnnotations: sinon.stub(),
+      clearAnnotations: sinon.spy(),
+      getState: sinon.stub().returns({}),
+      routeParams: sinon.stub().returns({ id: 'test' }),
+      setSortKey: sinon.spy(),
     };
 
-    fakeStreamFilter = {
-      resetFilter: sinon.stub().returnsThis(),
-      setMatchPolicyIncludeAll: sinon.stub().returnsThis(),
-      getFilter: sinon.stub(),
+    fakeToastMessenger = {
+      error: sinon.stub(),
     };
 
-    fakeRootThread = new FakeRootThread();
-
-    angular.mock.module('h', {
-      store: fakeStore,
-      api: fakeApi,
-      rootThread: fakeRootThread,
-      searchFilter: fakeSearchFilter,
-      streamFilter: fakeStreamFilter,
-      streamer: fakeStreamer,
+    $imports.$mock(mockImportedComponents());
+    $imports.$mock({
+      '../store/use-store': callback => callback(fakeStore),
     });
-
-    angular.mock.inject(function (_$componentController_) {
-      $componentController = _$componentController_;
   });
+
+  afterEach(() => {
+    $imports.$restore();
   });
 
-  function createController() {
-    return $componentController('streamContent', {}, {});
+  function createComponent() {
+    return mount(
+      <StreamContent
+        api={fakeApi}
+        rootThread={fakeRootThread}
+        searchFilter={fakeSearchFilter}
+        toastMessenger={fakeToastMessenger}
+      />
+    );
   }
 
   it('clears any existing annotations when the /stream route is loaded', () => {
-    createController();
+    createComponent();
     assert.calledOnce(fakeStore.clearAnnotations);
   });
 
-  it('calls the search API with `_separate_replies: true`', function () {
-    createController();
+  it('calls the search API with `_separate_replies: true`', () => {
+    createComponent();
     assert.equal(fakeApi.search.firstCall.args[0]._separate_replies, true);
   });
 
-  it('passes the annotations and replies from search to loadAnnotations()', function () {
-    fakeApi.search = function () {
-      return Promise.resolve({
+  it('loads the annotations and replies into the store', async () => {
+    fakeApi.search.resolves({
       rows: ['annotation_1', 'annotation_2'],
       replies: ['reply_1', 'reply_2', 'reply_3'],
     });
-    };
 
-    createController();
+    createComponent();
+    await waitFor(() => fakeStore.addAnnotations.called);
 
-    return Promise.resolve().then(function () {
     assert.calledOnce(fakeStore.addAnnotations);
     assert.calledWith(fakeStore.addAnnotations, [
       'annotation_1',
@@ -106,29 +87,43 @@ describe('StreamContentController', function () {
       'reply_3',
     ]);
   });
+
+  it('displays an error if fetching annotations fails', async () => {
+    fakeApi.search.rejects(new Error('Server error'));
+
+    createComponent();
+    await waitFor(() => fakeToastMessenger.error.called);
+
+    assert.calledWith(
+      fakeToastMessenger.error,
+      'Unable to fetch annotations: Server error'
+    );
   });
 
-  context('when route parameters change', function () {
-    it('updates annotations if the query changed', function () {
+  context('when route parameters change', () => {
+    it('updates annotations if the query changed', () => {
       fakeStore.routeParams.returns({ q: 'test query' });
-      createController();
+      const wrapper = createComponent();
       fakeStore.clearAnnotations.resetHistory();
       fakeApi.search.resetHistory();
 
       fakeStore.routeParams.returns({ q: 'new query' });
-      fakeStore.subscribe.lastCall.callback();
+      // Force update. `useStore` handles this in the real app.
+      wrapper.setProps({});
 
       assert.called(fakeStore.clearAnnotations);
       assert.called(fakeApi.search);
     });
 
-    it('does not clear annotations if the query did not change', function () {
+    it('does not clear annotations if the query did not change', () => {
       fakeStore.routeParams.returns({ q: 'test query' });
-      createController();
+      const wrapper = createComponent();
       fakeApi.search.resetHistory();
       fakeStore.clearAnnotations.resetHistory();
 
-      fakeStore.subscribe.lastCall.callback();
+      fakeStore.routeParams.returns({ q: 'test query', other_param: 'foo' });
+      // Force update. `useStore` handles this in the real app.
+      wrapper.setProps({});
 
       assert.notCalled(fakeStore.clearAnnotations);
       assert.notCalled(fakeApi.search);

src/sidebar/index.js

@@ -129,6 +129,7 @@ import SearchStatusBar from './components/search-status-bar';
 import SelectionTabs from './components/selection-tabs';
 import ShareAnnotationsPanel from './components/share-annotations-panel';
 import SidebarContentError from './components/sidebar-content-error';
+import StreamContent from './components/stream-content';
 import SvgIcon from '../shared/components/svg-icon';
 import Thread from './components/thread';
 import ThreadList from './components/thread-list';
@@ -139,7 +140,6 @@ import TopBar from './components/top-bar';
 
 import hypothesisApp from './components/hypothesis-app';
 import sidebarContent from './components/sidebar-content';
-import streamContent from './components/stream-content';
 
 // Services.
 
@@ -271,7 +271,7 @@ function startAngularApp(config) {
     .component('sidebarContent', sidebarContent)
     .component('sidebarContentError', wrapComponent(SidebarContentError))
     .component('shareAnnotationsPanel', wrapComponent(ShareAnnotationsPanel))
-    .component('streamContent', streamContent)
+    .component('streamContent', wrapComponent(StreamContent))
     .component('svgIcon', wrapComponent(SvgIcon))
     .component('thread', wrapComponent(Thread))
     .component('threadList', wrapComponent(ThreadList))

src/sidebar/templates/stream-content.html

-<thread-list
-  on-change-collapsed="vm.setCollapsed(id, collapsed)"
-  show-document-info="true"
-  thread="vm.rootThread()">
-</thread-list>