Filter objects UI #489

peterbe · 2018-10-02T18:39:44Z

A more complete list of suggestions for input. Something better than this.
Unit test(s) for QueryRecipeFilters.js
Get rid of any leftover debugging included debugging commented out.
Do we have tests for the getRecipeFilters selector?
(unconfirmed but jotting down) If you try to create a new recipe using only filter objects, it might error saying the extra_filter_expression may not be null.

By the way, the file FilterObjectForm.js has 92% test coverage at the time of writing. The only (interesting) lines that aren't covered is because I don't know how to simulate clicking on options in Antd <Select/> components.

Screenshots

*Recipe details page when a bunch of filter objects have been set*

*Recipe details page when 0 filter objects have been set*

*Recipe editing when there are country, locale, versions, (no channels!), and one sampling filter object*

Note! In ^ this screenshot, I have many countries but it only counts as 1 filter object for all countries. The "Geo (2)" is 2 because there are >=1 country and >=1 locales.

Fixes mozilla#176

mythmon

I have some small issues with this, throughout and one bigger question. You seem to be going through a lot of effort and introducing a lot of complexity to convert to and from the representation used on the server. Why not use that representation directly? Working in the native format seems a lot easier to me. Even if you keep the transformations, the special casing for bucket and stable sampling seems very fragile. It would be nice to have a more unified data flow here.

If the format used by the server is really a burden for implementing UIs with, then we should explore how to make it better. If it is hard for us, it's going to be even harder for anyone else trying to use this API.

mythmon · 2018-10-02T21:32:05Z

src/workflows/recipes/components/FilterObjectForm.js

+    },
+    // XXX THIS LIST IS BASED ON SCRAPING https://normandy.cdn.mozilla.net/api/v1/recipe/
+    // AND IT'S VERY DIFFERENT FROM THE AVAILABLE OPTIONS HERE:
+    // https://normandy.readthedocs.io/en/latest/user/filters.html#filter-context


The goal of this UI should not be making every possible option available, but instead it should be to make the common things easy. Most of the options available in the docs are massively over complicated, and have literally never been used as inputs to sampling. It does not make logical sense to filter on most of the available options.

For example, it does something to use normandy.isDefaultBrowser or normandy.searchEngine in input to a sampling function, but it doesn't do anything sensical.

The list you have here is only lacking in constant strings and constant numbers. None of the other options in the context need to be available, and wouldn't really make sense.

mythmon · 2018-10-02T21:34:27Z

src/workflows/recipes/components/FilterObjectForm.js

+    defaultInputOptions: [
+      'normandy.clientId',
+      'normandy.recipe.id',
+      'normandy.request_time',


This should not be one of the quick choices. Since it varies constantly (by definition), it isn't useful as a sampling input, except when used in complicated expressions.

So, can you bless a good default array and then I'll remove all these XXX comments and just leave it as that.

These three?

normandy.clientId

normandy.recipe.id

normandy.userId

@mythmon Can you respond to this question ^

normandy.clientId doesn't exist, according to the docs. Besides the constant strings and numbers, normandy.userId and normandy.recipe.id are the only options that should be here.

mythmon · 2018-10-02T21:41:14Z

src/setupTests.js

 import Adapter from 'enzyme-adapter-react-16';
 import * as immutableMatchers from 'jest-immutable-matchers';
 import fetchMock from 'fetch-mock';
-
+import { Headers } from 'whatwg-fetch';


It would probably be good to leave the space between imports and the rest of the code that used to be here.

mythmon · 2018-10-02T21:41:55Z

src/setupTests.js

@@ -1,10 +1,11 @@
 import React from 'react';
 import ReactDOM from 'react-dom';
 import { configure, mount, shallow } from 'enzyme';
+import { cleanup } from 'react-testing-library';


It isn't clear below what cleanup refers to. Can this be import * as reactTestingLibrary from 'react-testing-library instead?

Because JavaScript.
Look at the line above. It's called configure :)

I don't like import * as ... from '...' because when tree shaking and partial imports can be used that's an anti-pattern.

Also, I've seen a lot of code where they accept this mess and leave it as import { commonword } from 'javascript-lacks-namespaces'

Incidentally and whimsically, how about is :)

