Forced builds should say who (which continuum user) forced the build

Initial investigation

DefaultBuildController.java has method
public void build( int projectId, int buildDefinitionId, int trigger, ScmResult scmResult )

'build' is called from BuildProjectTaskExecutor.java:

    public void executeTask( Task task ) throws TaskExecutionException {
        BuildProjectTask buildProjectTask = (BuildProjectTask) task;
        controller.build( buildProjectTask.getProjectId(), buildProjectTask.getBuildDefinitionId(), buildProjectTask.getTrigger(), buildProjectTask.getScmResult() );
    }

• 'trigger' is an integer, no additional info such as username is passed in
• Brett suggested 'trigger' should be a class rather than an int
• username needs to get into BuildProjectTask so it's available in BuildProjectTaskExecutor when calling controller.build

more in 20090514 #continuum irc log

This should be solved by CONTINUUM-2265 (Need information on who executed a build or release)

2009-05-14 irc conversation from my logs:

[4:56pm] wsmoak: does anyone know how anything actually gets built in Continuum?
[4:56pm] wsmoak: all I want to do is add "by <whoever>" on the build results page to show who forced a build
[4:57pm] brett: there's a build executor in core
[4:57pm] brett: you probably want to do a find usages on the add method of the build result dao
[4:57pm] wsmoak: BuildProjectTaskExecutor is one of the things I found
[4:58pm] brett: that's him
[4:58pm] wsmoak: all he does is call controller.build
[4:59pm] wsmoak: BuildController is an interface
[4:59pm] wsmoak: where's the code for pete's sake
[4:59pm] brett:
[4:59pm] wsmoak: there's no 'add' in the BuildResult class
[4:59pm] brett: BR, or BRDao?
[5:00pm] brett: if you're in eclipse, you can get an outline on the interface cass that shows subclasses
[5:01pm] wsmoak: just BR. hadn't found the Dao one.
[5:01pm] wsmoak: yeah, alt-F7
[5:02pm] wsmoak: DefaultBuildController. possibly.
[5:05pm] marica joined the chat room.
[5:07pm] wsmoak: looks like it would mean changing a method signature on DefaultBuildController.build.
[5:08pm] wsmoak: last time I tried something like that, it rippled through the entire app.
[5:09pm] brett: yah
[5:09pm] brett: it doesn't take a context of extra info?
[5:11pm] wsmoak: not afaict - public void build( int projectId, int buildDefinitionId, int trigger, ScmResult scmResult )
[5:12pm] brett: boo
[5:12pm] brett: trigger should be a class
[5:12pm] wsmoak: there's private void updateBuildResult( BuildContext context, String error ) that has a context...
[5:12pm] brett: might only be called on error
[5:14pm] wsmoak: darn. the build context gets created inside this class, it isn't passed in
[5:14pm] wsmoak: so... turn Trigger into a class and see what else has to change...
[5:14pm] brett: (everything)
[5:14pm] brett:
[5:15pm] wsmoak: yeah. here's the part where I decide changing stuff in continuum is way too hard and just go open more issues
[5:15pm] brett: I would probably overload it, not change it
[5:16pm] wsmoak: yeah. but I bet it's still going to be messy changing it on the other side where things are calling it.
[5:18pm] wsmoak: hm. we may be in luck... the only usage I can find is the aforementioned controller.build(...) in BuildProjectTaskExecutor
[5:19pm] wsmoak: which means BuildProjectTask has to change... and we'll leave figuring out who fills him up for another day
[5:22pm] wsmoak: thanks

Fixed in:
r786863 in 1.3.x branch
r786876 in trunk

When we were discussing this on the list I wasn't aware this was going on the 1.3.x branch, and the issue was marked for "1.x" so no clue there either. As a new feature/improvement, IMO it should have only gone on trunk.

Wendy, I have overlooked the fix for version, my mistake. Should I revert the commit in 1.3.x branch instead?

Reverted r786863 commit (r786891).

Well I wouldn't have insisted on reverting it since it was already done.

Thanks, though. I'm looking forward to seeing this in our first 1.4.x release!

I see you updated the unit tests already; I'll see if I can find the right place in the Selenium tests if no one beats me to it.

do any docs (esp. screen shots) need to be updated for this?

Wendy, yes. The build result screen shot should be replaced to reflect the added field (Triggered by).

I will be updating the docs too.

Thanks!