NCIOCPL / glossary-api

API for Dictionary of Cancer Terms, Dictionary of Genetics Terms, and other Glossary documents.
0 stars 5 forks source link

Change includedFields to includeAdditionalInfo on Search and Expand endpoints #132

Closed bryanpizzillo closed 3 years ago

bryanpizzillo commented 3 years ago

Issue description

Devs on the team are really confused about how to use the includedFields endpoint. Additionally, there are certain fields that we force anyway (non-nullable like audience), this is more confusing because the shape of the response does not change, just the data in that response. For example if there are 10 media items, when not specifying the includedFields, you get media: [] back making seem like there is no media for that item.

What would be easier is to just have a flag that indicates that you want this additional information (relatedResources and media) returned back with a search or expand response. (I don't want to call it includeRelatedInfo because it will be confused as to if it only includes relatedResources.)

ESTIMATE NN

What's the expected change?

-

What's the current functionality?

-

What's the updated acceptance criteria?

Additional details / screenshot

Related Tickets

blairlearn commented 3 years ago

To be clear, the includeAdditionalInfo parameter is a replacement for includedFields? If there are any external users, this will be a breaking change. (Unlikely, since the API has not been publicized.)

bryanpizzillo commented 3 years ago

To be clear, the includeAdditionalInfo parameter is a replacement for includedFields? If there are any external users, this will be a breaking change. (Unlikely, since the API has not been publicized.)

Yep. Let's assume there are no external users yet. This was a bad parameter and it overly complicates the code as well. Once the API is promoted, etc. then we can worry about multiple versions. (and deprecation schedules, etc)