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

Make it possible to hide fields from the table #3532

Merged
merged 4 commits into from
Jan 16, 2025
Merged

Make it possible to hide fields from the table #3532

merged 4 commits into from
Jan 16, 2025

Conversation

fhennig
Copy link
Contributor

@fhennig fhennig commented Jan 15, 2025

resolves #3325

preview URL: http://column-hiding.loculus.org

Summary

  • Adds a new metadata config setting: hideInSearchResultsTable which you can set to hide a column in the Column visibility dialog.

Screenshot

Nothing to see really.

PR Checklist

  • All necessary documentation has been adapted.
  • The implemented feature is covered by an appropriate test.

@fhennig fhennig added the preview Triggers a deployment to argocd label Jan 15, 2025
@fhennig fhennig self-assigned this Jan 15, 2025
@fhennig fhennig requested a review from theosanderson January 15, 2025 15:45
@fhennig fhennig marked this pull request as ready for review January 15, 2025 15:46
theosanderson
theosanderson previously approved these changes Jan 15, 2025
Copy link
Member

@theosanderson theosanderson left a comment

Choose a reason for hiding this comment

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

Looks very reasonable to me! If one wanted to test this one way would be:

  • add a field with this to e.g. test-organism in values.yaml
  • in the integration tests, assert that it is not displayed when you go and list the fields for this organism (and assert that some other field is, as a positive control)

But not demanding that

@fhennig
Copy link
Contributor Author

fhennig commented Jan 16, 2025

Thanks! I added a test, maybe you can have a look again?

@fhennig fhennig dismissed theosanderson’s stale review January 16, 2025 14:34

Added test, pls view again

Copy link
Member

@theosanderson theosanderson left a comment

Choose a reason for hiding this comment

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

Nice - formatting issue but otherwise good.

My inclination is that we should aim in say a year's time to have most of what is currently the "E2E" tests instead moved to the "integration" tests, so if I were adding this I'd have probably added it to the integration tests, but I appreciate we don't have e.g. a searchPage set up there so it's more work / less nice. We can probably move whole sets of tests over in due course.

@theosanderson
Copy link
Member

huh or maybe the format issue is unrelated?

@fhennig fhennig merged commit 16b07af into main Jan 16, 2025
20 checks passed
@fhennig fhennig deleted the column-hiding branch January 16, 2025 15:47
@fhennig
Copy link
Contributor Author

fhennig commented Jan 16, 2025

My inclination is that we should aim in say a year's time to have most of what is currently the "E2E" tests instead moved to the "integration" tests, so if I were adding this I'd have probably added it to the integration tests, but I appreciate we don't have e.g. a searchPage set up there so it's more work / less nice. We can probably move whole sets of tests over in due course.

Ah! I'll keep that in mind. I'd tend to agree actually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Triggers a deployment to argocd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Support making a field hidden from the column selection
2 participants