-
Notifications
You must be signed in to change notification settings - Fork 15
DRYD-1832: New API Usage #279
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
base: develop
Are you sure you want to change the base?
Conversation
11572f6
to
70c360c
Compare
This requires some prop drilling to work correctly, it would be nice find a better way to handle this
70c360c
to
b12df38
Compare
Doing a QA check, I have found the following issue:
|
@spirosdi Thanks, I was still thinking about the best way to get the |
QA issue: The Traverser is not shown when navigating to the record editor after clicking the list item in the "grid" layout. It works fine in the "table". |
Actually the issue is not related to the current updates. It already appears in |
What does this do?
Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1832
This allows us to query against the new API and test what the views look like with the new response data. In addition it has updated the grid and detail list views to incorporate the thumbnail of an object when possible. There's still work around those views which will need to be done, such as displaying an image if it is not found or providing a warning for NAGPRA related media.
Additional changes were made to deal with passing the listType as a prop everything. I found that as I was testing different functionality, either the
search
action was being called incorrectly or something else would happen because thelistType
and potentially theisNewSearch
flag were incorrect. I opted to refactor a little bit and allow us to get thelistType
from either thesearchDescriptor
or thesearchResult
so that thelistType
/isNewSearch
props aren't as necessary and can be omitted. Most of this work was done in e2891ae and tests were updated in 4548504.How should this be tested? Do these changes have associated tests?
To test locally:
npm run lint
npm run test
develop
branch, e.g.npm run devserver
Once the 403 issue with dev is resolved, instead of deploying locally this can be validated by using
npm run devserver --back-end=https://core.dev.collectionspace.org
Dependencies for merging? Releasing to production?
The only thing I can think of is that it would be nice to have the tests use the same configuration as the application does when it starts up. I have work for that in another branch, which hopefully can be shared soon.
Has the application documentation been updated for these changes?
No
Did someone actually run this code to verify it works?
@mikejritter has only tested locally