From 4e7e5177e16e76ace36012e9d0cfdc31e60af736 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Thu, 9 Aug 2018 15:09:53 -0700 Subject: [PATCH 01/17] SHIBUI-668 Added icon to nav --- .../wizard/component/wizard.component.html | 5 ++-- .../app/wizard/component/wizard.component.ts | 24 ++++++++++++++----- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/ui/src/app/wizard/component/wizard.component.html b/ui/src/app/wizard/component/wizard.component.html index 3d62f7d7a..587605407 100644 --- a/ui/src/app/wizard/component/wizard.component.html +++ b/ui/src/app/wizard/component/wizard.component.html @@ -16,8 +16,9 @@

  • {{ (current$ | async).index }} - - {{ (current$ | async).index }}. + + {{ (current$ | async).index }}. + {{ (current$ | async).label }}

    diff --git a/ui/src/app/wizard/component/wizard.component.ts b/ui/src/app/wizard/component/wizard.component.ts index da9cdafb0..a75a3bb2d 100644 --- a/ui/src/app/wizard/component/wizard.component.ts +++ b/ui/src/app/wizard/component/wizard.component.ts @@ -3,6 +3,13 @@ import { Store } from '@ngrx/store'; import { Wizard, WizardStep } from '../model'; import * as fromWizard from '../reducer'; import { Observable } from 'rxjs'; +import { withLatestFrom, map } from 'rxjs/operators'; + +export enum ICONS { + CHECK = 'CHECK', + INDEX = 'INDEX' +} + /*tslint:disable:component-selector */ @Component({ @@ -10,7 +17,7 @@ import { Observable } from 'rxjs'; templateUrl: './wizard.component.html', styleUrls: ['./wizard.component.scss'] }) -export class WizardComponent implements OnChanges { +export class WizardComponent { @Output() onNext = new EventEmitter(); @Output() onPrevious = new EventEmitter(); @Output() onLast = new EventEmitter(); @@ -29,6 +36,10 @@ export class WizardComponent implements OnChanges { current$: Observable; last$: Observable; + currentIcon$: Observable; + + icons = ICONS; + constructor( private store: Store ) { @@ -39,11 +50,12 @@ export class WizardComponent implements OnChanges { this.next$ = this.store.select(fromWizard.getNext); this.current$ = this.store.select(fromWizard.getCurrent); this.last$ = this.store.select(fromWizard.getLast); - } - ngOnChanges(): void { - // this.currentPage = this.wizard.find(r => r.index === this.index); - // this.previousPage = this.wizard.find(r => r.index === this.index - 1); - // this.nextPage = this.wizard.find(r => r.index === this.index + 1); + this.currentIcon$ = this.current$.pipe( + withLatestFrom(this.last$), + map(([current, last]) => { + return (last && current.index === last.index) ? ICONS.CHECK : ICONS.INDEX; + }) + ); } } From a7274db8d32ca687cc101e8919f710a1351a17db Mon Sep 17 00:00:00 2001 From: Jodie Muramoto Date: Thu, 9 Aug 2018 16:56:21 -0700 Subject: [PATCH 02/17] SHIBUI-668: Modifying switch code to include the large font text inside the wizard nav; --- ui/src/app/wizard/component/wizard.component.html | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/ui/src/app/wizard/component/wizard.component.html b/ui/src/app/wizard/component/wizard.component.html index 587605407..ed670ea60 100644 --- a/ui/src/app/wizard/component/wizard.component.html +++ b/ui/src/app/wizard/component/wizard.component.html @@ -15,12 +15,13 @@

  • - {{ (current$ | async).index }} - - {{ (current$ | async).index }}. - - - {{ (current$ | async).label }} + + + {{ (current$ | async).index }} + + + + {{ (current$ | async).index }}. {{ (current$ | async).label }}

  • From f48ba27210152729e77c11ba8f60cc10179e61ca Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Fri, 10 Aug 2018 12:57:39 -0700 Subject: [PATCH 03/17] SHIBUI-751 Fixed issue with duplicate filters --- ui/package-lock.json | 5 +++ ui/package.json | 1 + ui/src/app/app.module.ts | 6 +++ .../metadata/domain/service/filter.service.ts | 1 + .../filter/action/collection.action.ts | 37 +++++++------------ .../filter/container/edit-filter.component.ts | 2 +- .../filter/container/new-filter.component.ts | 29 +++++++++++---- .../filter/effect/collection.effect.ts | 16 ++++---- .../filter/reducer/collection.reducer.spec.ts | 4 +- .../filter/reducer/collection.reducer.ts | 4 +- .../filter/reducer/filter.reducer.spec.ts | 23 +----------- .../metadata/filter/reducer/filter.reducer.ts | 22 +---------- ui/src/app/metadata/filter/reducer/index.ts | 3 +- ui/src/app/metadata/metadata.component.ts | 1 - .../provider-filter-list.component.ts | 26 ++++++++----- .../container/provider-select.component.ts | 1 - .../provider/effect/collection.effect.ts | 13 +------ .../app/metadata/provider/provider.module.ts | 4 +- .../provider/reducer/collection.reducer.ts | 2 +- .../provider/resolver/provider.resolver.ts | 34 ----------------- 20 files changed, 85 insertions(+), 149 deletions(-) delete mode 100644 ui/src/app/metadata/provider/resolver/provider.resolver.ts diff --git a/ui/package-lock.json b/ui/package-lock.json index 0fd8aeb12..1228abd2d 100644 --- a/ui/package-lock.json +++ b/ui/package-lock.json @@ -1366,6 +1366,11 @@ "resolved": "https://registry.npmjs.org/@ngrx/store/-/store-5.2.0.tgz", "integrity": "sha1-Yn7XTJzZVGKTBIXZEqVXEXsjkD4=" }, + "@ngrx/store-devtools": { + "version": "6.1.0", + "resolved": "https://registry.npmjs.org/@ngrx/store-devtools/-/store-devtools-6.1.0.tgz", + "integrity": "sha512-Uc0g/NCbJIbzvIMuCy3skiZVD5hoIrOAAvaninXkVHt7bXpbsSAdvJlmnozuGQqTbC0UQhYRwAR7InRSrzIbMQ==" + }, "@ngtools/webpack": { "version": "6.0.0-rc.11", "resolved": "https://registry.npmjs.org/@ngtools/webpack/-/webpack-6.0.0-rc.11.tgz", diff --git a/ui/package.json b/ui/package.json index c0199ee2d..6dc630ba9 100644 --- a/ui/package.json +++ b/ui/package.json @@ -31,6 +31,7 @@ "@ngrx/entity": "^5.2.0", "@ngrx/router-store": "^5.2.0", "@ngrx/store": "^5.2.0", + "@ngrx/store-devtools": "^6.1.0", "bootstrap": "4.1.1", "core-js": "^2.4.1", "deep-object-diff": "^1.1.0", diff --git a/ui/src/app/app.module.ts b/ui/src/app/app.module.ts index e51a919f2..aea2a20f6 100644 --- a/ui/src/app/app.module.ts +++ b/ui/src/app/app.module.ts @@ -2,6 +2,7 @@ import { BrowserModule } from '@angular/platform-browser'; import { NgModule } from '@angular/core'; import { StoreModule } from '@ngrx/store'; import { EffectsModule } from '@ngrx/effects'; +import { StoreDevtoolsModule } from '@ngrx/store-devtools'; import { StoreRouterConnectingModule, RouterStateSerializer } from '@ngrx/router-store'; import { HTTP_INTERCEPTORS, HttpClientModule } from '@angular/common/http'; @@ -22,6 +23,7 @@ import { ContentionModule } from './contention/contention.module'; import { SharedModule } from './shared/shared.module'; import { WizardModule } from './wizard/wizard.module'; import { FormModule } from './schema-form/schema-form.module'; +import { environment } from '../environments/environment.prod'; @NgModule({ declarations: [ @@ -31,6 +33,10 @@ import { FormModule } from './schema-form/schema-form.module'; StoreModule.forRoot(reducers, { metaReducers }), + StoreDevtoolsModule.instrument({ + maxAge: 25, // Retains last 25 states + logOnly: environment.production, // Restrict extension to log-only mode + }), EffectsModule.forRoot([]), BrowserModule, CoreModule.forRoot(), diff --git a/ui/src/app/metadata/domain/service/filter.service.ts b/ui/src/app/metadata/domain/service/filter.service.ts index ac9bc8ce1..7334ee301 100644 --- a/ui/src/app/metadata/domain/service/filter.service.ts +++ b/ui/src/app/metadata/domain/service/filter.service.ts @@ -27,6 +27,7 @@ export class MetadataFilterService { } save(providerId: string, filter: MetadataFilter): Observable { + console.log(providerId, filter); return this.http.post(`${this.base}${this.endpoint}/${providerId}/Filters`, filter); } } diff --git a/ui/src/app/metadata/filter/action/collection.action.ts b/ui/src/app/metadata/filter/action/collection.action.ts index f220d84bb..c93c563d8 100644 --- a/ui/src/app/metadata/filter/action/collection.action.ts +++ b/ui/src/app/metadata/filter/action/collection.action.ts @@ -3,37 +3,29 @@ import { MetadataFilter } from '../../domain/model/metadata-filter'; import { Update } from '@ngrx/entity'; export enum FilterCollectionActionTypes { - FIND = '[Metadata Filter] Find', - SELECT_FILTER = '[Metadata Filter] Select Request', - SELECT_FILTER_SUCCESS = '[Metadata Filter] Select Success', - SELECT_FILTER_FAIL = '[Metadata Filter] Select Fail', + SELECT_FILTER_REQUEST = '[Metadata Filter Collection] Select Filter Request', + SELECT_FILTER_SUCCESS = '[Metadata Filter Collection] Select Filter Success', + SELECT_FILTER_FAIL = '[Metadata Filter Collection] Select Filter Fail', - UPDATE_FILTER_REQUEST = '[Metadata Filter] Update Request', - UPDATE_FILTER_SUCCESS = '[Metadata Filter] Update Success', - UPDATE_FILTER_FAIL = '[Metadata Filter] Update Fail', + UPDATE_FILTER_REQUEST = '[Metadata Filter Collection] Update Filter Request', + UPDATE_FILTER_SUCCESS = '[Metadata Filter Collection] Update Filter Success', + UPDATE_FILTER_FAIL = '[Metadata Filter Collection] Update Filter Fail', - LOAD_FILTER_REQUEST = '[Metadata Filter Collection] Filter REQUEST', - LOAD_FILTER_SUCCESS = '[Metadata Filter Collection] Filter SUCCESS', - LOAD_FILTER_ERROR = '[Metadata Filter Collection] Filter ERROR', + LOAD_FILTER_REQUEST = '[Metadata Filter Collection] Load Filter Request', + LOAD_FILTER_SUCCESS = '[Metadata Filter Collection] Load Filter Success', + LOAD_FILTER_ERROR = '[Metadata Filter Collection] Load Filter Error', - ADD_FILTER = '[Metadata Filter Collection] Add Filter', + ADD_FILTER_REQUEST = '[Metadata Filter Collection] Add Filter Request', ADD_FILTER_SUCCESS = '[Metadata Filter Collection] Add Filter Success', ADD_FILTER_FAIL = '[Metadata Filter Collection] Add Filter Fail', - REMOVE_FILTER = '[Metadata Filter Collection] Remove Filter', + REMOVE_FILTER_REQUEST = '[Metadata Filter Collection] Remove Filter Request', REMOVE_FILTER_SUCCESS = '[Metadata Filter Collection] Remove Filter Success', REMOVE_FILTER_FAIL = '[Metadata Filter Collection] Remove Filter Fail' } - -export class FindFilter implements Action { - readonly type = FilterCollectionActionTypes.FIND; - - constructor(public payload: string) { } -} - export class SelectFilter implements Action { - readonly type = FilterCollectionActionTypes.SELECT_FILTER; + readonly type = FilterCollectionActionTypes.SELECT_FILTER_REQUEST; constructor(public payload: string) { } } @@ -87,7 +79,7 @@ export class UpdateFilterFail implements Action { } export class AddFilterRequest implements Action { - readonly type = FilterCollectionActionTypes.ADD_FILTER; + readonly type = FilterCollectionActionTypes.ADD_FILTER_REQUEST; constructor(public payload: MetadataFilter) { } } @@ -105,7 +97,7 @@ export class AddFilterFail implements Action { } export class RemoveFilterRequest implements Action { - readonly type = FilterCollectionActionTypes.REMOVE_FILTER; + readonly type = FilterCollectionActionTypes.REMOVE_FILTER_REQUEST; constructor(public payload: MetadataFilter) { } } @@ -132,7 +124,6 @@ export type FilterCollectionActionsUnion = | RemoveFilterRequest | RemoveFilterSuccess | RemoveFilterFail - | FindFilter | SelectFilter | SelectFilterSuccess | SelectFilterFail diff --git a/ui/src/app/metadata/filter/container/edit-filter.component.ts b/ui/src/app/metadata/filter/container/edit-filter.component.ts index f12429000..50184abf8 100644 --- a/ui/src/app/metadata/filter/container/edit-filter.component.ts +++ b/ui/src/app/metadata/filter/container/edit-filter.component.ts @@ -39,7 +39,7 @@ export class EditFilterComponent { this.definition = MetadataFilterTypes.EntityAttributesFilter; this.schema$ = this.schemaService.get(this.definition.schema); - this.isSaving$ = this.store.select(fromFilter.getSaving); + this.isSaving$ = this.store.select(fromFilter.getCollectionSaving); this.model$ = this.store.select(fromFilter.getSelectedFilter); this.valueChangeEmitted$.subscribe(changes => this.store.dispatch(new UpdateFilterChanges(changes.value))); diff --git a/ui/src/app/metadata/filter/container/new-filter.component.ts b/ui/src/app/metadata/filter/container/new-filter.component.ts index 244cb3a85..542126491 100644 --- a/ui/src/app/metadata/filter/container/new-filter.component.ts +++ b/ui/src/app/metadata/filter/container/new-filter.component.ts @@ -1,6 +1,7 @@ -import { Component } from '@angular/core'; +import { Component, OnDestroy, OnInit } from '@angular/core'; import { Store } from '@ngrx/store'; import { Subject, Observable, of } from 'rxjs'; +import { takeUntil } from 'rxjs/operators'; import * as fromFilter from '../reducer'; import { MetadataFilterTypes } from '../model'; @@ -14,7 +15,9 @@ import { CancelCreateFilter, UpdateFilterChanges } from '../action/filter.action selector: 'new-filter-page', templateUrl: './new-filter.component.html' }) -export class NewFilterComponent { +export class NewFilterComponent implements OnDestroy, OnInit { + + private ngUnsubscribe: Subject = new Subject(); valueChangeSubject = new Subject>(); private valueChangeEmitted$ = this.valueChangeSubject.asObservable(); @@ -37,19 +40,31 @@ export class NewFilterComponent { this.definition = MetadataFilterTypes.EntityAttributesFilter; this.schema$ = this.schemaService.get(this.definition.schema); - this.isSaving$ = this.store.select(fromFilter.getSaving); + this.isSaving$ = this.store.select(fromFilter.getCollectionSaving); this.model$ = of({}); + } - this.valueChangeEmitted$.subscribe(changes => this.store.dispatch(new UpdateFilterChanges(changes.value))); - this.statusChangeEmitted$.subscribe(valid => { - this.isValid = valid.value ? valid.value.length === 0 : true; - }); + ngOnInit(): void { + this.valueChangeEmitted$ + .pipe(takeUntil(this.ngUnsubscribe)) + .subscribe(changes => this.store.dispatch(new UpdateFilterChanges(changes.value))); + this.statusChangeEmitted$ + .pipe(takeUntil(this.ngUnsubscribe)) + .subscribe(valid => { + this.isValid = valid.value ? valid.value.length === 0 : true; + }); this.store .select(fromFilter.getFilter) + .pipe(takeUntil(this.ngUnsubscribe)) .subscribe(filter => this.filter = filter); } + ngOnDestroy(): void { + this.ngUnsubscribe.next(); + this.ngUnsubscribe.complete(); + } + save(): void { this.store.dispatch(new AddFilterRequest(this.filter)); } diff --git a/ui/src/app/metadata/filter/effect/collection.effect.ts b/ui/src/app/metadata/filter/effect/collection.effect.ts index 44651d0c2..cc41754c2 100644 --- a/ui/src/app/metadata/filter/effect/collection.effect.ts +++ b/ui/src/app/metadata/filter/effect/collection.effect.ts @@ -4,7 +4,7 @@ import { Store } from '@ngrx/store'; import { Router } from '@angular/router'; import { of } from 'rxjs'; -import { switchMap, map, catchError, tap, combineLatest, skipWhile } from 'rxjs/operators'; +import { switchMap, map, catchError, tap, combineLatest, skipWhile, debounceTime, withLatestFrom } from 'rxjs/operators'; import * as actions from '../action/collection.action'; import { FilterCollectionActionTypes } from '../action/collection.action'; @@ -35,9 +35,9 @@ export class FilterCollectionEffects { ); @Effect() selectFilterRequest$ = this.actions$.pipe( - ofType(FilterCollectionActionTypes.SELECT_FILTER), + ofType(FilterCollectionActionTypes.SELECT_FILTER_REQUEST), map(action => action.payload), - combineLatest(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), + withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), switchMap(([filterId, providerId]) => { return this.filterService .find(providerId, filterId) @@ -51,7 +51,7 @@ export class FilterCollectionEffects { @Effect() addFilter$ = this.actions$.pipe( - ofType(FilterCollectionActionTypes.ADD_FILTER), + ofType(FilterCollectionActionTypes.ADD_FILTER_REQUEST), map(action => action.payload), map(filter => { return { @@ -59,7 +59,7 @@ export class FilterCollectionEffects { relyingPartyOverrides: removeNulls(new EntityAttributesFilterEntity(filter).relyingPartyOverrides) }; }), - combineLatest(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), + withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), switchMap(([unsaved, providerId]) => { return this.filterService .save(providerId, unsaved as MetadataFilter) @@ -73,7 +73,7 @@ export class FilterCollectionEffects { addFilterSuccessRedirect$ = this.actions$.pipe( ofType(FilterCollectionActionTypes.ADD_FILTER_SUCCESS), map(action => action.payload), - combineLatest(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), + withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), tap(([filter, provider]) => this.router.navigate(['/', 'metadata', 'provider', provider, 'filters'])) ); @@ -81,7 +81,7 @@ export class FilterCollectionEffects { updateFilter$ = this.actions$.pipe( ofType(FilterCollectionActionTypes.UPDATE_FILTER_REQUEST), map(action => action.payload), - combineLatest(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), + withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), switchMap(([filter, providerId]) => { delete filter.modifiedDate; delete filter.createdDate; @@ -100,7 +100,7 @@ export class FilterCollectionEffects { updateFilterSuccessRedirect$ = this.actions$.pipe( ofType(FilterCollectionActionTypes.UPDATE_FILTER_SUCCESS), map(action => action.payload), - combineLatest(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), + withLatestFrom(this.store.select(fromProvider.getSelectedProviderId).pipe(skipWhile(id => !id))), tap(([filter, provider]) => this.router.navigate(['/', 'metadata', 'provider', provider, 'filters'])) ); diff --git a/ui/src/app/metadata/filter/reducer/collection.reducer.spec.ts b/ui/src/app/metadata/filter/reducer/collection.reducer.spec.ts index 32c46601c..dfe5fc107 100644 --- a/ui/src/app/metadata/filter/reducer/collection.reducer.spec.ts +++ b/ui/src/app/metadata/filter/reducer/collection.reducer.spec.ts @@ -23,7 +23,7 @@ describe('Filter Reducer', () => { }); }); - describe(`${FilterCollectionActionTypes.SELECT_FILTER}`, () => { + describe(`${FilterCollectionActionTypes.SELECT_FILTER_REQUEST}`, () => { it('should set the selected id in the store', () => { const selectedFilterId = 'foo'; const action = new SelectFilter(selectedFilterId); @@ -55,7 +55,7 @@ describe('Filter Reducer', () => { }); }); - describe(`${FilterCollectionActionTypes.ADD_FILTER}`, () => { + describe(`${FilterCollectionActionTypes.ADD_FILTER_REQUEST}`, () => { it('should set saving to true', () => { const filter = new EntityAttributesFilterEntity({ resourceId: 'foo', createdDate: new Date().toLocaleDateString() }); const action = new AddFilterRequest(filter); diff --git a/ui/src/app/metadata/filter/reducer/collection.reducer.ts b/ui/src/app/metadata/filter/reducer/collection.reducer.ts index e71f0947c..35074b696 100644 --- a/ui/src/app/metadata/filter/reducer/collection.reducer.ts +++ b/ui/src/app/metadata/filter/reducer/collection.reducer.ts @@ -41,7 +41,7 @@ export function reducer(state = initialState, action: FilterCollectionActionsUni }); } - case FilterCollectionActionTypes.ADD_FILTER: + case FilterCollectionActionTypes.ADD_FILTER_REQUEST: case FilterCollectionActionTypes.UPDATE_FILTER_REQUEST: { return { ...state, @@ -65,7 +65,7 @@ export function reducer(state = initialState, action: FilterCollectionActionsUni }; } - case FilterCollectionActionTypes.SELECT_FILTER: { + case FilterCollectionActionTypes.SELECT_FILTER_REQUEST: { return { ...state, selectedFilterId: action.payload, diff --git a/ui/src/app/metadata/filter/reducer/filter.reducer.spec.ts b/ui/src/app/metadata/filter/reducer/filter.reducer.spec.ts index f9ed5b7a5..ff832e198 100644 --- a/ui/src/app/metadata/filter/reducer/filter.reducer.spec.ts +++ b/ui/src/app/metadata/filter/reducer/filter.reducer.spec.ts @@ -1,4 +1,4 @@ -import { reducer } from './filter.reducer'; +import { reducer, initialState as snapshot } from './filter.reducer'; import * as fromFilter from './filter.reducer'; import { SelectId, LoadEntityPreviewSuccess, UpdateFilterChanges, FilterActionTypes, CancelCreateFilter } from '../action/filter.action'; import { SearchActionTypes } from '../action/search.action'; @@ -18,14 +18,6 @@ import { MDUI } from '../../domain/model'; import { MetadataFilter } from '../../domain/model'; import { EntityAttributesFilterEntity } from '../../domain/entity/filter/entity-attributes-filter'; - -const snapshot: fromFilter.FilterState = { - selected: null, - changes: null, - preview: null, - saving: false -}; - const mdui: MDUI = { displayName: 'foo', informationUrl: 'bar', @@ -70,19 +62,6 @@ describe('Filter Reducer', () => { }); }); - describe(`${FilterCollectionActionTypes.ADD_FILTER} action`, () => { - it('should set saving to true', () => { - const result = reducer(snapshot, new AddFilterRequest(new EntityAttributesFilterEntity())); - expect(result.saving).toBe(true); - }); - }); - describe(`${FilterCollectionActionTypes.UPDATE_FILTER_REQUEST} action`, () => { - it('should set saving to true', () => { - const result = reducer(snapshot, new UpdateFilterRequest(new EntityAttributesFilterEntity())); - expect(result.saving).toBe(true); - }); - }); - describe(`${FilterCollectionActionTypes.ADD_FILTER_SUCCESS} action`, () => { it('should set saving to true', () => { const result = reducer(snapshot, new AddFilterSuccess(new EntityAttributesFilterEntity())); diff --git a/ui/src/app/metadata/filter/reducer/filter.reducer.ts b/ui/src/app/metadata/filter/reducer/filter.reducer.ts index 3bce96ae3..653d53740 100644 --- a/ui/src/app/metadata/filter/reducer/filter.reducer.ts +++ b/ui/src/app/metadata/filter/reducer/filter.reducer.ts @@ -13,14 +13,12 @@ export interface FilterState { selected: string | null; changes: MetadataFilter | null; preview: MDUI | null; - saving: boolean; } export const initialState: FilterState = { selected: null, changes: null, - preview: null, - saving: false + preview: null }; export function reducer(state = initialState, action: FilterActionsUnion | SearchActionsUnion | FilterCollectionActionsUnion): FilterState { @@ -46,23 +44,6 @@ export function reducer(state = initialState, action: FilterActionsUnion | Searc } }; } - case FilterCollectionActionTypes.ADD_FILTER: - case FilterCollectionActionTypes.UPDATE_FILTER_REQUEST: { - return { - ...state, - saving: true - }; - } - case FilterCollectionActionTypes.ADD_FILTER_FAIL: - case FilterCollectionActionTypes.UPDATE_FILTER_FAIL: { - return { - ...state, - saving: false - }; - } - case FilterCollectionActionTypes.ADD_FILTER_SUCCESS: - case FilterCollectionActionTypes.UPDATE_FILTER_SUCCESS: - case SearchActionTypes.CLEAR_SEARCH: case FilterActionTypes.CANCEL_CREATE_FILTER: { return { ...initialState @@ -77,4 +58,3 @@ export function reducer(state = initialState, action: FilterActionsUnion | Searc export const getSelected = (state: FilterState) => state.selected; export const getFilterChanges = (state: FilterState) => state.changes; export const getPreview = (state: FilterState) => state.preview; -export const getSaving = (state: FilterState) => state.saving; diff --git a/ui/src/app/metadata/filter/reducer/index.ts b/ui/src/app/metadata/filter/reducer/index.ts index 951c05e51..5d9a4ecbe 100644 --- a/ui/src/app/metadata/filter/reducer/index.ts +++ b/ui/src/app/metadata/filter/reducer/index.ts @@ -32,7 +32,6 @@ export const getFilterFromState = createSelector(getFilterState, getFiltersFromS export const getSelected = createSelector(getFilterFromState, fromFilter.getSelected); export const getFilter = createSelector(getFilterFromState, fromFilter.getFilterChanges); export const getPreview = createSelector(getFilterFromState, fromFilter.getPreview); -export const getSaving = createSelector(getFilterFromState, fromFilter.getSaving); /* * Select pieces of Search Collection @@ -50,7 +49,7 @@ export const getViewingMore = createSelector(getSearchFromState, fromSearch.getV */ export const getCollectionState = createSelector(getFilterState, getCollectionFromStateFn); export const getAllFilters = createSelector(getCollectionState, fromCollection.selectAllFilters); -export const getFiltersSaving = createSelector(getCollectionState, fromCollection.getIsSaving); +export const getCollectionSaving = createSelector(getCollectionState, fromCollection.getIsSaving); export const notAddtlFilters = ['RequiredValidUntil', 'SignatureValidation', 'EntityRoleWhiteList']; export const filterTypeFn = filters => filters.filter(f => notAddtlFilters.indexOf(f['@type']) === -1); diff --git a/ui/src/app/metadata/metadata.component.ts b/ui/src/app/metadata/metadata.component.ts index 1eb87324f..df271e2fd 100644 --- a/ui/src/app/metadata/metadata.component.ts +++ b/ui/src/app/metadata/metadata.component.ts @@ -2,7 +2,6 @@ import { Component, ChangeDetectionStrategy } from '@angular/core'; import { Store } from '@ngrx/store'; import { LoadResolverRequest } from './resolver/action/collection.action'; -import { LoadFilterRequest } from './filter/action/collection.action'; import { LoadDraftRequest } from './resolver/action/draft.action'; import * as fromRoot from '../app.reducer'; import { LoadProviderRequest } from './provider/action/collection.action'; diff --git a/ui/src/app/metadata/provider/container/provider-filter-list.component.ts b/ui/src/app/metadata/provider/container/provider-filter-list.component.ts index 25dbcb0f1..92c346505 100644 --- a/ui/src/app/metadata/provider/container/provider-filter-list.component.ts +++ b/ui/src/app/metadata/provider/container/provider-filter-list.component.ts @@ -1,7 +1,7 @@ -import { Component } from '@angular/core'; +import { Component, OnInit, OnDestroy } from '@angular/core'; import { Store } from '@ngrx/store'; -import { Observable } from 'rxjs'; -import { skipWhile } from 'rxjs/operators'; +import { Observable, Subject } from 'rxjs'; +import { skipWhile, distinctUntilChanged, takeUntil, map } from 'rxjs/operators'; import * as fromProvider from '../reducer'; import * as fromFilter from '../../filter/reducer'; import { MetadataFilter, MetadataProvider } from '../../domain/model'; @@ -14,7 +14,9 @@ import { UpdateFilterRequest, LoadFilterRequest } from '../../filter/action/coll templateUrl: './provider-filter-list.component.html', styleUrls: ['./provider-filter-list.component.scss'] }) -export class ProviderFilterListComponent { +export class ProviderFilterListComponent implements OnDestroy { + + private ngUnsubscribe: Subject = new Subject(); filters$: Observable; provider$: Observable; @@ -27,17 +29,23 @@ export class ProviderFilterListComponent { ) { this.filters$ = this.store.select(fromFilter.getAdditionalFilters); this.provider$ = this.store.select(fromProvider.getSelectedProvider).pipe(skipWhile(p => !p)); + this.provider$ + .pipe(takeUntil(this.ngUnsubscribe)) + .subscribe(p => { + this.store.dispatch(new LoadFilterRequest(p.resourceId)); + }); this.store.dispatch(new SetIndex('filters')); - this.provider$.subscribe(p => { - this.store.dispatch(new LoadFilterRequest(p.resourceId)); - }); - - this.isSaving$ = this.store.select(fromFilter.getFiltersSaving); + this.isSaving$ = this.store.select(fromFilter.getCollectionSaving); } toggleEnabled(filter: MetadataFilter): void { this.store.dispatch(new UpdateFilterRequest({ ...filter, filterEnabled: !filter.filterEnabled })); } + + ngOnDestroy(): void { + this.ngUnsubscribe.next(); + this.ngUnsubscribe.complete(); + } } diff --git a/ui/src/app/metadata/provider/container/provider-select.component.ts b/ui/src/app/metadata/provider/container/provider-select.component.ts index 7ae2b180c..a540df912 100644 --- a/ui/src/app/metadata/provider/container/provider-select.component.ts +++ b/ui/src/app/metadata/provider/container/provider-select.component.ts @@ -26,7 +26,6 @@ export class ProviderSelectComponent implements OnDestroy { private route: ActivatedRoute ) { this.actionsSubscription = this.route.params.pipe( - distinctUntilChanged(), map(params => new SelectProviderRequest(params.providerId)) ).subscribe(store); diff --git a/ui/src/app/metadata/provider/effect/collection.effect.ts b/ui/src/app/metadata/provider/effect/collection.effect.ts index 6d135bdd1..fbf4a6c1e 100644 --- a/ui/src/app/metadata/provider/effect/collection.effect.ts +++ b/ui/src/app/metadata/provider/effect/collection.effect.ts @@ -118,21 +118,10 @@ export class CollectionEffects { map(provider => new LoadProviderRequest()) ); - @Effect({ dispatch: false }) - newFilterSuccessUpdate = this.actions$.pipe( - ofType(FilterCollectionActionTypes.ADD_FILTER_SUCCESS), - map(action => action.payload), - withLatestFrom(this.store.select(fromProvider.getSelectedProviderId)), - map(([filter, id]) => id), - tap(id => { - this.store.dispatch(new SelectProviderRequest(id)); - }) - ); - constructor( private actions$: Actions, private router: Router, private store: Store, private providerService: MetadataProviderService ) { } -} /* istanbul ignore next */ +} diff --git a/ui/src/app/metadata/provider/provider.module.ts b/ui/src/app/metadata/provider/provider.module.ts index afd8409c4..dfd95fa8f 100644 --- a/ui/src/app/metadata/provider/provider.module.ts +++ b/ui/src/app/metadata/provider/provider.module.ts @@ -28,7 +28,6 @@ import { EntityEffects } from './effect/entity.effect'; import { ProviderFilterListComponent } from './container/provider-filter-list.component'; import { ProviderEditorNavComponent } from './component/provider-editor-nav.component'; -import { ProviderResolver } from './resolver/provider.resolver'; @NgModule({ declarations: [ @@ -61,8 +60,7 @@ export class ProviderModule { return { ngModule: RootProviderModule, providers: [ - { provide: WidgetRegistry, useClass: CustomWidgetRegistry }, - ProviderResolver + { provide: WidgetRegistry, useClass: CustomWidgetRegistry } ] }; } diff --git a/ui/src/app/metadata/provider/reducer/collection.reducer.ts b/ui/src/app/metadata/provider/reducer/collection.reducer.ts index 8cc0c6f0c..7d74cb5af 100644 --- a/ui/src/app/metadata/provider/reducer/collection.reducer.ts +++ b/ui/src/app/metadata/provider/reducer/collection.reducer.ts @@ -26,7 +26,7 @@ export function reducer(state = initialState, action: ProviderCollectionActionsU case ProviderCollectionActionTypes.SELECT_PROVIDER_SUCCESS: { return adapter.upsertOne(action.payload, { ...state, - selectedProviderId: action.payload.id.toString() + selectedProviderId: action.payload.id as string }); } diff --git a/ui/src/app/metadata/provider/resolver/provider.resolver.ts b/ui/src/app/metadata/provider/resolver/provider.resolver.ts deleted file mode 100644 index 9a527b8a9..000000000 --- a/ui/src/app/metadata/provider/resolver/provider.resolver.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { Injectable } from '@angular/core'; -import { Resolve, ActivatedRoute } from '@angular/router'; -import { Store } from '@ngrx/store'; -import { Observable, of } from 'rxjs'; -import { map, distinctUntilChanged, skipWhile } from 'rxjs/operators'; - -import * as fromCollection from '../reducer'; -import { MetadataProviderService } from '../../domain/service/provider.service'; -import { MetadataProvider } from '../../domain/model'; -import { SelectProviderRequest } from '../action/collection.action'; - -/** - * Guards are hooks into the route resolution process, providing an opportunity - * to inform the router's navigation process whether the route should continue - * to activate this route. Guards must return an of true or false. - */ -@Injectable() -export class ProviderResolver implements Resolve> { - constructor( - private store: Store, - private route: ActivatedRoute - ) { } - - resolve() { - this.route.params.pipe( - distinctUntilChanged(), - map(params => { - return new SelectProviderRequest(params.providerId); - }) - ).subscribe(this.store); - - return this.store.select(fromCollection.getSelectedProvider).pipe(skipWhile(id => !id)); - } -} From e6ef569d3f1565ada545d7d24484d6284e446c32 Mon Sep 17 00:00:00 2001 From: Jodie Muramoto Date: Fri, 10 Aug 2018 22:32:04 +0000 Subject: [PATCH 04/17] Merged in bug/SHIBUI-712 (pull request #152) SHIBUI-712: Updated $gray-700 value to meet color contrast accessibility guidelines; --- ui/src/theme/_palette.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/theme/_palette.scss b/ui/src/theme/_palette.scss index c94195318..4a8adb95a 100644 --- a/ui/src/theme/_palette.scss +++ b/ui/src/theme/_palette.scss @@ -9,7 +9,7 @@ $gray-300: #dee2e6; $gray-400: #ced4da; $gray-500: #adb5bd; $gray-600: #868e96; -$gray-700: #697581; +$gray-700: #495057; $gray-800: #343a40; $gray-900: #212529; $black: #000; From 3549b7c3b0ef4be7d8077d1726f9a2fd9b3d6732 Mon Sep 17 00:00:00 2001 From: Jodie Muramoto Date: Fri, 10 Aug 2018 16:11:55 -0700 Subject: [PATCH 05/17] SHIBUI-668: Made the wizard summary page current navigation label consistent across sources and providers; --- .../metadata/resolver/component/wizard-nav.component.html | 8 +++++++- ui/src/app/wizard/component/wizard.component.html | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ui/src/app/metadata/resolver/component/wizard-nav.component.html b/ui/src/app/metadata/resolver/component/wizard-nav.component.html index cba134ecf..02ff3865f 100644 --- a/ui/src/app/metadata/resolver/component/wizard-nav.component.html +++ b/ui/src/app/metadata/resolver/component/wizard-nav.component.html @@ -11,12 +11,18 @@
    • -
  • +

  • {{ currentPage.index }}

    • +
  • +

    + + +

    +
  • From 741c86258bd494e1c77bab148a9758b1bbbc42e5 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Mon, 13 Aug 2018 12:00:05 -0400 Subject: [PATCH 06/17] Unit test to demo the incorrect version when updating resolver --- ...ResolversControllerIntegrationTests.groovy | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy index 25071cb98..3ceb55417 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversControllerIntegrationTests.groovy @@ -255,6 +255,33 @@ class MetadataResolversControllerIntegrationTests extends Specification { createdResolver.metadataFilters[0] instanceof EntityAttributesFilter } + def "PUT MetadataResolver with one EntityAttributesFilters attached and check version -> /api/MetadataResolvers"() { + given: 'MetadataResolver with attached entity attributes is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + when: 'GET request is made with resource Id matching the existing resolver' + def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", String) + def existingMetadataResolverMap = new JsonSlurper().parseText(result.body) + def existingMetadataVersion = existingMetadataResolverMap.version + + and: 'PUT call is made with' + existingMetadataResolverMap.name = 'Updated' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/${existingMetadataResolverMap.resourceId}", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingMetadataResolverMap) }, + String) + def updatedResultFromGET = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId", String) + def updatedVersionReturnedFromPUT = new JsonSlurper().parseText(updatedResultFromPUT.body).version + def updatedVersionReturnedFromGET = new JsonSlurper().parseText(updatedResultFromGET.body).version + + then: + updatedVersionReturnedFromPUT == updatedVersionReturnedFromGET + } + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) } From 221133de60a2acb84408e25eebea93f960be3bfa Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 09:28:56 -0400 Subject: [PATCH 07/17] Incremental changes related to hashcod/version refactoring based on code review --- .../ui/controller/MetadataResolversController.java | 14 ++++---------- .../ui/domain/resolvers/MetadataResolver.java | 13 +++++++++---- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 52f1a50ac..15ae5d236 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -61,7 +61,6 @@ public ResponseEntity unableToParseJson(Exception ex) { @Transactional(readOnly = true) public ResponseEntity getAll() { List resolvers = positionOrderContainerService.getAllMetadataResolversInDefinedOrderOrUnordered(); - resolvers.forEach(MetadataResolver::updateVersion); return ResponseEntity.ok(resolvers); } @@ -86,7 +85,6 @@ public ResponseEntity getOne(@PathVariable String resourceId) { if (resolver == null) { return ResponseEntity.notFound().build(); } - resolver.updateVersion(); return ResponseEntity.ok(resolver); } @@ -103,11 +101,9 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) { } newResolver.convertFiltersFromTransientRepresentationIfNecessary(); - resolverRepository.save(newResolver); - MetadataResolver persistedResolver = resolverRepository.findByResourceId(newResolver.getResourceId()); + MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - persistedResolver.updateVersion(); persistedResolver.convertFiltersIntoTransientRepresentationIfNecessary(); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } @@ -119,9 +115,9 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me if (existingResolver == null) { return ResponseEntity.notFound().build(); } - if (existingResolver.hashCode() != updatedResolver.getVersion()) { + if (existingResolver.getVersion() != updatedResolver.getVersion()) { log.info("Metadata Resolver version conflict. Latest resolver in database version: {}. Resolver version sent from UI: {}", - existingResolver.hashCode(), updatedResolver.getVersion()); + existingResolver.getVersion(), updatedResolver.getVersion()); return ResponseEntity.status(HttpStatus.CONFLICT).build(); } @@ -133,10 +129,8 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me updatedResolver.setAudId(existingResolver.getAudId()); updatedResolver.convertFiltersFromTransientRepresentationIfNecessary(); - resolverRepository.save(updatedResolver); - MetadataResolver persistedResolver = resolverRepository.findByResourceId(updatedResolver.getResourceId()); + MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - persistedResolver.updateVersion(); persistedResolver.convertFiltersFromTransientRepresentationIfNecessary(); return ResponseEntity.ok(persistedResolver); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java index 7d0fc03d1..cb3b7c9d9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.resolvers; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -73,10 +74,14 @@ public class MetadataResolver extends AbstractAuditable { private List metadataFilters = new ArrayList<>(); @Transient - private int version; - - public void updateVersion() { - this.version = hashCode(); + private Integer version; + + @JsonGetter("version") + public int getVersion() { + if (this.version != null && this.version != 0 ) { + return this.version; + } + return this.hashCode(); } public void convertFiltersIntoTransientRepresentationIfNecessary() { From 4b0f3f669e2c7c50bdfbea2c9eb1c5474dccb955 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 14 Aug 2018 14:05:57 +0000 Subject: [PATCH 08/17] Merged in feature/SHIBUI-707 (pull request #154) * SHIBUI-707 Implemented provider re-ordering Approved-by: Shibui Jenkins Approved-by: Ryan Mathis --- .../preview-dialog.component.spec.ts | 46 +++++++++ .../metadata/domain/model/metadata-order.ts | 3 + .../domain/service/provider.service.ts | 10 ++ .../component/provider-item.component.html | 6 +- .../component/provider-item.component.ts | 6 ++ .../dashboard-providers-list.component.html | 9 +- .../dashboard-providers-list.component.ts | 28 ++++-- .../provider/action/collection.action.ts | 72 +++++++++++++- .../provider/effect/collection.effect.ts | 90 ++++++++++++++++- .../reducer/collection.reducer.spec.ts | 9 +- .../provider/reducer/collection.reducer.ts | 19 ++-- .../provider/reducer/entity.reducer.spec.ts | 21 +++- .../metadata/provider/reducer/index.spec.ts | 96 +++++++++++++++++++ ui/src/app/metadata/provider/reducer/index.ts | 12 +++ .../shared/switch/switch.component.spec.ts | 15 +++ 15 files changed, 410 insertions(+), 32 deletions(-) create mode 100644 ui/src/app/metadata/domain/component/preview-dialog.component.spec.ts create mode 100644 ui/src/app/metadata/domain/model/metadata-order.ts create mode 100644 ui/src/app/metadata/provider/reducer/index.spec.ts diff --git a/ui/src/app/metadata/domain/component/preview-dialog.component.spec.ts b/ui/src/app/metadata/domain/component/preview-dialog.component.spec.ts new file mode 100644 index 000000000..d08b873ba --- /dev/null +++ b/ui/src/app/metadata/domain/component/preview-dialog.component.spec.ts @@ -0,0 +1,46 @@ +import { TestBed, ComponentFixture } from '@angular/core/testing'; +import { ReactiveFormsModule } from '@angular/forms'; +import { NoopAnimationsModule } from '@angular/platform-browser/animations'; +import { ViewChild, Component } from '@angular/core'; +import { Observable, of } from 'rxjs'; +import { PreviewDialogComponent } from './preview-dialog.component'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { NgbActiveModalStub } from '../../../../testing/modal.stub'; + +@Component({ + template: `` +}) +class TestHostComponent { + @ViewChild(PreviewDialogComponent) + public formUnderTest: PreviewDialogComponent; +} + + +describe('Advanced Info Form Component', () => { + let fixture: ComponentFixture; + let instance: TestHostComponent; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + { provide: NgbActiveModal, useClass: NgbActiveModalStub } + ], + imports: [ + NoopAnimationsModule, + ReactiveFormsModule + ], + declarations: [ + PreviewDialogComponent, + TestHostComponent + ], + }); + + fixture = TestBed.createComponent(TestHostComponent); + instance = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should compile', () => { + expect(fixture).toBeDefined(); + }); +}); diff --git a/ui/src/app/metadata/domain/model/metadata-order.ts b/ui/src/app/metadata/domain/model/metadata-order.ts new file mode 100644 index 000000000..b365af3b0 --- /dev/null +++ b/ui/src/app/metadata/domain/model/metadata-order.ts @@ -0,0 +1,3 @@ +export interface ProviderOrder { + resourceIds: string[]; +} diff --git a/ui/src/app/metadata/domain/service/provider.service.ts b/ui/src/app/metadata/domain/service/provider.service.ts index 39258be42..3d83d3c55 100644 --- a/ui/src/app/metadata/domain/service/provider.service.ts +++ b/ui/src/app/metadata/domain/service/provider.service.ts @@ -4,11 +4,13 @@ import { Observable } from 'rxjs'; import { MetadataProvider } from '../../domain/model'; import { FileBackedHttpMetadataProvider } from '../model/providers'; +import { ProviderOrder } from '../model/metadata-order'; @Injectable() export class MetadataProviderService { readonly endpoint = '/MetadataResolvers'; + readonly order = '/MetadataResolversPositionOrder'; readonly base = '/api'; constructor( @@ -29,4 +31,12 @@ export class MetadataProviderService { save(provider: MetadataProvider): Observable { return this.http.post(`${this.base}${this.endpoint}`, provider); } + + getOrder(): Observable { + return this.http.get(`${this.base}${this.order}`); + } + + setOrder(order: ProviderOrder): Observable { + return this.http.post(`${this.base}${this.order}`, order); + } } diff --git a/ui/src/app/metadata/manager/component/provider-item.component.html b/ui/src/app/metadata/manager/component/provider-item.component.html index f8441c696..1ca8f575b 100644 --- a/ui/src/app/metadata/manager/component/provider-item.component.html +++ b/ui/src/app/metadata/manager/component/provider-item.component.html @@ -2,11 +2,11 @@
    -

    1

    - -
    diff --git a/ui/src/app/metadata/manager/component/provider-item.component.ts b/ui/src/app/metadata/manager/component/provider-item.component.ts index b8f676f40..ad8fa572b 100644 --- a/ui/src/app/metadata/manager/component/provider-item.component.ts +++ b/ui/src/app/metadata/manager/component/provider-item.component.ts @@ -12,6 +12,12 @@ import { EntityItemComponent } from './entity-item.component'; export class ProviderItemComponent extends EntityItemComponent { @Input() provider: MetadataProvider; + @Input() index: number; + @Input() first: boolean; + @Input() last: boolean; @Output() viewFilters: EventEmitter = new EventEmitter(); + + @Output() changeOrderUp: EventEmitter = new EventEmitter(); + @Output() changeOrderDown: EventEmitter = new EventEmitter(); } diff --git a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.html b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.html index 7d2eab50a..467538039 100644 --- a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.html +++ b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.html @@ -9,14 +9,19 @@
      -
    • + (toggle)="toggleEntity(provider)" + (changeOrderUp)="updateOrderUp($event)" + (changeOrderDown)="updateOrderDown($event)">
    diff --git a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts index bc2e609c4..1815d4ccf 100644 --- a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts +++ b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts @@ -1,20 +1,21 @@ -import { Component } from '@angular/core'; +import { Component, OnInit } from '@angular/core'; import { Router } from '@angular/router'; import { MetadataProvider } from '../../domain/model'; import { Observable } from '../../../../../node_modules/rxjs'; import { Store } from '@ngrx/store'; -import { ProviderState, getAllProviders } from '../../provider/reducer'; -import * as fromDashboard from '../reducer'; +import { ProviderState, getOrderedProviders } from '../../provider/reducer'; +import { getOpenProviders } from '../reducer'; import { ToggleEntityDisplay } from '../action/manager.action'; import { map } from 'rxjs/operators'; +import { ChangeOrderUp, ChangeOrderDown } from '../../provider/action/collection.action'; @Component({ selector: 'dashboard-providers-list', templateUrl: './dashboard-providers-list.component.html' }) -export class DashboardProvidersListComponent { +export class DashboardProvidersListComponent implements OnInit { providers$: Observable; providersOpen$: Observable<{ [key: string]: boolean }>; @@ -22,11 +23,12 @@ export class DashboardProvidersListComponent { constructor( private store: Store, private router: Router - ) { - this.providers$ = this.store.select(getAllProviders).pipe( - map(providers => providers.filter(p => p['@type'] !== 'BaseMetadataResolver')) - ); - this.providersOpen$ = store.select(fromDashboard.getOpenProviders); + ) { } + + ngOnInit(): void { + this.providers$ = this.store.select(getOrderedProviders); + this.providersOpen$ = this.store.select(getOpenProviders); + this.providers$.subscribe(p => console.log(p)); } view(id: string, page: string): void { @@ -44,4 +46,12 @@ export class DashboardProvidersListComponent { toggleEntity(provider: MetadataProvider): void { this.store.dispatch(new ToggleEntityDisplay(provider.resourceId)); } + + updateOrderUp(provider: MetadataProvider): void { + this.store.dispatch(new ChangeOrderUp(provider.resourceId)); + } + + updateOrderDown(provider: MetadataProvider): void { + this.store.dispatch(new ChangeOrderDown(provider.resourceId)); + } } diff --git a/ui/src/app/metadata/provider/action/collection.action.ts b/ui/src/app/metadata/provider/action/collection.action.ts index 6746ffad5..637c58115 100644 --- a/ui/src/app/metadata/provider/action/collection.action.ts +++ b/ui/src/app/metadata/provider/action/collection.action.ts @@ -1,6 +1,7 @@ import { Action } from '@ngrx/store'; import { MetadataProvider } from '../../domain/model/metadata-provider'; import { Update } from '@ngrx/entity'; +import { ProviderOrder } from '../../domain/model/metadata-order'; export enum ProviderCollectionActionTypes { UPDATE_PROVIDER_REQUEST = '[Metadata Provider] Update Request', @@ -21,7 +22,18 @@ export enum ProviderCollectionActionTypes { REMOVE_PROVIDER_REQUEST = '[Metadata Provider Collection] Remove Provider Request', REMOVE_PROVIDER_SUCCESS = '[Metadata Provider Collection] Remove Provider Success', - REMOVE_PROVIDER_FAIL = '[Metadata Provider Collection] Remove Provider Fail' + REMOVE_PROVIDER_FAIL = '[Metadata Provider Collection] Remove Provider Fail', + + SET_ORDER_PROVIDER_REQUEST = '[Metadata Provider Collection] Set Order Provider Request', + SET_ORDER_PROVIDER_SUCCESS = '[Metadata Provider Collection] Set Order Remove Provider Success', + SET_ORDER_PROVIDER_FAIL = '[Metadata Provider Collection] Set Order Remove Provider Fail', + + GET_ORDER_PROVIDER_REQUEST = '[Metadata Provider Collection] Get Order Remove Provider Request', + GET_ORDER_PROVIDER_SUCCESS = '[Metadata Provider Collection] Get Order Remove Provider Success', + GET_ORDER_PROVIDER_FAIL = '[Metadata Provider Collection] Get Order Remove Provider Fail', + + CHANGE_PROVIDER_ORDER_UP = '[Metadata Provider Collection] Change Order Up', + CHANGE_PROVIDER_ORDER_DOWN = '[Metadata Provider Collection] Change Order Down', } export class LoadProviderRequest implements Action { @@ -114,6 +126,54 @@ export class RemoveProviderFail implements Action { constructor(public payload: MetadataProvider) { } } +export class SetOrderProviderRequest implements Action { + readonly type = ProviderCollectionActionTypes.SET_ORDER_PROVIDER_REQUEST; + + constructor(public payload: ProviderOrder) { } +} + +export class SetOrderProviderSuccess implements Action { + readonly type = ProviderCollectionActionTypes.SET_ORDER_PROVIDER_SUCCESS; + + constructor() { } +} + +export class SetOrderProviderFail implements Action { + readonly type = ProviderCollectionActionTypes.SET_ORDER_PROVIDER_FAIL; + + constructor(public payload: Error) { } +} + +export class GetOrderProviderRequest implements Action { + readonly type = ProviderCollectionActionTypes.GET_ORDER_PROVIDER_REQUEST; + + constructor() { } +} + +export class GetOrderProviderSuccess implements Action { + readonly type = ProviderCollectionActionTypes.GET_ORDER_PROVIDER_SUCCESS; + + constructor(public payload: ProviderOrder) { } +} + +export class GetOrderProviderFail implements Action { + readonly type = ProviderCollectionActionTypes.GET_ORDER_PROVIDER_FAIL; + + constructor(public payload: Error) { } +} + +export class ChangeOrderUp implements Action { + readonly type = ProviderCollectionActionTypes.CHANGE_PROVIDER_ORDER_UP; + + constructor(public payload: string) { } +} + +export class ChangeOrderDown implements Action { + readonly type = ProviderCollectionActionTypes.CHANGE_PROVIDER_ORDER_DOWN; + + constructor(public payload: string) { } +} + export type ProviderCollectionActionsUnion = | LoadProviderRequest | LoadProviderSuccess @@ -129,4 +189,12 @@ export type ProviderCollectionActionsUnion = | RemoveProviderFail | UpdateProviderRequest | UpdateProviderSuccess - | UpdateProviderFail; + | UpdateProviderFail + | SetOrderProviderRequest + | SetOrderProviderSuccess + | SetOrderProviderFail + | GetOrderProviderRequest + | GetOrderProviderSuccess + | GetOrderProviderFail + | ChangeOrderUp + | ChangeOrderDown; diff --git a/ui/src/app/metadata/provider/effect/collection.effect.ts b/ui/src/app/metadata/provider/effect/collection.effect.ts index fbf4a6c1e..55528b395 100644 --- a/ui/src/app/metadata/provider/effect/collection.effect.ts +++ b/ui/src/app/metadata/provider/effect/collection.effect.ts @@ -19,7 +19,15 @@ import { SelectProviderError, UpdateProviderRequest, UpdateProviderSuccess, - UpdateProviderFail + UpdateProviderFail, + GetOrderProviderRequest, + GetOrderProviderSuccess, + GetOrderProviderFail, + SetOrderProviderRequest, + SetOrderProviderSuccess, + SetOrderProviderFail, + ChangeOrderUp, + ChangeOrderDown } from '../action/collection.action'; import { MetadataProviderService } from '../../domain/service/provider.service'; import * as fromProvider from '../reducer'; @@ -118,6 +126,86 @@ export class CollectionEffects { map(provider => new LoadProviderRequest()) ); + @Effect() + getOrderWithLoad$ = this.actions$.pipe( + ofType(ProviderCollectionActionTypes.LOAD_PROVIDER_SUCCESS), + map(() => new GetOrderProviderRequest()) + ); + + @Effect() + getProviderOrder$ = this.actions$.pipe( + ofType(ProviderCollectionActionTypes.GET_ORDER_PROVIDER_REQUEST), + switchMap(() => + this.providerService.getOrder().pipe( + map(order => new GetOrderProviderSuccess(order)), + catchError(err => of(new GetOrderProviderFail(err))) + ) + ) + ); + + @Effect() + reloadProviderOrderAfterChange$ = this.actions$.pipe( + ofType(ProviderCollectionActionTypes.SET_ORDER_PROVIDER_SUCCESS), + map(() => new GetOrderProviderRequest()) + ); + + @Effect() + setProviderOrder$ = this.actions$.pipe( + ofType(ProviderCollectionActionTypes.SET_ORDER_PROVIDER_REQUEST), + map(action => action.payload), + switchMap(order => + this.providerService.setOrder(order).pipe( + map(() => new SetOrderProviderSuccess()), + catchError(err => of(new SetOrderProviderFail(err))) + ) + ) + ); + + @Effect() + changeOrderUp$ = this.actions$.pipe( + ofType(ProviderCollectionActionTypes.CHANGE_PROVIDER_ORDER_UP), + map(action => action.payload), + withLatestFrom(this.store.select(fromProvider.getProviderOrder)), + map(([id, orderSet]) => { + const order = orderSet.resourceIds; + const index = order.indexOf(id); + if (index > 0) { + const newOrder = this.array_move(order, index, index - 1); + return new SetOrderProviderRequest({ resourceIds: newOrder }); + } else { + return new SetOrderProviderFail(new Error(`could not change order: ${ id }`)); + } + }) + ); + + @Effect() + changeOrderDown$ = this.actions$.pipe( + ofType(ProviderCollectionActionTypes.CHANGE_PROVIDER_ORDER_DOWN), + map(action => action.payload), + withLatestFrom(this.store.select(fromProvider.getProviderOrder)), + map(([id, orderSet]) => { + const order = orderSet.resourceIds; + const index = order.indexOf(id); + if (index < order.length - 1) { + const newOrder = this.array_move(order, index, index + 1); + return new SetOrderProviderRequest({ resourceIds: newOrder }); + } else { + return new SetOrderProviderFail(new Error(`could not change order: ${id}`)); + } + }) + ); + + array_move(arr, old_index, new_index): any[] { + if (new_index >= arr.length) { + let k = new_index - arr.length + 1; + while (k--) { + arr.push(undefined); + } + } + arr.splice(new_index, 0, arr.splice(old_index, 1)[0]); + return arr; + } + constructor( private actions$: Actions, private router: Router, diff --git a/ui/src/app/metadata/provider/reducer/collection.reducer.spec.ts b/ui/src/app/metadata/provider/reducer/collection.reducer.spec.ts index 3e716c212..5feb8ed5b 100644 --- a/ui/src/app/metadata/provider/reducer/collection.reducer.spec.ts +++ b/ui/src/app/metadata/provider/reducer/collection.reducer.spec.ts @@ -1,4 +1,4 @@ -import { reducer } from './collection.reducer'; +import { reducer, initialState as snapshot } from './collection.reducer'; import * as fromProvider from './collection.reducer'; import { ProviderCollectionActionTypes, @@ -6,13 +6,6 @@ import { UpdateProviderSuccess } from '../action/collection.action'; -const snapshot: fromProvider.CollectionState = { - ids: [], - entities: {}, - selectedProviderId: null, - loaded: false -}; - describe('Provider Collection Reducer', () => { describe('undefined action', () => { it('should return the default state', () => { diff --git a/ui/src/app/metadata/provider/reducer/collection.reducer.ts b/ui/src/app/metadata/provider/reducer/collection.reducer.ts index 7d74cb5af..04dfb0297 100644 --- a/ui/src/app/metadata/provider/reducer/collection.reducer.ts +++ b/ui/src/app/metadata/provider/reducer/collection.reducer.ts @@ -1,24 +1,22 @@ import { EntityState, EntityAdapter, createEntityAdapter } from '@ngrx/entity'; import { ProviderCollectionActionTypes, ProviderCollectionActionsUnion } from '../action/collection.action'; import { MetadataProvider } from '../../domain/model'; +import { ProviderOrder } from '../../domain/model/metadata-order'; export interface CollectionState extends EntityState { selectedProviderId: string | null; loaded: boolean; -} - -export function sortByDate(a: MetadataProvider, b: MetadataProvider): number { - return a.createdDate.localeCompare(b.createdDate); + order: ProviderOrder; } export const adapter: EntityAdapter = createEntityAdapter({ - sortComparer: sortByDate, selectId: (model: MetadataProvider) => model.resourceId }); export const initialState: CollectionState = adapter.getInitialState({ selectedProviderId: null, - loaded: false + loaded: false, + order: { resourceIds: [] } }); export function reducer(state = initialState, action: ProviderCollectionActionsUnion): CollectionState { @@ -42,6 +40,13 @@ export function reducer(state = initialState, action: ProviderCollectionActionsU return adapter.updateOne(action.payload, state); } + case ProviderCollectionActionTypes.GET_ORDER_PROVIDER_SUCCESS: { + return { + ...state, + order: action.payload + }; + } + default: { return state; } @@ -56,3 +61,5 @@ export const { selectAll: selectAllProviders, selectTotal: selectProviderTotal } = adapter.getSelectors(); + +export const getProviderOrder = (state: CollectionState) => state.order; diff --git a/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts b/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts index 3079a99cd..a37687766 100644 --- a/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts +++ b/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts @@ -1,5 +1,6 @@ -import { reducer, initialState as snapshot } from './entity.reducer'; +import { reducer, initialState as snapshot, isEntitySaved } from './entity.reducer'; import { EntityActionTypes, ClearProvider } from '../action/entity.action'; +import { MetadataProvider } from '../../domain/model'; describe('Provider Editor Reducer', () => { describe('undefined action', () => { @@ -15,4 +16,22 @@ describe('Provider Editor Reducer', () => { expect(reducer(snapshot, new ClearProvider())).toEqual(snapshot); }); }); + + describe(`isEntitySaved method`, () => { + it('should return false if there are changes', () => { + expect(isEntitySaved({ + ...snapshot, + changes: { + name: 'bar' + } + })).toBe(false); + }); + + it('should return true if there are no changes', () => { + expect(isEntitySaved({ + ...snapshot, + changes: {} + })).toBe(true); + }); + }); }); diff --git a/ui/src/app/metadata/provider/reducer/index.spec.ts b/ui/src/app/metadata/provider/reducer/index.spec.ts new file mode 100644 index 000000000..f595dea38 --- /dev/null +++ b/ui/src/app/metadata/provider/reducer/index.spec.ts @@ -0,0 +1,96 @@ +import * as fromProvider from './'; +import { MetadataProvider } from '../../domain/model'; + +describe(`provider reducer/selector functions`, () => { + + describe('getSchemaParseFn', () => { + const schema = { + properties: { + foo: { + type: 'string' + } + } + }; + const schema2 = { + properties: { + foo: { + type: 'object', + properties: { + bar: { + type: 'string' + } + } + } + } + }; + it('should lock all properties', () => { + expect(fromProvider.getSchemaParseFn(schema, true)).toEqual({ + ...schema, + properties: { + ...schema.properties, + foo: { + ...schema.properties.foo, + readOnly: true + } + } + }); + }); + + it('should unlock all properties', () => { + expect(fromProvider.getSchemaParseFn(schema, false)).toEqual({ + ...schema, + properties: { + ...schema.properties, + foo: { + type: 'string', + readOnly: false + } + } + }); + }); + + it('should lock child properties properties', () => { + expect(fromProvider.getSchemaParseFn(schema2, true)).toEqual({ + ...schema, + properties: { + ...schema2.properties, + foo: { + ...schema2.properties.foo, + readOnly: true, + properties: { + bar: { + ...schema2.properties.foo.properties.bar, + readOnly: true + } + } + } + } + }); + }); + }); + + describe('getSchemaLockedFn', () => { + it('should return true if the step is locked', () => { + expect(fromProvider.getSchemaLockedFn({ locked: true }, false)).toEqual(false); + }); + }); + + describe('mergeProviderOrderFn', () => { + const providers = [ + { resourceId: 'foo', name: 'foo', '@type': 'foo', enabled: true, xmlId: 'id', sortKey: 1, metadataFilters: [] }, + { resourceId: 'bar', name: 'bar', '@type': 'bar', enabled: false, xmlId: 'id2', sortKey: 2, metadataFilters: [] }, + { resourceId: 'baz', name: 'baz', '@type': 'baz', enabled: false, xmlId: 'id3', sortKey: 3, metadataFilters: [] } + ]; + it('1 should sort the list accordingly', () => { + let order = {resourceIds: ['bar', 'foo', 'baz']}, + ordered = fromProvider.mergeProviderOrderFn([...providers], order); + expect(ordered.indexOf(providers[0])).toBe(1); + }); + + it('2 should sort the list accordingly', () => { + let order = { resourceIds: ['foo', 'bar', 'baz'] }, + ordered = fromProvider.mergeProviderOrderFn(providers, order); + expect(ordered.indexOf(providers[0])).toBe(0); + }); + }); +}); diff --git a/ui/src/app/metadata/provider/reducer/index.ts b/ui/src/app/metadata/provider/reducer/index.ts index d56ba8c0a..1fefd6406 100644 --- a/ui/src/app/metadata/provider/reducer/index.ts +++ b/ui/src/app/metadata/provider/reducer/index.ts @@ -9,6 +9,7 @@ import * as fromWizard from '../../../wizard/reducer'; import { MetadataProvider } from '../../domain/model'; import { WizardStep } from '../../../wizard/model'; +import { ProviderOrder } from '../../domain/model/metadata-order'; export interface ProviderState { editor: fromEditor.EditorState; @@ -87,6 +88,7 @@ export const getUpdatedEntity = createSelector(getEntityState, fromEntity.getUpd /* * Select pieces of Provider Collection */ +export const getProviderOrder = createSelector(getCollectionState, fromCollection.getProviderOrder); export const getAllProviders = createSelector(getCollectionState, fromCollection.selectAllProviders); export const getProviderEntities = createSelector(getCollectionState, fromCollection.selectProviderEntities); export const getSelectedProviderId = createSelector(getCollectionState, fromCollection.getSelectedProviderId); @@ -100,3 +102,13 @@ export const getProviderFilters = createSelector(getSelectedProvider, provider = export const getProviderXmlIds = createSelector(getAllProviders, (providers: MetadataProvider[]) => providers.map(p => p.xmlId)); +export const mergeProviderOrderFn = (providers: MetadataProvider[], order: ProviderOrder): MetadataProvider[] => { + return [...providers.sort( + (a: MetadataProvider, b: MetadataProvider) => { + const aIndex = order.resourceIds.indexOf(a.resourceId); + const bIndex = order.resourceIds.indexOf(b.resourceId); + return aIndex > bIndex ? 1 : bIndex > aIndex ? -1 : 0; + } + )]; +}; +export const getOrderedProviders = createSelector(getAllProviders, getProviderOrder, mergeProviderOrderFn); diff --git a/ui/src/app/shared/switch/switch.component.spec.ts b/ui/src/app/shared/switch/switch.component.spec.ts index 144dd8b71..16933e729 100644 --- a/ui/src/app/shared/switch/switch.component.spec.ts +++ b/ui/src/app/shared/switch/switch.component.spec.ts @@ -20,6 +20,7 @@ class TestHostComponent { describe('Toggle Switch Component', () => { let fixture: ComponentFixture; let instance: TestHostComponent; + let cmp: ToggleSwitchComponent; beforeEach(() => { TestBed.configureTestingModule({ @@ -37,10 +38,24 @@ describe('Toggle Switch Component', () => { fixture = TestBed.createComponent(TestHostComponent); instance = fixture.componentInstance; + cmp = instance.instanceUnderTest; fixture.detectChanges(); }); it('should compile', () => { expect(fixture).toBeDefined(); }); + + describe('value getter', () => { + it('should return the checked value', () => { + expect(cmp.value).toBe(false); + }); + }); + + describe('setDisabledState', () => { + it('should set the disabled property', () => { + cmp.setDisabledState(true); + expect(cmp.disabled).toBe(true); + }) + }); }); From 2b6988242775f3e288d8f71c7df131bd992e088f Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 10:55:17 -0400 Subject: [PATCH 09/17] Refactoring filters persistance --- .../controller/MetadataFiltersController.java | 58 ++++------- .../filters/EntityAttributesFilter.java | 9 +- .../ui/domain/filters/MetadataFilter.java | 15 ++- ...taFiltersControllerIntegrationTests.groovy | 97 +++++++++++++++++++ 4 files changed, 134 insertions(+), 45 deletions(-) create mode 100644 backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java index 7d873afa6..a6491fa86 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java @@ -6,10 +6,10 @@ import edu.internet2.tier.shibboleth.admin.ui.domain.filters.RequiredValidUntilFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.SignatureValidationFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver; +import edu.internet2.tier.shibboleth.admin.ui.repository.FilterRepository; import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository; import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -24,7 +24,6 @@ import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import java.net.URI; -import java.util.List; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -32,14 +31,15 @@ @RequestMapping("/api/MetadataResolvers/{metadataResolverId}") public class MetadataFiltersController { - private static Logger LOGGER = LoggerFactory.getLogger(MetadataFiltersController.class); - @Autowired private MetadataResolverRepository repository; @Autowired private MetadataResolverService metadataResolverService; + @Autowired + private FilterRepository filterRepository; + @GetMapping("/Filters") @Transactional(readOnly = true) public ResponseEntity getAll(@PathVariable String metadataResolverId) { @@ -78,8 +78,7 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques // we reload the filters here after save metadataResolverService.reloadFilters(persistedMr.getName()); - MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); + MetadataFilter persistedFilter = newlyPersistedFilter(persistedMr.getMetadataFilters().stream(), createdFilter.getResourceId()); return ResponseEntity .created(getResourceUriFor(persistedMr, createdFilter.getResourceId())) @@ -91,30 +90,28 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques public ResponseEntity update(@PathVariable String metadataResolverId, @PathVariable String resourceId, @RequestBody MetadataFilter updatedFilter) { + MetadataFilter filterTobeUpdated = filterRepository.findByResourceId(resourceId); + if (filterTobeUpdated == null) { + return ResponseEntity.notFound().build(); + } MetadataResolver metadataResolver = repository.findByResourceId(metadataResolverId); if(metadataResolver == null) { return ResponseEntity.notFound().build(); } - if (!resourceId.equals(updatedFilter.getResourceId())) { - return new ResponseEntity(HttpStatus.CONFLICT); + // check to make sure that the relationship exists + if (!metadataResolver.getMetadataFilters().contains(filterTobeUpdated)) { + // TODO: find a better response + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } - List filters = - metadataResolver.getMetadataFilters().stream() - .filter(f -> f.getResourceId().equals(updatedFilter.getResourceId())) - .collect(Collectors.toList()); - if (filters.size() > 1) { - // TODO: I don't think this should ever happen, but... if it does... - // do something? throw exception, return error? - LOGGER.warn("More than one filter was found for id {}! This is probably a bad thing.\n" + - "We're going to go ahead and use the first one, but .. look in to this!", updatedFilter.getResourceId()); + if (!resourceId.equals(updatedFilter.getResourceId())) { + return new ResponseEntity(HttpStatus.CONFLICT); } - MetadataFilter filterTobeUpdated = filters.get(0); // Verify we're the only one attempting to update the filter - if (updatedFilter.getVersion() != filterTobeUpdated.hashCode()) { + if (updatedFilter.getVersion() != filterTobeUpdated.getVersion()) { return new ResponseEntity(HttpStatus.CONFLICT); } @@ -122,32 +119,19 @@ public ResponseEntity update(@PathVariable String metadataResolverId, filterTobeUpdated.setFilterEnabled(updatedFilter.isFilterEnabled()); updateConcreteFilterTypeData(filterTobeUpdated, updatedFilter); - //convert before saving into database - if(filterTobeUpdated instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(filterTobeUpdated).fromTransientRepresentation(); - } + MetadataFilter persistedFilter = filterRepository.save(filterTobeUpdated); - MetadataResolver persistedMr = repository.save(metadataResolver); - - metadataResolverService.reloadFilters(persistedMr.getName()); - - MetadataFilter persistedFilter = - convertIntoTransientRepresentationIfNecessary(persistedMr.getMetadataFilters().stream(), updatedFilter.getResourceId()); - - persistedFilter.setVersion(persistedFilter.hashCode()); + // TODO: this is wrong + metadataResolverService.reloadFilters(metadataResolver.getName()); return ResponseEntity.ok().body(persistedFilter); } - private MetadataFilter convertIntoTransientRepresentationIfNecessary(Stream filters, final String filterResourceId) { + private MetadataFilter newlyPersistedFilter(Stream filters, final String filterResourceId) { MetadataFilter persistedFilter = filters .filter(f -> f.getResourceId().equals(filterResourceId)) .collect(Collectors.toList()).get(0); - //convert before saving into database - if(persistedFilter instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(persistedFilter).intoTransientRepresentation(); - } return persistedFilter; } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java index a62ed9a24..e5e923918 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java @@ -51,19 +51,22 @@ public EntityAttributesFilter() { @Transient private RelyingPartyOverridesRepresentation relyingPartyOverrides; + @PostLoad public void intoTransientRepresentation() { this.attributeRelease = getAttributeReleaseListFromAttributeList(this.attributes); this.relyingPartyOverrides = getRelyingPartyOverridesRepresentationFromAttributeList(attributes); - updateVersion(); } + @PrePersist + @PreUpdate public void fromTransientRepresentation() { List attributeList = new ArrayList<>(); attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); attributeList.addAll(getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); if(!attributeList.isEmpty()) { - this.attributes = (List) (List) attributeList; + this.attributes.clear(); + this.attributes.addAll((List) (List) attributeList); } } -} +} \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java index 0347de1f7..a80d538c9 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/MetadataFilter.java @@ -1,5 +1,6 @@ package edu.internet2.tier.shibboleth.admin.ui.domain.filters; +import com.fasterxml.jackson.annotation.JsonGetter; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -46,9 +47,13 @@ public class MetadataFilter extends AbstractAuditable { private boolean filterEnabled; @Transient - private int version; - - public void updateVersion() { - this.version = hashCode(); + private transient Integer version; + + @JsonGetter("version") + public int getVersion() { + if (version != null && version != 0) { + return this.version; + } + return this.hashCode(); } -} +} \ No newline at end of file diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy new file mode 100644 index 000000000..1804ff6f1 --- /dev/null +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy @@ -0,0 +1,97 @@ +package edu.internet2.tier.shibboleth.admin.ui.controller + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.SerializationFeature +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule +import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository +import edu.internet2.tier.shibboleth.admin.ui.util.TestObjectGenerator +import edu.internet2.tier.shibboleth.admin.util.AttributeUtility +import groovy.json.JsonOutput +import groovy.json.JsonSlurper +import org.opensaml.saml.metadata.resolver.ChainingMetadataResolver +import org.opensaml.saml.metadata.resolver.MetadataResolver +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.boot.test.context.TestConfiguration +import org.springframework.boot.test.web.client.TestRestTemplate +import org.springframework.context.annotation.Bean +import org.springframework.http.HttpEntity +import org.springframework.http.HttpHeaders +import org.springframework.test.context.ActiveProfiles +import spock.lang.Specification + +import static org.springframework.http.HttpMethod.PUT + +/** + * @author Dmitriy Kopylenko + */ +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +@ActiveProfiles("no-auth") +class MetadataFiltersControllerIntegrationTests extends Specification { + + @Autowired + private TestRestTemplate restTemplate + + @Autowired + MetadataResolverRepository metadataResolverRepository + + @Autowired + AttributeUtility attributeUtility + + ObjectMapper mapper + TestObjectGenerator generator + + JsonSlurper jsonSlurper = new JsonSlurper() + + static BASE_URI = '/api/MetadataResolvers' + + def setup() { + generator = new TestObjectGenerator(attributeUtility) + mapper = new ObjectMapper() + mapper.enable(SerializationFeature.INDENT_OUTPUT) + mapper.registerModule(new JavaTimeModule()) + } + + def cleanup() { + metadataResolverRepository.deleteAll() + } + + def "PUT EntityAttributesFilter"() { + given: 'MetadataResolver with attached entity attributes is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + def filterResourceId = resolver.metadataFilters[0].resourceId + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + + when: 'GET request is made with resource Id matching the existing filter' + def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) + def existingFilterMap = jsonSlurper.parseText(result.body) + + and: 'PUT call is made with unmodified filter state' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/$resolverResourceId/Filters/$filterResourceId", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingFilterMap) }, String) + + then: + updatedResultFromPUT.statusCode.value() == 200 + } + + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { + new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) + } + + @TestConfiguration + static class Config { + @Bean + MetadataResolver metadataResolver() { + new ChainingMetadataResolver().with { + it.id = 'tester' + it.initialize() + return it + } + } + } +} \ No newline at end of file From ed5fe9f325e74b3076145676a92e99c28f0305dd Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 14 Aug 2018 07:55:23 -0700 Subject: [PATCH 10/17] SHIBUI-331 Implemented unsaved changes modal in provider editor --- .../domain/service/provider.service.ts | 6 +++- .../dashboard-providers-list.component.ts | 1 - .../component/unsaved-provider.dialog.html | 16 +++++++++ .../component/unsaved-provider.dialog.ts | 28 +++++++++++++++ .../container/provider-edit.component.ts | 34 +++++++++++++++++-- .../app/metadata/provider/provider.module.ts | 8 +++-- .../app/metadata/provider/provider.routing.ts | 7 ++-- .../provider/reducer/entity.reducer.ts | 2 +- .../resolver/container/editor.component.ts | 1 - 9 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 ui/src/app/metadata/provider/component/unsaved-provider.dialog.html create mode 100644 ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts diff --git a/ui/src/app/metadata/domain/service/provider.service.ts b/ui/src/app/metadata/domain/service/provider.service.ts index 3d83d3c55..ede4bb172 100644 --- a/ui/src/app/metadata/domain/service/provider.service.ts +++ b/ui/src/app/metadata/domain/service/provider.service.ts @@ -1,11 +1,13 @@ import { Injectable } from '@angular/core'; import { HttpClient } from '@angular/common/http'; import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; import { MetadataProvider } from '../../domain/model'; import { FileBackedHttpMetadataProvider } from '../model/providers'; import { ProviderOrder } from '../model/metadata-order'; + @Injectable() export class MetadataProviderService { @@ -17,7 +19,9 @@ export class MetadataProviderService { private http: HttpClient ) {} query(): Observable { - return this.http.get(`${this.base}${this.endpoint}`, {}); + return this.http.get(`${this.base}${this.endpoint}`).pipe( + map(providers => providers.filter(p => p['@type'] !== 'BaseMetadataResolver')) + ); } find(id: string): Observable { diff --git a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts index 1815d4ccf..88ec0b19e 100644 --- a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts +++ b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts @@ -28,7 +28,6 @@ export class DashboardProvidersListComponent implements OnInit { ngOnInit(): void { this.providers$ = this.store.select(getOrderedProviders); this.providersOpen$ = this.store.select(getOpenProviders); - this.providers$.subscribe(p => console.log(p)); } view(id: string, page: string): void { diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.html b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.html new file mode 100644 index 000000000..f556f49cd --- /dev/null +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.html @@ -0,0 +1,16 @@ + +
    +
    +

    Save your information?

    +
    +
    +

    You have not saved your changes. If you exit this screen, your changes will be lost.

    +
    +
    + + +
    +
    +     \ No newline at end of file diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts new file mode 100644 index 000000000..beafacf87 --- /dev/null +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts @@ -0,0 +1,28 @@ +import { Component, Input } from '@angular/core'; + +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { Store, Action } from '@ngrx/store'; +import { Subject } from 'rxjs/Subject'; + +import * as fromEditor from '../reducer'; + +@Component({ + selector: 'unsaved-provider', + templateUrl: './unsaved-provider.dialog.html' +}) +export class UnsavedProviderComponent { + readonly subject: Subject = new Subject(); + + constructor( + public activeModal: NgbActiveModal, + private store: Store + ) { } + + close(): void { + this.activeModal.close(); + } + + dismiss(): void { + this.activeModal.dismiss(); + } +} diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.ts b/ui/src/app/metadata/provider/container/provider-edit.component.ts index 07a79a7cf..ea7114022 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.ts @@ -1,6 +1,6 @@ import { Component, OnDestroy } from '@angular/core'; -import { Router, ActivatedRoute } from '@angular/router'; -import { Observable } from 'rxjs'; +import { Router, ActivatedRoute, ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; +import { Observable, of } from 'rxjs'; import { skipWhile, map, combineLatest } from 'rxjs/operators'; import { Store } from '@ngrx/store'; import * as fromWizard from '../../../wizard/reducer'; @@ -12,6 +12,9 @@ import { ClearProvider } from '../action/entity.action'; import { Wizard } from '../../../wizard/model'; import { UpdateProviderRequest } from '../action/collection.action'; import { NAV_FORMATS } from '../component/provider-editor-nav.component'; +import { NgbModal } from '../../../../../node_modules/@ng-bootstrap/ng-bootstrap'; +import { UnsavedDialogComponent } from '../../resolver/component/unsaved-dialog.component'; +import { UnsavedProviderComponent } from '../component/unsaved-provider.dialog'; @Component({ selector: 'provider-edit', @@ -36,7 +39,8 @@ export class ProviderEditComponent implements OnDestroy { constructor( private store: Store, private router: Router, - private route: ActivatedRoute + private route: ActivatedRoute, + private modalService: NgbModal ) { this.provider$ = this.store.select(fromProvider.getSelectedProvider).pipe(skipWhile(d => !d)); this.definition$ = this.store.select(fromWizard.getWizardDefinition).pipe(skipWhile(d => !d)); @@ -77,6 +81,10 @@ export class ProviderEditComponent implements OnDestroy { } ngOnDestroy() { + this.clear(); + } + + clear(): void { this.store.dispatch(new ClearProvider()); this.store.dispatch(new ClearWizard()); this.store.dispatch(new ClearEditor()); @@ -87,7 +95,27 @@ export class ProviderEditComponent implements OnDestroy { } cancel(): void { + this.clear(); this.router.navigate(['metadata', 'manager', 'providers']); } + + canDeactivate( + currentRoute: ActivatedRouteSnapshot, + currentState: RouterStateSnapshot, + nextState: RouterStateSnapshot + ): Observable { + if (nextState.url.match('edit')) { return of(true); } + if (Object.keys({ ...this.latest }).length > 0) { + let modal = this.modalService.open(UnsavedProviderComponent); + modal.result.then( + () => { + this.clear(); + this.router.navigate([nextState.url]); + }, + () => console.warn('denied') + ); + } + return this.store.select(fromProvider.getEntityIsSaved); + } } diff --git a/ui/src/app/metadata/provider/provider.module.ts b/ui/src/app/metadata/provider/provider.module.ts index dfd95fa8f..95d037256 100644 --- a/ui/src/app/metadata/provider/provider.module.ts +++ b/ui/src/app/metadata/provider/provider.module.ts @@ -28,6 +28,7 @@ import { EntityEffects } from './effect/entity.effect'; import { ProviderFilterListComponent } from './container/provider-filter-list.component'; import { ProviderEditorNavComponent } from './component/provider-editor-nav.component'; +import { UnsavedProviderComponent } from './component/unsaved-provider.dialog'; @NgModule({ declarations: [ @@ -40,9 +41,12 @@ import { ProviderEditorNavComponent } from './component/provider-editor-nav.comp ProviderSelectComponent, ProviderFilterListComponent, SummaryPropertyComponent, - ProviderEditorNavComponent + ProviderEditorNavComponent, + UnsavedProviderComponent + ], + entryComponents: [ + UnsavedProviderComponent ], - entryComponents: [], imports: [ ReactiveFormsModule, CommonModule, diff --git a/ui/src/app/metadata/provider/provider.routing.ts b/ui/src/app/metadata/provider/provider.routing.ts index 8e6ecc3f5..7acd52fec 100644 --- a/ui/src/app/metadata/provider/provider.routing.ts +++ b/ui/src/app/metadata/provider/provider.routing.ts @@ -10,6 +10,7 @@ import { ProviderFilterListComponent } from './container/provider-filter-list.co import { NewFilterComponent } from '../filter/container/new-filter.component'; import { FilterComponent } from '../filter/container/filter.component'; import { EditFilterComponent } from '../filter/container/edit-filter.component'; +import { CanDeactivateGuard } from '../../core/service/can-deactivate.guard'; export const ProviderRoutes: Routes = [ { @@ -44,6 +45,9 @@ export const ProviderRoutes: Routes = [ path: ':form', component: ProviderEditStepComponent } + ], + canDeactivate: [ + CanDeactivateGuard ] }, { @@ -61,8 +65,7 @@ export const ProviderRoutes: Routes = [ children: [ { path: 'edit', - component: EditFilterComponent, - canDeactivate: [] + component: EditFilterComponent } ] } diff --git a/ui/src/app/metadata/provider/reducer/entity.reducer.ts b/ui/src/app/metadata/provider/reducer/entity.reducer.ts index d5b63eca1..175c769d8 100644 --- a/ui/src/app/metadata/provider/reducer/entity.reducer.ts +++ b/ui/src/app/metadata/provider/reducer/entity.reducer.ts @@ -41,7 +41,7 @@ export function reducer(state = initialState, action: EntityActionUnion): Entity } } -export const isEntitySaved = (state: EntityState) => !Object.keys(state.changes).length && !state.saving; +export const isEntitySaved = (state: EntityState) => state.changes ? !Object.keys(state.changes).length && !state.saving : true; export const getEntityChanges = (state: EntityState) => state.changes; export const isEditorSaving = (state: EntityState) => state.saving; export const getUpdatedEntity = (state: EntityState) => state.changes; diff --git a/ui/src/app/metadata/resolver/container/editor.component.ts b/ui/src/app/metadata/resolver/container/editor.component.ts index 6aa252abe..5d4b6e1c3 100644 --- a/ui/src/app/metadata/resolver/container/editor.component.ts +++ b/ui/src/app/metadata/resolver/container/editor.component.ts @@ -76,7 +76,6 @@ export class EditorComponent implements OnInit, OnDestroy { ); this.providerName$ = this.resolver$.pipe(map(p => p.serviceProviderName)); - this.changes$ = this.store.select(fromResolver.getEditorChanges); this.editorIndex$ = this.route.params.pipe(map(params => Number(params.index))); this.currentPage$ = this.editorIndex$.pipe(map(index => EditorDef.find(r => r.index === index))); this.editor = EditorDef; From e83f17fb5cd2ca4547584afb176b115597f110a6 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 11:22:33 -0400 Subject: [PATCH 11/17] Add test for updated entity attributes filter --- ...taFiltersControllerIntegrationTests.groovy | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy index 1804ff6f1..557fdf56e 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerIntegrationTests.groovy @@ -79,6 +79,30 @@ class MetadataFiltersControllerIntegrationTests extends Specification { updatedResultFromPUT.statusCode.value() == 200 } + def "PUT EntityAttributesFilter and update it"() { + given: 'MetadataResolver with attached entity attributes is available in data store' + def resolver = generator.buildRandomMetadataResolverOfType('FileBacked') + resolver.metadataFilters << generator.entityAttributesFilter() + def filterResourceId = resolver.metadataFilters[0].resourceId + def resolverResourceId = resolver.resourceId + metadataResolverRepository.save(resolver) + + + when: 'GET request is made with resource Id matching the existing filter' + def result = this.restTemplate.getForEntity("$BASE_URI/$resolverResourceId/Filters/$filterResourceId", String) + def existingFilterMap = jsonSlurper.parseText(result.body) + + and: 'PUT call is made with modified filter state' + existingFilterMap.name = 'Entity Attributes Filter Updated' + def updatedResultFromPUT = this.restTemplate.exchange( + "$BASE_URI/$resolverResourceId/Filters/$filterResourceId", + PUT, + createRequestHttpEntityFor { JsonOutput.toJson(existingFilterMap) }, String) + + then: + updatedResultFromPUT.statusCode.value() == 200 + } + private HttpEntity createRequestHttpEntityFor(Closure jsonBodySupplier) { new HttpEntity(jsonBodySupplier(), ['Content-Type': 'application/json'] as HttpHeaders) } From ce19055806395aea94ffd911eda65f7b9088c3a3 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 14 Aug 2018 09:27:29 -0700 Subject: [PATCH 12/17] SHIBUI-331 Implemented unsaved changes modal and tests --- .../filter/entity-attributes-filter.spec.ts | 4 ++ .../component/unsaved-provider.dialog.spec.ts | 46 +++++++++++++ .../component/unsaved-provider.dialog.ts | 3 +- .../container/provider-edit.component.spec.ts | 50 ++++++++++++++- .../container/provider-edit.component.ts | 3 +- .../wizard/component/wizard.component.spec.ts | 64 +++++++++++++++++++ .../app/wizard/component/wizard.component.ts | 8 ++- 7 files changed, 169 insertions(+), 9 deletions(-) create mode 100644 ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts diff --git a/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts b/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts index 04d1234be..22eae4cda 100644 --- a/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts +++ b/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts @@ -13,5 +13,9 @@ describe('EntityAttributesFilter Entity', () => { expect(entity).toBeDefined(); expect(entity.resourceId).toBe('foo'); expect(entity.enabled).toBe(entity.filterEnabled); + expect(entity.id).toBe(entity.resourceId); + expect(entity.getId()).toBe(entity.entityId); + expect(entity.getDisplayId()).toBe(entity.entityId); + expect(entity.isDraft()).toBe(false); }); }); diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts new file mode 100644 index 000000000..e9f0e4554 --- /dev/null +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts @@ -0,0 +1,46 @@ +import { Component, ViewChild } from '@angular/core'; +import { TestBed, async, ComponentFixture} from '@angular/core/testing'; +import { RouterTestingModule } from '@angular/router/testing'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { SharedModule } from '../../../shared/shared.module'; +import { UnsavedProviderComponent } from './unsaved-provider.dialog'; +import { NgbActiveModalStub } from '../../../../testing/modal.stub'; + +@Component({ + template: ` + + ` +}) +class TestHostComponent { + @ViewChild(UnsavedProviderComponent) + public componentUnderTest: UnsavedProviderComponent; +} + +describe('Unsaved Provider Dialog Component', () => { + + let fixture: ComponentFixture; + let instance: TestHostComponent; + let cmp: UnsavedProviderComponent; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [], + declarations: [ + UnsavedProviderComponent, + TestHostComponent + ], + providers: [ + { provide: NgbActiveModal, useClass: NgbActiveModalStub } + ] + }).compileComponents(); + + fixture = TestBed.createComponent(TestHostComponent); + instance = fixture.componentInstance; + cmp = instance.componentUnderTest; + fixture.detectChanges(); + })); + + it('should instantiate the component', async(() => { + expect(cmp).toBeTruthy(); + })); +}); diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts index beafacf87..4b8d12da3 100644 --- a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts @@ -14,8 +14,7 @@ export class UnsavedProviderComponent { readonly subject: Subject = new Subject(); constructor( - public activeModal: NgbActiveModal, - private store: Store + public activeModal: NgbActiveModal ) { } close(): void { diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts b/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts index a62734833..05cb75832 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts @@ -1,10 +1,10 @@ import { Component, ViewChild } from '@angular/core'; -import { ActivatedRoute, Router } from '@angular/router'; +import { ActivatedRoute, Router, ActivatedRouteSnapshot } from '@angular/router'; import { APP_BASE_HREF } from '@angular/common'; -import { TestBed, async, ComponentFixture } from '@angular/core/testing'; +import { TestBed, async, ComponentFixture, fakeAsync } from '@angular/core/testing'; import { RouterTestingModule } from '@angular/router/testing'; import { StoreModule, Store, combineReducers } from '@ngrx/store'; -import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; +import { NgbDropdownModule, NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { ProviderEditComponent } from './provider-edit.component'; import * as fromRoot from '../reducer'; import * as fromWizard from '../../../wizard/reducer'; @@ -12,6 +12,9 @@ import { SharedModule } from '../../../shared/shared.module'; import { ActivatedRouteStub } from '../../../../testing/activated-route.stub'; import { FileBackedHttpMetadataProviderEditor } from '../model'; import { ProviderEditorNavComponent } from '../component/provider-editor-nav.component'; +import { NgbModalStub } from '../../../../testing/modal.stub'; +import { MetadataProvider } from '../../domain/model'; +import { of } from 'rxjs'; @Component({ template: ` @@ -32,6 +35,7 @@ describe('Provider Edit Component', () => { let router: Router; let activatedRoute: ActivatedRouteStub = new ActivatedRouteStub(); let child: ActivatedRouteStub = new ActivatedRouteStub(); + let modal: NgbModal; child.testParamMap = { form: 'common' }; activatedRoute.firstChild = child; @@ -60,6 +64,7 @@ describe('Provider Edit Component', () => { ProviderEditorNavComponent ], providers: [ + { provide: NgbModal, useClass: NgbModalStub }, { provide: ActivatedRoute, useValue: activatedRoute }, { provide: APP_BASE_HREF, useValue: '/' } ] @@ -67,6 +72,7 @@ describe('Provider Edit Component', () => { store = TestBed.get(Store); router = TestBed.get(Router); + modal = TestBed.get(NgbModal); spyOn(store, 'dispatch'); fixture = TestBed.createComponent(TestHostComponent); @@ -104,8 +110,46 @@ describe('Provider Edit Component', () => { describe('cancel method', () => { it('should route to the metadata manager', () => { spyOn(router, 'navigate'); + spyOn(app, 'clear'); app.cancel(); expect(router.navigate).toHaveBeenCalled(); + expect(app.clear).toHaveBeenCalled(); + }); + }); + + describe('clear method', () => { + it('should dispatch actions to clear the reducer state', () => { + app.clear(); + expect(store.dispatch).toHaveBeenCalled(); + }); + }); + + describe('canDeactivate method', () => { + it('should check if the current route is another edit page', (done) => { + let route = new ActivatedRouteStub(), + snapshot = route.snapshot; + let result = app.canDeactivate(null, { url: 'edit', root: null }, { url: 'edit', root: null }); + result.subscribe(can => { + expect(can).toBe(true); + done(); + }); + fixture.detectChanges(); + }); + + it('should open a modal', (done) => { + app.latest = { name: 'bar' }; + spyOn(store, 'select').and.returnValue(of(false)); + spyOn(modal, 'open').and.returnValue({ result: Promise.resolve('closed') }); + fixture.detectChanges(); + let route = new ActivatedRouteStub(), + snapshot = route.snapshot; + let result = app.canDeactivate(null, { url: 'edit', root: null }, { url: 'foo', root: null }); + result.subscribe(can => { + expect(can).toBe(false); + expect(modal.open).toHaveBeenCalled(); + done(); + }); + fixture.detectChanges(); }); }); }); diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.ts b/ui/src/app/metadata/provider/container/provider-edit.component.ts index ea7114022..56616fa8b 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.ts @@ -15,6 +15,7 @@ import { NAV_FORMATS } from '../component/provider-editor-nav.component'; import { NgbModal } from '../../../../../node_modules/@ng-bootstrap/ng-bootstrap'; import { UnsavedDialogComponent } from '../../resolver/component/unsaved-dialog.component'; import { UnsavedProviderComponent } from '../component/unsaved-provider.dialog'; +import { CanComponentDeactivate } from '../../../core/service/can-deactivate.guard'; @Component({ selector: 'provider-edit', @@ -22,7 +23,7 @@ import { UnsavedProviderComponent } from '../component/unsaved-provider.dialog'; styleUrls: [] }) -export class ProviderEditComponent implements OnDestroy { +export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate { provider$: Observable; definition$: Observable>; diff --git a/ui/src/app/wizard/component/wizard.component.spec.ts b/ui/src/app/wizard/component/wizard.component.spec.ts index e69de29bb..ef513015e 100644 --- a/ui/src/app/wizard/component/wizard.component.spec.ts +++ b/ui/src/app/wizard/component/wizard.component.spec.ts @@ -0,0 +1,64 @@ +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 * as fromWizard from '../reducer'; +import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; +import { WizardComponent, ICONS } from './wizard.component'; + +@Component({ + template: ` + + ` +}) +class TestHostComponent { + @ViewChild(WizardComponent) + public componentUnderTest: WizardComponent; +} + +describe('Wizard Component', () => { + + let fixture: ComponentFixture; + let instance: TestHostComponent; + let app: WizardComponent; + let store: Store; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [ + NgbDropdownModule.forRoot(), + RouterTestingModule, + StoreModule.forRoot({ + wizard: combineReducers(fromWizard.reducers) + }) + ], + declarations: [ + WizardComponent, + TestHostComponent + ], + }).compileComponents(); + + store = TestBed.get(Store); + spyOn(store, 'dispatch'); + + fixture = TestBed.createComponent(TestHostComponent); + instance = fixture.componentInstance; + app = instance.componentUnderTest; + fixture.detectChanges(); + })); + + it('should compile without error', () => { + expect(app).toBeTruthy(); + }); + + describe('getIcon method', () => { + it('should return the check string for the last index', () => { + expect(app.getIcon({ index: 'foo' }, { index: 'foo' })).toEqual(ICONS.CHECK); + }); + it('should return the index icon for other indexes', () => { + expect(app.getIcon({ index: 'foo' }, { index: 'bar' })).toEqual(ICONS.INDEX); + expect(app.getIcon({ index: 'foo' }, null)).toEqual(ICONS.INDEX); + }); + }); +}); diff --git a/ui/src/app/wizard/component/wizard.component.ts b/ui/src/app/wizard/component/wizard.component.ts index a75a3bb2d..7b4aa8ff4 100644 --- a/ui/src/app/wizard/component/wizard.component.ts +++ b/ui/src/app/wizard/component/wizard.component.ts @@ -53,9 +53,11 @@ export class WizardComponent { this.currentIcon$ = this.current$.pipe( withLatestFrom(this.last$), - map(([current, last]) => { - return (last && current.index === last.index) ? ICONS.CHECK : ICONS.INDEX; - }) + map(([current, last]) => this.getIcon(current, last)) ); } + + getIcon(current, last): string { + return (last && current.index === last.index) ? ICONS.CHECK : ICONS.INDEX; + } } From 10c1241abc53837044b176fec6213d604517ddaa Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 14 Aug 2018 17:11:36 +0000 Subject: [PATCH 13/17] Merged in feature/SHIBUI-331 (pull request #155) SHIBUI-331 Implemented unsaved changes modal in provider editor * SHIBUI-331 Implemented unsaved changes modal in provider editor Approved-by: Shibui Jenkins Approved-by: Ryan Mathis --- .../filter/entity-attributes-filter.spec.ts | 4 ++ .../domain/service/provider.service.ts | 6 +- .../dashboard-providers-list.component.ts | 1 - .../component/unsaved-provider.dialog.html | 16 +++++ .../component/unsaved-provider.dialog.spec.ts | 46 +++++++++++++ .../component/unsaved-provider.dialog.ts | 27 ++++++++ .../container/provider-edit.component.spec.ts | 50 ++++++++++++++- .../container/provider-edit.component.ts | 37 +++++++++-- .../app/metadata/provider/provider.module.ts | 8 ++- .../app/metadata/provider/provider.routing.ts | 7 +- .../provider/reducer/entity.reducer.ts | 2 +- .../resolver/container/editor.component.ts | 1 - .../wizard/component/wizard.component.spec.ts | 64 +++++++++++++++++++ .../app/wizard/component/wizard.component.ts | 8 ++- 14 files changed, 259 insertions(+), 18 deletions(-) create mode 100644 ui/src/app/metadata/provider/component/unsaved-provider.dialog.html create mode 100644 ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts create mode 100644 ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts diff --git a/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts b/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts index 04d1234be..22eae4cda 100644 --- a/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts +++ b/ui/src/app/metadata/domain/entity/filter/entity-attributes-filter.spec.ts @@ -13,5 +13,9 @@ describe('EntityAttributesFilter Entity', () => { expect(entity).toBeDefined(); expect(entity.resourceId).toBe('foo'); expect(entity.enabled).toBe(entity.filterEnabled); + expect(entity.id).toBe(entity.resourceId); + expect(entity.getId()).toBe(entity.entityId); + expect(entity.getDisplayId()).toBe(entity.entityId); + expect(entity.isDraft()).toBe(false); }); }); diff --git a/ui/src/app/metadata/domain/service/provider.service.ts b/ui/src/app/metadata/domain/service/provider.service.ts index 3d83d3c55..ede4bb172 100644 --- a/ui/src/app/metadata/domain/service/provider.service.ts +++ b/ui/src/app/metadata/domain/service/provider.service.ts @@ -1,11 +1,13 @@ import { Injectable } from '@angular/core'; import { HttpClient } from '@angular/common/http'; import { Observable } from 'rxjs'; +import { map } from 'rxjs/operators'; import { MetadataProvider } from '../../domain/model'; import { FileBackedHttpMetadataProvider } from '../model/providers'; import { ProviderOrder } from '../model/metadata-order'; + @Injectable() export class MetadataProviderService { @@ -17,7 +19,9 @@ export class MetadataProviderService { private http: HttpClient ) {} query(): Observable { - return this.http.get(`${this.base}${this.endpoint}`, {}); + return this.http.get(`${this.base}${this.endpoint}`).pipe( + map(providers => providers.filter(p => p['@type'] !== 'BaseMetadataResolver')) + ); } find(id: string): Observable { diff --git a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts index 1815d4ccf..88ec0b19e 100644 --- a/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts +++ b/ui/src/app/metadata/manager/container/dashboard-providers-list.component.ts @@ -28,7 +28,6 @@ export class DashboardProvidersListComponent implements OnInit { ngOnInit(): void { this.providers$ = this.store.select(getOrderedProviders); this.providersOpen$ = this.store.select(getOpenProviders); - this.providers$.subscribe(p => console.log(p)); } view(id: string, page: string): void { diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.html b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.html new file mode 100644 index 000000000..f556f49cd --- /dev/null +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.html @@ -0,0 +1,16 @@ + +
    +
    +

    Save your information?

    +
    +
    +

    You have not saved your changes. If you exit this screen, your changes will be lost.

    +
    +
    + + +
    +
    +     \ No newline at end of file diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts new file mode 100644 index 000000000..e9f0e4554 --- /dev/null +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.spec.ts @@ -0,0 +1,46 @@ +import { Component, ViewChild } from '@angular/core'; +import { TestBed, async, ComponentFixture} from '@angular/core/testing'; +import { RouterTestingModule } from '@angular/router/testing'; +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { SharedModule } from '../../../shared/shared.module'; +import { UnsavedProviderComponent } from './unsaved-provider.dialog'; +import { NgbActiveModalStub } from '../../../../testing/modal.stub'; + +@Component({ + template: ` + + ` +}) +class TestHostComponent { + @ViewChild(UnsavedProviderComponent) + public componentUnderTest: UnsavedProviderComponent; +} + +describe('Unsaved Provider Dialog Component', () => { + + let fixture: ComponentFixture; + let instance: TestHostComponent; + let cmp: UnsavedProviderComponent; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [], + declarations: [ + UnsavedProviderComponent, + TestHostComponent + ], + providers: [ + { provide: NgbActiveModal, useClass: NgbActiveModalStub } + ] + }).compileComponents(); + + fixture = TestBed.createComponent(TestHostComponent); + instance = fixture.componentInstance; + cmp = instance.componentUnderTest; + fixture.detectChanges(); + })); + + it('should instantiate the component', async(() => { + expect(cmp).toBeTruthy(); + })); +}); diff --git a/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts new file mode 100644 index 000000000..4b8d12da3 --- /dev/null +++ b/ui/src/app/metadata/provider/component/unsaved-provider.dialog.ts @@ -0,0 +1,27 @@ +import { Component, Input } from '@angular/core'; + +import { NgbActiveModal } from '@ng-bootstrap/ng-bootstrap'; +import { Store, Action } from '@ngrx/store'; +import { Subject } from 'rxjs/Subject'; + +import * as fromEditor from '../reducer'; + +@Component({ + selector: 'unsaved-provider', + templateUrl: './unsaved-provider.dialog.html' +}) +export class UnsavedProviderComponent { + readonly subject: Subject = new Subject(); + + constructor( + public activeModal: NgbActiveModal + ) { } + + close(): void { + this.activeModal.close(); + } + + dismiss(): void { + this.activeModal.dismiss(); + } +} diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts b/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts index a62734833..05cb75832 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.spec.ts @@ -1,10 +1,10 @@ import { Component, ViewChild } from '@angular/core'; -import { ActivatedRoute, Router } from '@angular/router'; +import { ActivatedRoute, Router, ActivatedRouteSnapshot } from '@angular/router'; import { APP_BASE_HREF } from '@angular/common'; -import { TestBed, async, ComponentFixture } from '@angular/core/testing'; +import { TestBed, async, ComponentFixture, fakeAsync } from '@angular/core/testing'; import { RouterTestingModule } from '@angular/router/testing'; import { StoreModule, Store, combineReducers } from '@ngrx/store'; -import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; +import { NgbDropdownModule, NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { ProviderEditComponent } from './provider-edit.component'; import * as fromRoot from '../reducer'; import * as fromWizard from '../../../wizard/reducer'; @@ -12,6 +12,9 @@ import { SharedModule } from '../../../shared/shared.module'; import { ActivatedRouteStub } from '../../../../testing/activated-route.stub'; import { FileBackedHttpMetadataProviderEditor } from '../model'; import { ProviderEditorNavComponent } from '../component/provider-editor-nav.component'; +import { NgbModalStub } from '../../../../testing/modal.stub'; +import { MetadataProvider } from '../../domain/model'; +import { of } from 'rxjs'; @Component({ template: ` @@ -32,6 +35,7 @@ describe('Provider Edit Component', () => { let router: Router; let activatedRoute: ActivatedRouteStub = new ActivatedRouteStub(); let child: ActivatedRouteStub = new ActivatedRouteStub(); + let modal: NgbModal; child.testParamMap = { form: 'common' }; activatedRoute.firstChild = child; @@ -60,6 +64,7 @@ describe('Provider Edit Component', () => { ProviderEditorNavComponent ], providers: [ + { provide: NgbModal, useClass: NgbModalStub }, { provide: ActivatedRoute, useValue: activatedRoute }, { provide: APP_BASE_HREF, useValue: '/' } ] @@ -67,6 +72,7 @@ describe('Provider Edit Component', () => { store = TestBed.get(Store); router = TestBed.get(Router); + modal = TestBed.get(NgbModal); spyOn(store, 'dispatch'); fixture = TestBed.createComponent(TestHostComponent); @@ -104,8 +110,46 @@ describe('Provider Edit Component', () => { describe('cancel method', () => { it('should route to the metadata manager', () => { spyOn(router, 'navigate'); + spyOn(app, 'clear'); app.cancel(); expect(router.navigate).toHaveBeenCalled(); + expect(app.clear).toHaveBeenCalled(); + }); + }); + + describe('clear method', () => { + it('should dispatch actions to clear the reducer state', () => { + app.clear(); + expect(store.dispatch).toHaveBeenCalled(); + }); + }); + + describe('canDeactivate method', () => { + it('should check if the current route is another edit page', (done) => { + let route = new ActivatedRouteStub(), + snapshot = route.snapshot; + let result = app.canDeactivate(null, { url: 'edit', root: null }, { url: 'edit', root: null }); + result.subscribe(can => { + expect(can).toBe(true); + done(); + }); + fixture.detectChanges(); + }); + + it('should open a modal', (done) => { + app.latest = { name: 'bar' }; + spyOn(store, 'select').and.returnValue(of(false)); + spyOn(modal, 'open').and.returnValue({ result: Promise.resolve('closed') }); + fixture.detectChanges(); + let route = new ActivatedRouteStub(), + snapshot = route.snapshot; + let result = app.canDeactivate(null, { url: 'edit', root: null }, { url: 'foo', root: null }); + result.subscribe(can => { + expect(can).toBe(false); + expect(modal.open).toHaveBeenCalled(); + done(); + }); + fixture.detectChanges(); }); }); }); diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.ts b/ui/src/app/metadata/provider/container/provider-edit.component.ts index 07a79a7cf..56616fa8b 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.ts @@ -1,6 +1,6 @@ import { Component, OnDestroy } from '@angular/core'; -import { Router, ActivatedRoute } from '@angular/router'; -import { Observable } from 'rxjs'; +import { Router, ActivatedRoute, ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'; +import { Observable, of } from 'rxjs'; import { skipWhile, map, combineLatest } from 'rxjs/operators'; import { Store } from '@ngrx/store'; import * as fromWizard from '../../../wizard/reducer'; @@ -12,6 +12,10 @@ import { ClearProvider } from '../action/entity.action'; import { Wizard } from '../../../wizard/model'; import { UpdateProviderRequest } from '../action/collection.action'; import { NAV_FORMATS } from '../component/provider-editor-nav.component'; +import { NgbModal } from '../../../../../node_modules/@ng-bootstrap/ng-bootstrap'; +import { UnsavedDialogComponent } from '../../resolver/component/unsaved-dialog.component'; +import { UnsavedProviderComponent } from '../component/unsaved-provider.dialog'; +import { CanComponentDeactivate } from '../../../core/service/can-deactivate.guard'; @Component({ selector: 'provider-edit', @@ -19,7 +23,7 @@ import { NAV_FORMATS } from '../component/provider-editor-nav.component'; styleUrls: [] }) -export class ProviderEditComponent implements OnDestroy { +export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate { provider$: Observable; definition$: Observable>; @@ -36,7 +40,8 @@ export class ProviderEditComponent implements OnDestroy { constructor( private store: Store, private router: Router, - private route: ActivatedRoute + private route: ActivatedRoute, + private modalService: NgbModal ) { this.provider$ = this.store.select(fromProvider.getSelectedProvider).pipe(skipWhile(d => !d)); this.definition$ = this.store.select(fromWizard.getWizardDefinition).pipe(skipWhile(d => !d)); @@ -77,6 +82,10 @@ export class ProviderEditComponent implements OnDestroy { } ngOnDestroy() { + this.clear(); + } + + clear(): void { this.store.dispatch(new ClearProvider()); this.store.dispatch(new ClearWizard()); this.store.dispatch(new ClearEditor()); @@ -87,7 +96,27 @@ export class ProviderEditComponent implements OnDestroy { } cancel(): void { + this.clear(); this.router.navigate(['metadata', 'manager', 'providers']); } + + canDeactivate( + currentRoute: ActivatedRouteSnapshot, + currentState: RouterStateSnapshot, + nextState: RouterStateSnapshot + ): Observable { + if (nextState.url.match('edit')) { return of(true); } + if (Object.keys({ ...this.latest }).length > 0) { + let modal = this.modalService.open(UnsavedProviderComponent); + modal.result.then( + () => { + this.clear(); + this.router.navigate([nextState.url]); + }, + () => console.warn('denied') + ); + } + return this.store.select(fromProvider.getEntityIsSaved); + } } diff --git a/ui/src/app/metadata/provider/provider.module.ts b/ui/src/app/metadata/provider/provider.module.ts index dfd95fa8f..95d037256 100644 --- a/ui/src/app/metadata/provider/provider.module.ts +++ b/ui/src/app/metadata/provider/provider.module.ts @@ -28,6 +28,7 @@ import { EntityEffects } from './effect/entity.effect'; import { ProviderFilterListComponent } from './container/provider-filter-list.component'; import { ProviderEditorNavComponent } from './component/provider-editor-nav.component'; +import { UnsavedProviderComponent } from './component/unsaved-provider.dialog'; @NgModule({ declarations: [ @@ -40,9 +41,12 @@ import { ProviderEditorNavComponent } from './component/provider-editor-nav.comp ProviderSelectComponent, ProviderFilterListComponent, SummaryPropertyComponent, - ProviderEditorNavComponent + ProviderEditorNavComponent, + UnsavedProviderComponent + ], + entryComponents: [ + UnsavedProviderComponent ], - entryComponents: [], imports: [ ReactiveFormsModule, CommonModule, diff --git a/ui/src/app/metadata/provider/provider.routing.ts b/ui/src/app/metadata/provider/provider.routing.ts index 8e6ecc3f5..7acd52fec 100644 --- a/ui/src/app/metadata/provider/provider.routing.ts +++ b/ui/src/app/metadata/provider/provider.routing.ts @@ -10,6 +10,7 @@ import { ProviderFilterListComponent } from './container/provider-filter-list.co import { NewFilterComponent } from '../filter/container/new-filter.component'; import { FilterComponent } from '../filter/container/filter.component'; import { EditFilterComponent } from '../filter/container/edit-filter.component'; +import { CanDeactivateGuard } from '../../core/service/can-deactivate.guard'; export const ProviderRoutes: Routes = [ { @@ -44,6 +45,9 @@ export const ProviderRoutes: Routes = [ path: ':form', component: ProviderEditStepComponent } + ], + canDeactivate: [ + CanDeactivateGuard ] }, { @@ -61,8 +65,7 @@ export const ProviderRoutes: Routes = [ children: [ { path: 'edit', - component: EditFilterComponent, - canDeactivate: [] + component: EditFilterComponent } ] } diff --git a/ui/src/app/metadata/provider/reducer/entity.reducer.ts b/ui/src/app/metadata/provider/reducer/entity.reducer.ts index d5b63eca1..175c769d8 100644 --- a/ui/src/app/metadata/provider/reducer/entity.reducer.ts +++ b/ui/src/app/metadata/provider/reducer/entity.reducer.ts @@ -41,7 +41,7 @@ export function reducer(state = initialState, action: EntityActionUnion): Entity } } -export const isEntitySaved = (state: EntityState) => !Object.keys(state.changes).length && !state.saving; +export const isEntitySaved = (state: EntityState) => state.changes ? !Object.keys(state.changes).length && !state.saving : true; export const getEntityChanges = (state: EntityState) => state.changes; export const isEditorSaving = (state: EntityState) => state.saving; export const getUpdatedEntity = (state: EntityState) => state.changes; diff --git a/ui/src/app/metadata/resolver/container/editor.component.ts b/ui/src/app/metadata/resolver/container/editor.component.ts index 6aa252abe..5d4b6e1c3 100644 --- a/ui/src/app/metadata/resolver/container/editor.component.ts +++ b/ui/src/app/metadata/resolver/container/editor.component.ts @@ -76,7 +76,6 @@ export class EditorComponent implements OnInit, OnDestroy { ); this.providerName$ = this.resolver$.pipe(map(p => p.serviceProviderName)); - this.changes$ = this.store.select(fromResolver.getEditorChanges); this.editorIndex$ = this.route.params.pipe(map(params => Number(params.index))); this.currentPage$ = this.editorIndex$.pipe(map(index => EditorDef.find(r => r.index === index))); this.editor = EditorDef; diff --git a/ui/src/app/wizard/component/wizard.component.spec.ts b/ui/src/app/wizard/component/wizard.component.spec.ts index e69de29bb..ef513015e 100644 --- a/ui/src/app/wizard/component/wizard.component.spec.ts +++ b/ui/src/app/wizard/component/wizard.component.spec.ts @@ -0,0 +1,64 @@ +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 * as fromWizard from '../reducer'; +import { NgbDropdownModule } from '@ng-bootstrap/ng-bootstrap'; +import { WizardComponent, ICONS } from './wizard.component'; + +@Component({ + template: ` + + ` +}) +class TestHostComponent { + @ViewChild(WizardComponent) + public componentUnderTest: WizardComponent; +} + +describe('Wizard Component', () => { + + let fixture: ComponentFixture; + let instance: TestHostComponent; + let app: WizardComponent; + let store: Store; + + beforeEach(async(() => { + TestBed.configureTestingModule({ + imports: [ + NgbDropdownModule.forRoot(), + RouterTestingModule, + StoreModule.forRoot({ + wizard: combineReducers(fromWizard.reducers) + }) + ], + declarations: [ + WizardComponent, + TestHostComponent + ], + }).compileComponents(); + + store = TestBed.get(Store); + spyOn(store, 'dispatch'); + + fixture = TestBed.createComponent(TestHostComponent); + instance = fixture.componentInstance; + app = instance.componentUnderTest; + fixture.detectChanges(); + })); + + it('should compile without error', () => { + expect(app).toBeTruthy(); + }); + + describe('getIcon method', () => { + it('should return the check string for the last index', () => { + expect(app.getIcon({ index: 'foo' }, { index: 'foo' })).toEqual(ICONS.CHECK); + }); + it('should return the index icon for other indexes', () => { + expect(app.getIcon({ index: 'foo' }, { index: 'bar' })).toEqual(ICONS.INDEX); + expect(app.getIcon({ index: 'foo' }, null)).toEqual(ICONS.INDEX); + }); + }); +}); diff --git a/ui/src/app/wizard/component/wizard.component.ts b/ui/src/app/wizard/component/wizard.component.ts index a75a3bb2d..7b4aa8ff4 100644 --- a/ui/src/app/wizard/component/wizard.component.ts +++ b/ui/src/app/wizard/component/wizard.component.ts @@ -53,9 +53,11 @@ export class WizardComponent { this.currentIcon$ = this.current$.pipe( withLatestFrom(this.last$), - map(([current, last]) => { - return (last && current.index === last.index) ? ICONS.CHECK : ICONS.INDEX; - }) + map(([current, last]) => this.getIcon(current, last)) ); } + + getIcon(current, last): string { + return (last && current.index === last.index) ? ICONS.CHECK : ICONS.INDEX; + } } From dcf2d2481b2af10700f30ee3cd61901f06d05848 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 14 Aug 2018 10:33:21 -0700 Subject: [PATCH 14/17] SHIBUI-331 Fixed issue with modal opening up always --- .../container/provider-edit.component.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.ts b/ui/src/app/metadata/provider/container/provider-edit.component.ts index 56616fa8b..a29949bb5 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.ts @@ -16,6 +16,7 @@ import { NgbModal } from '../../../../../node_modules/@ng-bootstrap/ng-bootstrap import { UnsavedDialogComponent } from '../../resolver/component/unsaved-dialog.component'; import { UnsavedProviderComponent } from '../component/unsaved-provider.dialog'; import { CanComponentDeactivate } from '../../../core/service/can-deactivate.guard'; +import { DifferentialService } from '../../../core/service/differential.service'; @Component({ selector: 'provider-edit', @@ -34,6 +35,7 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate status$: Observable; latest: MetadataProvider; + provider: MetadataProvider; formats = NAV_FORMATS; @@ -41,7 +43,8 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate private store: Store, private router: Router, private route: ActivatedRoute, - private modalService: NgbModal + private modalService: NgbModal, + private diffService: DifferentialService ) { this.provider$ = this.store.select(fromProvider.getSelectedProvider).pipe(skipWhile(d => !d)); this.definition$ = this.store.select(fromWizard.getWizardDefinition).pipe(skipWhile(d => !d)); @@ -70,6 +73,7 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate } }); + this.provider$.subscribe(p => this.provider = p); this.store.select(fromProvider.getEntityChanges).subscribe(changes => this.latest = changes); } @@ -105,8 +109,11 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate currentState: RouterStateSnapshot, nextState: RouterStateSnapshot ): Observable { - if (nextState.url.match('edit')) { return of(true); } - if (Object.keys({ ...this.latest }).length > 0) { + if (nextState.url.match('edit')) { + return of(true); + } + const diff = this.diffService.updatedDiff(this.provider, this.latest); + if (diff && Object.keys(diff).length > 0) { let modal = this.modalService.open(UnsavedProviderComponent); modal.result.then( () => { @@ -115,8 +122,9 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate }, () => console.warn('denied') ); + return this.store.select(fromProvider.getEntityIsSaved); } - return this.store.select(fromProvider.getEntityIsSaved); + return of(true); } } From cf7f2197a644803ef80de480f038f34897748245 Mon Sep 17 00:00:00 2001 From: Ryan Mathis Date: Tue, 14 Aug 2018 17:48:03 +0000 Subject: [PATCH 15/17] Merged in feature/SHIBUI-599 (pull request #156) SHIBUI-599 Implemented saving loader on provider/source Approved-by: Shibui Jenkins Approved-by: Ryan Mathis --- .../container/provider-edit.component.html | 8 +++- .../container/provider-edit.component.ts | 2 + .../container/provider-wizard.component.ts | 1 + .../provider/reducer/entity.reducer.spec.ts | 45 +++++++++++++++++++ .../provider/reducer/entity.reducer.ts | 20 ++++++++- .../component/wizard-nav.component.html | 13 +++++- .../component/wizard-nav.component.ts | 2 + .../wizard/component/wizard.component.html | 17 +++++-- 8 files changed, 100 insertions(+), 8 deletions(-) diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.html b/ui/src/app/metadata/provider/container/provider-edit.component.html index 6f15638b7..85fc2bfc9 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.html +++ b/ui/src/app/metadata/provider/container/provider-edit.component.html @@ -18,10 +18,14 @@
      diff --git a/ui/src/app/metadata/provider/container/provider-edit.component.ts b/ui/src/app/metadata/provider/container/provider-edit.component.ts index a29949bb5..a2c2e0b86 100644 --- a/ui/src/app/metadata/provider/container/provider-edit.component.ts +++ b/ui/src/app/metadata/provider/container/provider-edit.component.ts @@ -33,6 +33,7 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate valid$: Observable; isInvalid$: Observable; status$: Observable; + isSaving$: Observable; latest: MetadataProvider; provider: MetadataProvider; @@ -52,6 +53,7 @@ export class ProviderEditComponent implements OnDestroy, CanComponentDeactivate this.valid$ = this.store.select(fromProvider.getEditorIsValid); this.isInvalid$ = this.valid$.pipe(map(v => !v)); this.status$ = this.store.select(fromProvider.getInvalidEditorForms); + this.isSaving$ = this.store.select(fromProvider.getEntityIsSaving); let startIndex$ = this.route.firstChild ? this.route.firstChild.params.pipe(map(p => p.form || 'filters')) : diff --git a/ui/src/app/metadata/provider/container/provider-wizard.component.ts b/ui/src/app/metadata/provider/container/provider-wizard.component.ts index f7f958db8..8a4e481f5 100644 --- a/ui/src/app/metadata/provider/container/provider-wizard.component.ts +++ b/ui/src/app/metadata/provider/container/provider-wizard.component.ts @@ -86,6 +86,7 @@ export class ProviderWizardComponent implements OnDestroy { } save(): void { + this.store.dispatch(new SetDisabled(true)); this.store.dispatch(new AddProviderRequest(this.provider)); } diff --git a/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts b/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts index a37687766..2f1bcff62 100644 --- a/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts +++ b/ui/src/app/metadata/provider/reducer/entity.reducer.spec.ts @@ -1,6 +1,15 @@ import { reducer, initialState as snapshot, isEntitySaved } from './entity.reducer'; import { EntityActionTypes, ClearProvider } from '../action/entity.action'; import { MetadataProvider } from '../../domain/model'; +import { + ProviderCollectionActionTypes, + UpdateProviderRequest, + AddProviderRequest, + UpdateProviderSuccess, + UpdateProviderFail, + AddProviderFail, + AddProviderSuccess +} from '../action/collection.action'; describe('Provider Editor Reducer', () => { describe('undefined action', () => { @@ -17,6 +26,42 @@ describe('Provider Editor Reducer', () => { }); }); + describe(`${ProviderCollectionActionTypes.UPDATE_PROVIDER_REQUEST}`, () => { + it('should set to `saving`', () => { + expect(reducer(snapshot, new UpdateProviderRequest({})).saving).toBe(true); + }); + }); + + describe(`${ProviderCollectionActionTypes.ADD_PROVIDER_REQUEST}`, () => { + it('should set to `saving`', () => { + expect(reducer(snapshot, new AddProviderRequest({})).saving).toBe(true); + }); + }); + + describe(`${ProviderCollectionActionTypes.UPDATE_PROVIDER_SUCCESS}`, () => { + it('should set to not `saving`', () => { + expect(reducer(snapshot, new UpdateProviderSuccess({id: 'foo', changes: {} })).saving).toBe(false); + }); + }); + + describe(`${ProviderCollectionActionTypes.ADD_PROVIDER_FAIL}`, () => { + it('should set to not `saving`', () => { + expect(reducer(snapshot, new AddProviderFail({})).saving).toBe(false); + }); + }); + + describe(`${ProviderCollectionActionTypes.ADD_PROVIDER_SUCCESS}`, () => { + it('should set to not `saving`', () => { + expect(reducer(snapshot, new AddProviderSuccess({})).saving).toBe(false); + }); + }); + + describe(`${ProviderCollectionActionTypes.UPDATE_PROVIDER_FAIL}`, () => { + it('should set to not `saving`', () => { + expect(reducer(snapshot, new UpdateProviderFail({})).saving).toBe(false); + }); + }); + describe(`isEntitySaved method`, () => { it('should return false if there are changes', () => { expect(isEntitySaved({ diff --git a/ui/src/app/metadata/provider/reducer/entity.reducer.ts b/ui/src/app/metadata/provider/reducer/entity.reducer.ts index 175c769d8..faa90afd5 100644 --- a/ui/src/app/metadata/provider/reducer/entity.reducer.ts +++ b/ui/src/app/metadata/provider/reducer/entity.reducer.ts @@ -1,5 +1,6 @@ import { MetadataProvider } from '../../domain/model'; import { EntityActionTypes, EntityActionUnion } from '../action/entity.action'; +import { ProviderCollectionActionsUnion, ProviderCollectionActionTypes } from '../action/collection.action'; export interface EntityState { saving: boolean; @@ -11,8 +12,25 @@ export const initialState: EntityState = { changes: null }; -export function reducer(state = initialState, action: EntityActionUnion): EntityState { +export function reducer(state = initialState, action: EntityActionUnion | ProviderCollectionActionsUnion): EntityState { switch (action.type) { + case ProviderCollectionActionTypes.ADD_PROVIDER_REQUEST: + case ProviderCollectionActionTypes.UPDATE_PROVIDER_REQUEST: { + return { + ...state, + saving: true + }; + } + + case ProviderCollectionActionTypes.UPDATE_PROVIDER_FAIL: + case ProviderCollectionActionTypes.UPDATE_PROVIDER_SUCCESS: + case ProviderCollectionActionTypes.ADD_PROVIDER_FAIL: + case ProviderCollectionActionTypes.ADD_PROVIDER_SUCCESS: { + return { + ...state, + saving: false + }; + } case EntityActionTypes.CLEAR_PROVIDER: { return { ...initialState diff --git a/ui/src/app/metadata/resolver/component/wizard-nav.component.html b/ui/src/app/metadata/resolver/component/wizard-nav.component.html index 02ff3865f..ca6c3cccc 100644 --- a/ui/src/app/metadata/resolver/component/wizard-nav.component.html +++ b/ui/src/app/metadata/resolver/component/wizard-nav.component.html @@ -35,10 +35,19 @@

  • - diff --git a/ui/src/app/metadata/resolver/component/wizard-nav.component.ts b/ui/src/app/metadata/resolver/component/wizard-nav.component.ts index ab9dd677e..c55a30191 100644 --- a/ui/src/app/metadata/resolver/component/wizard-nav.component.ts +++ b/ui/src/app/metadata/resolver/component/wizard-nav.component.ts @@ -22,6 +22,7 @@ export class WizardNavComponent implements OnChanges { isFirstPage = false; wizardIsValid$: Observable; wizardIsInvalid$: Observable; + wizardIsSaving$: Observable; wizard: EditorFlowDefinition[] = WizardDef; @@ -38,6 +39,7 @@ export class WizardNavComponent implements OnChanges { this.wizard = WizardDef; this.wizardIsValid$ = this.store.select(fromEditor.getEditorIsValid); + this.wizardIsSaving$ = this.store.select(fromEditor.getEditorIsSaving); this.wizardIsInvalid$ = this.wizardIsValid$.pipe(map(valid => !valid)); } } /* istanbul ignore next */ diff --git a/ui/src/app/wizard/component/wizard.component.html b/ui/src/app/wizard/component/wizard.component.html index cf59c451f..ec12ca657 100644 --- a/ui/src/app/wizard/component/wizard.component.html +++ b/ui/src/app/wizard/component/wizard.component.html @@ -42,10 +42,21 @@

  • - From 70c7b4651ec0750b8b5e67d4ab53c1f881e0188e Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Tue, 14 Aug 2018 15:25:54 -0400 Subject: [PATCH 16/17] Fix test --- .../MetadataFiltersControllerTests.groovy | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy index 0443a7e54..e28a05cb3 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy @@ -9,6 +9,7 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.SearchConfiguration import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter import edu.internet2.tier.shibboleth.admin.ui.domain.resolvers.MetadataResolver +import edu.internet2.tier.shibboleth.admin.ui.repository.FilterRepository import edu.internet2.tier.shibboleth.admin.ui.repository.MetadataResolverRepository import edu.internet2.tier.shibboleth.admin.ui.service.FilterService import edu.internet2.tier.shibboleth.admin.ui.service.MetadataResolverService @@ -53,6 +54,8 @@ class MetadataFiltersControllerTests extends Specification { def metadataResolverRepository = Mock(MetadataResolverRepository) + def metadataFilterRepository = Mock(FilterRepository) + def controller def mockMvc @@ -67,6 +70,7 @@ class MetadataFiltersControllerTests extends Specification { controller = new MetadataFiltersController ( repository: metadataResolverRepository, + filterRepository: metadataFilterRepository, metadataResolverService: new MetadataResolverService() { @Override void reloadFilters(String metadataResolverName) { @@ -98,8 +102,8 @@ class MetadataFiltersControllerTests extends Specification { then: result.andExpect(expectedHttpResponseStatus) - .andExpect(content().contentType(expectedResponseContentType)) - .andExpect(content().json(mapper.writeValueAsString(expectedContent))) + .andExpect(content().contentType(expectedResponseContentType)) + .andExpect(content().json(mapper.writeValueAsString(expectedContent))) } def "FilterController.getOne gets the desired filter"() { @@ -156,11 +160,11 @@ class MetadataFiltersControllerTests extends Specification { .andExpect(header().string(expectedResponseHeader, containsString(expectedResponseHeaderValue))) where: - filterType | _ - 'entityAttributes' | _ - 'entityRoleWhiteList' | _ - 'signatureValidation' | _ - 'requiredValidUntil' | _ + filterType | _ + 'entityAttributes' | _ + 'entityRoleWhiteList' | _ + 'signatureValidation' | _ + 'requiredValidUntil' | _ } @Unroll @@ -173,7 +177,7 @@ class MetadataFiltersControllerTests extends Specification { def postedJsonBody = mapper.writeValueAsString(updatedFilter) def originalMetadataResolver = new MetadataResolver() - originalMetadataResolver.setResourceId(randomGenerator.randomId()) + originalMetadataResolver.setResourceId('foo') originalMetadataResolver.setMetadataFilters(testObjectGenerator.buildAllTypesOfFilterList()) originalMetadataResolver.metadataFilters.add(originalFilter) @@ -183,7 +187,8 @@ class MetadataFiltersControllerTests extends Specification { updatedMetadataResolver.getMetadataFilters().add(updatedFilter) 1 * metadataResolverRepository.findByResourceId(_) >> originalMetadataResolver - 1 * metadataResolverRepository.save(_) >> updatedMetadataResolver + 1 * metadataFilterRepository.findByResourceId(_) >> originalFilter + 1 * metadataFilterRepository.save(_) >> updatedFilter def filterUUID = updatedFilter.getResourceId() @@ -198,16 +203,16 @@ class MetadataFiltersControllerTests extends Specification { if (filterType == 'entityAttributes') { EntityAttributesFilter.cast(updatedFilter).fromTransientRepresentation() } - expectedJson << [version: updatedFilter.hashCode()] + expectedJson << [version: updatedFilter.getVersion()] result.andExpect(status().isOk()) .andExpect(content().json(JsonOutput.toJson(expectedJson), true)) where: - filterType | _ - 'entityAttributes' | _ - 'entityRoleWhiteList' | _ - 'signatureValidation' | _ - 'requiredValidUntil' | _ + filterType | _ + 'entityAttributes' | _ + 'entityRoleWhiteList' | _ + 'signatureValidation' | _ + 'requiredValidUntil' | _ } def "FilterController.update filter 409's if the version numbers don't match"() { @@ -223,6 +228,7 @@ class MetadataFiltersControllerTests extends Specification { originalMetadataResolver.getMetadataFilters().add(randomFilter) 1 * metadataResolverRepository.findByResourceId(_) >> originalMetadataResolver + 1 * metadataFilterRepository.findByResourceId(_) >> randomFilter def filterUUID = randomFilter.getResourceId() @@ -235,4 +241,4 @@ class MetadataFiltersControllerTests extends Specification { then: result.andExpect(status().is(409)) } -} +} \ No newline at end of file From b3f93bebfd3981f413d23cf6e6c2f6d2d549e9f6 Mon Sep 17 00:00:00 2001 From: Dmitriy Kopylenko Date: Wed, 15 Aug 2018 09:10:12 -0400 Subject: [PATCH 17/17] Refactor persistence implementation for attribute filter --- .../controller/MetadataFiltersController.java | 5 --- .../MetadataResolversController.java | 4 --- .../filters/EntityAttributesFilter.java | 32 ++++++++++--------- .../ui/domain/resolvers/MetadataResolver.java | 18 ----------- .../MetadataFiltersControllerTests.groovy | 3 -- ...ymorphicFiltersJacksonHandlingTests.groovy | 1 - .../MetadataResolverRepositoryTests.groovy | 5 --- 7 files changed, 17 insertions(+), 51 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java index a6491fa86..52122b64f 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersController.java @@ -68,11 +68,6 @@ public ResponseEntity create(@PathVariable String metadataResolverId, @Reques return ResponseEntity.notFound().build(); } metadataResolver.getMetadataFilters().add(createdFilter); - - //convert before saving into database - if(createdFilter instanceof EntityAttributesFilter) { - EntityAttributesFilter.class.cast(createdFilter).fromTransientRepresentation(); - } MetadataResolver persistedMr = repository.save(metadataResolver); // we reload the filters here after save diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java index 15ae5d236..be6ab51a4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataResolversController.java @@ -100,11 +100,9 @@ public ResponseEntity create(@RequestBody MetadataResolver newResolver) { return validationErrorResponse; } - newResolver.convertFiltersFromTransientRepresentationIfNecessary(); MetadataResolver persistedResolver = resolverRepository.save(newResolver); positionOrderContainerService.appendPositionOrderForNew(persistedResolver); - persistedResolver.convertFiltersIntoTransientRepresentationIfNecessary(); return ResponseEntity.created(getResourceUriFor(persistedResolver)).body(persistedResolver); } @@ -128,10 +126,8 @@ public ResponseEntity update(@PathVariable String resourceId, @RequestBody Me updatedResolver.setAudId(existingResolver.getAudId()); - updatedResolver.convertFiltersFromTransientRepresentationIfNecessary(); MetadataResolver persistedResolver = resolverRepository.save(updatedResolver); - persistedResolver.convertFiltersFromTransientRepresentationIfNecessary(); return ResponseEntity.ok(persistedResolver); } diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java index e5e923918..8492e745d 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/filters/EntityAttributesFilter.java @@ -14,8 +14,6 @@ import javax.persistence.OneToOne; import javax.persistence.OrderColumn; import javax.persistence.PostLoad; -import javax.persistence.PrePersist; -import javax.persistence.PreUpdate; import javax.persistence.Transient; import java.util.ArrayList; @@ -48,25 +46,29 @@ public EntityAttributesFilter() { @Transient private List attributeRelease = new ArrayList<>(); + public void setAttributeRelease(List attributeRelease) { + this.attributeRelease = attributeRelease; + this.rebuildAttributes(); + } + @Transient private RelyingPartyOverridesRepresentation relyingPartyOverrides; + public void setRelyingPartyOverrides(RelyingPartyOverridesRepresentation relyingPartyOverridesRepresentation) { + this.relyingPartyOverrides = relyingPartyOverridesRepresentation; + this.rebuildAttributes(); + } + + //TODO: yeah, I'm not too happy, either + private void rebuildAttributes() { + this.attributes.clear(); + this.attributes.addAll((List) (List)getAttributeListFromAttributeReleaseList(this.attributeRelease)); + this.attributes.addAll((List) (List)getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); + } + @PostLoad public void intoTransientRepresentation() { this.attributeRelease = getAttributeReleaseListFromAttributeList(this.attributes); this.relyingPartyOverrides = getRelyingPartyOverridesRepresentationFromAttributeList(attributes); } - - @PrePersist - @PreUpdate - public void fromTransientRepresentation() { - List attributeList = new ArrayList<>(); - attributeList.addAll(getAttributeListFromAttributeReleaseList(this.attributeRelease)); - attributeList.addAll(getAttributeListFromRelyingPartyOverridesRepresentation(this.relyingPartyOverrides)); - - if(!attributeList.isEmpty()) { - this.attributes.clear(); - this.attributes.addAll((List) (List) attributeList); - } - } } \ No newline at end of file diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java index cb3b7c9d9..599503f75 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/domain/resolvers/MetadataResolver.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import edu.internet2.tier.shibboleth.admin.ui.domain.AbstractAuditable; -import edu.internet2.tier.shibboleth.admin.ui.domain.filters.EntityAttributesFilter; import edu.internet2.tier.shibboleth.admin.ui.domain.filters.MetadataFilter; import lombok.EqualsAndHashCode; import lombok.Getter; @@ -25,8 +24,6 @@ import java.util.List; import java.util.UUID; -import static java.util.stream.Collectors.toList; - @Entity @Inheritance(strategy = InheritanceType.TABLE_PER_CLASS) @EqualsAndHashCode(callSuper = true, exclude = {"version"}) @@ -83,19 +80,4 @@ public int getVersion() { } return this.hashCode(); } - - public void convertFiltersIntoTransientRepresentationIfNecessary() { - getAvailableEntityAttributesFilters().forEach(EntityAttributesFilter::intoTransientRepresentation); - } - - public void convertFiltersFromTransientRepresentationIfNecessary() { - getAvailableEntityAttributesFilters().forEach(EntityAttributesFilter::fromTransientRepresentation); - } - - private List getAvailableEntityAttributesFilters() { - return this.metadataFilters.stream() - .filter(EntityAttributesFilter.class::isInstance) - .map(EntityAttributesFilter.class::cast) - .collect(toList()); - } } diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy index e28a05cb3..40c6cefef 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/controller/MetadataFiltersControllerTests.groovy @@ -200,9 +200,6 @@ class MetadataFiltersControllerTests extends Specification { then: def expectedJson = new JsonSlurper().parseText(postedJsonBody) - if (filterType == 'entityAttributes') { - EntityAttributesFilter.cast(updatedFilter).fromTransientRepresentation() - } expectedJson << [version: updatedFilter.getVersion()] result.andExpect(status().isOk()) .andExpect(content().json(JsonOutput.toJson(expectedJson), true)) diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy index 56112a350..7841e045b 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/domain/filters/PolymorphicFiltersJacksonHandlingTests.groovy @@ -73,7 +73,6 @@ class PolymorphicFiltersJacksonHandlingTests extends Specification { def simulatedPrePersistentFilter = new EntityAttributesFilter() simulatedPrePersistentFilter.attributeRelease = simulatedPersistentFilter.attributeRelease simulatedPrePersistentFilter.relyingPartyOverrides = simulatedPersistentFilter.relyingPartyOverrides - simulatedPrePersistentFilter.fromTransientRepresentation() expect: simulatedPersistentFilter.attributes.size() == simulatedPrePersistentFilter.attributes.size() diff --git a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy index 646d3e4aa..872182ce8 100644 --- a/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy +++ b/backend/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/repository/MetadataResolverRepositoryTests.groovy @@ -88,9 +88,6 @@ class MetadataResolverRepositoryTests extends Specification { } MetadataResolver metadataResolver = metadataResolverRepository.findAll().iterator().next() - //convert before saving into database - filter.fromTransientRepresentation() - metadataResolver.getMetadataFilters().add(filter) MetadataResolver persistedMr = metadataResolverRepository.save(metadataResolver) @@ -141,8 +138,6 @@ class MetadataResolverRepositoryTests extends Specification { filterToBeUpdated.relyingPartyOverrides = filter.relyingPartyOverrides filterToBeUpdated.attributeRelease = filter.attributeRelease - //convert before saving into database - filterToBeUpdated.fromTransientRepresentation() entityManager.clear() persistedMr = metadataResolverRepository.save(metadataResolver)