Implement request filter pipeline #819

daltonmatos · 2017-12-08T13:00:07Z

Here is a implementation candidate for a "filters pipeline" feature. This will enrich plugin even more, as long as the core Marathon UI adds some extension points where it calls the filter pipeline.

Let me know what do you think about this implementation, so I can adjust the code to be able to eventually merge.

Things I didn't like the way it's done, but don't know how to do it differently.

The constants that define the name of the filters: Don't know if there'a a way to continue to use PipelineStore.PRE_AJAX_REQUEST without the need of the constructor.
The clearPipeline() method is only used in the tests. I could not clean the pipeline before each tests without this method.
Ideally the object that holds all pipelines (here FILTERS_PIPELINE) should be visible only to the PipelineStore module, and not be an attribute of the PipelineStore class, also don't know how to achieve this.

Thanks a lot!

daltonmatos · 2017-12-11T18:27:25Z

Hello @orlandohohmeier,

I will adjust some things that I learned how to do (like change module.exports to export const) and will update the commits. Then you can take a look and start the review.

The overall idea is still the same, however.

Thanks,

daltonmatos · 2017-12-13T16:07:05Z

Hello @orlandohohmeier,

the implementation is a little better now and is ready for you to start reviewing.

Thanks,

daltonmatos · 2017-12-14T10:31:46Z

Hello @orlandohohmeier,

made some changes and the implementation is getting better. If you could take a look.

About the new instance of the Pipeline passed to every loaded plugin: If you think this is a problem, we can extract this instance to a variable and pass the same Pipeline object to every plugin.

Also, if you think this is better we can extract all hook names to a separated file, for example: plugin/sdk/stores/hooks.js.

Let me know what you think.

Thanks,

orlandohohmeier · 2017-12-19T10:14:56Z

src/js/plugin/sdk/stores/PipelineStore.js

