Skip to content

Commit

Permalink
fix(1607): fix some bugs on build page (screwdriver-cd#447)
Browse files Browse the repository at this point in the history
* fix: resolve uncaught errors from parent build route

* fix: attempt to fix build log
  • Loading branch information
DekusDenial authored and kumada626 committed Jul 12, 2019
1 parent 02f6ef7 commit 759b258
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 21 deletions.
4 changes: 4 additions & 0 deletions app/components/build-banner/styles.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
padding: 0 25px;
}

summary {
display: list-item;
}

ul {
list-style: none;
padding: 0;
Expand Down
59 changes: 43 additions & 16 deletions app/components/build-log/component.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Promise } from 'rsvp';
import { set, computed } from '@ember/object';
import { set, computed, observer } from '@ember/object';
import { scheduleOnce, later } from '@ember/runloop';
import { inject as service } from '@ember/service';
import Component from '@ember/component';
Expand All @@ -13,25 +13,37 @@ export default Component.extend({
autoscroll: true,
isFetching: false,
isDownloading: false,
inProgress: false,
justFinished: false,
timeFormat: 'datetime',
lastScrollTop: 0,
lastScrollHeight: 0,
inProgress: computed('totalLine', {
get() {
return this.totalLine === undefined;
// eslint-disable-next-line ember/no-observers
inProgressObserver: observer('totalLine', function inProgressObserver() {
const inProgress = this.totalLine === undefined;

// step just finished
if (this.inProgress && !inProgress) {
this.set('justFinished', true);
}

this.set('inProgress', inProgress);
}),
sortOrder: computed('inProgress', {
get() {
return this.inProgress ? 'ascending' : 'descending';
return this.inProgress || this.justFinished ? 'ascending' : 'descending';
}
}),
getPageSize(fetchMax = false) {
const { totalLine } = this;
const itemSize = this.logService.getCache(this.buildId, this.stepName, 'nextLine') || totalLine;
const { totalLine, inProgress, justFinished } = this;
let itemSize = this.logService.getCache(this.buildId, this.stepName, 'nextLine') || totalLine;

if (justFinished) {
itemSize = totalLine - itemSize + 1;
}

// for running step, fetch regular page size
if (this.inProgress) {
if (inProgress) {
return ENV.APP.DEFAULT_LOG_PAGE_SIZE;
}

Expand Down Expand Up @@ -148,6 +160,11 @@ export default Component.extend({
return [{ m: `Loading logs for step ${stepName}...` }];
}

if (this.justFinished) {
// there were logs in the cache, fetch the last batch of logs
this.getLogs(true);
}

scheduleOnce('afterRender', this, 'scrollDown');

return logs;
Expand Down Expand Up @@ -199,10 +216,15 @@ export default Component.extend({
const newStepId = `${this.buildId}/${this.stepName}`;

if (newStepId !== this.lastStepId) {
set(this, 'autoscroll', true);
set(this, 'lastStepId', newStepId);
set(this, 'lastScrollTop', 0);
set(this, 'lastScrollHeight', 0);
this.setProperties({
autoscroll: true,
lastStepId: newStepId,
lastScrollTop: 0,
lastScrollHeight: 0,
isFetching: false,
isDownloading: false,
justFinished: false
});
}
},

Expand Down Expand Up @@ -258,7 +280,7 @@ export default Component.extend({
*/
getLogs(fetchMax = false) {
if (!this.isFetching && this.shouldLoad) {
const { buildId, stepName, totalLine, inProgress } = this;
const { buildId, stepName, totalLine } = this;
const started = !!this.stepStartTime;

set(this, 'isFetching', true);
Expand All @@ -277,6 +299,7 @@ export default Component.extend({
// prevent updating logs when component is being destroyed
if (!this.isDestroyed && !this.isDestroying) {
const container = this.$('.wrap')[0];
const { inProgress, justFinished } = this;

set(this, 'isFetching', false);
set(this, 'lastScrollTop', container.scrollTop);
Expand All @@ -288,10 +311,14 @@ export default Component.extend({
cb = inProgress ? 'scrollDown' : 'scrollStill';
}

if (justFinished) {
cb = 'scrollDown';
}

scheduleOnce('afterRender', this, cb);

if (inProgress && !done) {
later(this, 'getLogs', ENV.APP.LOG_RELOAD_TIMER);
if ((justFinished || inProgress) && !done) {
later(this, 'getLogs', justFinished, ENV.APP.LOG_RELOAD_TIMER);
}
}
});
Expand Down Expand Up @@ -345,7 +372,7 @@ export default Component.extend({
}

// autoscroll when the bottom of the logs is roughly in view
set(this, 'autoscroll', this.$('.bottom')[0].getBoundingClientRect().top < 1000);
set(this, 'autoscroll', this.$('.bottom')[0].getBoundingClientRect().top < 1500);
},
toggleTimeDisplay() {
let index = timeTypes.indexOf(this.timeFormat);
Expand Down
2 changes: 1 addition & 1 deletion app/pipeline/build/controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export default Controller.extend({
const pipelineId = this.get('pipeline.id');
const activeStep = getActiveStep(get(build, 'steps'));

if (this.get('preselectedStepName') !== activeStep) {
if (activeStep && this.get('preselectedStepName') !== activeStep) {
this.transitionToRoute('pipeline.build.step', pipelineId, build.get('id'), activeStep);
}
}
Expand Down
12 changes: 8 additions & 4 deletions app/pipeline/build/step/route.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import BuildRoute from '../route';
import Route from '@ember/routing/route';

export default BuildRoute.extend({
export default Route.extend({
routeAfterAuthentication: 'pipeline.build',
model(params) {
this.controllerFor('pipeline.build').set('preselectedStepName', params.step_id);

return this._super(this.paramsFor('pipeline.build'));
// return parent route model
return this.modelFor('pipeline.build');
},
afterModel(model) {
this._super(model);
if (!model) {
return;
}

const stepName = this.controllerFor('pipeline.build').get('preselectedStepName');

Expand Down

0 comments on commit 759b258

Please sign in to comment.