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

Replace default string representation of Iterable fields with pipe-delimited format for some Accepts types #380

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

alexdunnjpl
Copy link
Contributor

affects application/csv, application/kvp+json, and text/csv return types fixes #375

🗒️ Summary

Previously, array-like properties would be formatted [a, collection, of, elements]
Now, they are formatted a|collection|of|elements, per @tloubrieu-jpl request in #375

⚙️ Test Data and/or Report

Manually tested

♻️ Related Issues

fixes #375

@alexdunnjpl alexdunnjpl force-pushed the 375-csv-array-format branch 2 times, most recently from 55cc717 to 9e18908 Compare September 26, 2023 19:13
…limited format

affects application/csv, application/kvp+json, and text/csv return types
fixes #375
@alexdunnjpl
Copy link
Contributor Author

Two branch tests failures

  1. AssertionError C2488912 Collection DOI is [10.17189/1517568]
    expected '10.17189/1517568' to equal '[10.17189/1517568]'
    at assertion:3 in test-script
    inside "cookbook examples / search for collection of an observtional product, kvp response"

  2. AssertionError C2488844 Response contains same number of properties
    expected 141 to deeply equal 140
    at assertion:2 in test-script
    inside "other tickets / Product summary object has an incomplete "properties" set #277 /properties endpoint"

Error 1 has a PR open. Error 2 appears to be unrelated to this PR and is probably the result of changes to test data.

@alexdunnjpl
Copy link
Contributor Author

@tloubrieu-jpl @al-niessner could I trouble y'all for a review when you get a chance?

Copy link
Member

@tloubrieu-jpl tloubrieu-jpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alexdunnjpl , that sounds simple enough

@alexdunnjpl alexdunnjpl merged commit b06b648 into main Oct 9, 2023
1 check failed
@alexdunnjpl alexdunnjpl deleted the 375-csv-array-format branch October 9, 2023 17:09
@tloubrieu-jpl tloubrieu-jpl changed the title replace default string representation of Iterable fields with pipe-delimited format for some Accepts types Replace default string representation of Iterable fields with pipe-delimited format for some Accepts types Oct 9, 2023
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.

text/csv format is impacted by the repairkit script (apparently)
2 participants