QueryLibrary model and reducer #1239
Conversation
Signed-off-by: Mason Fish <mason@looky.cloud>
mason-fish
force-pushed
the
1198-query_lib_reducer
branch
from
96a1412
to
a086573
Compare
|
|
||
| export default { | ||
| getRaw: (state: State): QueryLibraryState => state.queryLibrary | ||
| } |
There was a problem hiding this comment.
We may want more selectors here, and will likely return the library as a parsed tree model in one of them. For now I am only returning the raw state and the tests are all built off of that, and as I am wiring this up to the components that consume this data then I may add more
src/js/state/QueryLibrary/initial.ts
Outdated
| const init = (): QueryLibraryState => ({ | ||
| id: "root", | ||
| name: "root", | ||
| items: [] |
There was a problem hiding this comment.
this will actually be populated by #1201
src/js/state/QueryLibrary/reducer.ts
Outdated
| // cause an off by one issue since the destination index will be affected after | ||
| // removal (e.g. an item cannot be moved to the end of its current group because of this). | ||
| // For this situation we instead remove the item first, and then insert its copy | ||
| if (srcItemPath.length === destItemPath.length) { |
There was a problem hiding this comment.
Is this just comparing the depths? Should check to see if the two paths (minus the last item) are equal?
There was a problem hiding this comment.
Oh thank you, yes this is not enough as is. will fix
There was a problem hiding this comment.
actually, this does work as is: removing before or after only matters if it IS the same group. But I still prefer the handling to be only during the special case so am adding an isEqual check in the next commit
src/js/state/QueryLibrary/types.ts
Outdated
| export interface Query { | ||
| id: string | ||
| name: string | ||
| value: string |
There was a problem hiding this comment.
What do you think about naming this field zql instead of value?
There was a problem hiding this comment.
Sure, that works
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
src/js/state/QueryLibrary/reducer.ts
Outdated
| @@ -71,7 +71,7 @@ const moveItem = ( | |||
| // cause an off by one issue since the destination index will be affected after | |||
| // removal (e.g. an item cannot be moved to the end of its current group because of this). | |||
| // For this situation we instead remove the item first, and then insert its copy | |||
| if (srcItemPath.length === destItemPath.length) { | |||
| if (isEqual(initial(srcItemPath), initial(destItemPath))) { | |||
There was a problem hiding this comment.
I didn't know about that "initial" function. Very cool
|
@jameskerr I agree qlib is a very insider term. I'm only using it in code for var names and for the action strings, and never show it to users. I shortened it to save some typing and I think it's mostly used within folders that have the name expanded so the context for future devs should be clear, but if you feel strongly I can change |
|
Thanks for your comment Mason. You know, it appears I do feel strongly enough to request the change to be QUERIES. I think the top-level folder in the state should also match and just be "queries". Thanks! |
Signed-off-by: Mason Fish <mason@looky.cloud>
|
Thanks for making that name change for me! |
fixes #1198
Adds a new QueryLibrary reducer. This code is all additive and is not yet being used, though it will be once we link together the remaining ui work tracked in #1199
The underlying data structures we are using to represent this lib are simply an object tree and arrays. The arrays provide the order which we need while also keep the structure easily serializable into JSON, and the tree structure provides the grouping hierarchy which we will want to use later on. The issue/inefficiency of re copying elements on array resize/reorder/remove is moot since we are using redux, which depends on data being immutable and when changes occur they must be done by creating new resources anyway.
Signed-off-by: Mason Fish mason@looky.cloud