Skip to content
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.

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

Admin pop-up to create/edit a DSO #367

Merged
merged 16 commits into from
Mar 21, 2019

Conversation

LotteHofstede
Copy link
Contributor

@LotteHofstede LotteHofstede commented Mar 7, 2019

This PR adds the ability to quickly go to a create or edit page as an administrator, using the admin menu.
To use this functionality, login and click a Community/Collection/Item menu item in the New or Edit menu section.

Create

For Communities a list of Communities will be shown in which you can create a subcommunity - or you can choose to add a new Top-level community instead.
For Collections a list of Communities will also be shown in which you can create a the collection.
For Items a list of Collections will be shown in which you can create a new Item.*

Edit:

For edit a list of the respective object type is shown.

*) These create item links do not work yet, because there's no direct link (even in the submission PR) to submit an item in a specific collection

Note that in the mockups, a list of parents was shown for the communities/collections like a trail.
However, this functionality is not yet implemented yet because DSpace objects do not have any parents in the backend yet. This relates to this comment on the Submission PR

This PR now also closes #219

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, this looks great overall. I've tested it, and it works. I'm nearly a 👍 , but I have a few (very minor) requests for TypeDocs, comments etc. inline below.

function: () => {
this.modalService.open(CreateCommunityParentSelectorComponent);
}
} as OnClickMenuItemModel,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely, even though Create Community is listed first in this component, it shows up last under the "New" menu. I'm not sure that's a fault of this PR though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strangely enough, this behaviour only occurs when preboot is enabled and is not related to this PR.
I'm not sure why this happens, but before launching the Admin Menu PR I discussed this with @artlowel and we decided it's not really a bug: if you want to force a specific order of menu items, you can explicitly pass a manual "index" when creating a new menu item.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this isn't the fault of this PR. I see the same behavior on master.

However, I'm going to log it as a bug, as I feel these menus should retain an expected ordering. At the very least, we need to document the "force a specific order of menu items" option you noted, but ideally we should find a way to get ordering to remain consistent.

Nonetheless, it's a bug not caused by this PR. So, this PR can obviously move forward as-is.

src/app/+search-page/search-page.module.ts Outdated Show resolved Hide resolved
src/app/+search-page/search-page.module.ts Outdated Show resolved Hide resolved
Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Gave this a second review/test. It looks great to me now. Thanks @LotteHofstede !

@paulo-graca
Copy link
Contributor

I'm having a weird behavior. A UUID is showed just after creating a new Top Level community and I can't do anything (create a sub-community or collection)
2019-03-12 14_51_39-The 3 dalmatians

When I'm trying to create a new collection on a nearly created community, a UUID is showed and I can't create a collection:
2019-03-12 14_52_05-The 3 dalmatians

@paulo-graca
Copy link
Contributor

@LotteHofstede a doubt I have, "new item" is same as "submission", right? It will be created another PR to integrate this work with the submission?

@LotteHofstede
Copy link
Contributor Author

LotteHofstede commented Mar 14, 2019

@paulo-graca If I understand correctly, you create a Community which will redirect you to the detail page of that Community. Then you open the admin menu and try to create/edit another Community or create a Collection. Because you are on a community detail page, the current community's UUID will already be filled out to help you.

Unfortunately, this will give you 0 results, because the SOLR search core has not yet indexed the newly created community and therefore cannot find it yet. You should be able to find the Community if you wait a bit longer and search for the UUID again.

I understand this is confusing behaviour, but it's nothing we can do anything about in the frontend.
We could request to turn the backend's SOLR indexing delay off or minimise it, or at least force a SOLR commit after a new DSO is created.

Thanks to your report I did find another issue with request caching. The cache was almost never invalidated because of some bugs in the request service code. I did fix this, which should also fix issue #219.

To answer your second question: yes, the New item menu item should link to the submission page. I asked @atarix83 to look into creating a link to immediately submit a new item into an existing collection, however this is not yet available (neither is it in the current Submission PR, as far as I'm aware).

@paulo-graca
Copy link
Contributor

When creating a new Item, I was trying to searched for an existing collection. I noticed that, for every typed letter, the service makes a request. After several typed letters I've got a 403 HTTP Error (Forbidden) when searching for that collection.
And after that error, the service stopped performing requests.
Console error:
ERROR TypeError: Cannot read property 'objects' of undefined
at MapSubscriber.project (search.service.ts:115)
at MapSubscriber../node_modules/rxjs/_esm5/internal/operators/map.js.MapSubscriber._next (map.js:35)
at MapSubscriber../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next (Subscriber.js:54)
at MapSubscriber../node_modules/rxjs/_esm5/internal/operators/map.js.MapSubscriber._next (map.js:41)
at MapSubscriber../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next (Subscriber.js:54)
at MapSubscriber../node_modules/rxjs/_esm5/internal/operators/map.js.MapSubscriber._next (map.js:41)
at MapSubscriber../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next (Subscriber.js:54)
at FilterSubscriber../node_modules/rxjs/_esm5/internal/operators/filter.js.FilterSubscriber._next (filter.js:38)
at FilterSubscriber../node_modules/rxjs/_esm5/internal/Subscriber.js.Subscriber.next (Subscriber.js:54)
at SwitchMapSubscriber../node_modules/rxjs/_esm5/internal/operators/switchMap.js.SwitchMapSubscriber.notifyNext (switchMap.js:66)

@artlowel
Copy link
Member

@paulo-graca we weren't able to reproduce the 403, perhaps you can give more details about what you typed to get it? @LotteHofstede did adjust the search service to ensure an unexpected result can no longer break the UI for follow up queries. She also added a debounce to ensure that not every keypress gets sent to the server if you're typing quickly.

@paulo-graca
Copy link
Contributor

paulo-graca commented Mar 19, 2019

@LotteHofstede did adjust the search service to ensure an unexpected result can no longer break the UI for follow up queries

That is great! I will perform another test.

Copy link
Contributor

@paulo-graca paulo-graca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I performed another test and now it works properly, the code looks ok to me. @LotteHofstede thank you for your work!

@tdonohue
Copy link
Member

This PR is now at +2. I gave it another quick code review (of more recent changes). Merging. Thanks @LotteHofstede!

@tdonohue tdonohue merged commit 6aa0ec4 into DSpace:master Mar 21, 2019
@ghost ghost removed the needs review label Mar 21, 2019
@benbosman benbosman deleted the Admin-select-DSO-modal branch September 11, 2020 07:44
@tdonohue tdonohue added this to the 7.0preview milestone Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants