Move checking for errors to router rather than template

Previously we were checking if we should display an error message by
adding if statements in a template. This is not the best way to do
it, because it clutters a template and makes code harder to follow.

In this commit I move rendering error templates to the router. Code for
rendering error when there is no builds is not the best way to do it
either, but it can be improved when new router changes are merged to
Ember's master and a way Ember Data is handling promises is revised and
improved.
This commit is contained in:
Piotr Sarnacki 2013-06-05 11:27:47 +02:00
parent f87e4108a8
commit 50cdc4cf98
7 changed files with 84 additions and 80 deletions

View File

@ -3,7 +3,6 @@ Travis.RepoController = Travis.Controller.extend
needs: ['repos', 'currentUser']
currentUserBinding: 'controllers.currentUser'
isError: (-> @get('repo.isError') ).property('repo.isError')
slug: (-> @get('repo.slug') ).property('repo.slug')
isLoading: (-> @get('repo.isLoading') ).property('repo.isLoading')

View File

@ -123,11 +123,19 @@ 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('repo.lastBuild')
@render('builds/not_found', outlet: 'pane', into: 'repo')
lastBuildDidChange: ->
build = @controllerFor('repo').get('repo.lastBuild')
@controllerFor('build').set('build', build)
@ -238,15 +246,15 @@ Travis.RepoRoute = Ember.Route.extend Travis.DontSetupModelForControllerMixin,
repos = Travis.Repo.bySlug(slug)
self = this
observer = ->
if repos.get 'isLoaded'
repos.removeObserver 'isLoaded', observer
proxy.set 'isLoading', false
if repos.get('length') == 0
# isError is also used in DS.Model, but maybe we should use something
# more focused like notFound later
proxy.set 'isError', true
self.render('repos/not_found', outlet: 'main')
else
proxy.set 'content', repos.objectAt(0)

View File

@ -0,0 +1 @@
There are no builds for this repository.

View File

