Link decorator fixes (#4742)

* Fix check when fieldValue is not an object The `in` operator is not available for all types, and can make the condition throw an error. * Move renderForDisplay method Fixing a linting error * Small styling changes Preparing for the changes to come * Introduce DecorationStats This will help us to check if a field was decorated without passing around functions. * Ensure LinkFieldDecorator is applied in message table Move logic decorating message to MessageTableEntry, and apply the same logic to the message details.
Graylog2 · Apr 20, 2018 · 3b2efc6 · 3b2efc6
1 parent fcf400e
commit 3b2efc6
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 54 deletions.
diff --git a/graylog2-web-interface/src/components/search/MessageField.jsx b/graylog2-web-interface/src/components/search/MessageField.jsx
@@ -4,6 +4,7 @@ import React from 'react';
 import createReactClass from 'create-react-class';
 
 import { MessageFieldDescription } from 'components/search';
+import DecorationStats from 'logic/message/DecorationStats';
 
 const MessageField = createReactClass({
   displayName: 'MessageField',
@@ -19,38 +20,24 @@ const MessageField = createReactClass({
 
   SPECIAL_FIELDS: ['full_message', 'level'],
 
-  _isAdded(key) {
-    const decorationStats = this.props.message.decoration_stats;
-    return decorationStats && decorationStats.added_fields && decorationStats.added_fields[key] !== undefined;
-  },
-
-  _isChanged(key) {
-    const decorationStats = this.props.message.decoration_stats;
-    return decorationStats && decorationStats.changed_fields && decorationStats.changed_fields[key] !== undefined;
-  },
-
-  _isDecorated(key) {
-    return this._isAdded(key) || this._isChanged(key);
-  },
-
   render() {
+    const message = this.props.message;
     let innerValue = this.props.value;
     const key = this.props.fieldName;
     if (this.SPECIAL_FIELDS.indexOf(key) !== -1) {
-      innerValue = this.props.message.fields[key];
+      innerValue = message.fields[key];
     }
 
     return (
       <span>
         <dt key={`${key}Title`}>{key}</dt>
         <MessageFieldDescription key={`${key}Description`}
-                                 message={this.props.message}
+                                 message={message}
                                  fieldName={key}
                                  fieldValue={innerValue}
                                  renderForDisplay={this.props.renderForDisplay}
-                                 disableFieldActions={this._isAdded(key) || this.props.disableFieldActions}
-                                 customFieldActions={this.props.customFieldActions}
-                                 isDecorated={this._isDecorated(key)} />
+                                 disableFieldActions={DecorationStats.isFieldAddedByDecorator(message, key) || this.props.disableFieldActions}
+                                 customFieldActions={this.props.customFieldActions} />
       </span>
     );
   },

diff --git a/graylog2-web-interface/src/components/search/MessageFieldDescription.jsx b/graylog2-web-interface/src/components/search/MessageFieldDescription.jsx
@@ -4,16 +4,16 @@ import { Alert } from 'react-bootstrap';
 import Immutable from 'immutable';
 
 import StoreProvider from 'injection/StoreProvider';
+import ActionsProvider from 'injection/ActionsProvider';
+import { DecoratedMessageFieldMarker } from 'components/search';
+import DecorationStats from 'logic/message/DecorationStats';
+import MessageFieldSearchActions from './MessageFieldSearchActions';
+
 const SearchStore = StoreProvider.getStore('Search');
 // eslint-disable-next-line no-unused-vars
 const MessagesStore = StoreProvider.getStore('Messages');
-
-import ActionsProvider from 'injection/ActionsProvider';
 const MessagesActions = ActionsProvider.getActions('Messages');
 
-import MessageFieldSearchActions from './MessageFieldSearchActions';
-
-import { DecoratedMessageFieldMarker } from 'components/search';
 
 class MessageFieldDescription extends React.Component {
   static propTypes = {
@@ -23,7 +23,6 @@ class MessageFieldDescription extends React.Component {
     renderForDisplay: PropTypes.func.isRequired,
     disableFieldActions: PropTypes.bool,
     customFieldActions: PropTypes.node,
-    isDecorated: PropTypes.bool,
   };
 
   state = {
@@ -50,14 +49,6 @@ class MessageFieldDescription extends React.Component {
     SearchStore.addSearchTerm(this.props.fieldName, this.props.fieldValue);
   };
 
-  _getFieldValue = () => {
-    if (this.props.isDecorated && ('type' in this.props.fieldValue) && (this.props.fieldValue.type === 'a')) {
-      const link = this.props.fieldValue.href;
-      return React.createElement('a', { href: link }, link);
-    }
-    return this.props.renderForDisplay(this.props.fieldName);
-  };
-
   _getFormattedTerms = () => {
     const termsMarkup = [];
     this.state.messageTerms.forEach((term, idx) => {
@@ -88,18 +79,20 @@ class MessageFieldDescription extends React.Component {
   };
 
   render() {
-    const className = this.props.fieldName === 'message' || this.props.fieldName === 'full_message' ? 'message-field' : '';
+    const fieldName = this.props.fieldName;
+    const className = fieldName === 'message' || fieldName === 'full_message' ? 'message-field' : '';
+    const isDecorated = DecorationStats.isFieldDecorated(this.props.message, fieldName);
 
     return (
-      <dd className={className} key={`${this.props.fieldName}dd`}>
+      <dd className={className} key={`${fieldName}dd`}>
         {this._getFormattedFieldActions()}
-        <div className="field-value">{this._getFieldValue()}</div>
+        <div className="field-value">{this.props.renderForDisplay(this.props.fieldName)}</div>
         {this._shouldShowTerms() &&
         <Alert bsStyle="info" onDismiss={() => this.setState({ messageTerms: Immutable.Map() })}>
           Field terms: &nbsp;{this._getFormattedTerms()}
         </Alert>
         }
-        {this.props.isDecorated && <DecoratedMessageFieldMarker />}
+        {isDecorated && <DecoratedMessageFieldMarker />}
       </dd>
     );
   }

diff --git a/graylog2-web-interface/src/components/search/MessageTableEntry.jsx b/graylog2-web-interface/src/components/search/MessageTableEntry.jsx
@@ -4,10 +4,11 @@ import Immutable from 'immutable';
 import ImmutablePropTypes from 'react-immutable-proptypes';
 import { Popover, OverlayTrigger } from 'react-bootstrap';
 
-import MessageDetail from './MessageDetail';
 import { Timestamp } from 'components/common';
 import StringUtils from 'util/StringUtils';
 import DateTime from 'logic/datetimes/DateTime';
+import DecorationStats from 'logic/message/DecorationStats';
+import MessageDetail from './MessageDetail';
 import style from './MessageTableEntry.css';
 
 class MessageTableEntry extends React.Component {
@@ -65,22 +66,6 @@ class MessageTableEntry extends React.Component {
     return false;
   }
 
-  renderForDisplay = (fieldName, truncate) => {
-    const fullOrigValue = this.props.message.fields[fieldName];
-
-    if (fullOrigValue === undefined) {
-      return '';
-    }
-
-    /* Timestamp can not be highlighted by elastic search. So we can safely
-     * skip them from highlighting. */
-    if (fieldName === 'timestamp') {
-      return this._toTimestamp(fullOrigValue);
-    } else {
-      return this.possiblyHighlight(fieldName, fullOrigValue, truncate);
-    }
-  };
-
   possiblyHighlight = (fieldName, fullOrigValue, truncate) => {
     // Ensure the field is a string for later processing
     const fullStringOrigValue = StringUtils.stringify(fullOrigValue);
@@ -137,6 +122,28 @@ class MessageTableEntry extends React.Component {
     );
   };
 
+  renderForDisplay = (fieldName, truncate) => {
+    const fullOrigValue = this.props.message.fields[fieldName];
+    const isDecorated = DecorationStats.isFieldDecorated(this.props.message, fieldName);
+
+    if (isDecorated && (typeof fullOrigValue === 'object') && (fullOrigValue.type === 'a')) {
+      const link = fullOrigValue.href;
+      return React.createElement('a', { href: link }, link);
+    }
+
+    if (fullOrigValue === undefined) {
+      return '';
+    }
+
+    /* Timestamp can not be highlighted by elastic search. So we can safely
+     * skip them from highlighting. */
+    if (fieldName === 'timestamp') {
+      return this._toTimestamp(fullOrigValue);
+    } else {
+      return this.possiblyHighlight(fieldName, fullOrigValue, truncate);
+    }
+  };
+
   render() {
     const colSpanFixup = this.props.selectedFields.size + 1;
 

diff --git a/graylog2-web-interface/src/logic/message/DecorationStats.js b/graylog2-web-interface/src/logic/message/DecorationStats.js
@@ -0,0 +1,17 @@
+const DecorationStats = {
+  isFieldAddedByDecorator(message, fieldName) {
+    const decorationStats = message.decoration_stats;
+    return decorationStats && decorationStats.added_fields && decorationStats.added_fields[fieldName] !== undefined;
+  },
+
+  isFieldChangedByDecorator(message, fieldName) {
+    const decorationStats = message.decoration_stats;
+    return decorationStats && decorationStats.changed_fields && decorationStats.changed_fields[fieldName] !== undefined;
+  },
+
+  isFieldDecorated(message, fieldName) {
+    return this.isFieldAddedByDecorator(message, fieldName) || this.isFieldChangedByDecorator(message, fieldName);
+  },
+};
+
+export default DecorationStats;