diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java index 899a5b9e5..6a035d9e2 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/support/RestControllersSupport.java @@ -6,6 +6,7 @@ import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; import org.hibernate.exception.ConstraintViolationException; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; @@ -33,8 +34,8 @@ public MetadataResolver findResolverOrThrowHttp404(String resolverResourceId) { return resolver; } - - @ExceptionHandler + //TODO: Review this handler and update accordingly. Do we still need it? + @ExceptionHandler(HttpClientErrorException.class) public ResponseEntity notFoundHandler(HttpClientErrorException ex) { if(ex.getStatusCode() == NOT_FOUND) { return ResponseEntity.status(NOT_FOUND).body(ex.getStatusText()); @@ -46,4 +47,10 @@ public ResponseEntity notFoundHandler(HttpClientErrorException ex) { public ResponseEntity handleDatabaseConstraintViolation(ConstraintViolationException ex) { return ResponseEntity.status(BAD_REQUEST).body(new ErrorResponse("400", "message.database-constraint")); } + + @ExceptionHandler(Exception.class) + public final ResponseEntity handleAllOtherExceptions(Exception ex) { + ErrorResponse errorResponse = new ErrorResponse("400", ex.getLocalizedMessage()); + return new ResponseEntity<>(errorResponse, HttpStatus.BAD_REQUEST); + } } diff --git a/ui/src/app/metadata/domain/model/wizards/metadata-source-base.ts b/ui/src/app/metadata/domain/model/wizards/metadata-source-base.ts index 8a6604c5a..ebc8921d8 100644 --- a/ui/src/app/metadata/domain/model/wizards/metadata-source-base.ts +++ b/ui/src/app/metadata/domain/model/wizards/metadata-source-base.ts @@ -85,7 +85,6 @@ export class MetadataSourceBase implements Wizard { errors.push(error); } }); - console.log(errors, form_current); return errors; }, '/entityId': (value, property, form) => { diff --git a/ui/src/app/metadata/domain/service/draft.service.ts b/ui/src/app/metadata/domain/service/draft.service.ts index b43713359..824e8ec64 100644 --- a/ui/src/app/metadata/domain/service/draft.service.ts +++ b/ui/src/app/metadata/domain/service/draft.service.ts @@ -29,6 +29,10 @@ export class EntityDraftService { ); } + exists(id: string, attr: string = 'id'): boolean { + return this.storage.query().some(entity => entity[attr] === id); + } + save(provider: MetadataResolver): Observable { this.storage.add(provider); return of(provider); diff --git a/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.spec.ts b/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.spec.ts index a7e0dd900..0469b62ca 100644 --- a/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.spec.ts +++ b/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.spec.ts @@ -127,7 +127,7 @@ describe('Dashboard Resolvers List Page', () => { it('should route to the wizard page', () => { spyOn(router, 'navigate'); instance.edit(draft); - expect(router.navigate).toHaveBeenCalledWith(['metadata', 'resolver', 'new', 'blank', 'org-info'], { + expect(router.navigate).toHaveBeenCalledWith(['metadata', 'resolver', 'new', 'blank', 'common'], { queryParams: { id: '1' } }); }); diff --git a/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.ts b/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.ts index e6577886d..537cf198c 100644 --- a/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.ts +++ b/ui/src/app/metadata/manager/container/dashboard-resolvers-list.component.ts @@ -72,7 +72,7 @@ export class DashboardResolversListComponent implements OnInit { edit(entity: MetadataEntity): void { if (entity.isDraft()) { - this.router.navigate(['metadata', 'resolver', 'new', 'blank', 'org-info'], { + this.router.navigate(['metadata', 'resolver', 'new', 'blank', 'common'], { queryParams: { id: entity.getId() } diff --git a/ui/src/app/metadata/resolver/container/new-resolver.component.html b/ui/src/app/metadata/resolver/container/new-resolver.component.html index 924321a93..efaea7239 100644 --- a/ui/src/app/metadata/resolver/container/new-resolver.component.html +++ b/ui/src/app/metadata/resolver/container/new-resolver.component.html @@ -19,7 +19,8 @@

How are you ad class="btn btn-lg btn-block btn-secondary" aria-label="Upload local metadata file or use a metadata URL" role="button" - routerLink="upload" + [routerLink]="['./', 'upload']" + queryParamsHandling="merge" routerLinkActive="btn-success"> Upload/URL @@ -33,7 +34,8 @@

