DRYD-1330: Updates to new claim #224
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #224 +/- ##
=======================================
Coverage 98.18% 98.18%
=======================================
Files 590 590
Lines 12679 12679
Branches 2593 2593
=======================================
Hits 12449 12449
Misses 227 227
Partials 3 3 ☔ View full report in Codecov by Sentry. |
| id: 'field.repatriationclaims_common.status.fullName', | ||
| defaultMessage: 'Claim status state', | ||
| }, | ||
| name: { | ||
| id: 'field.nagpraclaims_common.status.name', | ||
| id: 'field.repatriationclaims_common.status.name', | ||
| defaultMessage: 'State', |
There was a problem hiding this comment.
One thing I realized I left out was updating the message for the status field. Since the group message is Claim status I originally went with Claim status state, which Kristina mentioned feels awkward compared to the Claim documentation status below. I'm not sure if just Claim status/Status would be ok or if we should switch to Value. Just a minor thing and I believe it's the last thing to sort out.
There was a problem hiding this comment.
Maybe Description/Claim status description?
There was a problem hiding this comment.
I ended up going with Value; we'll see how it goes. It feels a little awkward but feels ok when referring to the full name of claim status value.
| id: 'field.repatriationclaims_common.status.fullName', | ||
| defaultMessage: 'Claim status state', | ||
| }, | ||
| name: { | ||
| id: 'field.nagpraclaims_common.status.name', | ||
| id: 'field.repatriationclaims_common.status.name', | ||
| defaultMessage: 'State', |
There was a problem hiding this comment.
Maybe Description/Claim status description?
| [config]: { | ||
| messages: defineMessages({ | ||
| name: { | ||
| id: 'field.nagpraclaims_common.nagpraDocumentationGroup.name', | ||
| fullName: { |
There was a problem hiding this comment.
I think this should just be name.
There was a problem hiding this comment.
@ray-lee I was just testing everything and realized I switched these to fullName to prevent the group label from displaying because it's the same as the panel.
Maybe it shouldn't have a defined message at all in this case?
There was a problem hiding this comment.
Ah, that makes sense. In that case just having fullName is fine, so that it won't appear on the record editor, but will appear when selecting the group for search on the search page. This is how Textual Inscription and Non-textual Inscription work on collectionobjects.
| name: { | ||
| id: 'field.nagpraclaims_common.nagpraDocumentationGroup.name', | ||
| fullName: { | ||
| id: 'field.repatriationclaims_common.documentationGroup.fullName', |
What does this do?
Why are we doing this? (with JIRA link)
Jira: https://collectionspace.atlassian.net/browse/DRYD-1330
These are updates from data QA done for the claim procedure. It renames the nagpraclaim procedure as well as various fields so that they can be applied more generically.
A few fields were also meant to be controlled and have been updated to be TermPickerInputs - claimType, statusGroup, and documentationGroup. These are controlled by the claimtype and deaccessionapprovalgroup term lists.
How should this be tested? Do these changes have associated tests?
npm run lintandnpm run testas a sanity checkDependencies for merging? Releasing to production?
None
Has the application documentation been updated for these changes?
No
Did someone actually run this code to verify it works?
@mikejritter tested against the devserver