+export const FILTERS_PIPELINE = {};
+const PRE_AJAX_REQUEST = "PRE_AJAX_REQUEST";
+
+export const Hooks = {


I'd move these const to a dedicated filter const file.

Sure! Will do it.

orlandohohmeier · 2017-12-19T10:20:57Z

src/js/plugin/sdk/stores/PipelineStore.js

@@ -0,0 +1,44 @@
+
+export const FILTERS_PIPELINE = {};


Please don't export FILTERS_PIPELINE as you otherwise risk that other components or plugins accidentally tamper with the internal state which could lead to bugs. I'd move the map into a closure make sure that only the pipeline has access.

Sure! The idea is to let only the PipelineStore modify the internal state of the pipelines. Currently exporting this variable makes it possible to other components of the core UI do modify it (that's what I'm currently doing in the test cases) but this is not true for plugins, this is because plugins cannot import the module directly, all that plugins see are the objects exported to its pluginWindow.marathonPluginInterface, so no plugin could ever reach the original exported FILTERS_PIPELINE, right? Does this make sense?

orlandohohmeier · 2017-12-19T11:15:10Z

Hi @daltonmatos,

Please excuse the late response. I've been on the road for the last two weeks and wasn't able to review your code.

I like the idea of a filter/pipeline mechanism to enrich the plugin system and I'm happy to help move this one forward. We might want to think about ways to ensure that the input and output types are consistent as I've seen such mechanism fail many times due to the lack of type safety and the related set of errors.

Making PipelineStore instantiable was a great idea and is the way to go to circumvent the need for a clearPipeline method. Using the plugin interface to share a new instance of the pipeline store is fine— think of it as an IOC container.

I'd refactor the pipeline store as follows to address the open issues you've initial mentioned and ensure data integrity.

export default class PipelineStore {

  constructor() {
    /**
     * Private Context
     *
     * @typedef {Object} PipelineStore~Context
     * @property {PipelineStore} instance - Current pipeline store instance
     * @property {Map} pipelines
     */
    const context = {
      instance: this,
      pipelines: new Map()
    };

    this.applyPipeline = this.applyPipeline.bind(context);
    this.registerOperator = this.registerOperator.bind(context);
    this.unregisterOperator = this.unregisterOperator.bind(context);

  }

  /**
   * Apply pipeline
   *
   * @this PipelineStore~Context
   * @param {Symbol|String} id - Pipeline identifier
   * @param {*} input
   * @return {*} output
   */
  applyPipeline(id, input) {
    if (!this.pipelines.has(id)) {
      return input;
    }

    return this.pipelines.get(id)
      .reduce((accumulator, operator) => {
        try {
          return operator(accumulator);
        } catch (error) {
          // We might want to log a proper error to help users debug  
          return accumulator;
        }
      }, input);

  };

  /**
   * @this PipelineStore~Context
   * @param {Symbol|String} id
   * @param {function} operator
   */
  registerOperator(id, operator) {
    if (!this.pipelines.has(id)) {
      this.pipelines.set(id, []);
    }

    const pipeline = this.pipelines.get(id);

    if (!pipeline.includes(operator)) {
      pipeline.push(operator);
    }
  };

  /**
   * @this PipelineStore~Context
   * @param {Symbol|String} id
   * @param {function} operator
   */
  unregisterOperator(id, operator) {
    if (!this.pipelines.has(id)) {
      return;
    }

    const pipeline = this.pipelines.get(id);

    if (pipeline.includes(operator)) {
      pipeline.splice(pipeline.indexOf(operator), 1);
    }
  }
};

N.B.This code is illustrative and I haven't tested it at all.

Creating and binding a private context helps us to ensure data integrity. Please note that it's rather hard to properly extend this class due to the usage of the private context which in this case shouldn't be a problem.
You also don't need to run through the list of filters in you're tests as you get a fresh copy every-time you instantiate a new pipeline store.

daltonmatos · 2017-12-19T14:43:05Z

Hello @orlandohohmeier,

Thank you very much for your time to review this PR.

Making PipelineStore instantiable was a great idea and is the way to go to circumvent the
need for a clearPipeline method. Using the plugin interface to share a new instance of the pipeline store is fine— think of it as an IOC container.

Indeed this is great, but if we have a new pipelines (new Map()) object for each new PipelineStore the following scenario won't work:

Plugin A registers a PRE_AJAX_REQUEST operator;
Plugin B also registers a PRE_AJAX_REQUEST operator;

When ajaxWrapper calls pipeline.applyPipeline(PRE_AJAX_REQUEST, options) the two registered operators (one for each plugin) won't run, because Plugin A, Plugin B and ajaxWrapper have a different instance of PipelineStore. Even if we take care of passing the same instance to all plugins, that is, PluginLoader instantiates PipelineStore only once, ajaxWrappeer would still have a different instance, because whoever imports PipelineStore must create an instance before using it.

That's why I choosed to have FILTERS_PIPELINE as a global object inside the module, so every instance of PipelineStore would use it and the tests could prepare it before each testcase.

I don't know how is the best way to solve this. What do you think would be the best way to achieve this behavior: No plugin can mess with the internal pipelines state, but all plugins share the same pipeline internal state.

I thought that having the FILTERS_PIPELINE importable only by the core UI code would be acceptable, since we control all of the core UI code and we could agree not to modify the internal state directly and always use PipelineStore for this. The only exception would be the unit tests code.

Let me know what do you think.

Thanks a lot.

orlandohohmeier · 2017-12-21T15:26:49Z

Hi @daltonmatos,

you're right, with the current proposal we would run into issues as the plugins and marathon UI wouldn't share the same instance of the PipelineStore — I forgot that we don't use the plugin interface to share instance internally as well.
I'd rename the current PipelineStore to PipelineModel and create a PipelineStore which acts as a singleton and ensures that all parties share the same instance of the PipelineModel to circumvent the issues. This would allow you to properly test the PipelineModel without opening its internals to the world. What do you think?

daltonmatos · 2017-12-21T18:39:06Z

Could work but I have some doubts:

Are you suggesting the PipelineModel to be an internal attribute of PipelineStore?

Because if yes, even if they share the same instance of PipelineModel (which is the correct behavior), being an internal attribute (this.pipeline = ... in the getInstance() singleton method, for example) would allow any plugin do modify the internal state of the pipeline.

When I created the FILTERS_PIPELINE as a global variable in the module scope I was trying to hide this variable from all plugins, since this is not even exported to them. And they can't do anything like:

const {
  PipelineStore
} = window.marathonPluginInterface;

PipelineStore.pipeline = {};

becasue there is no such attribute inside PipelineStore instance, since it uses the module scoped FILTERS_PIPELINE object.

If your are not suggesting the PipelineModel to be an internal attribute of PipelineStore, I think we return to the same point, that will be one object published to all plugins that uses a module scoped object that stores the internal state of the pipeline, we just have different names: PipelineStore using FILTERS_PIPELINE or PipelineStore using PipelineModel.

About the tests on this approach, the test code should still be able to import the instance of PipelineModel (thus it would be exportable to the core UI code) because it needs to do two things (when testing PipelineStore behavior):

I have a test that instantiate two PipelineStore, adds a filter to each one and then checks that both filters are in the internal pipeline state;
The tests need to clear the pipeline state so that tests do not interfere one another.

Do you think is it too bad to let an object importable to the core UI code?

Thanks a lot!!

daltonmatos · 2017-12-27T10:31:05Z

Hello @orlandohohmeier, any thoughts?

Thanks,

orlandohohmeier · 2017-12-27T12:40:31Z

Hi @daltonmatos,

Imaging the pipeline model throws an error – where would you look for bugs? In case your local state is importable to the core UI code you would need to check not only the pipeline model but also the entire core UI to ensure that none of the components mutates that the state by accident.
Making sure that the state is guarded drastically decreases the scope of errors and makes it easier to reason about the respective implementation. Let's try to work-around the need for an unguarded state even though that many of the old stores leak state.

I was thinking about the PipelineStore being nothing more than a proxy sharing the PipelineModel instance as illustrated in the following example:

// PipelineStore.js

export default  new PipelineModel();

You could also use Object.defineProperty to define a read-only property.

// PipelineStore.js

export default new class {
  constructor() {
    Object.defineProperty(this, "pipelines", {value: new PiplelineModel()});
  }
}

Both of these approaches make it impossible for any plugin to tamper with the PipelineStore or PipelineModel.

Your test would then test that the PipelineStore always provide the same instance of PipelineModel and that the PipelineModel works properly.

daltonmatos · 2017-12-28T12:41:08Z

Hello @orlandohohmeier,

I implemented the a idea of PipelineStore being a wrapper for PipelineModel. Let me know if this was what you were thinking of.

Also extracted the constants to a separated file. Don't know if the names are ok, but let's focus on the idea and then we can change the namings, if needed.

Let me know what do you think.

Thanks.

orlandohohmeier · 2017-12-28T13:25:25Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+
+};
+
+export default new PipelineModel();


