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

EES-4856 Add csv preview, variables, and footnotes to GetDataSetFile #4865

Merged
merged 8 commits into from
May 24, 2024

Conversation

mmyoungman
Copy link
Collaborator

@mmyoungman mmyoungman commented May 16, 2024

This PR includes additional data in the response returned by DataSetFileService#GetDataSetFile:

  • Data set preview - The headers and first five rows of the data set
  • Variables - The column names and descriptions - basically the same as provided on data guidance pages
  • Footnotes - Any footnotes for the data set

To have these changes work with integration tests, Testcontainers.Azurite has been added as a dependency to the Content.Api.Tests project. This is used to locally spins up a docker container which are then used by integration tests.

The appsettings.IntegrationTest.json file has also been added to the Content.Api.Tests project. This was done to ensure that if someone is running the integration tests while they have the data-storage docker container running in the background, the tests don't pass due to reliance on the data-storage container.

I've tested the fetching of the first five rows of the data set CSV locally to ensure it's performance is acceptable, and it seemed fine. I've held off doing more exhaustive testing as it'll be easy for us to test this once it's deployed on Prod by visiting the currently hidden Data set details pages - and that'll be testing against current Prod data which seems superior anyway.

I've also moved DataSetFileMetaViewModel into DatSetFileFileViewModel, since the meta is related to a specific data set file, unlike footnotes, for example, that are linked to a specific data set file/release version combo.

@mmyoungman mmyoungman added the do not merge Don't merge just yet label May 20, 2024
@mmyoungman
Copy link
Collaborator Author

I've put a do not merge because I want to merge #4871 and resolve any conflicts here, rather than vice versa.

@mmyoungman mmyoungman force-pushed the EES-4856 branch 2 times, most recently from 619c01a to 0ccf518 Compare May 21, 2024 12:18
@mmyoungman mmyoungman removed the do not merge Don't merge just yet label May 23, 2024
@mmyoungman mmyoungman force-pushed the EES-4856 branch 2 times, most recently from 9087812 to 47ebbb6 Compare May 24, 2024 09:25
@mmyoungman mmyoungman merged commit 42fbf91 into dev May 24, 2024
3 of 7 checks passed
@mmyoungman mmyoungman deleted the EES-4856 branch May 24, 2024 15:39
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