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
Item edit page metadatafields suggestion & validation #860
Item edit page metadatafields suggestion & validation #860
Conversation
This pull request introduces 7 alerts when merging dcbb002 into cd6c5b7 - view on LGTM.com new alerts:
|
There was a problem hiding this 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...
- 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?
- I have a few minor requests for TypeDocs/comments inline below.
Beyond those two things, this looks good to me!
removed unneeded service
@tdonohue 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. |
@MarieVerdonck : Just a quick note. It looks like Travis CI is hitting build errors based on your latest commit. |
There was a problem hiding this 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 !
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
yarn run lint
package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.