-
Notifications
You must be signed in to change notification settings - Fork 2
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
Revert display for objectCount #14
Conversation
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.
This almost works. The field displays and saves on the record editor, but selecting and performing a search on the field in advanced search doesn't work. You can actually do this using ONE WEIRD TRICK!
src/messages.js
Outdated
@@ -12,6 +12,7 @@ export default { | |||
'column.collectionobject.narrow.objectNumber': 'Acc. number', | |||
|
|||
'field.collectionobjects_common.objectNumber.name': 'Accession number', | |||
'field.collectionobjects_common.objectCountGroup.name': 'Number of objects', |
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.
For the search page, the field.collectionobjects_common.objectCount.fullName
and field.collectionobjects_common.objectCount.name
messages need to be set to 'Number of objects'. You don't need to change field.collectionobjects_common.objectCountGroup.name
, because of the trick...
objectCountGroupList: { | ||
objectCountGroup: { | ||
[config]: { | ||
repeating: false, |
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.
This is the thing that causes search to break. In order to generate the right search query, we need to know that objectCountGroup
is repeating, so this needs to stay true. The trick is to make it not repeating only in the form.
[config]: { | ||
repeating: false, | ||
props: { | ||
tabular: false, |
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.
We also don't need this, because of the trick.
<Field name="objectCountGroup" tabular={false}> | ||
<Field name="objectCount" label="" /> | ||
</Field> | ||
</Field> |
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.
Here's the trick. We just want to render a field that displays the objectCount
from the first objectCountGroup
, without rendering anything for the intervening fields. This is how to do that:
<Field name="objectCount" subpath={["ns2:collectionobjects_common", "objectCountGroupList", "objectCountGroup", "0"]} />
The subpath
prop refers to the path under the enclosing field (in this case, <Field name="document">
) in which to find this field. It effectively specifies intervening fields that you want to skip over rendering.
What does this do?
Revert the display for objectCount to numberOfObjects
Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1405
For material order, object count is aligned with library holdings and as such is better thought of with the pre-8.0 numberOfObjects.
How should this be tested? Do these changes have associated tests?
Number of objects
under object nameNumber of objects
can save/loadDependencies for merging? Releasing to production?
Since this is relabeling the objectCount field, it might be good to update other messages so that search and export are consistent.
Has the application documentation been updated for these changes?
No, but I believe the chanelog will need be updated to reflect this.
Did someone actually run this code to verify it works?
@mikejritter tested locally