ENH Better versioned history

client/lang/en.js

@@ -21,7 +21,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

@@ -14,7 +14,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

@@ -5,7 +5,6 @@ import { connect } from 'react-redux';
 import { DndContext, closestCenter, PointerSensor, useSensor, useSensors } from '@dnd-kit/core';
 import { arrayMove, SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable';
 import { restrictToVerticalAxis, restrictToParentElement } from '@dnd-kit/modifiers';
-import { injectGraphql } from 'lib/Injector';
 import fieldHolder from 'components/FieldHolder/FieldHolder';
 import LinkPicker from 'components/LinkPicker/LinkPicker';
 import LinkPickerTitle from 'components/LinkPicker/LinkPickerTitle';
@@ -38,11 +37,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,
-  onNonPublishedVersionedState,
+  onChange = () => {},
   types = {},
   actions,
   isMulti = false,
@@ -53,10 +52,12 @@ const LinkField = ({
   ownerClass,
   ownerRelation,
   excludeLinkTextField = false,
+  inHistoryViewer,
 }) => {
   const [data, setData] = useState({});
   const [editingID, setEditingID] = useState(0);
   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}));
@@ -94,16 +95,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]);
 
@@ -316,14 +317,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
@@ -350,7 +362,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,
@@ -361,6 +373,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 */
 import React from 'react';
-import { render, act } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 import { Component as LinkField } from '../LinkField';
 
 let doResolve;
@@ -28,7 +28,16 @@ function makeProps(obj = {}) {
   return {
     value: 123,
     onChange: () => {},
-    types: {},
+    types: {
+      mylink: {
+        key: 'mylink',
+        title: 'My Link',
+        handlerName: 'FormBuilderModal',
+        priority: 100,
+        icon: 'font-icon-link',
+        allowed: true
+      }
+    },
     actions: {
       toasts: {
         success: () => {},
@@ -39,6 +48,7 @@ function makeProps(obj = {}) {
     canCreate: true,
     readonly: false,
     disabled: false,
+    inHistoryViewer: false,
     ownerID: 123,
     ownerClass: 'Page',
     ownerRelation: 'MyRelation',
@@ -66,16 +76,21 @@ 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();
-  // Short wait - we can't use screen.find* because we're waiting for something to be removed, not added to the DOM
-  await act(async () => {
-    await new Promise((resolve) => setTimeout(resolve, 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

@@ -375,7 +375,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/LinkField.php

@@ -7,6 +7,7 @@
 use SilverStripe\LinkField\Form\Traits\AllowedLinkClassesTrait;
 use SilverStripe\LinkField\Form\Traits\LinkFieldGetOwnerTrait;
 use SilverStripe\Forms\HasOneRelationFieldInterface;
+use SilverStripe\VersionedAdmin\Controllers\HistoryViewerController;
 
 /**
  * Allows CMS users to edit a Link object.
@@ -52,6 +53,7 @@ protected function getDefaultAttributes(): array
         $attributes['data-owner-class'] = $ownerFields['Class'];
         $attributes['data-owner-relation'] = $ownerFields['Relation'];
         $attributes['data-exclude-linktext-field'] = $this->getExcludeLinkTextField();
+        $attributes['data-in-history-viewer'] = $this->getInHistoryViewer();
         return $attributes;
     }
 
@@ -64,9 +66,15 @@ public function getSchemaDataDefaults()
         $data['ownerClass'] = $ownerFields['Class'];
         $data['ownerRelation'] = $ownerFields['Relation'];
         $data['excludeLinkTextField'] = $this->getExcludeLinkTextField();
+        $data['inHistoryViewer'] = $this->getInHistoryViewer();
         return $data;
     }
 
+    private function getInHistoryViewer(): bool
+    {
+        return is_a($this->getForm()->getController(), HistoryViewerController::class);
+    }
+
     /**
      * Changes this field to the readonly field.
      */

src/Form/MultiLinkField.php

@@ -9,6 +9,7 @@
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\Relation;
 use SilverStripe\ORM\SS_List;
+use SilverStripe\VersionedAdmin\Controllers\HistoryViewerController;
 
 /**
  * Allows CMS users to edit a Link object.
@@ -47,6 +48,7 @@ public function getSchemaDataDefaults()
         $data['ownerClass'] = $ownerFields['Class'];
         $data['ownerRelation'] = $ownerFields['Relation'];
         $data['excludeLinkTextField'] = $this->getExcludeLinkTextField();
+        $data['inHistoryViewer'] = $this->getInHistoryViewer();
         return $data;
     }
 
@@ -72,9 +74,15 @@ protected function getDefaultAttributes(): array
         $attributes['data-owner-class'] = $ownerFields['Class'];
         $attributes['data-owner-relation'] = $ownerFields['Relation'];
         $attributes['data-exclude-linktext-field'] = $this->getExcludeLinkTextField();
+        $attributes['data-in-history-viewer'] = $this->getInHistoryViewer();
         return $attributes;
     }
 
+    private function getInHistoryViewer(): bool
+    {
+        return is_a($this->getForm()->getController(), HistoryViewerController::class);
+    }
+
     /**
      * Extracts the value of this field, normalised as a non-associative array.
      */