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

Revert display for objectCount #14

Merged
merged 4 commits into from
Mar 20, 2024

Conversation

mikejritter
Copy link
Contributor

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?

  • Run the devserver
  • See Number of objects under object name
  • Verify that Number of objects can save/load

Dependencies 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

Copy link

@ray-lee ray-lee left a 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',
Copy link

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,
Copy link

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,
Copy link

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>
Copy link

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.

@ray-lee ray-lee merged commit 6159dd1 into collectionspace:master Mar 20, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants