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

[BUG] [Data Explorer] Modify columns on switch #5268

Closed
abbyhu2000 opened this issue Oct 10, 2023 · 7 comments
Closed

[BUG] [Data Explorer] Modify columns on switch #5268

abbyhu2000 opened this issue Oct 10, 2023 · 7 comments
Assignees
Labels
bug Something isn't working data explorer Issues related to the Data Explorer project discover for discover reinvent v2.12.0

Comments

@abbyhu2000
Copy link
Member

Describe the bug
Advanced setting discover:modifyColumnsOnSwitch does not work for the new data explorer.

To Reproduce

Screen.Recording.2023-10-10.at.12.31.03.PM.mov

Expected behavior
When the setting discover:modifyColumnsOnSwitch is turned off, the columns from the previous index pattern should not be automatically cleared.

Screen.Recording.2023-10-10.at.12.33.58.PM.mov
@abbyhu2000 abbyhu2000 added bug Something isn't working untriaged de-angular de-angularize work data explorer Issues related to the Data Explorer project labels Oct 10, 2023
@abbyhu2000 abbyhu2000 changed the title [BUG] Modify columns on switch [BUG] [Data Explorer] Modify columns on switch Oct 10, 2023
@ananzh ananzh added v2.12.0 and removed untriaged labels Oct 10, 2023
@ananzh ananzh self-assigned this Oct 10, 2023
@wbeckler
Copy link

wbeckler commented Nov 3, 2023

What is the purpose of this switch?

@MadaniKK
Copy link
Contributor

May I work on this issue? Ty.

@MadaniKK
Copy link
Contributor

MadaniKK commented Nov 16, 2023

The workflow of how the fields are updated in the left side nav:

in the use effect, when a new indexPattern is selected, all the variables (fields, selected, unpopular, popular) used for rendering the elements in the left sidebar will get updated

const {
selected: selectedFields,
popular: popularFields,
unpopular: unpopularFields,
} = useMemo(() => groupFields(fields, columns, popularLimit, fieldCounts, fieldFilterState), [
fields,
columns,
popularLimit,
fieldCounts,
fieldFilterState,
]);

we care most about the state of fields and columns(columns refers to the selected fields) because if MODIFY_COLUMN_ON_SWITCH is off, we want them to keep the fields & selected columns from the previously chosen data sample(indexPattern)

The state of fields gets modified here: function'getIndexPatternFieldList generates fields`

useEffect(() => {
const newFields = getIndexPatternFieldList(selectedIndexPattern, fieldCounts);
setFields(newFields);
}, [selectedIndexPattern, fieldCounts, hits, services]);

The state of columns gets modified here everytime the indexPattern is changed to a new one:

const { columns } = useSelector((state) => state.discover);
const filteredColumns = filterColumns(
columns,
indexPattern,
uiSettings.get(DEFAULT_COLUMNS_SETTING)
);
const dispatch = useDispatch();
const prevIndexPattern = useRef(indexPattern);

useEffect(() => {
if (indexPattern !== prevIndexPattern.current) {
dispatch(setColumns({ columns: filteredColumns }));
prevIndexPattern.current = indexPattern;
}
}, [dispatch, filteredColumns, indexPattern]);

Proposed solution

If we want to implement MODIFY_COLUMNS_ON_SWITCH, we just need to add in conditioning in the function'getIndexPatternFieldList generates fieldsand the functionfilterColumns` to keep previous column and fields

@ananzh ananzh added discover for discover reinvent and removed de-angular de-angularize work labels Nov 17, 2023
@ananzh
Copy link
Member

ananzh commented Nov 17, 2023

What is the purpose of this switch?

One user story I could think about is that user has some similar index pattern, with most fields same but 1-2 key fields diff. When they nav from one index pattern to another one, they would like to keep all fields even though some of them will be grey. So they could know what index pattern they are using without distinguishing the index pattern name.

@ananzh
Copy link
Member

ananzh commented Nov 17, 2023

@MadaniKK since we have go through the workflow today, could you help us add a clear summary on

  • how index pattern is changed in Discover
  • how does discover.state.columns update/refresh when index pattern is change

Thanks~ ❤️

@MadaniKK
Copy link
Contributor

Updated the workflow

@ananzh
Copy link
Member

ananzh commented Dec 22, 2023

closed it as it is resolved in #5508

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working data explorer Issues related to the Data Explorer project discover for discover reinvent v2.12.0
Projects
None yet
Development

No branches or pull requests

4 participants