Browse Source

fix(discover) Don't include issue tags in events search bar (#16747)

When searching events we shouldn't show the default issue attributes
that act like tags. I would like to take these issue attributes out of
TagStore but that is a more invasive change that I don't have the
capacity to test right now.
Mark Story 5 years ago
parent
commit
647bffd1fb

+ 6 - 2
src/sentry/static/sentry/app/stores/tagStore.jsx

@@ -28,9 +28,9 @@ const TagStore = Reflux.createStore({
     this.reset();
   },
 
-  reset() {
+  getIssueAttributes() {
     // TODO(mitsuhiko): what do we do with translations here?
-    this.tags = {
+    return {
       is: {
         key: 'is',
         name: 'Status',
@@ -96,6 +96,10 @@ const TagStore = Reflux.createStore({
         predefined: true,
       },
     };
+  },
+
+  reset() {
+    this.tags = this.getIssueAttributes();
 
     this.trigger(this.tags);
   },

+ 28 - 3
src/sentry/static/sentry/app/utils/withTags.tsx

@@ -16,8 +16,33 @@ type State = {
   tags: TagCollection;
 };
 
+type Options = {
+  /**
+   * Set to true if you want to include issue attributes in the tag listt
+   * that is forwarded to the wrapped component.
+   */
+  includeIssueAttributes?: boolean;
+};
+
+const ISSUE_TAGS: TagCollection = TagStore.getIssueAttributes();
+
+function filterTags(tags: TagCollection, includeIssueAttributes: boolean): TagCollection {
+  if (includeIssueAttributes) {
+    return tags;
+  }
+  const out = Object.keys(tags).reduce((acc, name) => {
+    if (!ISSUE_TAGS.hasOwnProperty(name)) {
+      acc[name] = tags[name];
+    }
+
+    return acc;
+  }, {});
+  return out;
+}
+
 const withTags = <P extends InjectedTagsProps>(
-  WrappedComponent: React.ComponentType<P>
+  WrappedComponent: React.ComponentType<P>,
+  {includeIssueAttributes = false}: Options = {}
 ) =>
   createReactClass<Omit<P, keyof InjectedTagsProps>, State>({
     displayName: `withTags(${getDisplayName(WrappedComponent)})`,
@@ -25,12 +50,12 @@ const withTags = <P extends InjectedTagsProps>(
 
     getInitialState() {
       return {
-        tags: TagStore.getAllTags(),
+        tags: filterTags(TagStore.getAllTags(), includeIssueAttributes),
       };
     },
 
     onTagsUpdate(tags: TagCollection) {
-      this.setState({tags});
+      this.setState({tags: filterTags(tags, includeIssueAttributes)});
     },
 
     render() {

+ 25 - 0
tests/js/spec/utils/withTags.spec.jsx

@@ -18,8 +18,33 @@ describe('withTags HoC', function() {
     expect(wrapper.find('MyComponent').prop('other')).toEqual('value');
 
     TagStore.onLoadTagsSuccess([{name: 'Mechanism', key: 'mechanism', count: 1}]);
+    await wrapper.update();
+
+    // Should forward prop
+    expect(wrapper.find('MyComponent').prop('other')).toEqual('value');
+
     const tagsProp = wrapper.find('MyComponent').prop('tags');
+    // includes custom tags
     expect(tagsProp.mechanism).toBeTruthy();
+    // excludes issue tags by default
+    expect(tagsProp.is).toBeUndefined();
+  });
+
+  it('can include issue attributes', async function() {
+    const MyComponent = () => null;
+    const Container = withTags(MyComponent, {includeIssueAttributes: true});
+    const wrapper = mount(<Container other="value" />);
+
+    TagStore.onLoadTagsSuccess([{name: 'Mechanism', key: 'mechanism', count: 1}]);
+    await wrapper.update();
+
+    // Should forward props.
     expect(wrapper.find('MyComponent').prop('other')).toEqual('value');
+
+    const tagsProp = wrapper.find('MyComponent').prop('tags');
+    // includes custom tags
+    expect(tagsProp.mechanism).toBeTruthy();
+    // includes issue tags
+    expect(tagsProp.is).toBeTruthy();
   });
 });