How are you ad class="btn btn-lg btn-block btn-secondary" aria-label="Create metadata source using the wizard" role="button" - [routerLink]="['./']" + [routerLink]="['./', 'blank', 'common']" + queryParamsHandling="merge" routerLinkActive="btn-info"> Create @@ -47,7 +49,8 @@

How are you ad class="btn btn-lg btn-block btn-secondary" aria-label="Copy a metadata source" role="button" - routerLink="copy" + [routerLink]="['./', 'copy']" + queryParamsHandling="merge" routerLinkActive="btn-warning"> Copy diff --git a/ui/src/app/metadata/resolver/container/new-resolver.component.spec.ts b/ui/src/app/metadata/resolver/container/new-resolver.component.spec.ts index bf0f91506..ca735a319 100644 --- a/ui/src/app/metadata/resolver/container/new-resolver.component.spec.ts +++ b/ui/src/app/metadata/resolver/container/new-resolver.component.spec.ts @@ -23,6 +23,7 @@ describe('New Resolver Page', () => { let instance: NewResolverComponent; let activatedRoute: ActivatedRouteStub = new ActivatedRouteStub(); activatedRoute.testParamMap = { id: 'foo', events: of({}) }; + activatedRoute.data = of('foo'); beforeEach(() => { TestBed.configureTestingModule({ diff --git a/ui/src/app/metadata/resolver/container/new-resolver.component.ts b/ui/src/app/metadata/resolver/container/new-resolver.component.ts index ab4f09103..fa51a426d 100644 --- a/ui/src/app/metadata/resolver/container/new-resolver.component.ts +++ b/ui/src/app/metadata/resolver/container/new-resolver.component.ts @@ -27,13 +27,13 @@ export class NewResolverComponent { debounceTime(10), map(url => { let child = this.route.snapshot.firstChild; - return child.routeConfig.path.match('blank').length === 0 || child.params.index === 'common'; + return !child.routeConfig.path.match('blank') || child.params.index === 'common'; }) ); - this.actionsSubscription = this.route.queryParams.pipe( + this.actionsSubscription = this.route.data.pipe( distinctUntilChanged(), - map(params => new SelectDraftRequest(params.id)) + map(data => new SelectDraftRequest(data.draft)) ).subscribe(this.store); } } diff --git a/ui/src/app/metadata/resolver/container/resolver-wizard-step.component.ts b/ui/src/app/metadata/resolver/container/resolver-wizard-step.component.ts index 566291404..bd5dfced5 100644 --- a/ui/src/app/metadata/resolver/container/resolver-wizard-step.component.ts +++ b/ui/src/app/metadata/resolver/container/resolver-wizard-step.component.ts @@ -77,14 +77,10 @@ export class ResolverWizardStepComponent implements OnDestroy { this.valueChangeEmitted$.pipe( withLatestFrom(this.definition$), filter(([ changes, definition ]) => (!!definition && !!changes)), - map(([ changes, definition ]) => definition.parser(changes.value)), - withLatestFrom(this.store.select(fromResolver.getSelectedDraft)), - map(([changes, original]) => ({ ...original, ...changes })) + map(([ changes, definition ]) => definition.parser(changes.value)) ) .subscribe(changes => { - if (changes.id) { - this.store.dispatch(new UpdateChanges(changes)); - } + this.store.dispatch(new UpdateChanges(changes)); }); this.statusChangeEmitted$.pipe(distinctUntilChanged()).subscribe(errors => this.updateStatus(errors)); diff --git a/ui/src/app/metadata/resolver/container/resolver-wizard.component.spec.ts b/ui/src/app/metadata/resolver/container/resolver-wizard.component.spec.ts index fdd16fd5f..877a7298c 100644 --- a/ui/src/app/metadata/resolver/container/resolver-wizard.component.spec.ts +++ b/ui/src/app/metadata/resolver/container/resolver-wizard.component.spec.ts @@ -2,24 +2,22 @@ import { Component, ViewChild } from '@angular/core'; import { TestBed, async, ComponentFixture } from '@angular/core/testing'; import { RouterTestingModule } from '@angular/router/testing'; import { StoreModule, Store, combineReducers } from '@ngrx/store'; - -import { NgbDropdownModule, NgbPopoverModule, NgbModal, NgbModalModule } from '@ng-bootstrap/ng-bootstrap'; +import { RouterStateSnapshot } from '@angular/router'; +import { NgbDropdownModule, NgbPopoverModule, NgbModal } from '@ng-bootstrap/ng-bootstrap'; +import { of } from 'rxjs'; import { ResolverWizardComponent } from './resolver-wizard.component'; import * as fromRoot from '../reducer'; -import { WizardModule } from '../../../wizard/wizard.module'; -import { WizardSummaryComponent } from '../../domain/component/wizard-summary.component'; -import { SummaryPropertyComponent } from '../../domain/component/summary-property.component'; import * as fromWizard from '../../../wizard/reducer'; import { MockI18nModule } from '../../../../testing/i18n.stub'; import { METADATA_SOURCE_WIZARD } from '../wizard-definition'; import { MetadataSourceWizard } from '../../domain/model/wizards/metadata-source-wizard'; import { initialState } from '../reducer/entity.reducer'; import { MockWizardModule } from '../../../../testing/wizard.stub'; -import { RouterStateSnapshot, ActivatedRouteSnapshot } from '@angular/router'; + import { NgbModalStub } from '../../../../testing/modal.stub'; -import { of } from 'rxjs'; import { MetadataResolver } from '../../domain/model'; +import { DifferentialService } from '../../../core/service/differential.service'; @Component({ template: ` @@ -94,6 +92,7 @@ describe('Resolver Wizard Component', () => { TestHostComponent ], providers: [ + DifferentialService, { provide: NgbModal, useClass: NgbModalStub }, { provide: METADATA_SOURCE_WIZARD, useValue: MetadataSourceWizard } ] diff --git a/ui/src/app/metadata/resolver/container/resolver-wizard.component.ts b/ui/src/app/metadata/resolver/container/resolver-wizard.component.ts index d6cac6053..c3c7881f1 100644 --- a/ui/src/app/metadata/resolver/container/resolver-wizard.component.ts +++ b/ui/src/app/metadata/resolver/container/resolver-wizard.component.ts @@ -12,6 +12,7 @@ import { import { Observable, Subject, of, combineLatest as combine } from 'rxjs'; import { skipWhile, startWith, distinctUntilChanged, map, takeUntil, combineLatest } from 'rxjs/operators'; import { Store } from '@ngrx/store'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { MetadataResolver } from '../../domain/model/metadata-resolver'; import * as fromCollections from '../reducer'; @@ -26,8 +27,8 @@ import { SetDefinition, SetIndex, SetDisabled, ClearWizard } from '../../../wiza import * as fromWizard from '../../../wizard/reducer'; import { LoadSchemaRequest } from '../../../wizard/action/wizard.action'; import { UnsavedEntityComponent } from '../../domain/component/unsaved-entity.dialog'; -import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { Clear } from '../action/entity.action'; +import { DifferentialService } from '../../../core/service/differential.service'; @Component({ selector: 'resolver-wizard-page', @@ -66,6 +67,7 @@ export class ResolverWizardComponent implements OnDestroy, CanComponentDeactivat private route: ActivatedRoute, private router: Router, private modalService: NgbModal, + private diffService: DifferentialService, @Inject(METADATA_SOURCE_WIZARD) private sourceWizard: Wizard ) { this.store @@ -113,6 +115,8 @@ export class ResolverWizardComponent implements OnDestroy, CanComponentDeactivat combineLatest(this.resolver$, (changes, base) => ({ ...base, ...changes })) ).subscribe(latest => this.latest = latest); + // this.changes$.subscribe(c => console.log(c)); + this.summary$ = combine( this.store.select(fromWizard.getWizardDefinition), this.store.select(fromWizard.getSchemaCollection), @@ -128,6 +132,7 @@ export class ResolverWizardComponent implements OnDestroy, CanComponentDeactivat ); this.changes$.pipe(takeUntil(this.ngUnsubscribe)).subscribe(c => this.changes = c); + this.resolver$.pipe(takeUntil(this.ngUnsubscribe)).subscribe(r => this.resolver = r); } next(): void { @@ -160,6 +165,18 @@ export class ResolverWizardComponent implements OnDestroy, CanComponentDeactivat this.store.dispatch(new SetIndex(page)); } + hasChanges(changes: MetadataResolver): boolean { + // const updated = this.diffService.updatedDiff(this.resolver, changes); + // const deleted = this.diffService.deletedDiff(this.resolver, changes); + let blacklist = ['id', 'resourceId']; + return Object.keys(changes).filter(key => !(blacklist.indexOf(key) > -1)).length > 0; + } + + isNew(changes: MetadataResolver): boolean { + let blacklist = ['id', 'resourceId']; + return Object.keys(changes).filter(key => !(blacklist.indexOf(key) > -1)).length === 0; + } + ngOnDestroy(): void { this.ngUnsubscribe.next(); this.ngUnsubscribe.complete(); @@ -171,18 +188,23 @@ export class ResolverWizardComponent implements OnDestroy, CanComponentDeactivat currentState: RouterStateSnapshot, nextState: RouterStateSnapshot ): Observable { - if (nextState.url.match('blank') && !!nextState.root.queryParams.id) { return of(true); } - if (Object.keys(this.changes).length > 0) { + if (nextState.url.match('blank') && !!nextState.root.queryParams.id) { + return of(true); + } + if (this.hasChanges(this.changes)) { let modal = this.modalService.open(UnsavedEntityComponent); modal.componentInstance.message = 'resolver'; modal.result.then( () => { this.store.dispatch(new Clear()); - this.router.navigate([nextState.url]); + this.router.navigateByUrl(nextState.url); }, () => console.warn('denied') ); } + if (this.isNew(this.latest)) { + return of(true); + } return this.store.select(fromResolver.getEntityIsSaved); } } diff --git a/ui/src/app/metadata/resolver/effect/collection.effects.ts b/ui/src/app/metadata/resolver/effect/collection.effects.ts index 8824a02b3..d56110a7e 100644 --- a/ui/src/app/metadata/resolver/effect/collection.effects.ts +++ b/ui/src/app/metadata/resolver/effect/collection.effects.ts @@ -111,7 +111,10 @@ export class ResolverCollectionEffects { addResolverSuccessRemoveDraft$ = this.actions$.pipe( ofType(ResolverCollectionActionTypes.ADD_RESOLVER_SUCCESS), map(action => action.payload), - map(provider => new draftActions.RemoveDraftRequest(provider)) + map(provider => { + console.log(provider); + return new draftActions.RemoveDraftRequest(provider); + }) ); @Effect() diff --git a/ui/src/app/metadata/resolver/effect/draft-collection.effects.ts b/ui/src/app/metadata/resolver/effect/draft-collection.effects.ts index f6e203381..ac94d2410 100644 --- a/ui/src/app/metadata/resolver/effect/draft-collection.effects.ts +++ b/ui/src/app/metadata/resolver/effect/draft-collection.effects.ts @@ -63,13 +63,6 @@ export class DraftCollectionEffects { ) ); - @Effect({ dispatch: false }) - addDraftSuccessRedirect$ = this.actions$.pipe( - ofType(DraftActionTypes.ADD_DRAFT_SUCCESS), - map(getPayload), - tap(provider => this.router.navigate(['metadata', 'resolver', provider.entityId, 'wizard'])) - ); - @Effect() updateDraft$ = this.actions$.pipe( ofType(DraftActionTypes.UPDATE_DRAFT_REQUEST), @@ -107,24 +100,6 @@ export class DraftCollectionEffects { map(id => new actions.LoadDraftRequest()) ); - @Effect() - selectDraftError$ = this.actions$.pipe( - ofType(DraftActionTypes.SELECT_ERROR), - map(getPayload), - switchMap(id => - this.draftService - .save({ id: `r-${ Date.now() }`, serviceProviderName: '' }) - .pipe( - map(p => new SelectDraftRequest(p.id)), - catchError(e => of(new SelectDraftError())) - ) - ), - tap(() => { - // this.store.dispatch(new ClearWizard()); - this.store.dispatch(new Clear()); - }) - ); - @Effect() removeDraft$ = this.actions$.pipe( ofType(DraftActionTypes.REMOVE_DRAFT), diff --git a/ui/src/app/metadata/resolver/effect/wizard.effect.ts b/ui/src/app/metadata/resolver/effect/wizard.effect.ts index 69d24f732..1632fb441 100644 --- a/ui/src/app/metadata/resolver/effect/wizard.effect.ts +++ b/ui/src/app/metadata/resolver/effect/wizard.effect.ts @@ -1,7 +1,7 @@ import { Injectable } from '@angular/core'; import { Effect, Actions, ofType } from '@ngrx/effects'; import { ActivatedRoute, Router } from '@angular/router'; -import { map, filter, tap } from 'rxjs/operators'; +import { map, filter, tap, withLatestFrom } from 'rxjs/operators'; import { Store } from '@ngrx/store'; import { @@ -29,7 +29,8 @@ export class WizardEffects { ofType(ResolverEntityActionTypes.UPDATE_CHANGES), map(action => action.payload), filter(provider => !provider.createdDate), - map(provider => new UpdateDraftRequest(provider)) + withLatestFrom(this.store.select(fromResolver.getSelectedDraft)), + map(([provider, draft]) => new UpdateDraftRequest({ ...draft, ...provider })) ); @Effect() diff --git a/ui/src/app/metadata/resolver/resolver.module.ts b/ui/src/app/metadata/resolver/resolver.module.ts index 39f4b67f1..c5290431f 100644 --- a/ui/src/app/metadata/resolver/resolver.module.ts +++ b/ui/src/app/metadata/resolver/resolver.module.ts @@ -35,6 +35,7 @@ import { ResolverSelectComponent } from './container/resolver-select.component'; import { MetadataSourceEditor } from '../domain/model/wizards/metadata-source-editor'; import { FinishFormComponent } from './component/finish-form.component'; import { ProviderFormFragmentComponent } from './component/provider-form-fragment.component'; +import { CreateDraftResolverService } from './service/create-draft.resolver'; @NgModule({ declarations: [ @@ -75,7 +76,8 @@ export class ResolverModule { return { ngModule: RootResolverModule, providers: [ - CopyIsSetGuard + CopyIsSetGuard, + CreateDraftResolverService ] }; } diff --git a/ui/src/app/metadata/resolver/resolver.routing.ts b/ui/src/app/metadata/resolver/resolver.routing.ts index 1d080bd5f..9916e2206 100644 --- a/ui/src/app/metadata/resolver/resolver.routing.ts +++ b/ui/src/app/metadata/resolver/resolver.routing.ts @@ -13,6 +13,7 @@ import { ResolverWizardStepComponent } from './container/resolver-wizard-step.co import { ResolverEditComponent } from './container/resolver-edit.component'; import { ResolverEditStepComponent } from './container/resolver-edit-step.component'; import { ResolverSelectComponent } from './container/resolver-select.component'; +import { CreateDraftResolverService } from './service/create-draft.resolver'; export const ResolverRoutes: Routes = [ { @@ -21,6 +22,9 @@ export const ResolverRoutes: Routes = [ { path: 'new', component: NewResolverComponent, + resolve: { + draft: CreateDraftResolverService + }, children: [ { path: '', redirectTo: 'blank/common', pathMatch: 'prefix' }, { diff --git a/ui/src/app/metadata/resolver/service/create-draft.resolver.ts b/ui/src/app/metadata/resolver/service/create-draft.resolver.ts new file mode 100644 index 000000000..1c6cb84a0 --- /dev/null +++ b/ui/src/app/metadata/resolver/service/create-draft.resolver.ts @@ -0,0 +1,35 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; +import { Observable, of } from 'rxjs'; +import { Store } from '@ngrx/store'; +import { MetadataResolver } from '../../domain/model'; +import { AddDraftRequest } from '../action/draft.action'; +import * as fromResolver from '../reducer'; +import { EntityDraftService } from '../../domain/service/draft.service'; + +@Injectable() +export class CreateDraftResolverService { + constructor( + private store: Store, + private draftService: EntityDraftService + ) { } + + resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable | Observable { + let id = route.queryParamMap.get('id'); + if (id) { + let exists = this.draftService.exists(id); + if (!exists) { + let resolver = {id}; + this.store.dispatch(new AddDraftRequest(resolver)); + } + } + if (!id) { + let resolver = { + id: `r-${Date.now()}` + }; + id = resolver.id; + this.store.dispatch(new AddDraftRequest(resolver)); + } + return of(id); + } +} diff --git a/ui/src/testing/activated-route.stub.ts b/ui/src/testing/activated-route.stub.ts index 0620096d5..062493069 100644 --- a/ui/src/testing/activated-route.stub.ts +++ b/ui/src/testing/activated-route.stub.ts @@ -3,6 +3,7 @@ import { Injectable } from '@angular/core'; import { BehaviorSubject } from 'rxjs/BehaviorSubject'; import { convertToParamMap, ParamMap, ActivatedRoute } from '@angular/router'; +import { Observable, of } from 'rxjs'; @Injectable() export class ActivatedRouteStub { @@ -16,6 +17,8 @@ export class ActivatedRouteStub { private _firstChild: ActivatedRouteStub; + private _data: Observable; + get testParamMap() { return this._testParamMap; } set testParamMap(params: {}) { this._testParamMap = convertToParamMap(params); @@ -44,4 +47,12 @@ export class ActivatedRouteStub { set firstChild(stub: ActivatedRouteStub) { this._firstChild = stub; } + + get data(): Observable { + return this._data; + } + + set data(d: Observable) { + this._data = d; + } }