Item relations edit delete #402
Conversation
…and item-relationships
…and fix performance issues
…rmissions Conflicts: src/app/core/cache/object-cache.service.ts src/app/core/core.module.ts src/app/core/data/request.service.ts src/app/core/shared/item.model.ts
There was a problem hiding this comment.
Thank you @Atmire-Kristof for adding this important feature! The basic feature is implemented but there are some issues regarding this implementation, like when you remove a relationship, the related item is still displayed. Or, on a specific Entity page (author, for instance) the related items listing nor the
Now showing 1 - 10 of 215 is updated. You need to refresh the page to reload all components.
| <ng-container *ngVar="(updates$ | async) as updates"> | ||
| <div *ngIf="updates"> | ||
| <h5>{{getRelationshipMessageKey(relationshipLabel) | translate}}</h5> | ||
| <ng-container *ngVar="(updates | dsObjectValues) as updateValues"> |
There was a problem hiding this comment.
Thank you @Atmire-Kristof! I found out If you try to edit a Person entity that has more than 20 related items, not all of them are showed. I think we need some kind of paging for each entity type.
There was a problem hiding this comment.
Hi @paulo-graca
I would like to handle that use case in a separate PR as it will be dependent on DSpace/DSpace#2450
Once that's in master, we can use paging per relationship type, which would be important for this functionality. This can also be used for item display pages once merged
| * Sends a new remove update for this field to the object updates service | ||
| */ | ||
| remove(): void { | ||
| this.objectUpdatesService.saveRemoveFieldUpdate(this.url, this.item); |
There was a problem hiding this comment.
just after removing the relationship, the related item still appears in the "Items relationships" tab. And you can even try to repeat the same action (you will get an error -
).
You need to switch to any other tab and then go back to Items relationships tab in order to be updated.
There was a problem hiding this comment.
Hi @paulo-graca
This was apparently caused by some updates in master from REST
Kristof has solved these errors now, and I was able to use delete relationships
There was a problem hiding this comment.
Ok, thank you @benbosman! I will perform some more tests.
There was a problem hiding this comment.
Thank you @benbosman and @Atmire-Kristof , that fix that issue.
But just after removing that relationship, that item is still showed as a part of Works associated with that Person.
and you click on that entry, you will see that the item is no longer related:
It seems to me that is a caching issue.
There was a problem hiding this comment.
Thanks @paulo-graca
I've asked Kristof to verify it in detail, it seems that no new REST discovery is created for displaying the item page. The discovery cache would need to be invalidated here.
Do you know how this is handled for other situations where the discovery cache has to be invalidated (e.g. a new item is submitted, item metadata has been edited, …)?
There was a problem hiding this comment.
It was indeed a caching issue. This should be fixed now.
There was a problem hiding this comment.
Ok, thank you @Atmire-Kristof and @benbosman! I will perform another test.
|
I'm sorry but I still see the related items in the list just after delete the relationship between them. |
Conflicts: src/app/+item-page/simple/item-types/shared/item-relationships-utils.ts
I think this: #458 might be related with what I'm experiencing. |
Conflicts: resources/i18n/en.json src/app/+item-page/edit-item-page/item-metadata/item-metadata.component.ts src/app/+search-page/search-service/search.service.ts src/app/core/core.module.ts src/app/core/shared/item.model.ts
There was a problem hiding this comment.
👍 I've reviewed the code and tested this today. It works as expected. However, I didn't test for large numbers of relationships (i.e. the prior performance issues mentioned above). As discussed in yesterday's Entities Meeting, we felt the performance issues are less specific to this PR, and should therefore be handled separately in #458
So, I'm fine with merging this as-is. Performance issues seem more general, and should be in a separate PR.
There was a problem hiding this comment.
I gave a second look to the code and I don't have any code suggestions. 👍 Thank you @Atmire-Kristof !
On our last entities meeting (https://wiki.duraspace.org/display/DSPACE/2019-09-03+DSpace+7+Entities+WG+Meeting) I noted that the process took you almost 5 minutes to complete in your video. There are performance and scalability issues and the usage will be unsustainable on my point of view. I've been reporting some issues that might address these performance issues:
https://jira.duraspace.org/browse/DS-4335
https://jira.duraspace.org/browse/DS-4248
or #458
|
@paulo-graca Thanks for the analysis on the performance, we'll also verify these tickets |
This PR adds the possibility to remove relationships from an item.
UI
A new tab labeled “Item Relationships” has been added to the item edit page (under sub-route
/relationships)The content of this tab functions much like the “Item Metadata” tab, except it is limited to deleting relationships (no adding or editing yet).
Relationships are also displayed as lists of items instead of a table, separated by relationship type.
Selecting items by clicking the trash bin icon will mark them with a red background.
Clicking “save” will remove the item relationships, display a notification stating the removal was successful (or an error notification for each failed delete request) and eventually reload the lists of relationships.
The other options (such as undo or discard) function the exact same way as it does on the edit item metadata page.
Code changes
A notable code change that was made in this PR is a new abstract item-update component, which contains all the shared code between item-metadata and item-relationships. This is to avoid code duplication between the two components (and open for expansion).