Consistency is good, but repeating mistakes for the sake of consistency isn't useful. We can improve over time. Just because there are bad examples already in the code doesn't mean we can't do better going forward.

mythmon · 2018-10-02T22:01:53Z

src/tests/workflows/recipes/components/FilterObjectForm.test.js

+import { wrapMockStore } from 'console/tests/mockStore';
+
+describe('<FilterObjectForm>', () => {
+  it('render editing form with an empty existing filter object', () => {


The testing style used throughout the rest of the code reads the whole line as a sentence, like "It render editing form...", notably including the "it" at the beginning. "It should..." is a common phrasing. The use of the tense "render" here makes that sound really awkward.

I've now rewritten them all to read more pleasantly. (not pushed yet)

mythmon · 2018-10-02T22:31:44Z

src/workflows/recipes/components/FilterObjectForm.js

+          if (name === undefined) {
+            throw new Error(`Key '${key}' no in known mapping.`);
+          }
+          return { [key]: value, type: name };


The usage here suggests that reverseMapping is semantically a mapping, so it should probably be a stdlib Map or Immutable.Map.

Yeah, mayhaps. This is kinda annoying. The purpose of this is data here is that it gets converted to a JSON string and sent as part of the HTTP request with fetch. I don't see a big win in not keeping it simple.

mythmon · 2018-10-02T22:32:07Z

src/workflows/recipes/components/FilterObjectForm.js

+  static propTypes = {
+    allChannels: PropTypes.instanceOf(List),
+    allCountries: PropTypes.instanceOf(List),
+    allLocales: PropTypes.instanceOf(List),


👍 to calling this allLocales, etc.

mythmon · 2018-10-02T22:33:43Z

src/workflows/recipes/components/FilterObjectForm.js

+    window.sessionStorage.setItem('filterObjectActiveTabKey', key);
+  };
+
+  getDefaultActiveTabKey = (default_ = 'geo') => {


Geo is likely the least used type of filter. Browser or sampling would would be a much better default.

I can easily rearrange the order. But I think the left-most one should be visible by default.

So, can you confirm this preferred order?:

Browser

Sampling

Geo

mythmon · 2018-10-02T22:39:21Z

src/workflows/recipes/components/FilterObjectForm.js

+          onClick={this.onSubmitSamplingInput}
+          // Needed to be able to find button DOM in testing.
+          // https://github.com/kentcdodds/dom-testing-library/issues/108
+          // data-testid="add-button"


Is this needed or not?

Not needed. I went for using CSS selectors even though Kent recommended against it. What he doesn't know is that Antd is not your grandfathers React component library.

mythmon · 2018-10-02T22:43:35Z

src/workflows/recipes/components/RecipeForm.js

+    // Error thrown here will be automatically turned into a notification message in the UI.
+    throw new Error('Have you have at least one filter object or a filter expression.');
+  }
+  // console.log('DATA.filter_object', JSON.stringify(data.filter_object));


peterbe · 2018-10-03T14:15:53Z

I have some small issues with this, throughout and one bigger question. You seem to be going through a lot of effort and introducing a lot of complexity to convert to and from the representation used on the server. Why not use that representation directly? Working in the native format seems a lot easier to me.

What do you mean by "Working in the native format"?
I have the <FormItem/> which expects an array (with the exception of sampling) as the initial value and when whatever component is wraps, it sends back an array and when you press the "Save changes" button you get those arrays, keyed by the name prop to <FormItem/>.

I actually don't mind this pattern. I do agree that it feels like a lot of work and it would be nice if it was easier. Visual UIs often are different from the deep underlying data structures that need to exist so it's sensible to go through a serializer and deserializer when mapping between database data an form input widgets and back.

Even if you keep the transformations, the special casing for bucket and stable sampling seems very fragile. It would be nice to have a more unified data flow here.

Yeah, it sucks to have to have an exception. I picked the magic keyword _sampling out of thin air. However, suppose if it was like this instead:

[{"type": "sampling", "subtype": "stable", "rate": 0.33, "input": ["normandy.foo"]}]

...it would make things slightly less fragile.

If the format used by the server is really a burden for implementing UIs with, then we should explore how to make it better. If it is hard for us, it's going to be even harder for anyone else trying to use this API.

I never really considered pushing back on the format because I assumed you A) had put in a lot of thought into it and B) it's already used.

Why can't the recipe.filter_object look like this for example...:

{'countries': ['SE'], 'sampling': {'type': 'bucket', 'start': 50, 'count': 70, 'total': 100, 'input': ['normandy.foo']}

It would make the serializing/deserializing to turn that into <FormItem/> components a LOT easier.
The time to make that pivot is now. We'd have to write a pretty hefty django migration but it shouldn't be too complex since it's easy to unit test for.

Last but not least, writing the serializers wasn't particularly hard. It might look like a lot but it's shadowed by the messy jumping back and forth between native JavaScript arrays/objects to Immutable Map/List.

peterbe · 2018-10-03T14:21:33Z

One option would be to do something like this instead of serializing the array to an object first...:

let initialValueLocales = []
if (this.props.filterObject.find(o => o.type==='locale')) {
  initialValueLocales = this.props.filterObject.find(o => o.type==='locale').map(obj => obj.locales)
}
...
<FormItem
  label="Locale"
  name="filter_object.locales"
  required={false}
  rules={[{ required: false }]}
  initialValue={initialValueLocales}
>
  <Select
    disabled={disabled}
    mode="multiple"
    allowClear
    showArrow={false}
    filterOption={this.filterOption}
  >
  <Select>...
  </Select>
</FormItem>

But when saving the form, I would still need to deserialize an object into an array of these filter object data structures.

mythmon · 2018-10-03T19:45:28Z

The reason for the shape the data is in currently (an array of filter objects) is two fold.

First, the UI I was envisioning at the time was rather different. Instead of a fixed set of filters that were always visible, I was expecting it would start with no filters, and then users could add or remove individual filters (such as browser or channel). These would then translate very directly to and from the data format in the API currently. To me, this makes more sense than the current UI.

The current UI make me think that all filters are active, even if they aren't. It makes me think that if I don't select a locale or a country, then the recipe would apply to no-one. It makes me think that I must select a channel and that I must select a version. None of these should be true. All the filters should be optional, and that should be very clear in the UI. Besides my issues about data processing, I am very concerned that this UI is not the right abstraction for users.

The second reason I chose the data format that is used is because it is reasonable to have two filters objects with the same type. It doesn't make a lot of sense for country or locale, or really for any of the locales we have here, but there are other filter object types I'd like to add. For example, a semi-common thing in the current filter expressions is filtering by preferences the user has set. It is also reasonable to want to check two of these preference at once. We could make a complicate filter object that can deal with many different combinations of filters, or we could do something simple:

[
  {"type": "preferenceEquals", "preferenceName": "app.feature.enable", "value": true},
  {"type": "preferenceEquals", "preferenceName": "app.feature.setUp", "value": false}
]

This type of thing would be hard to represent compactly in a key/value mapping. Another idea that I pondered is nested filter objects. I'm not sure this is a good idea or not, but I'd like to leave the option for it in the future. Something

[
  {"type": "orGroup", "filters": [
    {"type": "country", "countries": ["US"]},
    {"type": "locale", "locales": ["en-US", "en-CA", "en-GB", "en-ZA"]}
  ]}
]

An orGroup, if we ever made it, should definitely be allowed to exist multiple times. Please note: This is not something I definitely want to do. I want to give this a lot more thought and discussion before going forward with it. But it represents well the ways I'd like filter objects to be flexible.

I also have a minor bias against APIs that change their type, and having to do "if key exists on object" checks makes me sad. I've been writing too much Rust. This is a personal bias though, and isn't really applicable to JS and Python.

All that said, if this API schema isn't reasonable to work with, it won't achieve its goals. One of the goals of filter objects is to make a filtering system that is flexible and machine readable. I want people that make dashboards and automation tools and data analysis to be able to consume filter objects and make programs that can understand them. If that isn't the case, we need to rethink this.

I did put a lot of thought into this, but that doesn't mean it is the best solution. APIs are inherently social and involve many people, and we need many ideas to get them right.

We won't have to worry about doing any migrations. No one is writing to the Normandy API except Delivery Console, so we can be sure there aren't any filter objects in place that need migrated.

To answer one of your questions directly, by "native format" I mean in the format used by the server. That wasn't a very clear term to use, sorry.

peterbe · 2018-10-03T20:24:11Z

I blame Rehan for making the wireframes. :)

Did you see the feature I put in that each tab gets a number of things set? The little numbers in brackets. This wasn't in the wireframes but it's something I made to solve the discoverability of seeing that you have some filter objects set.
One thing we could do is display it as "Geo (0)" if neither Country or Locale has any. Do you think that would help the UI? Or, is that putting a band-aid on bigger problem.

Your underlying motive and gut-feelings about the UI and that example about "makes me think that if I don't select a locale or a country, then the recipe would apply to no-one" is real. I'm not sure what to do about it. The, perhaps obvious, alternative is that user first has to click to "I want to enable filter by Country" and once clicked it shows a Country input widget.

The example you mentioned with two filter objects of (repeated) type preferenceEquals is a real challenge. However, we have the power to build that. We can build a widget where you type in preferenceName and value and click a big plus icon and they get listed below (each with a little remove icon) like we're currently doing with the {"type": "version", ...} filter object.

mythmon · 2018-10-03T21:18:23Z

Did you see the feature I put in that each tab gets a number of things set? The little numbers in brackets. This wasn't in the wireframes but it's something I made to solve the discoverability of seeing that you have some filter objects set.
One thing we could do is display it as "Geo (0)" if neither Country or Locale has any. Do you think that would help the UI? Or, is that putting a band-aid on bigger problem.

I like the numbers, and if we keep this tabbed model, I think we should definitely add the 0 marker. That helps, but still leaves some uncertainty IMO.

Here's a quick sketch of the kind of UI I was expecting: rendered (code)

It's ugly and the code is a bit rough, but it gets the idea by pretty well, I think. I like that in this model, things that don't affect the filtering aren't shown, and things that do are shown explicitly. It also maps directly to the server's idea of filters. One thing that I think we would want to do is prevent a user from adding multiple filters that don't make sense to duplicate (probably all of the current ones).

I'd also probably replace the "Add X filter" with a dropdown or something. The core part is that you explicitly add in types of filters that apply.

peterbe · 2018-10-04T12:19:43Z

Here's a quick sketch of the kind of UI I was expecting: rendered (code)

I'm going to have to defer this one to @rehandalal . If you can "convince" him, I'm happy to go back and do a refactoring. Either way, we should allow him time to come back from PTO and absorb all of this.

I'll hold off on making more changes to the current code until then. I.e. adding " (0)" marker and moving the "Geo" tab after "Browser".

peterbe · 2018-10-05T15:14:48Z

src/setupTests.js

 global.mount = mount;
 global.shallow = shallow;
 global.React = React;
 global.ReactDOM = ReactDOM;
+global.Headers = Headers;


If you don't do this you get unhandled errors in api.js. In Jest they appear as warnings like this:

(node:646) UnhandledPromiseRejectionWarning: ReferenceError: Headers is not defined (node:646) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 4) (node:646) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. (node:646) UnhandledPromiseRejectionWarning: ReferenceError: Headers is not defined (node:646) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 8)

