Skip to content

Commit

Permalink
Merge pull request #383 from atmire/edit-metadata-bugfix
Browse files Browse the repository at this point in the history
Admin Edit Metadata - bugfix
  • Loading branch information
tdonohue committed Apr 15, 2019
2 parents 7d2ec90 + b5ab584 commit 642c091
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ describe('ObjectUpdatesEffects', () => {
let actions: Observable<any>;
let testURL = 'www.dspace.org/dspace7';
let testUUID = '20e24c2f-a00a-467c-bdee-c929e79bf08d';
const fakeID = 'id';
beforeEach(async(() => {
TestBed.configureTestingModule({
providers: [
Expand All @@ -44,7 +45,9 @@ describe('ObjectUpdatesEffects', () => {
testURL = 'www.dspace.org/dspace7';
testUUID = '20e24c2f-a00a-467c-bdee-c929e79bf08d';
updatesEffects = TestBed.get(ObjectUpdatesEffects);
(updatesEffects as any).actionMap[testURL] = new Subject<ObjectUpdatesAction>();
(updatesEffects as any).actionMap$[testURL] = new Subject<ObjectUpdatesAction>();
(updatesEffects as any).notificationActionMap$[fakeID] = new Subject<ObjectUpdatesAction>();
(updatesEffects as any).notificationActionMap$[(updatesEffects as any).allIdentifier] = new Subject<ObjectUpdatesAction>();
});

describe('mapLastActions$', () => {
Expand All @@ -57,7 +60,7 @@ describe('ObjectUpdatesEffects', () => {
it('should emit the action from the actionMap\'s value which key matches the action\'s URL', () => {
action = new RemoveObjectUpdatesAction(testURL);
actions = hot('--a-', { a: action });
(updatesEffects as any).actionMap[testURL].subscribe((act) => emittedAction = act);
(updatesEffects as any).actionMap$[testURL].subscribe((act) => emittedAction = act);
const expected = cold('--b-', { b: undefined });

expect(updatesEffects.mapLastActions$).toBeObservable(expected);
Expand Down
70 changes: 58 additions & 12 deletions src/app/core/data/object-updates/object-updates.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,74 @@ import {
ObjectUpdatesActionTypes,
RemoveObjectUpdatesAction
} from './object-updates.actions';
import { delay, map, switchMap, take, tap } from 'rxjs/operators';
import { delay, filter, map, switchMap, take, tap } from 'rxjs/operators';
import { of as observableOf, race as observableRace, Subject } from 'rxjs';
import { hasNoValue } from '../../../shared/empty.util';
import { NotificationsService } from '../../../shared/notifications/notifications.service';
import { INotification } from '../../../shared/notifications/models/notification.model';
import {
NotificationsActions,
NotificationsActionTypes,
RemoveNotificationAction
} from '../../../shared/notifications/notifications.actions';

/**
* NGRX effects for ObjectUpdatesActions
*/
@Injectable()
export class ObjectUpdatesEffects {

/**
* Identifier for when an action on all notifications is performed
*/
private allIdentifier = 'all';

/**
* Map that keeps track of the latest ObjectUpdatesAction for each page's url
*/
private actionMap: {
private actionMap$: {
/* Use Subject instead of BehaviorSubject:
we only want Actions that are fired while we're listening
actions that were previously fired do not matter anymore
*/
[url: string]: Subject<ObjectUpdatesAction>
} = {};

private notificationActionMap$: {
/* Use Subject instead of BehaviorSubject:
we only want Actions that are fired while we're listening
actions that were previously fired do not matter anymore
*/
[id: string]: Subject<NotificationsActions>
} = { all: new Subject() };
/**
* Effect that makes sure all last fired ObjectUpdatesActions are stored in the map of this service, with the url as their key
*/
@Effect({ dispatch: false }) mapLastActions$ = this.actions$
.pipe(
ofType(...Object.values(ObjectUpdatesActionTypes)),
map((action: DiscardObjectUpdatesAction) => {
map((action: ObjectUpdatesAction) => {
const url: string = action.payload.url;
if (hasNoValue(this.actionMap[url])) {
this.actionMap[url] = new Subject<ObjectUpdatesAction>();
if (hasNoValue(this.actionMap$[url])) {
this.actionMap$[url] = new Subject<ObjectUpdatesAction>();
}
this.actionMap$[url].next(action);
}
)
);

/**
* Effect that makes sure all last fired NotificationActions are stored in the notification map of this service, with the id as their key
*/
@Effect({ dispatch: false }) mapLastNotificationActions$ = this.actions$
.pipe(
ofType(...Object.values(NotificationsActionTypes)),
map((action: RemoveNotificationAction) => {
const id: string = action.payload.id || action.payload || this.allIdentifier;
if (hasNoValue(this.notificationActionMap$[id])) {
this.notificationActionMap$[id] = new Subject<NotificationsActions>();
}
this.actionMap[url].next(action);
this.notificationActionMap$[id].next(action);
}
)
);
Expand All @@ -61,17 +95,30 @@ export class ObjectUpdatesEffects {
// Either wait for the delay and perform a remove action
observableOf(new RemoveObjectUpdatesAction(action.payload.url)).pipe(delay(timeOut)),
// Or wait for a a user action
this.actionMap[url].pipe(
this.actionMap$[url].pipe(
take(1),
tap(() => this.notificationsService.remove(notification)),
tap(() => {
this.notificationsService.remove(notification);
}),
map((updateAction: ObjectUpdatesAction) => {
if (updateAction.type === ObjectUpdatesActionTypes.REINSTATE) {
// If someone reinstated, do nothing, just let the reinstating happen
return { type: 'NO_ACTION' }
} else {
// If someone performed another action, assume the user does not want to reinstate and remove all changes
return new RemoveObjectUpdatesAction(action.payload.url);
}
// If someone performed another action, assume the user does not want to reinstate and remove all changes
return new RemoveObjectUpdatesAction(action.payload.url);
})
),
this.notificationActionMap$[notification.id].pipe(
filter((notificationsAction: NotificationsActions) => notificationsAction.type === NotificationsActionTypes.REMOVE_NOTIFICATION),
map(() => {
return new RemoveObjectUpdatesAction(action.payload.url);
})
),
this.notificationActionMap$[this.allIdentifier].pipe(
filter((notificationsAction: NotificationsActions) => notificationsAction.type === NotificationsActionTypes.REMOVE_ALL_NOTIFICATIONS),
map(() => {
return new RemoveObjectUpdatesAction(action.payload.url);
})
)
)
Expand All @@ -81,7 +128,6 @@ export class ObjectUpdatesEffects {

constructor(private actions$: Actions,
private notificationsService: NotificationsService) {

}

}
3 changes: 2 additions & 1 deletion src/app/core/registry/registry.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ export class RegistryService {

const metadatafieldsObs: Observable<MetadataField[]> = rmrObs.pipe(
map((rmr: RegistryMetadatafieldsResponse) => rmr.metadatafields),
map((metadataFields: MetadataField[]) => metadataFields)
/* Make sure to explicitly cast this into a MetadataField object, on first page loads this object comes from the object cache created by the server and its prototype is unknown */
map((metadataFields: MetadataField[]) => metadataFields.map((metadataField: MetadataField) => Object.assign(new MetadataField(), metadataField)))
);

const pageInfoObs: Observable<PageInfo> = requestEntryObs.pipe(
Expand Down
1 change: 0 additions & 1 deletion src/app/shared/notifications/notifications.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export class NotificationsService {
options: Partial<NotificationOptions> = {},
html: boolean = false): INotification {
const notificationOptions = { ...this.getDefaultOptions(), ...options };
console.log(notificationOptions);
const notification = new Notification(uniqueId(), NotificationType.Info, title, content, notificationOptions, html);
this.add(notification);
return notification;
Expand Down

0 comments on commit 642c091

Please sign in to comment.