Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"@angular/common": "6.0.3",
"@angular/compiler": "6.0.3",
"@angular/core": "6.0.3",
"@angular/flex-layout": "^6.0.0-beta.16",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't look like this is used within Stratos, however removing results in the following error

ERROR in ../../angular6-json-schema-form/angular6-json-schema-form.ts(38,53): Error during template compile of 'MaterialDesignFrameworkModule'
  Could not resolve @angular/flex-layout relative to /home/richard/dev/github/stratos-ui-orange-cloudfoundry/node_modules/angular6-json-schema-form/angular6-json-schema-form.d.ts..

Should this be a dependency of angular6-json-schema-form?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the material design module uses @angular/flex-layout. I'm open to better approachs

"@angular/forms": "6.0.3",
"@angular/http": "6.0.3",
"@angular/material": "^6.1.0",
Expand All @@ -59,6 +60,7 @@
"@ngrx/store-devtools": "^6.0.1",
"@swimlane/ngx-charts": "^8.1.0",
"angular2-virtual-scroll": "^0.3.1",
"angular6-json-schema-form": "1.0.3",
"core-js": "^2.5.7",
"hammerjs": "^2.0.8",
"hyperlist": "^1.0.0-beta",
Expand Down
2 changes: 2 additions & 0 deletions src/frontend/app/core/cf-api-svc.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ export interface IServicePlan {
service?: APIResource<IService>;
guid?: string;
cfGuid?: string;
schemas?: any;
Comment thread
richard-cox marked this conversation as resolved.
}

export interface IServicePlanExtra {
displayName: string;
bullets: string[];
}

export interface IService {
label: string;
description: string;
Expand Down
46 changes: 32 additions & 14 deletions src/frontend/app/features/service-catalog/services-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { createEntityRelationPaginationKey } from '../../store/helpers/entity-re
import { getPaginationObservables } from '../../store/reducers/pagination-reducer/pagination-reducer.helper';
import { APIResource } from '../../store/types/api.types';
import { getIdFromRoute } from '../cloud-foundry/cf.helpers';
import { JsonPointer } from 'angular6-json-schema-form';


export const getSvcAvailability = (servicePlan: APIResource<IServicePlan>,
Expand Down Expand Up @@ -90,23 +91,40 @@ export const getServicePlans = (
cfGuid: string,
store: Store<AppState>,
paginationMonitorFactory: PaginationMonitorFactory
): Observable<APIResource<IServicePlan>[]> => {
): Observable<APIResource<IServicePlan>[]> => {
return service$.pipe(
filter(p => !!p),
switchMap(service => {
if (service.entity.service_plans && service.entity.service_plans.length > 0) {
return observableOf(service.entity.service_plans);
if (service.entity.service_plans && service.entity.service_plans.length > 0) {
return observableOf(service.entity.service_plans);
} else {
const guid = service.metadata.guid;
const paginationKey = createEntityRelationPaginationKey(servicePlanSchemaKey, guid);
const getServicePlansAction = new GetServicePlansForService(guid, cfGuid, paginationKey);
// Could be a space-scoped service, make a request to fetch the plan
return getPaginationObservables<APIResource<IServicePlan>>({
store: store,
action: getServicePlansAction,
paginationMonitor: paginationMonitorFactory.create(getServicePlansAction.paginationKey, entityFactory(servicePlanSchemaKey))
}, true)
.entities$.pipe(share(), first());
}
}));
};

export const prettyValidationErrors = (formValidationErrors) => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formValidationErrors needs a type, that would open up the easier formValidationErrors.forEach iteration function. If you're feeling fancy you could even use formValidationErrors.reduce instead, which would both iterate over formValidationErrors and output the errorArray

if (!formValidationErrors) { return null; }
const errorArray = [];
for (const error of formValidationErrors) {
const message = error.message;
const dataPathArray = JsonPointer.parse(error.dataPath);
if (dataPathArray.length) {
let field: any;
dataPathArray.forEach(elm => field += /^\d+$/.test(elm) ? `[${elm}]` : `.${elm}`);
errorArray.push(`${field}: ${message}`);
} else {
const guid = service.metadata.guid;
const paginationKey = createEntityRelationPaginationKey(servicePlanSchemaKey, guid);
const getServicePlansAction = new GetServicePlansForService(guid, cfGuid, paginationKey);
// Could be a space-scoped service, make a request to fetch the plan
return getPaginationObservables<APIResource<IServicePlan>>({
store: store,
action: getServicePlansAction,
paginationMonitor: paginationMonitorFactory.create(getServicePlansAction.paginationKey, entityFactory(servicePlanSchemaKey))
}, true)
.entities$.pipe(share(), first());
errorArray.push(message);
}
}));
}
return errorArray.join('<br>');
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<app-step title="Select Plan" [onNext]="selectPlan.onNext" [blocked]="initialisedService$ | async" [onEnter]="selectPlan.onEnter" [valid]="selectPlan.validate | async" cancelButtonText="Cancel">
<app-select-plan-step #selectPlan></app-select-plan-step>
</app-step>
<app-step [title]="bindAppStepperText" *ngIf="modeService.viewDetail.showBindApp" [skip]="skipApps$ | async" [onNext]="bindApp.submit " [valid]="bindApp.validate | async" cancelButtonText="Cancel" finishButtonText="Bind">
<app-step [title]="bindAppStepperText" *ngIf="modeService.viewDetail.showBindApp" [skip]="skipApps$ | async" [onEnter]="bindApp.onEnter" [onNext]="bindApp.submit " [valid]="bindApp.validate | async" cancelButtonText="Cancel" finishButtonText="Bind">
<app-bind-apps-step #bindApp [boundAppId]="appId"></app-bind-apps-step>
</app-step>
<app-step title="Service Instance" [onNext]="specifyDetails.onNext" [onEnter]="specifyDetails.onEnter" [blocked]="!!(specifyDetails.serviceInstancesInit$ | async)" [valid]=" specifyDetails.validate | async " cancelButtonText="Cancel " nextButtonText="Create ">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { SelectPlanStepComponent } from '../select-plan-step/select-plan-step.co
import { SelectServiceComponent } from '../select-service/select-service.component';
import { SpecifyDetailsStepComponent } from '../specify-details-step/specify-details-step.component';
import { AddServiceInstanceComponent } from './add-service-instance.component';
import { MaterialDesignFrameworkModule } from 'angular6-json-schema-form';