@ -1,66 +1,62 @@
{{#if loading}}
<span>Loading</span>
{{else}}
{{#if build}}
<dl id="summary">
<div class="left">
<dt>{{t builds.name}}</dt>
<dd class="number">
<span class="status"></span>
{{#if build.id}}
{{#if build.repo.slug}}
{{#linkTo build repo build}}{{build.number}}{{/linkTo}}
{{/if}}
<dl id="summary">
<div class="left">
<dt>{{t builds.name}}</dt>
<dd class="number">
<span class="status"></span>
{{#if build.id}}
{{#if build.repo.slug}}
{{#linkTo build repo build}}{{build.number}}{{/linkTo}}
{{/if}}
</dd>
<dt>{{t builds.state}}</dt>
<dd class="state">{{capitalize build.state}}</dd>
<dt class="finished_at_label">{{t builds.finished_at}}</dt>
<dd class="finished_at timeago" {{bindAttr title="finishedAt"}}>{{formatTime build.finishedAt}}</dd>
<dt>{{t builds.duration}}</dt>
<dd class="duration" {{bindAttr title="startedAt"}}>{{formatDuration build.duration}}</dd>
{{/if}}
</dd>
<dt>{{t builds.state}}</dt>
<dd class="state">{{capitalize build.state}}</dd>
<dt class="finished_at_label">{{t builds.finished_at}}</dt>
<dd class="finished_at timeago" {{bindAttr title="finishedAt"}}>{{formatTime build.finishedAt}}</dd>
<dt>{{t builds.duration}}</dt>
<dd class="duration" {{bindAttr title="startedAt"}}>{{formatDuration build.duration}}</dd>
</div>
{{#with build.commit}}
<div class="right">
<dt>{{t builds.commit}}</dt>
<dd class="commit"><a {{bindAttr href="controller.urlGithubCommit"}}>{{formatCommit this}}</a></dd>
{{#if build.pullRequest}}
<dt>{{t builds.pull_request}}</dt>
<dd class="pull_request"><a {{bindAttr href="compareUrl"}}>#{{build.pullRequestNumber}} {{build.pullRequestTitle}}</a></dd>
{{else}}
{{#if compareUrl}}
<dt>{{t builds.compare}}</dt>
<dd class="compare"><a {{bindAttr href="compareUrl"}}>{{pathFrom compareUrl}}</a></dd>
{{/if}}
{{/if}}
{{#if authorName}}
<dt>{{t builds.author}}</dt>
<dd class="author"><a {{bindAttr href="urlAuthor"}}>{{authorName}}</a></dd>
{{/if}}
{{#if committerName}}
<dt>{{t builds.committer}}</dt>
<dd class="committer"><a {{bindAttr href="urlCommitter"}}>{{committerName}}</a></dd>
{{/if}}
</div>
{{/with}}
{{#with build.commit}}
<div class="right">
<dt>{{t builds.commit}}</dt>
<dd class="commit"><a {{bindAttr href="controller.urlGithubCommit"}}>{{formatCommit this}}</a></dd>
{{#if build.pullRequest}}
<dt>{{t builds.pull_request}}</dt>
<dd class="pull_request"><a {{bindAttr href="compareUrl"}}>#{{build.pullRequestNumber}} {{build.pullRequestTitle}}</a></dd>
{{else}}
{{#if compareUrl}}
<dt>{{t builds.compare}}</dt>
<dd class="compare"><a {{bindAttr href="compareUrl"}}>{{pathFrom compareUrl}}</a></dd>
{{/if}}
{{/if}}
{{#if authorName}}
<dt>{{t builds.author}}</dt>
<dd class="author"><a {{bindAttr href="urlAuthor"}}>{{authorName}}</a></dd>
{{/if}}
{{#if committerName}}
<dt>{{t builds.committer}}</dt>
<dd class="committer"><a {{bindAttr href="urlCommitter"}}>{{committerName}}</a></dd>
{{/if}}
</div>
{{/with}}
<dt>{{t builds.message}}</dt>
<dd class="message">{{formatMessage build.commit.message}}</dd>
<dt>{{t builds.message}}</dt>
<dd class="message">{{formatMessage build.commit.message}}</dd>
{{#unless isMatrix}}
<dt>{{t builds.config}}</dt>
<dd class="config">{{formatConfig build.config}}</dd>
{{/unless}}
</dl>
{{#unless isMatrix}}
<dt>{{t builds.config}}</dt>
<dd class="config">{{formatConfig build.config}}</dd>
{{/unless}}
</dl>
{{#if build.isMatrix}}
{{view Travis.JobsView jobsBinding="build.requiredJobs" required="true"}}
{{view Travis.JobsView jobsBinding="build.allowedFailureJobs"}}
{{else}}
{{view Travis.LogView jobBinding="build.jobs.firstObject"}}
{{/if}}
{{#if build.isMatrix}}
{{view Travis.JobsView jobsBinding="build.requiredJobs" required="true"}}
{{view Travis.JobsView jobsBinding="build.allowedFailureJobs"}}
{{else}}
There are no builds for this repository.
{{view Travis.LogView jobBinding="build.jobs.firstObject"}}
{{/if}}
{{/if}}

View File

@ -0,0 +1,3 @@
<div id="repo">
<span class="not-found">The repository at {{slug}} was not found.</span>
</div>

View File

@ -2,28 +2,24 @@
{{#if view.isEmpty}}
{{view Travis.ReposEmptyView}}
{{else}}
{{#if isError}}
<span class="not-found">The repository at {{slug}} was not found.</span>
{{else}}
{{#if repo.isLoaded}}
{{#with repo}}
<div id="repo-header">
<h3>{{#linkTo "repo" this}}{{slug}}{{/linkTo}}</h3>
<div class="github-icon"><a {{bindAttr href="controller.urlGithub"}}><img src="/images/icons/github.png" width="21" height="21"/></a></div>
</div>
<p class="description">{{description}}</p>
{{view Travis.RepoShowTabsView}}
{{view Travis.RepoShowToolsView}}
{{/with}}
<div class="tab">
{{outlet pane}}
{{#if repo.isLoaded}}
{{#with repo}}
<div id="repo-header">
<h3>{{#linkTo "repo" this}}{{slug}}{{/linkTo}}</h3>
<div class="github-icon"><a {{bindAttr href="controller.urlGithub"}}><img src="/images/icons/github.png" width="21" height="21"/></a></div>
</div>
{{else}}
<span>Loading</span>
{{/if}}
<p class="description">{{description}}</p>
{{view Travis.RepoShowTabsView}}
{{view Travis.RepoShowToolsView}}
{{/with}}
<div class="tab">
{{outlet pane}}
</div>
{{else}}
<span>Loading</span>
{{/if}}
{{/if}}
</div>

View File

@ -17,6 +17,7 @@ Travis.reopen
# TODO: look into fixing it in more general way
pane = Ember.get('_outlets.pane')
if @get('controller.repo.isLoaded') && @state == 'inDOM' &&
@get('controller.repo.lastBuild') &&
@get('controller.tab') == 'current' && (!pane || pane.state == 'destroyed')
view = @get('controller.container').lookup('view:build')
view.set('controller', @get('controller.container').lookup('controller:build'))