ensure @Input bindings are not undefined before executing trigger logic

This commit is contained in:
alecmerdler 2017-05-26 11:50:11 -07:00
parent 4750d1c5ef
commit d5e35156e0
6 changed files with 64 additions and 53 deletions

View file

@ -1,34 +1,35 @@
import { LinearWorkflowSectionComponent } from './linear-workflow-section.component';
import { LinearWorkflowComponent } from './linear-workflow.component';
import { SimpleChanges } from 'ng-metadata/core';
import { Mock } from 'ts-mocks';
import Spy = jasmine.Spy;
describe("LinearWorkflowSectionComponent", () => {
var component: LinearWorkflowSectionComponent;
var parentMock: LinearWorkflowComponent;
var parentMock: Mock<LinearWorkflowComponent>;
beforeEach(() => {
parentMock = new LinearWorkflowComponent();
component = new LinearWorkflowSectionComponent(parentMock);
parentMock = new Mock<LinearWorkflowComponent>();
component = new LinearWorkflowSectionComponent(parentMock.Object);
component.sectionId = "mysection";
});
describe("ngOnInit", () => {
it("calls parent component to add itself as a section", () => {
var addSectionSpy: Spy = spyOn(parentMock, "addSection").and.returnValue(null);
parentMock.setup(mock => mock.addSection).is((section) => null);
component.ngOnInit();
expect(addSectionSpy.calls.argsFor(0)[0]).toBe(component);
expect((<Spy>parentMock.Object.addSection).calls.argsFor(0)[0]).toBe(component);
});
});
describe("ngOnChanges", () => {
var onSectionInvalidSpy: Spy;
var changesObj: SimpleChanges;
beforeEach(() => {
onSectionInvalidSpy = spyOn(parentMock, "onSectionInvalid").and.returnValue(null);
parentMock.setup(mock => mock.onSectionInvalid).is((section) => null);
changesObj = {
sectionValid: {
currentValue: true,
@ -46,51 +47,58 @@ describe("LinearWorkflowSectionComponent", () => {
it("does nothing if 'sectionValid' input not changed", () => {
component.ngOnChanges({});
expect(onSectionInvalidSpy).not.toHaveBeenCalled();
expect((<Spy>parentMock.Object.onSectionInvalid)).not.toHaveBeenCalled();
});
it("does nothing if 'sectionValid' input's previous value is falsy", () => {
changesObj['sectionValid'].previousValue = null;
component.ngOnChanges(changesObj);
expect((<Spy>parentMock.Object.onSectionInvalid)).not.toHaveBeenCalled();
});
it("does nothing if 'sectionValid' input is true", () => {
component.ngOnChanges(changesObj);
expect(onSectionInvalidSpy).not.toHaveBeenCalled();
expect((<Spy>parentMock.Object.onSectionInvalid)).not.toHaveBeenCalled();
});
it("calls parent method to inform that section is invalid if 'sectionValid' input changed to false", () => {
changesObj['sectionValid'].previousValue = true;
changesObj['sectionValid'].currentValue = false;
component.ngOnChanges(changesObj);
expect(onSectionInvalidSpy.calls.argsFor(0)[0]).toEqual(component.sectionId);
expect((<Spy>parentMock.Object.onSectionInvalid).calls.argsFor(0)[0]).toEqual(component.sectionId);
});
it("calls parent method to go to next section if 'skipSection' input is true and is current section", () => {
delete changesObj['sectionValid'];
const onNextSectionSpy: Spy = spyOn(parentMock, 'onNextSection').and.returnValue(null);
parentMock.setup(mock => mock.onNextSection).is(() => null);
component.isCurrentSection = true;
component.ngOnChanges(changesObj);
expect(onNextSectionSpy).toHaveBeenCalled();
expect(<Spy>parentMock.Object.onNextSection).toHaveBeenCalled();
});
});
describe("onSubmitSection", () => {
var onNextSectionSpy: Spy;
beforeEach(() => {
onNextSectionSpy = spyOn(parentMock, "onNextSection").and.returnValue(null);
parentMock.setup(mock => mock.onNextSection).is(() => null);
});
it("does nothing if section is invalid", () => {
component.sectionValid = false;
component.onSubmitSection();
expect(onNextSectionSpy).not.toHaveBeenCalled();
expect(<Spy>parentMock.Object.onNextSection).not.toHaveBeenCalled();
});
it("calls parent method to go to next section if section is valid", () => {
component.sectionValid = true;
component.onSubmitSection();
expect(onNextSectionSpy).toHaveBeenCalled();
expect(<Spy>parentMock.Object.onNextSection).toHaveBeenCalled();
});
});
});

View file

@ -24,7 +24,7 @@ export class LinearWorkflowSectionComponent implements OnChanges, OnInit {
constructor(@Host() @Inject(LinearWorkflowComponent) private parent: LinearWorkflowComponent) {
}
public ngOnInit(): void {
if (!this.skipSection) {
this.parent.addSection(this);
@ -34,7 +34,7 @@ export class LinearWorkflowSectionComponent implements OnChanges, OnInit {
public ngOnChanges(changes: SimpleChanges): void {
switch (Object.keys(changes)[0]) {
case 'sectionValid':
if (!changes['sectionValid'].currentValue && this.parent) {
if (changes['sectionValid'].previousValue && !changes['sectionValid'].currentValue && this.parent) {
this.parent.onSectionInvalid(this.sectionId);
}
break;

View file

@ -6,7 +6,7 @@
section-id="namespace"
section-title="::{{ 'Select ' + $ctrl.namespaceTitle }}"
section-valid="$ctrl.local.selectedNamespace"
skip-section="::$ctrl.githost == 'custom-git'">
skip-section="$ctrl.githost == 'custom-git'">
<div class="col-lg-7 col-md-7 col-sm-12 main-col"
ng-if="$ctrl.local.namespaces">
<h3>Select {{ ::$ctrl.namespaceTitle }}</h3>
@ -72,7 +72,7 @@
section-id="repo"
section-title="Select Repository"
section-valid="$ctrl.local.selectedRepository.full_name"
skip-section="::$ctrl.githost == 'custom-git'">
skip-section="$ctrl.githost == 'custom-git'">
<div class="col-lg-7 col-md-7 col-sm-12 main-col"
ng-if="$ctrl.local.repositories">
<h3>Select Repository</h3>
@ -164,7 +164,7 @@
section-id="repo"
section-title="Git Repository"
section-valid="$ctrl.local.selectedRepository.full_name"
skip-section="::$ctrl.githost != 'custom-git'">
skip-section="$ctrl.githost != 'custom-git'">
<div class="col-lg-7 col-md-7 col-sm-12 main-col">
<h3>Enter repository</h3>

View file

@ -31,7 +31,7 @@ describe("ManageTriggerComponent", () => {
component.trigger = {id: "2cac6317-754e-47d4-88d3-2a50b3f09ee3", service: "github"};
});
describe("ngOnInit", () => {
describe("ngOnChanges", () => {
beforeEach(() => {
apiServiceMock.setup(mock => mock.listTriggerBuildSourceNamespaces).is(() => Promise.resolve({}));
@ -40,7 +40,7 @@ describe("ManageTriggerComponent", () => {
});
it("sets default values for config and selected namespace", () => {
component.ngOnInit();
component.ngOnChanges({});
expect(component.config).toEqual({});
expect(component.local.selectedNamespace).toBe(null);

View file

@ -1,4 +1,4 @@
import { Input, Output, Component, Inject, EventEmitter, OnInit } from 'ng-metadata/core';
import { Input, Output, Component, Inject, EventEmitter, OnChanges, SimpleChanges } from 'ng-metadata/core';
import * as moment from 'moment';
import { Local, Trigger, TriggerConfig, Repository, Namespace } from '../../../types/common.types';
import { ContextChangeEvent } from '../context-path-select/context-path-select.component';
@ -12,7 +12,7 @@ import { PathChangeEvent } from '../dockerfile-path-select/dockerfile-path-selec
selector: 'manage-trigger',
templateUrl: '/static/js/directives/ui/manage-trigger/manage-trigger.component.html'
})
export class ManageTriggerComponent implements OnInit {
export class ManageTriggerComponent implements OnChanges {
@Input('<') public githost: string = 'custom-git';
@Input('<') public repository: Repository;
@ -43,26 +43,28 @@ export class ManageTriggerComponent implements OnInit {
}
public ngOnInit(): void {
this.config = this.trigger.config || {};
this.namespaceTitle = 'organization';
this.local.selectedNamespace = null;
if (this.githost != 'custom-git') {
this.loadNamespaces();
}
public ngOnChanges(changes: SimpleChanges): void {
if (this.githost && this.repository && this.trigger) {
this.config = this.trigger.config || {};
this.namespaceTitle = 'organization';
this.local.selectedNamespace = null;
if (this.githost != 'custom-git') {
this.loadNamespaces();
}
// FIXME: Need to have watchers here because cor-table doesn't have ng-change functionality yet
this.$scope.$watch(() => this.local.selectedNamespace, (namespace: Namespace) => {
if (namespace) {
this.loadRepositories(namespace);
}
});
this.$scope.$watch(() => this.local.selectedRepository, (selectedRepository: Repository) => {
if (selectedRepository && this.githost != 'custom-git') {
this.loadRepositoryRefs(selectedRepository);
this.loadDockerfileLocations(selectedRepository);
}
});
// FIXME (Alec 5/26/17): Need to have watchers here because cor-table doesn't have ng-change functionality yet
this.$scope.$watch(() => this.local.selectedNamespace, (namespace: Namespace) => {
if (namespace) {
this.loadRepositories(namespace);
}
});
this.$scope.$watch(() => this.local.selectedRepository, (selectedRepository: Repository) => {
if (selectedRepository && this.githost != 'custom-git') {
this.loadRepositoryRefs(selectedRepository);
this.loadDockerfileLocations(selectedRepository);
}
});
}
}
public getTriggerIcon(): any {
@ -120,20 +122,21 @@ export class ManageTriggerComponent implements OnInit {
return this.apiService.getRobots(this.repository.namespace, null, {'permissions': true});
} else {
this.local.triggerAnalysis = Object.assign({}, resp);
this.buildOrderedRobotAccounts();
}
})
.catch((error) => {
this.apiService.errorDisplay('Could not analyze trigger');
})
.then((resp) => {
this.local.triggerAnalysis = {
status: 'publicbase',
is_admin: true,
robots: resp.robots,
name: this.repository.name,
namespace: this.repository.namespace
};
if (resp) {
this.local.triggerAnalysis = {
status: 'publicbase',
is_admin: true,
robots: resp.robots,
name: this.repository.name,
namespace: this.repository.namespace
};
}
this.buildOrderedRobotAccounts();
})
.catch((error) => {

View file

@ -38,7 +38,7 @@
</div> <!-- /state = activated -->
<!-- state = managing or activating -->
<div ng-if="state == 'managing' || state == 'activating'"
<div ng-if="(state == 'managing' || state == 'activating') && trigger"
ng-class="{'activating': state == 'activating'}">
<manage-trigger githost="trigger.service"
trigger="trigger"