You can hopefully see an example of that here: https://circleci.com/gh/mozilla/delivery-console/3515

It saddens me that the tests don't fail on this blatant error (that global Headers obviously doesn't exist in NodeJS).

Also, when using globals, should one use window. instead? E.g. window.localStorage instead of localStorage?
You know, to avoid this happening again.

Or rather, to be more explicit.

peterbe · 2018-10-12T20:23:31Z

We chatted, at length about this and concluded that we can't decide.
To move this forward, we've decide to make 2 more branches of the UI for how decide which one is best.

Tabbed (current branch)
No tabs, all filter object choices displayed (even if empty). Similar to Google Advanced search
A drop-down thing that allows you to select a type (e.g. "Country") which adds an input widget. Similar to Socorro SuperSearch (click "New Line" button).

I'm going to build these branches, scrappily, take screenshots and share those screenshots for the sake of getting many other peoples opinions.

mythmon · 2018-10-15T20:20:08Z

I demoed this for Rob Rayborn, one of our power users. He was generally very excited about the feature, and thought it would cover many recipes' use cases. He wasn't confused by the problems I thought he might have, but did have a few issues I didn't forsee. I'd like to retract my previous concerns about usability, and I'm ok to land this with the tabbed UI, with the below problems dealt with:

First, some improvements that could be made. These can probably all wait for follow ups.

