[DSpace-CRIS] Bulk Import from Excel File (Frontend)#5135
[DSpace-CRIS] Bulk Import from Excel File (Frontend)#5135FrancescoMolinaro wants to merge 6 commits into
Conversation
|
Same here, @FrancescoMolinaro . Please specify in the description which features/differences this PR relates to. |
tdonohue
added
new feature
DSpace-CRIS merger
|
Hi @FrancescoMolinaro, |
|
I believe this requires backend PR DSpace/DSpace#11968. So, I've updated the description to note that |
tdonohue
changed the title
lgeggleston
moved this from 🙋 Needs Reviewers Assigned
to 👀 Under Review
in DSpace 10.0 Release
|
Hi @FrancescoMolinaro, |
|
Hi @FrancescoMolinaro, |
|
@tdonohue My colleague from PCG, @Emil-Domagala-pcg would like to perform a code review for this PR. Can you please add him as a reviewer to this task? |
|
@Dawnkai : Unfortunately, because of limitations with GitHub, I can only assign someone to a code review if they are a member of our "developer" team in GitHub or they comment on the PR directly. So, I've sent an invite to @Emil-Domagala-pcg to be added to the "developer" team for DSpace. In the meantime, @Emil-Domagala-pcg is welcome to review this PR (and the backend one) even if they are not formally assigned. |
| * A boolean representing if a create delete operation is pending | ||
| * @type {BehaviorSubject<boolean>} | ||
| */ | ||
| processingImport$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false); |
There was a problem hiding this comment.
why is this defined if its never updated?
i cant see anywhere in the file any change for it
| * Validates the form, sets the parameters to correct values and invokes the script with the correct parameters | ||
| * @param form | ||
| */ | ||
| submit() { |
There was a problem hiding this comment.
this method never checks if this.selectedFile is not null. In such cases this will throw error when trying to access .name
or form validity
| */ | ||
| submit() { | ||
|
|
||
| const values: any = this.form.value; |
|
|
||
| this.scriptService.invoke('bulk-import', stringParameters, [this.selectedFile]) | ||
| .pipe(getFirstCompletedRemoteData()) | ||
| .subscribe((rd: RemoteData<Process>) => { |
There was a problem hiding this comment.
i would prefer instead of using if to use something like:
.subscribe({
next: (rd) => { ... },
error: () => {this.notificationsService.error(...);}
});
5 participants
References
Description
Add new /bulk-import path under the collection route, to execute an import in bulk from file directly into the
collection.
The new page is accessible from the collection edit menu:
Instructions for Reviewers
The bulk import script allows to add, update or delete multiple items by uploading an excel file with a specific structure. All the items involved in a single import run must refer to a single collection: the updated and deleted objects must therefore be contained in that collection and the added items will be placed within the same collection.
List of changes in this PR:
Add new collection subroute and related page component.
Add new collection edit menu.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.