Skip to content

Fix support for adding and removing relationships in relationship-only fields in the submission #1096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 14, 2021
Merged

Fix support for adding and removing relationships in relationship-only fields in the submission #1096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a4a747e
fix issue where the submission form wouldn't update after a relations…
artlowel Apr 13, 2021
56a54ae
don't hide the lookup button for virtual relationship fields
artlowel Apr 13, 2021
43e9b14
78406: Test fixes
Atmire-Kristof Apr 13, 2021
6d612c3
hide hint for relationship fields that aren't repeatable but do have …
artlowel Apr 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 32 additions & 32 deletions src/app/core/shared/operators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,32 +45,32 @@ export const sendRequest = (requestService: RequestService) =>
(source: Observable<RestRequest>): Observable<RestRequest> =>
source.pipe(tap((request: RestRequest) => requestService.send(request)));

export const getRemoteDataPayload = () =>
<T>(source: Observable<RemoteData<T>>): Observable<T> =>
export const getRemoteDataPayload = <T>() =>
(source: Observable<RemoteData<T>>): Observable<T> =>
source.pipe(map((remoteData: RemoteData<T>) => remoteData.payload));

export const getPaginatedListPayload = () =>
<T>(source: Observable<PaginatedList<T>>): Observable<T[]> =>
export const getPaginatedListPayload = <T>() =>
(source: Observable<PaginatedList<T>>): Observable<T[]> =>
source.pipe(map((list: PaginatedList<T>) => list.page));

export const getAllCompletedRemoteData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const getAllCompletedRemoteData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(filter((rd: RemoteData<T>) => hasValue(rd) && rd.hasCompleted));

export const getFirstCompletedRemoteData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const getFirstCompletedRemoteData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(getAllCompletedRemoteData(), take(1));

export const takeUntilCompletedRemoteData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const takeUntilCompletedRemoteData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(takeWhile((rd: RemoteData<T>) => hasNoValue(rd) || rd.isLoading, true));

export const getFirstSucceededRemoteData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const getFirstSucceededRemoteData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(filter((rd: RemoteData<T>) => rd.hasSucceeded), take(1));

export const getFirstSucceededRemoteWithNotEmptyData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const getFirstSucceededRemoteWithNotEmptyData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(find((rd: RemoteData<T>) => rd.hasSucceeded && isNotEmpty(rd.payload)));