describe('AddServiceInstanceComponent', () => {
let component: AddServiceInstanceComponent;
Expand All @@ -40,6 +41,7 @@ describe('AddServiceInstanceComponent', () => {
imports: [
PageHeaderModule,
SteppersModule,
MaterialDesignFrameworkModule,
// CoreModule,
BaseTestModulesNoShared
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,25 @@
</mat-error>
</mat-form-field>
</form>
<div *ngIf="!!schema" class="json-schema">
<mat-card class="json-schema-form">
<mat-card-header>
<mat-card-title><b>Generated Form</b><button mat-button color="accent" (click)="showJsonSchema=!showJsonSchema">Json schema</button>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The button needs some padding-left, to ensure the 'on hover' button background has space between itself and text. Should also consider whether this button is something we should hide in production. @KlapTrap thoughts?

</mat-card-title>
</mat-card-header>
<mat-card-content>
<json-schema-form loadExternalAssets="true" [options]="jsonFormOptions" [schema]="schema" [framework]="selectedFramework" (validationErrors)="validationErrors($event)" (onChanges)="onFormChange($event)">
</json-schema-form>
<div *ngIf="!!prettyValidationErrors" class="data-bad" [innerHTML]="prettyValidationErrors"></div>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For class names we use BEM notation (for an intro see http://getbem.com/introduction/). At the moment we have BEM checking turned off in our pipeline, but at some point in the future we'd like to turn it back on again.

</mat-card-content>
</mat-card>
<mat-card *ngIf="showJsonSchema" class="json-schema-data">
<mat-card-header>
<mat-card-title><b>Json schema</b></mat-card-title>
</mat-card-header>
<mat-card-content>
<pre class="display-json">{{schema | json}} </pre>
</mat-card-content>
</mat-card>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,3 +1,31 @@
@import '../../../../../sass/mixins';
:host {
flex: 1;
}

@mixin json-schema-component() {
.data-bad { background-color: #fcc; }
.display-json {
white-space: -moz-pre-wrap;
white-space: -pre-wrap;
white-space: -o-pre-wrap;
white-space: pre-wrap;
word-wrap: break-word;
}
.json-schema {
display: flex;
flex-direction: column;
@include breakpoint(laptop) {
flex-direction: row;
}
}
.json-schema-data,
.json-schema-form {
width: 100%;
@include breakpoint(laptop) {
width: 50%;
}
}
}

@include json-schema-component();
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ServicesService } from '../../../../features/service-catalog/services.s
import { ServicesServiceMock } from '../../../../features/service-catalog/services.service.mock';
import { CsiGuidsService } from '../csi-guids.service';
import { PaginationMonitorFactory } from '../../../monitors/pagination-monitor.factory';
import { MaterialDesignFrameworkModule } from 'angular6-json-schema-form';

describe('BindAppsStepComponent', () => {
let component: BindAppsStepComponent;
Expand All @@ -14,7 +15,7 @@ describe('BindAppsStepComponent', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [BindAppsStepComponent],
imports: [BaseTestModulesNoShared],
imports: [BaseTestModulesNoShared , MaterialDesignFrameworkModule],
providers: [
{ provide: ServicesService, useClass: ServicesServiceMock },
CsiGuidsService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { PaginationMonitorFactory } from '../../../monitors/pagination-monitor.f
import { StepOnNextResult } from '../../stepper/step/step.component';
import { CsiGuidsService } from '../csi-guids.service';
import { SpecifyDetailsStepComponent } from '../specify-details-step/specify-details-step.component';
import { safeUnsubscribe, prettyValidationErrors } from '../../../../features/service-catalog/services-helper';

@Component({
selector: 'app-bind-apps-step',
Expand All @@ -35,6 +36,14 @@ export class BindAppsStepComponent implements OnDestroy, AfterContentInit {
stepperForm: FormGroup;
apps$: Observable<APIResource<IApp>[]>;
guideText = 'Specify the application to bind (Optional)';

selectedFramework = 'material-design';
schema: any;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible it would be nice to type the fields and function params in this class.

showJsonSchema: boolean;
jsonFormOptions: any = { addSubmit: false };
selectedServiceSubscription: Subscription;
formValidationErrors: any;
selectedService$: any;
constructor(
private store: Store<AppState>,
private paginationMonitorFactory: PaginationMonitorFactory
Expand All @@ -53,6 +62,21 @@ export class BindAppsStepComponent implements OnDestroy, AfterContentInit {
}
}

onFormChange(jsonData) {
if (!!jsonData) {
const stringData = JSON.stringify(jsonData);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For safety, in both places where JSON.stringify is used, this should be wrapped in a try catch block

this.stepperForm.get('params').setValue(stringData);
}
}

validationErrors(data: any): void {
this.formValidationErrors = data;
}

get prettyValidationErrors() {
return prettyValidationErrors(this.formValidationErrors);
}

ngAfterContentInit() {
this.validateSubscription = this.stepperForm.statusChanges.pipe(
map(() => {
Expand Down Expand Up @@ -80,9 +104,26 @@ export class BindAppsStepComponent implements OnDestroy, AfterContentInit {
this.setBoundApp();
}

onEnter = (selectedService$?) => {
this.selectedService$ = selectedService$;
if (selectedService$ instanceof Observable) {
this.selectedServiceSubscription = selectedService$
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't expect selectedService$ to change, or if this.schema never needs to be updated (which I don't think it does in this component), you can use a first() (closes source observable after it emits it's first value) instead of having to subscribers and unsubscribe. So it might look something like...

selectedService$.pipe(
    first()
).subscribe(selectedService => {
    this.schema = this.filterSchema(selectedService.entity.entity.schemas.service_binding.create.parameters);
})

Alternatively, this.schema can be changed to an Observable

this.schema$ = selectedService$.pipe(
    map(selectedService => 
     this.filterSchema(selectedService.entity.entity.schemas.service_binding.create.parameters);
)
*ngIf="!!schema" --> *ngIf="(schema$ | async) as schema

.subscribe(selectedService => {
this.schema = this.filterSchema(selectedService.entity.entity.schemas.service_binding.create.parameters);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the two places this is used need to check whether if schemas has a value (currently gives undefined exception for services without schemas`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full exception: Cannot read property 'service_binding' of undefined

});
}
}

private filterSchema = (schema: any): any => {
return Object.keys(schema).reduce((obj, key) => {
if (key !== '$schema') { obj[key] = schema[key]; }
return obj;
}, {});
}

submit = (): Observable<StepOnNextResult> => {
this.setApp();
return observableOf({ success: true });
return observableOf({ success: true, data: this.selectedService$ });
}

setApp = () => this.store.dispatch(
Expand All @@ -91,6 +132,7 @@ export class BindAppsStepComponent implements OnDestroy, AfterContentInit {

ngOnDestroy(): void {
this.validateSubscription.unsubscribe();
safeUnsubscribe(this.selectedServiceSubscription);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export class SelectPlanStepComponent implements OnDestroy {

onNext = (): Observable<StepOnNextResult> => {
this.store.dispatch(new SetCreateServiceInstanceServicePlan(this.stepperForm.controls.servicePlans.value));
return observableOf({ success: true });
return observableOf({ success: true, data: this.selectedService$ });
}

ngOnDestroy(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,26 @@
<mat-option *ngFor="let sI of bindableServiceInstances$ | async" [disabled]="!sI.metadata.guid" [value]="sI.metadata.guid">{{ sI.entity.name }}</mat-option>
</mat-select>
</mat-form-field>

</form>
<div *ngIf="!!schema" class="json-schema">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a fair bit of duplication between this step and bind app (html, input, error handling, etc). Could the common parts be pulled out into a new component ServiceBindingForm? This would remove a lot of duplicated html, plumming in the component's code and i think the possibly the need for the scss mixins

<mat-card class="json-schema-form">
<mat-card-header>
<mat-card-title><b>Generated Form</b><button mat-button color="accent" (click)="showJsonSchema=!showJsonSchema">Json schema</button>
</mat-card-title>
</mat-card-header>
<mat-card-content>
<json-schema-form loadExternalAssets="true" [options]="jsonFormOptions" [schema]="schema" [framework]="selectedFramework" (validationErrors)="validationErrors($event)" (onChanges)="onFormChange($event)">
</json-schema-form>
<div *ngIf="!!prettyValidationErrors" class="data-bad" [innerHTML]="prettyValidationErrors"></div>
</mat-card-content>
</mat-card>
<mat-card *ngIf="showJsonSchema" class="json-schema-data">
<mat-card-header>
<mat-card-title><b>Json schema</b></mat-card-title>
</mat-card-header>
<mat-card-content>
<pre class="display-json">{{schema | json}} </pre>
</mat-card-content>
</mat-card>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
@import '../bind-apps-step/bind-apps-step.component';
:host {
flex: 1;
}
Expand All @@ -23,3 +24,5 @@
}
}
}

@include json-schema-component();
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { CsiGuidsService } from '../csi-guids.service';
import { PaginationMonitorFactory } from '../../../monitors/pagination-monitor.factory';
import { EntityMonitorFactory } from '../../../monitors/entity-monitor.factory.service';
import { CsiModeService } from '../csi-mode.service';
import { MaterialDesignFrameworkModule } from 'angular6-json-schema-form';

describe('SpecifyDetailsStepComponent', () => {
let component: SpecifyDetailsStepComponent;
Expand All @@ -15,7 +16,7 @@ describe('SpecifyDetailsStepComponent', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [SpecifyDetailsStepComponent],
imports: [BaseTestModulesNoShared],
imports: [BaseTestModulesNoShared, MaterialDesignFrameworkModule],
providers: [
CreateServiceInstanceHelperServiceFactory,
CsiGuidsService,
Expand Down
Loading