Oops, just realized I forgot to sign the CLA. I just did that so it should be good now.

Thanks @lcs777! I can take a look at it today or tomorrow. If someone else is free before that, please feel free to review and comment.

Should keep #229 in mind, while looking at this.

@lcs777: I think this is looking good to be merged soon. I had a couple of points. Take a look at them. I really like the test names you've chosen and how everything looks. Thanks!

I've seen it in the UI and I'm running all the tests against this change.

Great idea about make a NotificationFilter.appliesTo() method. I fixed all of the issues. Hopefully should be good now.

Hi @lcs777,

I've accepted it, but I found that there is a small difference now between appliesTo and matchStage. Here's what I mean:

@Test
public void specificStageShouldMatchWithinAnyPipeline() {
    NotificationFilter filter = new NotificationFilter(GoConstants.ANY_PIPELINE, "dev", StageEvent.Breaks, false);
    assertThat(filter.matchStage(new StageConfigIdentifier("cruise1", "dev"), StageEvent.Breaks), is(true));
    assertThat(filter.matchStage(new StageConfigIdentifier("cruise2", "dev"), StageEvent.Breaks), is(true));
}

@Test
public void specificStageShouldApplyToAnyPipeline() {
    NotificationFilter filter = new NotificationFilter(GoConstants.ANY_PIPELINE, "dev", StageEvent.Breaks, false);
    assertThat(filter.appliesTo("cruise1", "dev"), is(true));
    assertThat(filter.appliesTo("cruise2", "dev"), is(true));
}

The first test (for matchStage) fails and the second test (for appliesTo) passes, given the same condition. The condition that I'm checking is "ANY_PIPELINE" + "Specific stage". I don't know if it makes sense. It might, for someone who uses pedantic names for their stages ("I want to be notified for all 'test' stage failures, across all pipelines").

I don't see why this difference in behavior needs to exist. I think shouldMatch should just call appliesTo. Like this:

public boolean matchStage(StageConfigIdentifier stageIdentifier, StageEvent event) {
    return this.event.include(event) && appliesTo(stageIdentifier.getPipelineName(), stageIdentifier.getStageName());
}

Let me know what you think.

I made your changes in my local repo and think it makes perfect sense. Do you want to put the changes in, since you already have the code, or should I make another pull request?

Hopefully I will be a little less sloppy in my next pull request so I will catch that.

thanks,
lcs777

Hi,

That's fine. I can make this change.

It's completely ok! That's why code gets seen before merging. So, we can all help each other. If you have comments about any other code that gets in, please feel free to comment. :) I appreciate the commits and the changes.

Cheers,
Aravind

Hey @lcs777,

The build for this is available at http://www.go.cd/download/ (build number 816 or later), if you want to download and check.

Cheers,
Aravind

PS: I just realized that "[Any Pipeline]" and "some-stage" is not going to be allowed from the UI. But, the change is still ok, I feel.

Awesome. Hopefully this will mean another instance of Go in a production environment!

Even though it is not currently allowed in the UI, it makes sense to do it anyways. Also, I definitely can see someone doing the "pedantic" naming you suggested and wanting to do what you mentioned in the future.

That build is bleeding-edge so to speak. It has gone through all the regression tests, of course. But, be careful. I'd recommend not setting up a production environment on it, yet. 14.3 should come out soon, and that will include this. That release will have gone through more rigorous verification, performance testing, etc. Just saying. :)

I mentioned it is available so that you can take a quick look at the feature you wrote, and see if it looks ok!

Works fine on 14.3.0(1081-da691d74e30a47)

Is there a way that we could add "Any Pipeline" and "Stage XYZ" ? I have many many pipelines that all use the same template and I have a particular Stage that I want to be notified on. It would be really nice if I could specify one rule that allowed me to match any pipeline but a specific stage, like "QA-Passed".

Are these notifications exposed anywhere that I can manipulate through an API call? If we can't have something like the above Any Pipeline + Specific Stage rule, then maybe I can write something that iterates through all my pipelines and creates new notification rules for each?