Rob said that sampling is the most used and most important thing, and should therefore be the first tab.
The range shortcut for versions is probably more work than its worth. It isn't clear that it is only a UI shortcut to adding many versions. I think it would be better overall to only have the specific version set, and make it work the same as the country and locale filter
The "input" fields for stable and bucket sample allowed values to be added twice. Although technically valid, this isn't a very useful UI. It would be nice but not needed to not allow duplicates.
The channels should be sorted (esr, release, beta, developer edition, nightly). Right now they seem random.

There were also some bugs. I think these need fixed before we land this.

Server validation, such as "this field is reqiured" when you leave out a field in the bucket sample form, doesn't get surfaced to the user at all, instead just returning a generic error "Recipe cannot be updated. APIError: Bad Request".
The "start" field of bucket sampling doesn't allow 0 as a value, which is valid value. I think there is probably a nonstrict falsey check going on somewhere?
The input fields for bucket and stable sample seemed a bit weird. Once we added one of the preset options, and it seemed to be added twice. Then later we tried removing all, and one of those that was doubled couldn't be removed. The one that couldn't be removed didn't get sent to the server either.
We had a few geo and locale filters, but the number in the tab still read 0.
The extra filter expression field was required by the server. I think this is Normandy server bug though.
Pressing tab when the last field in the browser tab is focused causes the whole tab's UI to scroll out of view. Shift-tab brings it partially back.

