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

Collection dropdown on MyDSpace & support enable/disable in Submission UI #708

Merged
merged 13 commits into from Jun 26, 2020

Conversation

ddinuzzo
Copy link
Contributor

@ddinuzzo ddinuzzo commented Jun 17, 2020

References

Fixes #671
Fixes #673

Description

In this PR has been added:

  • a new shared component used to display list of collections. This component is used as the collection dropdown in the Submission UI, as the collection selector in the Admin Sidebar menu and to start a new submission from the MyDSpace page
  • support to enable/disable the collection dropdown in the Submission UI

Instructions for Reviewers

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
  • My PR passes Checkstyle validation based on the Code Style Guide
  • My PR includes Javadoc for all new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
  • My PR passes all tests and includes new/updated Unit or Integration Tests for any bug fixes, improvements or new features. A few reminders about what constitutes good tests:
    • Include tests for different user types, including: (1) Anonymous user, (2) Logged in user (non-admin), and (3) Administrator.
    • Include tests for known error scenarios and error codes (e.g. 400 Bad Request, 401 Unauthorized, 403 Forbidden, 404 Not Found, etc)
    • For bug fixes, include a test that reproduces the bug and proves it is fixed. For clarity, it may be useful to provide the test in a separate commit from the bug fix.
  • N/A If my PR includes new, third-party dependencies (in any pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • N/A If my PR modifies the REST API, I've linked to the REST Contract page (or open PR) related to this change.

@atarix83 atarix83 mentioned this pull request Jun 18, 2020
5 tasks
@heathergreerklein heathergreerklein added this to Ready to review in DSpace 7 Beta 3 Jun 18, 2020
@tdonohue tdonohue changed the title Cst 3090 collection dropdown Collection dropdown on MyDSpace & support enable/disable in Submission UI Jun 23, 2020
@tdonohue tdonohue added this to the 7.0beta3 milestone Jun 23, 2020
@heathergreerklein heathergreerklein moved this from Needs Reviewers Assigned to Under Review in DSpace 7 Beta 3 Jun 23, 2020
@tdonohue
Copy link
Member

Just wanted to add a quick note here to say that I've reviewed the code and it all looks good/reasonable. I haven't had a chance to test this yet though, but will do so tomorrow.

Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

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

Thanks @ddinuzzo !

The performance of the collection select is much improved!

  • On mydspace, when I click the new submission button, I get the collection select, as was discussed. However if you drop a file on the dropzone, I get a notification that the workspace item was created: click HERE to edit it. If I click HERE I go straight to the submission. I would expect to also get the collection select first in this case.
  • Not new in this PR, but the word HERE in that notification is positioned strangely, it is lower than the rest of the text. There's also no need to put in in all caps.

@atarix83
Copy link
Contributor

thanks @artlowel for review

On mydspace, when I click the new submission button, I get the collection select, as was discussed. However if you drop a file on the dropzone, I get a notification that the workspace item was created: click HERE to edit it. If I click HERE I go straight to the submission. I would expect to also get the collection select first in this case.

This will be fixed in a follow-up PR regarding this issue

Not new in this PR, but the word HERE in that notification is positioned strangely, it is lower than the rest of the text. There's also no need to put in in all caps.

we will fix it

@ddinuzzo
Copy link
Contributor Author

Hi @artlowel !
I have fixed the position of the word "here" in the notification

@ddinuzzo ddinuzzo requested a review from artlowel June 26, 2020 07:42
@lgtm-com
Copy link

lgtm-com bot commented Jun 26, 2020

This pull request introduces 3 alerts when merging f37de54 into bb70591 - view on LGTM.com

new alerts:

  • 3 for Unused variable, import, function or class

Copy link
Member

@artlowel artlowel left a comment

Choose a reason for hiding this comment

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

Thanks

DSpace 7 Beta 3 automation moved this from Under Review to Reviewer approved Jun 26, 2020
@atarix83
Copy link
Contributor

@tdonohue since the related REST PR was merged I realized is urgent to merge this PR in the master, otherwise Angular app doesn't work properly
could you give a test as soon as you can? thanks

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.

👍 Looks good to me. Thanks for the friendly reminder @atarix83 on getting this reviewed & merged.

@tdonohue tdonohue merged commit ca564b2 into DSpace:master Jun 26, 2020
DSpace 7 Beta 3 automation moved this from Reviewer approved to Done Jun 26, 2020
@abollini abollini deleted the CST-3090-collection-dropdown branch July 6, 2020 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
4 participants