/**
Expand All @@ -83,8 +83,8 @@ export const getFirstSucceededRemoteWithNotEmptyData = () =>
* These operators were created as a first step in refactoring
* out all the instances where this is used incorrectly.
*/
export const getFirstSucceededRemoteDataPayload = () =>
<T>(source: Observable<RemoteData<T>>): Observable<T> =>
export const getFirstSucceededRemoteDataPayload = <T>() =>
(source: Observable<RemoteData<T>>): Observable<T> =>
source.pipe(
getFirstSucceededRemoteData(),
getRemoteDataPayload()
Expand All @@ -100,8 +100,8 @@ export const getFirstSucceededRemoteDataPayload = () =>
* These operators were created as a first step in refactoring
* out all the instances where this is used incorrectly.
*/
export const getFirstSucceededRemoteDataWithNotEmptyPayload = () =>
<T>(source: Observable<RemoteData<T>>): Observable<T> =>
export const getFirstSucceededRemoteDataWithNotEmptyPayload = <T>() =>
(source: Observable<RemoteData<T>>): Observable<T> =>
source.pipe(
getFirstSucceededRemoteWithNotEmptyData(),
getRemoteDataPayload()
Expand All @@ -117,8 +117,8 @@ export const getFirstSucceededRemoteDataWithNotEmptyPayload = () =>
* These operators were created as a first step in refactoring
* out all the instances where this is used incorrectly.
*/
export const getAllSucceededRemoteDataPayload = () =>
<T>(source: Observable<RemoteData<T>>): Observable<T> =>
export const getAllSucceededRemoteDataPayload = <T>() =>
(source: Observable<RemoteData<T>>): Observable<T> =>
source.pipe(
getAllSucceededRemoteData(),
getRemoteDataPayload()
Expand All @@ -138,8 +138,8 @@ export const getAllSucceededRemoteDataPayload = () =>
* These operators were created as a first step in refactoring
* out all the instances where this is used incorrectly.
*/
export const getFirstSucceededRemoteListPayload = () =>
<T>(source: Observable<RemoteData<PaginatedList<T>>>): Observable<T[]> =>
export const getFirstSucceededRemoteListPayload = <T>() =>
(source: Observable<RemoteData<PaginatedList<T>>>): Observable<T[]> =>
source.pipe(
getFirstSucceededRemoteData(),
getRemoteDataPayload(),
Expand All @@ -160,8 +160,8 @@ export const getFirstSucceededRemoteListPayload = () =>
* These operators were created as a first step in refactoring
* out all the instances where this is used incorrectly.
*/
export const getAllSucceededRemoteListPayload = () =>
<T>(source: Observable<RemoteData<PaginatedList<T>>>): Observable<T[]> =>
export const getAllSucceededRemoteListPayload = <T>() =>
(source: Observable<RemoteData<PaginatedList<T>>>): Observable<T[]> =>
source.pipe(
getAllSucceededRemoteData(),
getRemoteDataPayload(),
Expand All @@ -174,8 +174,8 @@ export const getAllSucceededRemoteListPayload = () =>
* @param router The router used to navigate to a new page
* @param authService Service to check if the user is authenticated
*/
export const redirectOn4xx = (router: Router, authService: AuthService) =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const redirectOn4xx = <T>(router: Router, authService: AuthService) =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
observableCombineLatest(source, authService.isAuthenticated()).pipe(
map(([rd, isAuthenticated]: [RemoteData<T>, boolean]) => {
if (rd.hasFailed) {
Expand Down Expand Up @@ -229,16 +229,16 @@ export const returnEndUserAgreementUrlTreeOnFalse = (router: Router, redirect: s
return hasAgreed ? hasAgreed : router.createUrlTree([getEndUserAgreementPath()], { queryParams });
}));

export const getFinishedRemoteData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const getFinishedRemoteData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(find((rd: RemoteData<T>) => !rd.isLoading));

export const getAllSucceededRemoteData = () =>
<T>(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
export const getAllSucceededRemoteData = <T>() =>
(source: Observable<RemoteData<T>>): Observable<RemoteData<T>> =>
source.pipe(filter((rd: RemoteData<T>) => rd.hasSucceeded));

export const toDSpaceObjectListRD = () =>
<T extends DSpaceObject>(source: Observable<RemoteData<PaginatedList<SearchResult<T>>>>): Observable<RemoteData<PaginatedList<T>>> =>
export const toDSpaceObjectListRD = <T extends DSpaceObject>() =>
(source: Observable<RemoteData<PaginatedList<SearchResult<T>>>>): Observable<RemoteData<PaginatedList<T>>> =>
source.pipe(
filter((rd: RemoteData<PaginatedList<SearchResult<T>>>) => rd.hasSucceeded),
map((rd: RemoteData<PaginatedList<SearchResult<T>>>) => {
Expand Down
4 changes: 2 additions & 2 deletions ...p/shared/form/builder/ds-dynamic-form-ui/ds-dynamic-form-control-container.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
'd-none': value?.isVirtual && (model.hasSelectableMetadata || context?.index > 0)}">
<div [ngClass]="getClass('grid', 'control')">
<ng-container #componentViewContainer></ng-container>
<small *ngIf="hasHint && (model.repeatable === false || context?.index === context?.context?.groups?.length - 1) && (!showErrorMessages || errorMessages.length === 0)"
<small *ngIf="hasHint && ((model.repeatable === false && (isRelationship === false || value?.value === null)) || (model.repeatable === true && context?.index === context?.context?.groups?.length - 1)) && (!showErrorMessages || errorMessages.length === 0)"
class="text-muted ds-hint" [innerHTML]="model.hint | translate" [ngClass]="getClass('element', 'hint')"></small>
<!-- In case of repeatable fields show empty space for all elements except the first -->
<div *ngIf="context?.index !== null
Expand All @@ -35,7 +35,7 @@
<option *ngFor="let lang of model.languageCodes" [value]="lang.code">{{lang.display}}</option>
</select>
</div>
<div *ngIf="isRelationship && !isVirtual()" class="col-auto text-center">
<div *ngIf="isRelationship" class="col-auto text-center">
<button class="btn btn-secondary"
type="button"
ngbTooltip="{{'form.lookup-help' | translate}}"
Expand Down
18 changes: 12 additions & 6 deletions src/app/submission/edit/submission-edit.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,24 @@ import { RouterStub } from '../../shared/testing/router.stub';
import { ActivatedRouteStub } from '../../shared/testing/active-router.stub';
import { mockSubmissionObject } from '../../shared/mocks/submission.mock';
import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils';
import { ItemDataService } from '../../core/data/item-data.service';

describe('SubmissionEditComponent Component', () => {

let comp: SubmissionEditComponent;
let fixture: ComponentFixture<SubmissionEditComponent>;
let submissionServiceStub: SubmissionServiceStub;
let itemDataService: ItemDataService;
let router: RouterStub;

const submissionId = '826';
const route: ActivatedRouteStub = new ActivatedRouteStub();
const submissionObject: any = mockSubmissionObject;

beforeEach(waitForAsync(() => {
itemDataService = jasmine.createSpyObj('itemDataService', {
findByHref: createSuccessfulRemoteDataObject$(submissionObject.item),
});
TestBed.configureTestingModule({
imports: [
TranslateModule.forRoot(),
Expand All @@ -41,6 +46,7 @@ describe('SubmissionEditComponent Component', () => {
providers: [
{ provide: NotificationsService, useClass: NotificationsServiceStub },
{ provide: SubmissionService, useClass: SubmissionServiceStub },
{ provide: ItemDataService, useValue: itemDataService },
{ provide: TranslateService, useValue: getMockTranslateService() },
{ provide: Router, useValue: new RouterStub() },
{ provide: ActivatedRoute, useValue: route },
Expand All @@ -63,7 +69,7 @@ describe('SubmissionEditComponent Component', () => {
router = null;
});

it('should init properly when a valid SubmissionObject has been retrieved', fakeAsync(() => {
it('should init properly when a valid SubmissionObject has been retrieved',() => {

route.testParams = { id: submissionId };
submissionServiceStub.retrieveSubmission.and.returnValue(
Expand All @@ -78,9 +84,9 @@ describe('SubmissionEditComponent Component', () => {
expect(comp.sections).toBe(submissionObject.sections);
expect(comp.submissionDefinition).toBe(submissionObject.submissionDefinition);

}));
});

it('should redirect to mydspace when an empty SubmissionObject has been retrieved', fakeAsync(() => {
it('should redirect to mydspace when an empty SubmissionObject has been retrieved',() => {

route.testParams = { id: submissionId };
submissionServiceStub.retrieveSubmission.and.returnValue(createSuccessfulRemoteDataObject$({})
Expand All @@ -90,9 +96,9 @@ describe('SubmissionEditComponent Component', () => {

expect(router.navigate).toHaveBeenCalled();

}));
});

it('should not has effects when an invalid SubmissionObject has been retrieved', fakeAsync(() => {
it('should not has effects when an invalid SubmissionObject has been retrieved',() => {

route.testParams = { id: submissionId };
submissionServiceStub.retrieveSubmission.and.returnValue(observableOf(null));
Expand All @@ -104,6 +110,6 @@ describe('SubmissionEditComponent Component', () => {
expect(comp.selfUrl).toBeUndefined();
expect(comp.sections).toBeUndefined();
expect(comp.submissionDefinition).toBeUndefined();
}));
});

});
82 changes: 56 additions & 26 deletions src/app/submission/edit/submission-edit.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@ import { ChangeDetectorRef, Component, OnDestroy, OnInit } from '@angular/core';
import { ActivatedRoute, ParamMap, Router } from '@angular/router';

import { Subscription } from 'rxjs';
import { filter, switchMap } from 'rxjs/operators';
import { filter, switchMap, debounceTime } from 'rxjs/operators';
import { TranslateService } from '@ngx-translate/core';

import { WorkspaceitemSectionsObject } from '../../core/submission/models/workspaceitem-sections.model';
import { hasValue, isEmpty, isNotNull } from '../../shared/empty.util';
import { hasValue, isEmpty, isNotNull, isNotEmptyOperator } from '../../shared/empty.util';
import { SubmissionDefinitionsModel } from '../../core/config/models/config-submission-definitions.model';
import { SubmissionService } from '../submission.service';
import { NotificationsService } from '../../shared/notifications/notifications.service';
import { SubmissionObject } from '../../core/submission/models/submission-object.model';
import { Collection } from '../../core/shared/collection.model';
import { RemoteData } from '../../core/data/remote-data';
import { Item } from '../../core/shared/item.model';
import { getAllSucceededRemoteData } from '../../core/shared/operators';
import { ItemDataService } from '../../core/data/item-data.service';
import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject';

/**
* This component allows to edit an existing workspaceitem/workflowitem.
Expand Down Expand Up @@ -60,6 +63,16 @@ export class SubmissionEditComponent implements OnDestroy, OnInit {
* @type {Array}
*/
private subs: Subscription[] = [];

/**
* BehaviorSubject containing the self link to the item for this submission
* @private
*/
private itemLink$: BehaviorSubject<string> = new BehaviorSubject(undefined);

/**
* The item for this submission.
*/
public item: Item;

/**
Expand All @@ -69,13 +82,15 @@ export class SubmissionEditComponent implements OnDestroy, OnInit {
* @param {NotificationsService} notificationsService
* @param {ActivatedRoute} route
* @param {Router} router
* @param {ItemDataService} itemDataService
* @param {SubmissionService} submissionService
* @param {TranslateService} translate
*/
constructor(private changeDetectorRef: ChangeDetectorRef,
private notificationsService: NotificationsService,
private route: ActivatedRoute,
private router: Router,
private itemDataService: ItemDataService,
private submissionService: SubmissionService,
private translate: TranslateService) {
}
Expand All @@ -84,32 +99,47 @@ export class SubmissionEditComponent implements OnDestroy, OnInit {
* Retrieve workspaceitem/workflowitem from server and initialize all instance variables
*/
ngOnInit() {
this.subs.push(this.route.paramMap.pipe(
switchMap((params: ParamMap) => this.submissionService.retrieveSubmission(params.get('id'))),
// NOTE new submission is retrieved on the browser side only, so get null on server side rendering
filter((submissionObjectRD: RemoteData<SubmissionObject>) => isNotNull(submissionObjectRD))
).subscribe((submissionObjectRD: RemoteData<SubmissionObject>) => {
if (submissionObjectRD.hasSucceeded) {
if (isEmpty(submissionObjectRD.payload)) {
this.notificationsService.info(null, this.translate.get('submission.general.cannot_submit'));
this.router.navigate(['/mydspace']);
this.subs.push(
this.route.paramMap.pipe(
switchMap((params: ParamMap) => this.submissionService.retrieveSubmission(params.get('id'))),
// NOTE new submission is retrieved on the browser side only, so get null on server side rendering
filter((submissionObjectRD: RemoteData<SubmissionObject>) => isNotNull(submissionObjectRD))
).subscribe((submissionObjectRD: RemoteData<SubmissionObject>) => {
if (submissionObjectRD.hasSucceeded) {
if (isEmpty(submissionObjectRD.payload)) {
this.notificationsService.info(null, this.translate.get('submission.general.cannot_submit'));
this.router.navigate(['/mydspace']);
} else {
this.submissionId = submissionObjectRD.payload.id.toString();
this.collectionId = (submissionObjectRD.payload.collection as Collection).id;
this.selfUrl = submissionObjectRD.payload._links.self.href;
this.sections = submissionObjectRD.payload.sections;
this.itemLink$.next(submissionObjectRD.payload._links.item.href);
this.item = submissionObjectRD.payload.item;
this.submissionDefinition = (submissionObjectRD.payload.submissionDefinition as SubmissionDefinitionsModel);
}
} else {
this.submissionId = submissionObjectRD.payload.id.toString();
this.collectionId = (submissionObjectRD.payload.collection as Collection).id;
this.selfUrl = submissionObjectRD.payload._links.self.href;
this.sections = submissionObjectRD.payload.sections;
this.item = submissionObjectRD.payload.item as Item;
this.submissionDefinition = (submissionObjectRD.payload.submissionDefinition as SubmissionDefinitionsModel);
this.changeDetectorRef.detectChanges();
}
} else {
if (submissionObjectRD.statusCode === 404) {
// redirect to not found page
this.router.navigate(['/404'], { skipLocationChange: true });
if (submissionObjectRD.statusCode === 404) {
// redirect to not found page
this.router.navigate(['/404'], { skipLocationChange: true });
}
// TODO handle generic error
}
// TODO handle generic error
}
}));
}),
this.itemLink$.pipe(
isNotEmptyOperator(),
switchMap((itemLink: string) =>
this.itemDataService.findByHref(itemLink)
),
getAllSucceededRemoteData(),
// Multiple sources can update the item in quick succession.
// We only want to rerender the form if the item is unchanged for some time
debounceTime(300),
).subscribe((itemRd: RemoteData<Item>) => {
this.item = itemRd.payload;
this.changeDetectorRef.detectChanges();
}),
);
}

/**
Expand Down
Loading