rehandalal

I think this looks fine for the most part. I'll do a styling pass on this once it lands to clean it up a bit visually. There are also a bunch of "XXX" comments that should be cleaned up.

rehandalal · 2018-10-15T20:21:37Z

src/workflows/recipes/components/FilterObjectForm.js

+      {"ages": [1,2,3]}
+
+  The list of valid keys needs to be known in advance. */
+export const serializeFilterObjectToMap = list => {


nit: i have a slight preference for a regular function.

rehandalal · 2018-10-15T20:22:55Z

src/workflows/recipes/components/FilterObjectForm.js

+import QueryRecipeFilters from 'console/components/data/QueryRecipeFilters';
+import FormItem from 'console/components/forms/FormItem';
+
+// XXX Why is this needed?!?


this is not "needed". im pretty sure you can also just use <Tabs.TabPane ...

peterbe · 2018-10-17T20:47:20Z

@mythmon ...

We had a few geo and locale filters, but the number in the tab still read 0.

I can't reproduce this. Not sure if it got fixed by one of the other fixes or something. I just can't get it to happen again. But I have seen it beforetimes but didn't think much about it because so many other things were in flux.

Pressing tab when the last field in the browser tab is focused causes the whole tab's UI to scroll out of view. Shift-tab brings it partially back.

Uh?? The "last field in the browser tab" is that the "Version" input. For me, when that input if focused and I press Tab it just moves the focus down to the textarea field below. As expected I would say.

rehandalal · 2018-10-17T20:51:01Z

@peterbe thanks for tackling these. in the interest of moving this along i'm okay with reviewing as-is, landing and improving. also to keep this PR from getting out of hand. we can file follow up issues.

peterbe · 2018-10-17T20:56:31Z

@rehandalal Apart from two points that @mythmon pointed out I can't think of anything else to work on right now. Perhaps you can help reproduce those mentioned bugs locally.

Also, when I run the unit tests I get a cryptic proptypes warning:

    console.error node_modules/react/node_modules/prop-types/checkPropTypes.js:19
      Warning: Failed prop type: The prop `children` is marked as required in `FormItem`, but its value is `undefined`.
          in FormItem (at forms.js:172)
          in Wrapper (at FilterObjectForm.js:323)
          in div (created by TabPane)
          in TabPane (at FilterObjectForm.js:322)
          in div (created by TabContent)
          in TabContent (created by Tabs)
          in div (created by Tabs)
          in Tabs (created by Tabs)
          in Tabs (at FilterObjectForm.js:318)
          in div (at FilterObjectForm.js:315)
          in FilterObjectForm (at forms.js:137)
          in WrappedForm (created by Form(WrappedForm))
          in Form(WrappedForm) (at FilterObjectForm.test.js:53)
          in Router (created by ConnectedRouter)
          in ConnectedRouter (created by Connect(ConnectedRouter))
          in Connect(ConnectedRouter) (at mockStore.js:27)
          in Provider (at mockStore.js:26)

I'm mixed-bag about fixing that. First of all, I spent some time but couldn't solve it. Perhaps the problem will just magically go away when we refactor this post-landing. Or, perhaps a second pair of eyes on it might spot it. I'm personally a bit drained on this work.

mythmon · 2018-10-17T21:10:20Z

Pressing tab when the last field in the browser tab is focused causes the whole tab's UI to scroll out of view. Shift-tab brings it partially back.

Uh?? The "last field in the browser tab" is that the "Version" input. For me, when that input if focused and I press Tab it just moves the focus down to the textarea field below. As expected I would say.

This happened for me when I had two version inputs on the screen, for a version range. Maybe you fixed it when you removed the ability to include a range?

rehandalal · 2018-10-17T22:29:37Z

src/state/recipes/actions.js

+    }
+    // XXX If we already had the filters in localStorage, can we avoid using this
+    // makeNormandyApiRequest() with an ID so that it doesn't cause any spinners to appear
+    // when the localStorage version almost definitely is good enough.


