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

feat: dataset-level disabling of data type querying by count #272

Closed
wants to merge 21 commits into from

Conversation

noctillion
Copy link
Contributor

@noctillion noctillion commented Jul 19, 2023

This PR introduces the handle of the data type-disabling by dataset.
Related to Feature #1675

@noctillion noctillion marked this pull request as ready for review July 19, 2023 18:07
src/components/manager/projects/RoutedProject.js Outdated Show resolved Hide resolved
src/components/manager/projects/RoutedProject.js Outdated Show resolved Hide resolved
src/modules/services/reducers.js Outdated Show resolved Hide resolved
src/modules/services/reducers.js Outdated Show resolved Hide resolved
src/modules/services/reducers.js Outdated Show resolved Hide resolved
src/utils/actions.js Outdated Show resolved Hide resolved
@davidlougheed davidlougheed changed the title Features/ds-level-data-type-disabling feat: dataset-level disabling of data type querying by count Jul 19, 2023
...state,
itemsByDatasetID: {
...state.itemsByDatasetID,
[datasetID]: [...existingItems, ...data],
Copy link
Member

Choose a reason for hiding this comment

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

this should replace, not combine I think - otherwise this could append forever

Copy link
Member

Choose a reason for hiding this comment

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

also, this has a different schema for itemsByDatasetID.datasetId items than the below redux error handling - which one is right?

Copy link
Member

Choose a reason for hiding this comment

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

as we discussed before, this action is not idempotent - meaning it doesn't give the same result if it is called again. a way to fix this would be to further break this down by either data type ID (nested object) and then optionally also create a flattened version but from this object so there arent duplicate values.

src/components/discovery/DiscoveryQueryBuilder.js Outdated Show resolved Hide resolved
src/components/discovery/DiscoveryQueryBuilder.js Outdated Show resolved Hide resolved
src/utils/actions.js Outdated Show resolved Hide resolved
...state,
itemsByDatasetID: {
...state.itemsByDatasetID,
[datasetID]: [...existingItems, ...data],
Copy link
Member

Choose a reason for hiding this comment

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

as we discussed before, this action is not idempotent - meaning it doesn't give the same result if it is called again. a way to fix this would be to further break this down by either data type ID (nested object) and then optionally also create a flattened version but from this object so there arent duplicate values.

@noctillion
Copy link
Contributor Author

Replaced by Features/table-removal-dataset-level-disable #283. Closed by refactoring for V13.

@noctillion noctillion closed this Aug 25, 2023
@davidlougheed davidlougheed deleted the features/ds-level-data-type-disabling branch October 31, 2024 14:49
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