From e7c5a2a29ced4dc9d414879fe4ec588e34f44445 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe <kristof.delanghe@atmire.com> Date: Wed, 5 Sep 2018 16:31:06 +0200 Subject: [PATCH] 54472: refactoring feedback pt1 --- .../create-collection-page.component.html | 2 +- .../create-collection-page.component.ts | 50 ++++++------ .../create-community-page.component.html | 2 +- .../create-community-page.component.ts | 48 ++++++------ src/app/core/data/collection-data.service.ts | 1 + src/app/core/data/comcol-data.service.spec.ts | 1 + src/app/core/data/comcol-data.service.ts | 45 ++++------- src/app/core/data/community-data.service.ts | 1 + src/app/core/data/data.service.ts | 76 ++++++++++++++----- src/app/core/data/item-data.service.ts | 10 ++- 10 files changed, 133 insertions(+), 103 deletions(-) diff --git a/src/app/+collection-page/create-collection-page/create-collection-page.component.html b/src/app/+collection-page/create-collection-page/create-collection-page.component.html index 9cf01f7c88..11f4f53ecd 100644 --- a/src/app/+collection-page/create-collection-page/create-collection-page.component.html +++ b/src/app/+collection-page/create-collection-page/create-collection-page.component.html @@ -1,7 +1,7 @@ <div class="container"> <div class="row"> <div class="col-12 pb-4"> - <ng-container *ngVar="(communityRDObs | async).payload as community"> + <ng-container *ngVar="(communityRDObs | async)?.payload as community"> <h2 *ngIf="!community" id="header" class="border-bottom pb-2">{{ 'collection.create.head' | translate }}</h2> <h2 *ngIf="community" id="sub-header" class="border-bottom pb-2">{{ 'collection.create.sub-head' | translate:{ parent: community.name } }}</h2> </ng-container> diff --git a/src/app/+collection-page/create-collection-page/create-collection-page.component.ts b/src/app/+collection-page/create-collection-page/create-collection-page.component.ts index 4f57482cbd..8f31d30441 100644 --- a/src/app/+collection-page/create-collection-page/create-collection-page.component.ts +++ b/src/app/+collection-page/create-collection-page/create-collection-page.component.ts @@ -12,6 +12,7 @@ import { Observable } from 'rxjs/Observable'; import { ResponseCacheEntry } from '../../core/cache/response-cache.reducer'; import { map } from 'rxjs/operators'; import { RemoteData } from '../../core/data/remote-data'; +import { isNotEmpty } from '../../shared/empty.util'; @Component({ selector: 'ds-create-collection', @@ -27,37 +28,34 @@ export class CreateCollectionPageComponent { public constructor(private collectionDataService: CollectionDataService, private communityDataService: CommunityDataService, private routeService: RouteService, private router: Router) { this.parentUUID$ = this.routeService.getQueryParameterValue('parent'); this.parentUUID$.subscribe((uuid: string) => { - this.communityRDObs = this.communityDataService.findById(uuid); + if (isNotEmpty(uuid)) { + this.communityRDObs = this.communityDataService.findById(uuid); + } }); } onSubmit(data: any) { - const collection = Object.assign(new Collection(), { - name: data.name, - metadata: [ - { key: 'dc.description', value: data.introductory }, - { key: 'dc.description.abstract', value: data.description }, - { key: 'dc.rights', value: data.copyright }, - { key: 'dc.rights.license', value: data.license } - // TODO: metadata for news and provenance - ] - }); this.parentUUID$.subscribe((uuid: string) => { - let response$: Observable<ResponseCacheEntry>; - if (uuid) { - response$ = this.collectionDataService.create(collection, uuid); - } else { - response$ = this.collectionDataService.create(collection); - } - this.error$ = response$.pipe( - map((response: ResponseCacheEntry) => { - if (!response.response.isSuccessful && response.response instanceof ErrorResponse) { - return response.response; - } else if (response.response instanceof DSOSuccessResponse) { - this.router.navigateByUrl(''); - } - }) - ); + const collection = Object.assign(new Collection(), { + name: data.name, + metadata: [ + { key: 'dc.description', value: data.introductory }, + { key: 'dc.description.abstract', value: data.description }, + { key: 'dc.rights', value: data.copyright }, + { key: 'dc.rights.license', value: data.license } + // TODO: metadata for news and provenance + ], + owner: Observable.of(new RemoteData(false, false, true, null, Object.assign(new Community(), { + id: uuid, + uuid: uuid + }))) + }); + this.collectionDataService.create(collection).subscribe((rd: RemoteData<Collection>) => { + console.log(rd); + if (rd.hasSucceeded) { + this.router.navigateByUrl(''); + } + }); }); } diff --git a/src/app/+community-page/create-community-page/create-community-page.component.html b/src/app/+community-page/create-community-page/create-community-page.component.html index 53ca4f5020..06fd643530 100644 --- a/src/app/+community-page/create-community-page/create-community-page.component.html +++ b/src/app/+community-page/create-community-page/create-community-page.component.html @@ -1,7 +1,7 @@ <div class="container"> <div class="row"> <div class="col-12 pb-4"> - <ng-container *ngVar="(communityRDObs | async).payload as community"> + <ng-container *ngVar="(communityRDObs | async)?.payload as community"> <h2 *ngIf="!community" id="header" class="border-bottom pb-2">{{ 'community.create.head' | translate }}</h2> <h2 *ngIf="community" id="sub-header" class="border-bottom pb-2">{{ 'community.create.sub-head' | translate:{ parent: community.name } }}</h2> </ng-container> diff --git a/src/app/+community-page/create-community-page/create-community-page.component.ts b/src/app/+community-page/create-community-page/create-community-page.component.ts index 8f2f61ab80..04d5b8fd39 100644 --- a/src/app/+community-page/create-community-page/create-community-page.component.ts +++ b/src/app/+community-page/create-community-page/create-community-page.component.ts @@ -10,6 +10,7 @@ import { map } from 'rxjs/operators'; import { RouteService } from '../../shared/services/route.service'; import { Router } from '@angular/router'; import { RemoteData } from '../../core/data/remote-data'; +import { isNotEmpty } from '../../shared/empty.util'; @Component({ selector: 'ds-create-community', @@ -25,36 +26,33 @@ export class CreateCommunityPageComponent { public constructor(private communityDataService: CommunityDataService, private routeService: RouteService, private router: Router) { this.parentUUID$ = this.routeService.getQueryParameterValue('parent'); this.parentUUID$.subscribe((uuid: string) => { - this.communityRDObs = this.communityDataService.findById(uuid); + if (isNotEmpty(uuid)) { + this.communityRDObs = this.communityDataService.findById(uuid); + } }); } onSubmit(data: any) { - const community = Object.assign(new Community(), { - name: data.name, - metadata: [ - { key: 'dc.description', value: data.introductory }, - { key: 'dc.description.abstract', value: data.description }, - { key: 'dc.rights', value: data.copyright } - // TODO: metadata for news - ] - }); this.parentUUID$.subscribe((uuid: string) => { - let response$: Observable<ResponseCacheEntry>; - if (uuid) { - response$ = this.communityDataService.create(community, uuid); - } else { - response$ = this.communityDataService.create(community); - } - this.error$ = response$.pipe( - map((response: ResponseCacheEntry) => { - if (!response.response.isSuccessful && response.response instanceof ErrorResponse) { - return response.response; - } else if (response.response instanceof DSOSuccessResponse) { - this.router.navigateByUrl(''); - } - }) - ); + const community = Object.assign(new Community(), { + name: data.name, + metadata: [ + { key: 'dc.description', value: data.introductory }, + { key: 'dc.description.abstract', value: data.description }, + { key: 'dc.rights', value: data.copyright } + // TODO: metadata for news + ], + owner: Observable.of(new RemoteData(false, false, true, null, Object.assign(new Community(), { + id: uuid, + uuid: uuid + }))) + }); + this.communityDataService.create(community).subscribe((rd: RemoteData<Community>) => { + console.log(rd); + if (rd.hasSucceeded) { + this.router.navigateByUrl(''); + } + }); }); } diff --git a/src/app/core/data/collection-data.service.ts b/src/app/core/data/collection-data.service.ts index 70c11ca5fc..dc3635595a 100644 --- a/src/app/core/data/collection-data.service.ts +++ b/src/app/core/data/collection-data.service.ts @@ -13,6 +13,7 @@ import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { AuthService } from '../auth/auth.service'; import { Community } from '../shared/community.model'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; @Injectable() export class CollectionDataService extends ComColDataService<NormalizedCollection, Collection> { diff --git a/src/app/core/data/comcol-data.service.spec.ts b/src/app/core/data/comcol-data.service.spec.ts index 5f440dd442..9cc44e65db 100644 --- a/src/app/core/data/comcol-data.service.spec.ts +++ b/src/app/core/data/comcol-data.service.spec.ts @@ -16,6 +16,7 @@ import { NormalizedObject } from '../cache/models/normalized-object.model'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { Community } from '../shared/community.model'; import { AuthService } from '../auth/auth.service'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; const LINK_NAME = 'test'; diff --git a/src/app/core/data/comcol-data.service.ts b/src/app/core/data/comcol-data.service.ts index 745879be1f..c47d48dc07 100644 --- a/src/app/core/data/comcol-data.service.ts +++ b/src/app/core/data/comcol-data.service.ts @@ -19,12 +19,12 @@ import { configureRequest, getResponseFromSelflink } from '../shared/operators'; import { AuthService } from '../auth/auth.service'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; import { HttpHeaders } from '@angular/common/http'; +import { RemoteData } from './remote-data'; export abstract class ComColDataService<TNormalized extends NormalizedObject, TDomain> extends DataService<TNormalized, TDomain> { protected abstract cds: CommunityDataService; protected abstract objectCache: ObjectCacheService; protected abstract halService: HALEndpointService; - protected abstract authService: AuthService; /** * Get the scoped endpoint URL by fetching the object with @@ -66,36 +66,21 @@ export abstract class ComColDataService<TNormalized extends NormalizedObject, TD } } - public create(comcol: TDomain, parentUUID?: string): Observable<ResponseCacheEntry> { - return this.halService.getEndpoint(this.linkPath).pipe( - isNotEmptyOperator(), - distinctUntilChanged(), - map((endpointURL: string) => { - const options: HttpOptions = Object.create({}); - const headers = new HttpHeaders(); - headers.append('Authentication', this.authService.buildAuthHeader()); - options.headers = headers; - return new PostRequest(this.requestService.generateRequestId(), endpointURL + ((parentUUID) ? this.buildCreateParams(comcol, parentUUID) : this.buildCreateParams(comcol)), options); - }), - configureRequest(this.requestService), - map((request: RestRequest) => request.href), - getResponseFromSelflink(this.responseCache) - ); - } - - public buildCreateParams(comcol: TDomain, parentUUID?: string): string { - if (comcol instanceof Community ||Â comcol instanceof Collection) { - let urlParams = '?name=' + comcol.name; - if (parentUUID) { - urlParams += '&parent=' + parentUUID; - } - if (comcol.metadata) { - for (const i of Object.keys(comcol.metadata)) { - urlParams += '&' + comcol.metadata[i].key + '=' + comcol.metadata[i].value; + public buildCreateParams(comcol): Observable<string> { + return comcol.owner.pipe( + map((rd: RemoteData<Community | Collection>) => { + let urlParams = '?name=' + comcol.name; + if (rd.payload.id) { + urlParams += '&parent=' + rd.payload.id; } - } - return urlParams; - } + if (comcol.metadata) { + for (const i of Object.keys(comcol.metadata)) { + urlParams += '&' + comcol.metadata[i].key + '=' + comcol.metadata[i].value; + } + } + return urlParams; + }) + ); } } diff --git a/src/app/core/data/community-data.service.ts b/src/app/core/data/community-data.service.ts index 9d6af5ee6f..d367b15f6b 100644 --- a/src/app/core/data/community-data.service.ts +++ b/src/app/core/data/community-data.service.ts @@ -12,6 +12,7 @@ import { ComColDataService } from './comcol-data.service'; import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; import { AuthService } from '../auth/auth.service'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; @Injectable() export class CommunityDataService extends ComColDataService<NormalizedCommunity, Community> { diff --git a/src/app/core/data/data.service.ts b/src/app/core/data/data.service.ts index f532ff05ba..b3b69fdd54 100644 --- a/src/app/core/data/data.service.ts +++ b/src/app/core/data/data.service.ts @@ -1,6 +1,6 @@ import { Store } from '@ngrx/store'; import { Observable } from 'rxjs/Observable'; -import { hasValue, isNotEmpty } from '../../shared/empty.util'; +import { hasValue, isNotEmpty, isNotEmptyOperator } from '../../shared/empty.util'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { ResponseCacheService } from '../cache/response-cache.service'; import { CoreState } from '../core.reducers'; @@ -8,9 +8,30 @@ import { HALEndpointService } from '../shared/hal-endpoint.service'; import { URLCombiner } from '../url-combiner/url-combiner'; import { PaginatedList } from './paginated-list'; import { RemoteData } from './remote-data'; -import { FindAllOptions, FindAllRequest, FindByIDRequest, GetRequest } from './request.models'; +import { + FindAllOptions, + FindAllRequest, + FindByIDRequest, + GetRequest, + PostRequest, + RestRequest +} from './request.models'; import { RequestService } from './request.service'; import { NormalizedObject } from '../cache/models/normalized-object.model'; +import { distinctUntilChanged, map, share, withLatestFrom } from 'rxjs/operators'; +import { + configureRequest, + filterSuccessfulResponses, + getRequestFromSelflink, + getResponseFromSelflink +} from '../shared/operators'; +import { ResponseCacheEntry } from '../cache/response-cache.reducer'; +import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; +import { HttpHeaders } from '@angular/common/http'; +import { ErrorResponse, GenericSuccessResponse } from '../cache/response-cache.models'; +import { BrowseEntry } from '../shared/browse-entry.model'; +import { AuthService } from '../auth/auth.service'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; export abstract class DataService<TNormalized extends NormalizedObject, TDomain> { protected abstract responseCache: ResponseCacheService; @@ -19,6 +40,7 @@ export abstract class DataService<TNormalized extends NormalizedObject, TDomain> protected abstract store: Store<CoreState>; protected abstract linkPath: string; protected abstract halService: HALEndpointService; + protected abstract authService: AuthService; public abstract getScopedEndpoint(scope: string): Observable<string> @@ -91,21 +113,37 @@ export abstract class DataService<TNormalized extends NormalizedObject, TDomain> return this.rdbService.buildSingle<TNormalized, TDomain>(href); } - // TODO implement, after the structure of the REST server's POST response is finalized - // create(dso: DSpaceObject): Observable<RemoteData<TDomain>> { - // const postHrefObs = this.getEndpoint(); - // - // // TODO ID is unknown at this point - // const idHrefObs = postHrefObs.map((href: string) => this.getFindByIDHref(href, dso.id)); - // - // postHrefObs - // .filter((href: string) => hasValue(href)) - // .take(1) - // .subscribe((href: string) => { - // const request = new RestRequest(this.requestService.generateRequestId(), href, RestRequestMethod.Post, dso); - // this.requestService.configure(request); - // }); - // - // return this.rdbService.buildSingle<TNormalized, TDomain>(idHrefObs, this.normalizedResourceType); - // } + public create(dso: TDomain): Observable<RemoteData<TDomain>> { + const request$ = this.halService.getEndpoint(this.linkPath).pipe( + isNotEmptyOperator(), + distinctUntilChanged(), + withLatestFrom(this.buildCreateParams(dso)), + map(([endpointURL, params]) => { + const options: HttpOptions = Object.create({}); + const headers = new HttpHeaders(); + headers.append('Authentication', this.authService.buildAuthHeader()); + options.headers = headers; + return new PostRequest(this.requestService.generateRequestId(), endpointURL + params, options); + }), + configureRequest(this.requestService), + share() + ); + + const href$ = request$.pipe(map((request: RestRequest) => request.href)); + + const requestEntry$ = href$.pipe(getRequestFromSelflink(this.requestService)); + const responseCache$ = href$.pipe(getResponseFromSelflink(this.responseCache)); + + const payload$ = responseCache$.pipe( + filterSuccessfulResponses(), + map((entry: ResponseCacheEntry) => entry.response), + map((response: GenericSuccessResponse<TDomain>) => response.payload), + distinctUntilChanged() + ); + + return this.rdbService.toRemoteDataObservable(requestEntry$, responseCache$, payload$); + } + + public abstract buildCreateParams(dso: TDomain): Observable<string>; + } diff --git a/src/app/core/data/item-data.service.ts b/src/app/core/data/item-data.service.ts index 6b0937d8e4..d1db71b78d 100644 --- a/src/app/core/data/item-data.service.ts +++ b/src/app/core/data/item-data.service.ts @@ -15,6 +15,8 @@ import { URLCombiner } from '../url-combiner/url-combiner'; import { DataService } from './data.service'; import { RequestService } from './request.service'; import { HALEndpointService } from '../shared/hal-endpoint.service'; +import { NotificationsService } from '../../shared/notifications/notifications.service'; +import { AuthService } from '../auth/auth.service'; @Injectable() export class ItemDataService extends DataService<NormalizedItem, Item> { @@ -26,7 +28,8 @@ export class ItemDataService extends DataService<NormalizedItem, Item> { protected rdbService: RemoteDataBuildService, protected store: Store<CoreState>, private bs: BrowseService, - protected halService: HALEndpointService) { + protected halService: HALEndpointService, + protected authService: AuthService) { super(); } @@ -41,4 +44,9 @@ export class ItemDataService extends DataService<NormalizedItem, Item> { } } + buildCreateParams(dso: Item): Observable<string> { + // TODO: Build parameters for creating an Item on the REST service + return undefined; + } + } -- GitLab