From 0327d39e2c65331281e62db207793aaa7819815a Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Thu, 12 Sep 2013 23:18:31 +0200 Subject: [PATCH] Move observing last build to controller We observe last build on the repo in order to show the freshest build on repo page. I moved it to router in order to keep such observers in the same place, but this was not a wise move. To make it work properly observer needs to be removed when moving to some other part (like build's page). The problem is that deactivate function is not called when we move to the other route in the same nesting. We have our own 'activate' function on repoController, which is better suited for handling this task. --- assets/scripts/app/controllers/repo.coffee | 14 ++++++++++++++ assets/scripts/app/routes.coffee | 14 ++------------ assets/scripts/app/views/repo/show.coffee | 1 - 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/assets/scripts/app/controllers/repo.coffee b/assets/scripts/app/controllers/repo.coffee index 89d22907..d6fa09af 100644 --- a/assets/scripts/app/controllers/repo.coffee +++ b/assets/scripts/app/controllers/repo.coffee @@ -23,12 +23,15 @@ Travis.RepoController = Travis.Controller.extend jobs.forEach (j) -> j.updateTimes() activate: (action) -> + @stopObservingLastBuild() this["view#{$.camelize(action)}"]() viewIndex: -> + @observeLastBuild() @connectTab('current') viewCurrent: -> + @observeLastBuild() @connectTab('current') viewBuilds: -> @@ -46,6 +49,17 @@ Travis.RepoController = Travis.Controller.extend viewJob: -> @connectTab('job') + lastBuildDidChange: -> + build = @get('repo.lastBuild') + @controllerFor('build').set('build', build) + + stopObservingLastBuild: -> + @removeObserver('repo.lastBuild', this, 'lastBuildDidChange') + + observeLastBuild: -> + @lastBuildDidChange() + @addObserver('repo.lastBuild', this, 'lastBuildDidChange') + connectTab: (tab) -> # TODO: such implementation seems weird now, because we render # in the renderTemplate function in routes diff --git a/assets/scripts/app/routes.coffee b/assets/scripts/app/routes.coffee index 899411fd..b1dac327 100644 --- a/assets/scripts/app/routes.coffee +++ b/assets/scripts/app/routes.coffee @@ -117,25 +117,15 @@ Travis.ApplicationRoute = Ember.Route.extend Travis.LineNumberParser, Travis.SetupLastBuild = Ember.Mixin.create setupController: -> - @lastBuildDidChange() - @controllerFor('repo').addObserver('repo.lastBuild', this, 'lastBuildDidChange') @repoDidLoad() @controllerFor('repo').addObserver('repo.isLoaded', this, 'repoDidLoad') - deactivate: -> - @_super.apply this, arguments - @controllerFor('repo').removeObserver('repo.lastBuild', this, 'lastBuildDidChange') - repoDidLoad: -> # TODO: it would be nicer to do it with promises repo = @controllerFor('repo').get('repo') if repo && repo.get('isLoaded') && !repo.get('lastBuild') @render('builds/not_found', outlet: 'pane', into: 'repo') - lastBuildDidChange: -> - build = @controllerFor('repo').get('repo.lastBuild') - @controllerFor('build').set('build', build) - Travis.GettingStartedRoute = Ember.Route.extend setupController: -> $('body').attr('id', 'home') @@ -168,7 +158,7 @@ Travis.IndexCurrentRoute = Ember.Route.extend Travis.SetupLastBuild, @_super.apply this, arguments @currentRepoDidChange() - @container.lookup('controller:repo').activate('index') + @controllerFor('repo').activate('index') @controllerFor('repos').addObserver('firstObject', this, 'currentRepoDidChange') deactivate: -> @@ -256,7 +246,7 @@ Travis.JobRoute = Ember.Route.extend Travis.RepoIndexRoute = Ember.Route.extend Travis.SetupLastBuild, setupController: (controller, model) -> @_super.apply this, arguments - @container.lookup('controller:repo').activate('current') + @controllerFor('repo').activate('current') renderTemplate: -> @render 'build', outlet: 'pane', into: 'repo' diff --git a/assets/scripts/app/views/repo/show.coffee b/assets/scripts/app/views/repo/show.coffee index 4129f2ba..21c8248c 100644 --- a/assets/scripts/app/views/repo/show.coffee +++ b/assets/scripts/app/views/repo/show.coffee @@ -17,7 +17,6 @@ Travis.reopen # TODO: look into fixing it in more general way Ember.run.schedule('afterRender', this, -> pane = Ember.get('_outlets.pane') - console.log('tab', @get('controller.tab')) if @get('controller.repo.isLoaded') && @state == 'inDOM' && @get('controller.repo.lastBuild') && @get('controller.tab') == 'current' && (!pane || pane.state == 'destroyed')