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

7.2 QA Updates #188

Merged
merged 5 commits into from
Aug 14, 2023
Merged

7.2 QA Updates #188

merged 5 commits into from
Aug 14, 2023

Conversation

mikejritter
Copy link
Contributor

What does this do?

  • Fixes some typos in chronology messages
  • Updates to chronology field names so that they're consistent with the ui names
  • Move the chronology fieldcollection authority to anthro
  • Remove the associated authority block so that it can be re-spec'd

Why are we doing this? (with JIRA link)

This brings in a few changes from QA which were identified in order to bring more consistency between the chronology field names and their default messages. In addition, the associated authorities block was seen as still needing more work before it's ready for production and as such has been removed from the chronology authority as well as having its vocabularies removed.

How should this be tested? Do these changes have associated tests?

  • Run the devserver with the application PR
  • Verify that the chronology fieldcollection authority is not available in core
  • Verify that the associated authorities do not display on chronology records
  • Verify that the chronology field names are updated
    • primaryDateRangeStructuredDateGroup
    • spatialCoverages + subfields
    • altDateGroupList + subfields

Dependencies for merging? Releasing to production?
I opted to leave the associatedAuthorities extension in as opposed to the application layer which removed it. I added a comment to note that it is currently unused, and can remove the extension if desired.

Actually the associatedAuthorities block is still referencing vocabularies which have now been removed, so I should probably remove those before merging.

Has the application documentation been updated for these changes?
No

Did someone actually run this code to verify it works?
@mikejritter tested against a local instance

@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (0c701ed) 96.58% compared to head (730ff98) 96.58%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
- Coverage   96.58%   96.58%   -0.01%     
==========================================
  Files         550      550              
  Lines       10936    10935       -1     
  Branches     1226     1226              
==========================================
- Hits        10563    10562       -1     
  Misses         38       38              
  Partials      335      335              
Files Changed Coverage Δ
src/plugins/recordTypes/chronology/fields.js 100.00% <ø> (ø)
...c/plugins/recordTypes/chronology/forms/default.jsx 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@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.

Looks good.

@ray-lee ray-lee merged commit 8518b00 into collectionspace:master Aug 14, 2023
3 checks passed
@mikejritter mikejritter deleted the 7.2-qa branch July 23, 2024 18:54
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