nit: can we address this?

Check this out. In practice it works.
Basically, if there was data in localStorage it makes the next XHR request without setting {[requestId]: {inProgress:true}}.

See this:

I put in a time.sleep(10) in the Filters(views.APIView).get method. Note how all the fields are not disabled whilst waiting for this request.

Then I delete the stuff from localStorage and refreshed. This time, all the fields are disabled whilst waiting.

mythmon · 2018-10-18T16:38:36Z

src/state/network/actions.js

-      type: REQUEST_SEND,
-      requestId,
-    });
+    !stealth &&


Can we write this as if (!stealth) { ... } instead? && version is useful when you need an expression, but we don't need an expression here.

Also, do we need/want to disable the REQUEST_SUCCESS/REQUEST_FAILURE? events when stealth is true as well?

rehandalal

phew! thanks for sticking with this!!

bors r+

489: Filter objects UI r=rehandalal a=peterbe - [x] A more complete list of suggestions for `input`. Something better than [this](https://github.com/mozilla/delivery-console/pull/489/files#diff-afaeb5a3b97ef82d4c08accda9fd72acR702). - [x] Unit test(s) for `QueryRecipeFilters.js` - [x] Get rid of any leftover debugging included debugging commented out. - [x] Do we have tests for the `getRecipeFilters` selector? - [x] (unconfirmed but jotting down) If you try to create a new recipe using only filter objects, it might error saying the `extra_filter_expression` may not be null. By the way, the file `FilterObjectForm.js` has 92% test coverage at the time of writing. The only (interesting) lines that aren't covered is because I don't know how to simulate clicking on options in Antd `<Select/>` components. ### Screenshots <img width="796" alt="screen shot 2018-10-03 at 4 09 26 pm" src="https://user-images.githubusercontent.com/26739/46436435-cbff7000-c726-11e8-9354-5de06e3f7d90.png"> *Recipe details page when a bunch of filter objects have been set* <img width="790" alt="screen shot 2018-10-03 at 4 10 41 pm" src="https://user-images.githubusercontent.com/26739/46436484-e6d1e480-c726-11e8-8e16-202b7c26ae7f.png"> *Recipe details page when 0 filter objects have been set* <img width="1043" alt="screen shot 2018-10-03 at 4 13 04 pm" src="https://user-images.githubusercontent.com/26739/46436600-3fa17d00-c727-11e8-9a7f-1e0f559a0e28.png"> *Recipe editing when there are country, locale, versions, (no channels!), and one sampling filter object* Note! In ^ this screenshot, I have many countries but it only counts as 1 filter object for *all* countries. The "Geo (2)" is 2 because there are >=1 country and >=1 locales. Co-authored-by: Peter Bengtsson <mail@peterbe.com>

bors · 2018-10-18T18:26:30Z

Build succeeded

peterbe added 2 commits October 2, 2018 14:27

Add UI for Filter objects

47f5187

Fixes mozilla#176

addin whatwg-fetch for ability to polyfill window.Headers

7352326

peterbe requested a review from rehandalal October 2, 2018 18:39

mythmon suggested changes Oct 2, 2018

View reviewed changes

peterbe added 3 commits October 3, 2018 10:49

feedbacked

0328c4e

tests for QueryRecipeFilters

9c86658

started actions.test.js

87abd55

peterbe commented Oct 5, 2018

View reviewed changes

peterbe added 2 commits October 15, 2018 15:36

change order of tabs

a41166d

and tests

615c8a8

rehandalal suggested changes Oct 15, 2018

View reviewed changes

peterbe added 11 commits October 16, 2018 12:49

Merge branch 'master' into filter-objects-with-react-testing-library

f94f73c

update yarn integrities

d51c9cd

'Sampling' tab first

5398297

one version at a time

81b5801

channels displayed alphabetically by label

daa56c7

leftover console log

3ac4de5

fix validation error rendering

6cbdf96

ability to select exactly '0'

7765403

inputs now require clicking the plus button

5589096

the right defaults for 'defaultInputOptions'

43f4cdd

ability to leave extra_filter_expression blank

a02ea37

mythmon approved these changes Oct 17, 2018

View reviewed changes

rehandalal suggested changes Oct 17, 2018

View reviewed changes

peterbe added 4 commits October 18, 2018 10:31

stealth api requests

4fe13f9

getting rid of component shortchuts

e787283

remove XXX comment on checkSampling validation function

0dc0f2d

remove last XXX comment

7f41d96

mythmon reviewed Oct 18, 2018

View reviewed changes

feedbacked

3425188

rehandalal approved these changes Oct 18, 2018

View reviewed changes

bors bot merged commit 3425188 into mozilla:master Oct 18, 2018

peterbe deleted the filter-objects-with-react-testing-library branch October 18, 2018 19:45

Filter objects UI #489

Filter objects UI #489

Conversation

peterbe commented Oct 2, 2018 • edited Loading

Screenshots

mythmon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbe commented Oct 3, 2018

peterbe commented Oct 3, 2018

mythmon commented Oct 3, 2018

peterbe commented Oct 3, 2018

mythmon commented Oct 3, 2018

peterbe commented Oct 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbe commented Oct 12, 2018

mythmon commented Oct 15, 2018 • edited by peterbe Loading

rehandalal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterbe commented Oct 17, 2018

rehandalal commented Oct 17, 2018

peterbe commented Oct 17, 2018

mythmon commented Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mythmon Oct 18, 2018 • edited Loading

Choose a reason for hiding this comment

rehandalal left a comment

Choose a reason for hiding this comment

bors bot commented Oct 18, 2018

Build succeeded

peterbe commented Oct 2, 2018 •

edited

Loading

mythmon commented Oct 15, 2018 •

edited by peterbe

Loading

mythmon commented Oct 17, 2018 •

edited

Loading

mythmon Oct 18, 2018 •

edited

Loading