Exporting an instance makes it unnecessary hard to test the PipelineModel and renders the store useless. Let's export the class here and use the store to create and provide the PipelineModel instance.

orlandohohmeier · 2017-12-28T13:35:03Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+    this.pipelines.clear();
+  };
+
+  applyFilter(operationName, filterParameters) {


I'd rename this method to applyPipeline as a pipeline could be used for many things other than filtering a result set otherwise I'd argue that we should restructure the model to prove a predicate that one can apply instead of applying the filter.

orlandohohmeier · 2017-12-28T13:35:51Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+    this.pipelines.clear();
+  };
+
+  applyFilter(operationName, filterParameters) {


We should also document that a pipeline can't handle more than one input parameter (could be an object or list though) as the output of one operation will be the input of the next operation.

Where is the best place to document this? As a comment in this method?

I'd add a JSDoc comment for now.

orlandohohmeier · 2017-12-28T13:51:20Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+  applyFilter(operationName, filterParameters) {
+    const pipeline = this.getPipeline(operationName);
+
+    pipeline.forEach((filter) => {


Let's rewrite this method to properly pass the input and output parameter so that applyPipeline(FOO, "bar") === c(b(a("bar")))) given we've registered the functions a, b, c as operators for "FOO".

function a(input){ return input.toUpperCase(); } function b(input){ return input.split("").reverse().join(""); } function c(input){ return input.replace(/[AEUIO]/g,""); } pipelines = new PipelineModel(); pipelines.registerOperator("FOO", a); pipelines.registerOperator("FOO", b); pipelines.registerOperator("FOO", c); pipelines.applyPipeline("FOO", "bar") === c(b(a("bar")))

#819 (comment) illustrates how to implement such a method.

orlandohohmeier · 2017-12-28T14:01:03Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+    }
+  };
+
+  getPipeline(pipelineName) {


I wouldn't expose this method as theoretically allows unwanted mutations. I'd also argue that as not necessary to extract the respective functionality as we only need to get the pipelines in a very few places and only in one place we want to actually create a new pipeline if there is none.

I still think that we still shouldn't expose this method due to the named reasons.

orlandohohmeier · 2017-12-28T14:03:28Z

src/js/plugin/sdk/pipeline/PipelineStore.js

@@ -0,0 +1,21 @@
+import PipelineModel from "./PipelineModel";
+
+export default class PipelineStore {


We should export and instance of PipelineStore as the pipeline store will create and manage the PipelineModel should be a singleton similar to all of our stores.

orlandohohmeier · 2017-12-28T14:10:14Z

Hi @daltonmatos,

I very much like where this is going! The current instance handling seems a little of though and doesn't solve the issues we've discussed. I've started to also comment on other parts of the implementations – feel free to ignore those comments for now. I was also wondering wether you want to implement a general purpose pipeline mechanism which one can use to apply a set of operators or provide a mechanism to filter data using the registered predicates. If you think we only need the latter I'd propose a slightly different implementation and would adjust the naming.

Once again, thanks for pushing the plugin interface.

daltonmatos · 2017-12-28T15:06:44Z

Hello @orlandohohmeier,

The idea, for now, is to just have the ability to filter some values in a very simple form. Only one parameter passed to the filters, no priority when running the registered callbacks.

The generic pipeline implementation could be postponed until really needed, which is not the case for now.

Let's do the minimal filtering implementation for now and think about the pipeline (if ever needed) in the future, what do you think?

Let me know what are you suggestions about the naming and the different implementation you mentioned above so we can move on.

Thanks a lot for your time reviewing my submissions. 🎉

daltonmatos · 2018-01-03T09:53:00Z

Hello @orlandohohmeier,

What do you think about making this implementation as simple as possible so we can merge it?

Let me know what are the suggestions you mentioned earlier so I can implement them on this PR.

Thanks,

orlandohohmeier · 2018-01-03T14:37:06Z

@daltonmatos please excuse that I didn't come back to you earlier. I've tried a few things but considering that you actually want to mutate an input object (e.g. options) I'd implement the pipeline as outlined in #819 (comment). What do you think?

daltonmatos · 2018-01-03T15:00:17Z

Sure @orlandohohmeier, no problem. I will write this implementation.

I will also use the names you suggested: applyPipeline(), registerOpetator() and unregisterOperator().

Thanks,

daltonmatos · 2018-01-03T16:03:52Z

Hello @orlandohohmeier,

please take a look at the implementation. Most of it was already done, just didn't push yet.

Thanks,

orlandohohmeier · 2018-01-05T18:58:10Z

@daltonmatos the last days were super busy and I haven't had the time to review your code. Will look into it next week. Hope you don't mind. 🙂

daltonmatos · 2018-01-08T10:28:43Z

No problem @orlandohohmeier. Let me know when you have some time to review this PR.

daltonmatos · 2018-01-10T18:37:43Z

Hello @orlandohohmeier,

have you had any time to review this PR?

Thanks,

orlandohohmeier · 2018-01-10T18:42:48Z

Hi @daltonmatos, unfortunately I haven't had the time to look into this. I'm super sorry. Will look into it first thing tomorrow morning.

daltonmatos · 2018-01-10T19:08:35Z

No problem, thanks!

orlandohohmeier · 2018-01-11T15:58:58Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+    }
+  };
+
+  getPipeline(pipelineName) {


I still think that we still shouldn't expose this method due to the named reasons.

orlandohohmeier · 2018-01-11T16:25:12Z

src/js/plugin/sdk/pipeline/PipelineOperations.js

+  PRE_AJAX_REQUEST
+};
+
+export default PipelineOperations;


I'd use named exports, instead of exporting an Object as illustrated below.

export const PRE_AJAX_REQUEST = "PRE_AJAX_REQUEST"

orlandohohmeier · 2018-01-11T17:48:51Z

src/js/plugin/sdk/pipeline/PipelineStore.js

+  };
+
+  deregisterOperator(operationName, callback) {
+    return this.deregisterOperator(operationName, callback);


Please change this to this.deregisterOperator(operationName, callback) as the method otherwise recurses infinitely.

Done. Thanks. Didn't spot this. Should be return this.pipeline.deregisterOperator(operationName, operator)

orlandohohmeier · 2018-01-11T17:50:23Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+         }, filterParameters);
+  };
+
+  registerOperator(operationName, callback) {


🎨 I'd rename callback to operator for all for all these methods.

orlandohohmeier · 2018-01-11T17:53:05Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+  deregisterOperator(operationName, callback) {
+    const pipeline = this.getPipeline(operationName);
+
+    var callbckPosition = pipeline.indexOf(callback);


🎨 I'd rename this one to operatorIndex

P.S. You could use a const as the index shouldn't change.

orlandohohmeier · 2018-01-11T18:26:48Z

src/test/units/PipelineModel.test.js

+describe("PipelineModel tests", function () {
+
+    beforeEach(function (){
+      pipelineModel.clearPipeline();


Instead of clearing the pipeline model I'd simple create a new instance before each run.

orlandohohmeier · 2018-01-11T18:27:30Z

src/test/units/PipelineModel.test.js

+
+const pipelineModel = new PipelineModel();
+
+describe("PipelineModel tests", function () {


Please add a test that verifies that the operators are called in the right order.

orlandohohmeier · 2018-01-11T18:28:16Z

src/test/units/PipelineModel.test.js

+      expect(pipelineModel.getPipeline(PipelineOperations.PRE_AJAX_REQUEST)).to.have.lengthOf(0);
+    });
+
+    it("should clear the pipeline when requested to", function () {


I don't think that we should add this functionality nor a test for it.

orlandohohmeier · 2018-01-11T18:29:21Z

src/test/units/PipelineModel.test.js

+      pipelineModel.clearPipeline();
+    });
+
+    it("should return an empty list for a non-existant pipeline", function () {


Please remove this test along with the getPipeline getter for the reasons detailed in https://github.com/mesosphere/marathon-ui/pull/819/files#r158946727

orlandohohmeier · 2018-01-11T18:32:26Z

src/test/units/PipelineModel.test.js

+      expect(pipelineModel.getPipeline(PipelineOperations.PRE_AJAX_REQUEST)).to.have.lengthOf(0);
+    });
+
+    it("should add a new function to the pipeline", function () {


🎨 Let's always refer to the functions/callback as operators to make it easier for other to understand the feature... "should register a new operator to the pipeline"

orlandohohmeier · 2018-01-11T19:35:14Z

@daltonmatos Thanks a lot for your effort to ship this new capability. I've added a few comments here and there including stylistic ones that I flagged with the 🎨 emoji.

daltonmatos · 2018-01-15T13:08:59Z

All done @orlandohohmeier. Please Take a look.

The suite for the PipelineStore is still broken, as I would like to know what should we do with it.

Also let me know if the publushing of PRE_AJAX_REQUEST to the plugins (inside PluginLoader.js) is correct.

awaiting for your response/review.

Thanks a lot!

orlandohohmeier · 2018-01-15T13:22:59Z

src/js/plugin/PluginLoader.js

@@ -13,7 +13,7 @@ import PluginDispatcherProxy from "./PluginDispatcherProxy";
 import MarathonService from "./sdk/services/MarathonService";
 import MarathonActions from "./sdk/actions/MarathonActions";
 import PipelineStore from "./sdk/pipeline/PipelineStore";
-import PipelineOperations from "./sdk/pipeline/PipelineOperations";
+import {PRE_AJAX_REQUEST} from "./sdk/pipeline/PipelineOperations";


Instead of explicitly importing PRE_AJAX_REQUEST I'd change the import to import * as PipelineOperations from './sdk/pipeline/PipelineOperations'; so that we don't need to manually add the operators here as well.

orlandohohmeier · 2018-01-15T18:21:18Z

src/js/plugin/sdk/pipeline/PipelineStore.js

@@ -0,0 +1,23 @@
+import PipelineModel from "./PipelineModel";
+
+class PipelineStore {


Wonder wether we should move this one and the related files to the stores directory as the pipeline store can be use used both by Marathon UI core as well as by plugins.

Makes sense, but I would leave it inside the plugin/sdk/stores folder and not in js/stores folder as I think the pipeline is feature provided by the plugin SDK (yet to be extracted from the core source-code). The UI core can use it, of couse, but importing from plugin/sdk/store/PipelineStore.

orlandohohmeier · 2018-01-15T18:49:09Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+    this.pipelines = new Map();
+  };
+
+  applyPipeline(operationName, filterParameters) {


🎨 I'd change operationName to pipelineName as we're allowing users to register operators for different pipelines.

orlandohohmeier · 2018-01-15T18:51:27Z

src/js/plugin/sdk/pipeline/PipelineOperations.js

@@ -0,0 +1,3 @@
+


🎨 I'd rename this file to PipelineNames as these const are used to identify different pipelines not operations on a pipeline. Please also see the comment in regards to the operationName param.

orlandohohmeier · 2018-01-15T19:00:01Z

src/js/plugin/sdk/pipeline/PipelineStore.js

+class PipelineStore {
+
+  constructor() {
+    Object.defineProperty(this, "pipeline", {value: new PipelineModel()});


🎨 I'd rename the value to "pipelines" as we're allowing users to register operators for multiple different pipelines.

orlandohohmeier · 2018-01-15T19:38:43Z

src/test/units/PipelineStore.test.js

+      const filter = function (data) {return data;};
+      const filter2 = function (data) {return data;};
+
+      const pipeline_one = PipelineStore;


You should be able to import the store as follows:

import { default as TestPipelineStore } from "../../js/plugin/sdk/pipeline/PipelineStore"; // ... PipelineStore.pipelines === TestPipelineStore.pipelines;

I'd use sinon to check wether the respective functions were called as illustrated in the following:

sinon.spy(PipelineStore.pipeline, "applyPipeline"); expect(PipelineStore.pipeline.applyPipeline).to.be.true;

orlandohohmeier · 2018-01-15T19:40:37Z

@daltonmatos thanks for addressing the comments. 💜 Found a few more minor things but those aside the code looks good.

daltonmatos · 2018-01-16T12:18:21Z

All Done @orlandohohmeier,

let me know what you think. I also implemented a ran locally an UI plugin that uses this new interface. Works as expected.

Thanks,

daltonmatos · 2018-01-19T12:27:30Z

Hello @orlandohohmeier,

any thoughts about this? I implemented all your suggested reviews.

Thanks,

orlandohohmeier · 2018-01-24T10:35:57Z

src/js/plugin/sdk/pipeline/PipelineModel.js

+      return filterParameters;
+    }
+    return this.pipelines.get(pipelineName)
+      .reduce((accumulator, pipelineCallback) => {


🎨 I'd rename pipelineCallback to operator to stick with the general naming scheme.

orlandohohmeier · 2018-01-24T10:43:07Z

@daltonmatos thanks a lot for your relentless work on this one. This is a great addition which will enable new plugins. Well done! 💯

Please squash your commits before we merge this one, as not all of them work independently of any later commits, and pass the linter as well as the test suite. Thanks!

It's now possible to plugins to change behaviors/values of the Core UI code. The main idea is to have pipelines offered by the Core and any plugins can register to these pipelines and they will be called when the pipelines run. The first pipeline implemented here makes it possible to modify any ajax request made by the UI. Here is how a plugin could register to be called when this pipeline runs: ``` const { PluginHelper, PluginMountPoints, PipelineNames, PipelineStore, } = window.marathonPluginInterface; PipelineStore.registerOperator(PipelineNames.PRE_AJAX_REQUEST, (data) => { /* Modify "data" and return a new version of it */ }); ```

daltonmatos · 2018-01-24T11:44:45Z

Great @orlandohohmeier,

Do you mind if I squash all commits into one single commit? This implementation is overall small and simple, so I think i will still be easy to review/understand even if contained in a single commit.

Let me know and I will update this PR and it will be ready do be merged.

Thanks,

daltonmatos · 2018-01-24T16:37:38Z

Hello @orlandohohmeier,

I squashed all commits into one single commit. Thought this was easier than trying to pick some commits to squash. If you prefer, I still have all original commits, if you think it's necessary I can push them back here in this PR and we can choose which commits to squash.

Thanks a lot for your reviews and for accepting this code change.

orlandohohmeier · 2018-01-24T18:19:13Z

@daltonmatos thanks for squashing your commits 🚀

orlandohohmeier reviewed Dec 19, 2017

View reviewed changes

orlandohohmeier suggested changes Dec 28, 2017

View reviewed changes

orlandohohmeier suggested changes Jan 11, 2018

View reviewed changes

orlandohohmeier suggested changes Jan 15, 2018

View reviewed changes

orlandohohmeier approved these changes Jan 24, 2018

View reviewed changes

orlandohohmeier merged commit 13bd17e into d2iq-archive:master Jan 24, 2018

		@@ -0,0 +1,21 @@
		import PipelineModel from "./PipelineModel";

		export default class PipelineStore {


		const pipelineModel = new PipelineModel();

		describe("PipelineModel tests", function () {

		@@ -0,0 +1,23 @@
		import PipelineModel from "./PipelineModel";

		class PipelineStore {

Implement request filter pipeline #819

Implement request filter pipeline #819

Conversation

daltonmatos commented Dec 8, 2017

daltonmatos commented Dec 11, 2017

daltonmatos commented Dec 13, 2017

daltonmatos commented Dec 14, 2017

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

orlandohohmeier commented Dec 19, 2017 • edited Loading

daltonmatos commented Dec 19, 2017

orlandohohmeier commented Dec 21, 2017

daltonmatos commented Dec 21, 2017

daltonmatos commented Dec 27, 2017

orlandohohmeier commented Dec 27, 2017

daltonmatos commented Dec 28, 2017

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

orlandohohmeier commented Dec 28, 2017

daltonmatos commented Dec 28, 2017

daltonmatos commented Jan 3, 2018

orlandohohmeier commented Jan 3, 2018 • edited Loading

daltonmatos commented Jan 3, 2018

daltonmatos commented Jan 3, 2018

orlandohohmeier commented Jan 5, 2018 • edited Loading

daltonmatos commented Jan 8, 2018

daltonmatos commented Jan 10, 2018

orlandohohmeier commented Jan 10, 2018

daltonmatos commented Jan 10, 2018

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

orlandohohmeier commented Jan 11, 2018

daltonmatos commented Jan 15, 2018

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

orlandohohmeier commented Jan 15, 2018

daltonmatos commented Jan 16, 2018

daltonmatos commented Jan 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orlandohohmeier commented Jan 24, 2018

daltonmatos commented Jan 24, 2018 • edited Loading

daltonmatos commented Jan 24, 2018

orlandohohmeier commented Jan 24, 2018

orlandohohmeier commented Dec 19, 2017 •

edited

Loading

orlandohohmeier commented Jan 3, 2018 •

edited

Loading

orlandohohmeier commented Jan 5, 2018 •

edited

Loading

daltonmatos commented Jan 24, 2018 •

edited

Loading