Skip to content

Conversation

@MarieVerdonck
Copy link
Contributor

@MarieVerdonck MarieVerdonck commented Sep 4, 2020

References

Works with endpoint created in: DSpace/RestContract#145
Fixes a functionality disabled here: 09f1b7e

Description

Metadata field auto complete on the edit item page now works with a new endpoint search, that utilises a search with the metadatafields indexed in discovery so they can be searched quicker. So the autocomplete has been re-implemented and a validator which uses the /metadatafields/search/byFieldName?exactName to check if there is 1 exact match to the user's input.

Instructions for Reviewers

Can be tested by going to the edit item page > metadata and adding a new metadata field, e.g. http://localhost:4000/items/047556d1-3d01-4c53-bc68-0cee7ad7ed4e/edit/metadata.
When you start putting in a metadata field it should give you a dropdown of suggestions. When your filled in metadata field is not a valid metadata field; when you go to fill in a value, an error message is shown underneath the metadata input field.

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes TSLint validation using yarn run lint
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

@artlowel artlowel added this to the 7.0beta4 milestone Sep 4, 2020
@lgtm-com
Copy link

lgtm-com bot commented Sep 4, 2020

This pull request introduces 7 alerts when merging dcbb002 into cd6c5b7 - view on LGTM.com

new alerts:

  • 7 for Unused variable, import, function or class

@tdonohue tdonohue added 1 APPROVAL pull request only requires a single approval to merge improvement labels Sep 8, 2020
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.

@MarieVerdonck : reviewed & tested today. Overall, this looks great, my only requests are minor...

  1. First, I tested this and it works well overall. However, I was surprised that it doesn't stop me from clicking save on an invalid field name. So, for example, I can type in "dc.titlenotvalid", and I'll see the "Please choose a valid metadata field" error. But, I can still add the field (by clicking the green checkbox) and I can even click the "Save" at the bottom of the page. The field is NOT added, but I get a success message back as if it all worked. If I refresh the page the field is gone though.
    • My expected behavior: I'd expect that I should not even be able to add an invalid field. Maybe the green checkbox should be disabled if you enter something invalid?
  2. I have a few minor requests for TypeDocs/comments inline below.

Beyond those two things, this looks good to me!

@MarieVerdonck
Copy link
Contributor Author

MarieVerdonck commented Sep 16, 2020

@tdonohue
The checkmark and general item edit metadata page save button is now disabled until the input for the metadata field is a valid, both for new metadata, and editing metadata.

The two places missing docs are actually no longer needed, they were needed in the previous implementation of endpoint to find exact metadatafield match or error if there was no match; (used by previous implementation of MetadataFieldDataService#findByExactFieldName, ref: 4a0444d#diff-7c72cde0ef249ec0c0a17d22b8e1a009R93) but this is now via the same endpoint /searchByFieldName but with query param exactName, so no longer needed since it just returns a paginated list (which can be empty indicating there is no exact match) containing the match, which gets parsed with the existing code. They've now been removed.

@tdonohue
Copy link
Member

@MarieVerdonck : Just a quick note. It looks like Travis CI is hitting build errors based on your latest commit.

@artlowel artlowel requested a review from tdonohue September 17, 2020 11:03
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.

👍 Retested today & it works great now. Code looks good too. Thanks @MarieVerdonck !

kosarko pushed a commit to ufal/dspace-angular that referenced this pull request Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1 APPROVAL pull request only requires a single approval to merge improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The metadata autocomplete queries should be sent to the server, not run on the client

3 participants