fix(ui): sort out table state #15944
Conversation
| @@ -117,6 +111,7 @@ export const getViewForTimeMachine = ( | |||
| } | |||
|
|
|||
| dispatch(setActiveTimeMachine(timeMachineID, {view})) | |||
| dispatch(executeQueries(dashboardID)) | |||
There was a problem hiding this comment.
i intended this to mean "whenever a timemachine loads a view, run the queries to populate its data"
| @@ -142,6 +135,40 @@ export const initialState = (): TimeMachinesState => ({ | |||
| }, | |||
| }) | |||
|
|
|||
| const getTableProperties = (view, files) => { | |||
There was a problem hiding this comment.
this is mostly a csv parser for now. since the csv can be very large, we dont want to make a lot of copies of it in memory, so we move a pointer around in place and try to extract only the line that has the meta data in it that we want
| csv | ||
| .slice(pointer, csv.indexOf('\r\n', pointer)) | ||
| .split(',') | ||
| .filter(o => !existing.hasOwnProperty(o)) |
There was a problem hiding this comment.
now that we have a hashmap of the existing column names and a list of ones we got from... where ever.. we can forget about the existing ones and have a default fill for the new ones
| return {...state, view} | ||
| } | ||
|
|
||
| case 'UPDATE_FIELD_OPTION': { |
There was a problem hiding this comment.
this seemed less heavy handed than replacing the entire array... as a side effect, it should be more resilient to data inconsistencies across the app. imagine one component updating the 3rd element of the array, while another one swaps the 3rd and 6th element.
|
I forgot that I can't effectively test this without goller's changes to the db flush mechanism as the meat of this code is dependent on transforming data from the resolution of backend calls, so.... i promise to write them later? 🤷♂ |
drdelambre
changed the title
| @@ -71,7 +71,7 @@ describe('Dashboards.Actions.getViewForTimeMachine', () => { | |||
| ) | |||
|
|
|||
| expect(mocked(getView)).toHaveBeenCalledTimes(1) | |||
| expect(mockedDispatch).toHaveBeenCalledTimes(2) | |||
| expect(mockedDispatch).toHaveBeenCalledTimes(3) | |||
There was a problem hiding this comment.
because now we populate the queries
Closes #15889
Right now there are multiple places within the app that are trying to convert the fields coming back from a query. All of these attempts tried to merge the multiple sources of truth (in turn becoming their own source of truth) from with the view layer. This has the drawback of being lost or needing replication when application state is required in a different part of the app, and resulting in a state that is hard to reason about. From the blame log, it looks like there have been a handful of bugs over the past 10mo or so circling this issue.
In this PR, I pull that logic closer towards its storage (into the reducer layer) to ensure that it is decoupled from any view, and is applied on any future transformation of the data. This simplified things a bit. All of this code still desperately needs to be normalized (see: https://egghead.io/lessons/javascript-redux-normalizing-the-state-shape) to reduce pass-by-reference errors and simplify the object assembly (in which most of this code is shuffled into a selector that builds the correct object instead of deciphering how to convert objects), but i wanted to shake this out in production for awhile to make sure I didn't introduce more bugs first.