ENH Better versioned history

client/lang/en.js

@@ -22,7 +22,7 @@ if (typeof(ss) === 'undefined' || typeof(ss.i18n) === 'undefined') {
     "LinkField.LINK_MODIFIED_TITLE": "Link has unpublished changes",
     "LinkField.LINK_MODIFIED_LABEL": "Modified",
     "LinkField.CANNOT_CREATE_LINK": "Cannot create link",
-    "LinkField.FAILED_TO_LOAD_LINKS": "Failed to load links",
+    "LinkField.FAILED_TO_LOAD_LINKS": "Failed to load link(s)",
     "LinkField.FAILED_TO_SAVE_LINK": "Failed to save link",
     "LinkField.SAVE_RECORD_FIRST": "Cannot add links until the record has been saved",
     "LinkField.SORT_SUCCESS": "Updated link sort order",

client/lang/src/en.json

@@ -15,7 +15,7 @@
     "LinkField.LINK_MODIFIED_TITLE": "Link has unpublished changes",
     "LinkField.LINK_MODIFIED_LABEL": "Modified",
     "LinkField.CANNOT_CREATE_LINK": "Cannot create link",
-    "LinkField.FAILED_TO_LOAD_LINKS": "Failed to load links",
+    "LinkField.FAILED_TO_LOAD_LINKS": "Failed to load link(s)",
     "LinkField.FAILED_TO_SAVE_LINK": "Failed to save link",
     "LinkField.SAVE_RECORD_FIRST": "Cannot add links until the record has been saved",
     "LinkField.SORT_SUCCESS": "Updated link sort order",

client/src/components/LinkField/LinkField.js

@@ -45,10 +45,11 @@ const section = 'SilverStripe\\LinkField\\Controllers\\LinkFieldController';
  * ownerID - ID of the owner DataObject
  * ownerClass - class name of the owner DataObject
  * ownerRelation - name of the relation on the owner DataObject
+ * inHistoryViewer - if the field is being viewed in the context of the history viewer
  */
 const LinkField = ({
   value = null,
-  onChange,
+  onChange = () => {},
   types = {},
   actions,
   isMulti = false,
@@ -59,6 +60,7 @@ const LinkField = ({
   ownerClass,
   ownerRelation,
   excludeLinkTextField = false,
+  inHistoryViewer,
 }) => {
   const [data, setData] = useState({});
   const [editingID, setEditingID] = useState(0);
@@ -67,6 +69,7 @@ const LinkField = ({
   const [focusOnNewLinkWhenClosed, setFocusOnNewLinkWhenClosed] = useState(false);
   const [focusOnNewLink, setFocusOnNewLink] = useState(false);
   const [loading, setLoading] = useState(false);
+  const [loadingError, setLoadingError] = useState(false);
   const [forceFetch, setForceFetch] = useState(0);
   const [isSorting, setIsSorting] = useState(false);
   const [linksClassName, setLinksClassName] = useState(classnames({'link-picker-links': true}));
@@ -156,16 +159,16 @@ const LinkField = ({
         .then(response => response.json())
         .then(responseJson => {
           setData(responseJson);
+        })
+        .catch(() => {
+          setLoadingError(true);
+        })
+        .finally(() => {
           setLoading(false);
           // isSorting is set to true on drag start and only set to false here to prevent
           // the loading indicator for flickering
           setIsSorting(false);
         })
-        .catch(() => {
-          actions.toasts.error(i18n._t('LinkField.FAILED_TO_LOAD_LINKS', 'Failed to load links'))
-          setLoading(false);
-          setIsSorting(false);
-        });
     }
   }, [editingID, value && value.length, forceFetch]);
 
@@ -441,14 +444,25 @@ const LinkField = ({
       });
   }
 
-  const saveRecordFirst = ownerID === 0;
-  const renderPicker = !saveRecordFirst && (isMulti || Object.keys(data).length === 0);
-  const renderModal = !saveRecordFirst && Boolean(editingID);
+  const saveRecordFirst = !loadingError && ownerID === 0;
+  const renderLoadingError = loadingError;
+  const renderPicker = !loadingError && !inHistoryViewer && !saveRecordFirst && (isMulti || Object.keys(data).length === 0);
+  const renderModal = !loadingError && !saveRecordFirst && Boolean(editingID);
+  const loadingErrorText = i18n._t('LinkField.FAILED_TO_LOAD_LINKS', 'Failed to load link(s)');
   const saveRecordFirstText = i18n._t('LinkField.SAVE_RECORD_FIRST', 'Cannot add links until the record has been saved');
   const links = renderLinks();
 
-  return <LinkFieldContext.Provider value={{ ownerID, ownerClass, ownerRelation, actions, loading, excludeLinkTextField }}>
+  return <LinkFieldContext.Provider value={{
+    ownerID,
+    ownerClass,
+    ownerRelation,
+    actions,
+    loading,
+    excludeLinkTextField,
+    inHistoryViewer
+  }}>
     <div className="link-field__container">
+      { renderLoadingError && <div className="link-field__loading-error">{loadingErrorText}</div> }
       { saveRecordFirst && <div className="link-field__save-record-first">{saveRecordFirstText}</div>}
       { loading && !isSorting && !saveRecordFirst && <Loading containerClass="link-field__loading"/> }
       { renderPicker && <LinkPicker
@@ -478,7 +492,7 @@ const LinkField = ({
 
 LinkField.propTypes = {
   value: PropTypes.oneOfType([PropTypes.arrayOf(PropTypes.number), PropTypes.number]),
-  onChange: PropTypes.func.isRequired,
+  onChange: PropTypes.func,
   types: PropTypes.object.isRequired,
   actions: PropTypes.object.isRequired,
   isMulti: PropTypes.bool,
@@ -489,6 +503,7 @@ LinkField.propTypes = {
   ownerClass: PropTypes.string.isRequired,
   ownerRelation: PropTypes.string.isRequired,
   excludeLinkTextField: PropTypes.bool,
+  inHistoryViewer: PropTypes.bool,
 };
 
 // redux actions loaded into props - used to get toast notifications

client/src/components/LinkField/LinkField.scss

@@ -17,7 +17,8 @@
   }
 }
 
-.link-field__save-record-first {
+.link-field__save-record-first,
+.link-field__loading_error {
   padding-top: 7px;
 }
 

client/src/components/LinkField/tests/LinkField-test.js

@@ -1,6 +1,6 @@
 /* global jest, test, expect, document */
 import React from 'react';
-import { render, screen, waitFor } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import '@testing-library/jest-dom';
 import { Component as LinkField } from '../LinkField';
@@ -50,6 +50,7 @@ function makeProps(obj = {}) {
     canCreate: true,
     readonly: false,
     disabled: false,
+    inHistoryViewer: false,
     ownerID: 123,
     ownerClass: 'Page',
     ownerRelation: 'MyRelation',
@@ -87,7 +88,7 @@ test('LinkField returns list of links if they exist', async () => {
 test('LinkField will render disabled state if disabled is true', async () => {
   const { container } = render(<LinkField {...makeProps({
     ownerID: 1,
-    disabled: true
+    disabled: true,
   })}
   />);
   doResolve();
@@ -99,10 +100,10 @@ test('LinkField will render disabled state if disabled is true', async () => {
 test('LinkField will render readonly state if readonly is true', async () => {
   const { container } = render(<LinkField {...makeProps({
     ownerID: 1,
-    readonly: true
+    readonly: true,
+    value: null,
   })}
   />);
-  doResolve();
   await screen.findByText('Cannot create link');
   expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
   expect(container.querySelectorAll('.link-picker')[0]).toHaveTextContent('Cannot create link');
@@ -180,15 +181,22 @@ test('LinkField will render loading indicator if ownerID is not 0', async () =>
   expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
 });
 
-test('LinkField will render link-picker if ownerID is not 0 and has finished loading', async () => {
+test('LinkField will render link-picker if ownerID is not 0 and isMulti and has finished loading', async () => {
   const { container } = render(<LinkField {...makeProps({
-    ownerID: 1
+    ownerID: 1,
+    isMulti: true,
   })}
   />);
-  doResolve();
-  await waitFor(() => {
-    expect(container.querySelectorAll('.link-field__save-record-first')).toHaveLength(0);
-    expect(container.querySelectorAll('.link-field__loading')).toHaveLength(0);
-    expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
-  }, { timeout: 100 });
+  await doResolve({ json: () => ({
+    123: {
+      Title: 'First title',
+      Sort: 1,
+      typeKey: 'mylink',
+    },
+  }) });
+  await screen.findByText('First title');
+
+  expect(container.querySelectorAll('.link-field__save-record-first')).toHaveLength(0);
+  expect(container.querySelectorAll('.link-field__loading')).toHaveLength(0);
+  expect(container.querySelectorAll('.link-picker')).toHaveLength(1);
 });

client/src/components/LinkModal/LinkModal.js

@@ -13,13 +13,16 @@ const buildSchemaUrl = (typeKey, linkID) => {
   const parsedURL = url.parse(schemaUrl);
   const parsedQs = qs.parse(parsedURL.query);
   parsedQs.typeKey = typeKey;
-  const { ownerID, ownerClass, ownerRelation, excludeLinkTextField } = useContext(LinkFieldContext);
+  const { ownerID, ownerClass, ownerRelation, excludeLinkTextField, inHistoryViewer } = useContext(LinkFieldContext);
   parsedQs.ownerID = ownerID;
   parsedQs.ownerClass = ownerClass;
   parsedQs.ownerRelation = ownerRelation;
   if (excludeLinkTextField) {
     parsedQs.excludeLinkTextField = true;
   }
+  if (inHistoryViewer) {
+    parsedQs.inHistoryViewer = '1';
+  }
   for (const prop of ['href', 'path', 'pathname']) {
     parsedURL[prop] = joinUrlPaths(parsedURL[prop], linkID.toString());
   }

client/src/containers/LinkModalContainer.js

@@ -23,6 +23,7 @@ const LinkModalContainer = ({ types, typeKey, linkID = 0, isOpen, onSuccess, onC
   // which will causes bugs with validation
   if (!LinkModal) {
     setLinkModal(() => loadComponent(`LinkModal.${handlerName}`));
+    return;
   }
 
   return <LinkModal

client/src/entwine/LinkField.js

@@ -58,6 +58,7 @@ jQuery.entwine('ss', ($) => {
         canCreate: inputField.data('can-create') ? true : false,
         readonly: inputField.data('readonly') ? true : false,
         disabled: inputField.data('disabled') ? true : false,
+        inHistoryViewer: inputField.data('in-history-viewer') ? true : false,
       };
     },
 

src/Controllers/LinkFieldController.php

@@ -372,7 +372,9 @@ private function createLinkForm(Link $link, string $operation, bool $excludeLink
         // Make readonly if fail can check
         if ($operation === 'create' && !$link->canCreate()
             || $operation === 'edit' && !$link->canEdit()
-            || $this->getFieldIsReadonlyOrDisabled()) {
+            || $this->getFieldIsReadonlyOrDisabled()
+            || $this->getRequest()->getVar('inHistoryViewer')
+        ) {
             $form->makeReadonly();
         }
 

src/Form/AbstractLinkField.php

@@ -13,6 +13,7 @@
 use SilverStripe\LinkField\Models\Link;
 use SilverStripe\LinkField\Services\LinkTypeService;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\VersionedAdmin\Controllers\HistoryViewerController;
 
 /**
  * Abstract form field for managing Link records
@@ -112,6 +113,7 @@ public function getSchemaDataDefaults(): array
         $data = parent::getSchemaDataDefaults();
         $data['types'] = json_decode($this->getTypesProp());
         $data['excludeLinkTextField'] = $this->getExcludeLinkTextField();
+        $data['inHistoryViewer'] = $this->getInHistoryViewer();
         $ownerFields = $this->getOwnerFields();
         $data['ownerID'] = $ownerFields['ID'];
         $data['ownerClass'] = $ownerFields['Class'];
@@ -136,13 +138,19 @@ protected function getDefaultAttributes(): array
         $attributes['data-readonly'] = $this->isReadonly();
         $attributes['data-disabled'] = $this->isDisabled();
         $attributes['data-exclude-linktext-field'] = $this->getExcludeLinkTextField();
+        $attributes['data-in-history-viewer'] = $this->getInHistoryViewer();
         $ownerFields = $this->getOwnerFields();
         $attributes['data-owner-id'] = $ownerFields['ID'];
         $attributes['data-owner-class'] = $ownerFields['Class'];
         $attributes['data-owner-relation'] = $ownerFields['Relation'];
         return $attributes;
     }
 
+    private function getInHistoryViewer(): bool
+    {
+        return is_a($this->getForm()->getController(), HistoryViewerController::class);
+    }
+
     protected function getOwner(): DataObject
     {
         /** @var Form $form */