Add a `isUniqueValuesLoading` key to columns in the vuex state #499

zegonz · 2020-04-07T13:37:29Z

state.dataset.headers contains elements like:
Before: {name: 'col1', type: 'number'}
Now: {name: 'col1', type: 'number', isUniqueValuesLoading: false}
Also, add the corresponding getter and mutation

Please read commit by commit.

NB:
This PR also fix story of DataViewer and remove the broken story of RenameStep. An issue has been written to create stories for the form steps: #501

codecov · 2020-04-07T14:07:42Z

Codecov Report

Merging #499 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #499   +/-   ##
=======================================
  Coverage   97.77%   97.78%           
=======================================
  Files         128      128           
  Lines        2925     2933    +8     
  Branches      410      410           
=======================================
+ Hits         2860     2868    +8     
  Misses         65       65

Impacted Files	Coverage Δ
src/components/ListUniqueValues.vue	`100.00% <100.00%> (ø)`
src/store/getters.ts	`95.12% <100.00%> (+0.25%)`	⬆️
src/store/mutations.ts	`96.73% <100.00%> (+0.14%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9382df...d24ac3c. Read the comment docs.

zegonz · 2020-04-07T14:16:07Z

(5) ETQDev le state "loading" n'est plus un boolean mais un "Enum(None, DataSet, UniqueValues)"

davinov · 2020-04-07T15:30:23Z

I'm not that sure it's such a good idea to introduce an enumeration here.
Why not simply add a "areUniqueValuesLoading" boolean?

rhuille · 2020-04-07T15:52:37Z

The idea is to limit the number of state properties and to keep inside the name loading the loading state.

adimascio · 2020-04-14T12:38:42Z

src/store/mutations.ts

+    state.dataset.headers[
+      state.dataset.headers.map(col => col.name).indexOf(column)
+    ].isUniqueValuesLoading = isLoading;
+  }


Suggested change

state.dataset.headers[

state.dataset.headers.map(col => col.name).indexOf(column)

].isUniqueValuesLoading = isLoading;

}

state.dataset.headers.find(hdr => hdr.name === column).isUniqueValuesLoading = isLoading;

src/lib/dataset/index.ts

adimascio · 2020-04-14T12:42:43Z

src/store/getters.ts

+   * helper that is True if unique values are loading
+   */
+  isUniqueValuesLoading: (state: VQBState) => (column: string) =>
+    state.dataset.headers[state.dataset.headers.map(col => col.name).indexOf(column)]


Suggested change

state.dataset.headers[state.dataset.headers.map(col => col.name).indexOf(column)]

state.dataset.headers.find(hdr => hdr.name === column).isUniqueValuesLoading

zegonz · 2020-04-15T14:22:47Z

@adimasci I push changes you ask for, can you have a look please ?

src/store/backend-plugin.ts

adimascio · 2020-04-19T19:51:33Z

tests/unit/plugins.spec.ts

+    ]);
+    const wrapper = mount(PipelineComponent, { store, localVue });
+    wrapper.find('.query-pipeline-queue__dot').trigger('click');
+    await flushPromises();


Genuine question: is there a reason why $nextTick() would not work here? Idem for other flushPromises() occurrences?

`state.dataset.headers` contains elements like: Before: `{name: 'col1', type: 'number'}` Now: `{name: 'col1', type: 'number', isUniqueValuesLoading: false}` Also, add the corresponding getter and mutation

adimascio · 2020-04-20T18:57:20Z

src/store/getters.ts

+   * helper that is True if unique values are loading
+   */
+  isUniqueValuesLoading: (state: VQBState) => (column: string) =>
+    (state.dataset.headers.find(hdr => hdr.name === column) as DataSetColumn).isUniqueValuesLoading,


I'm surprised that the explicit type annotation is necessary 🤔

The column names are not explicitly listed for typescript. Consequently, the .find() method may return an undefined value...

adimascio · 2020-04-20T18:58:37Z

tests/unit/list-unique-values.spec.ts

+  const shallowMountWrapper = (
+    filterValue: string[] = ['France', 'Spain'],
+    operator: 'in' | 'nin' = 'in',
+    // eslint-disable-next-line @typescript-eslint/no-inferrable-types


adimascio

Thanks for this work!

I've added 2 questions but I would be ok to merge the code as is.

- Add a spinner when colunm unique values are loading - Emit an event at click on "Load All Values" button

It must register `vqb` module and not setup directly the store This fix allow us make the storybook of ListUniqueValues

form-renam-step is removed because it was not working since a long time, implementation of this step has changed a lot

zegonz added enhancement New feature or request need review labels Apr 7, 2020

zegonz force-pushed the f/change-isloading-state branch 2 times, most recently from 7723379 to ca9872d Compare April 7, 2020 14:04

davinov changed the title ~~change isloading state~~ Multiple "loading" states Apr 7, 2020

rhuille force-pushed the f/change-isloading-state branch from ca9872d to ab4db9f Compare April 8, 2020 08:36

zegonz force-pushed the f/change-isloading-state branch from ab4db9f to bdbb3a1 Compare April 9, 2020 15:32

rhuille force-pushed the f/change-isloading-state branch from bdbb3a1 to 2184c98 Compare April 9, 2020 16:06

zegonz added 🔨 wip 🔨 and removed need review labels Apr 10, 2020

zegonz changed the title ~~Multiple "loading" states~~ Add "isUniqueValuesLoading" states Apr 10, 2020

rhuille force-pushed the f/change-isloading-state branch 4 times, most recently from 46b80db to 09b18d0 Compare April 10, 2020 12:37

zegonz force-pushed the f/change-isloading-state branch from 09b18d0 to 4b06e07 Compare April 10, 2020 14:18

rhuille force-pushed the f/change-isloading-state branch 2 times, most recently from c3d7d57 to fd991cb Compare April 10, 2020 14:38

rhuille changed the title ~~Add "isUniqueValuesLoading" states~~ @rhuille Add a isUniqueValuesLoading key to columns in the vuex state Apr 10, 2020

rhuille changed the title ~~@rhuille Add a isUniqueValuesLoading key to columns in the vuex state~~ Add a isUniqueValuesLoading key to columns in the vuex state Apr 10, 2020

rhuille added need review and removed 🔨 wip 🔨 labels Apr 10, 2020

rhuille force-pushed the f/change-isloading-state branch from fd991cb to 63ab186 Compare April 10, 2020 14:58

adimascio reviewed Apr 14, 2020

View reviewed changes

src/lib/dataset/index.ts Outdated Show resolved Hide resolved

adimascio reviewed Apr 14, 2020

View reviewed changes

zegonz force-pushed the f/change-isloading-state branch from 4cde97d to 03ea30b Compare April 15, 2020 13:41

adimascio reviewed Apr 19, 2020

View reviewed changes

src/store/backend-plugin.ts Outdated Show resolved Hide resolved

adimascio reviewed Apr 19, 2020

View reviewed changes

rhuille force-pushed the f/change-isloading-state branch 2 times, most recently from afbd51e to 96ce88a Compare April 20, 2020 14:46

Add a isUniqueValuesLoading key to columns in the vuex state

beeb123

`state.dataset.headers` contains elements like: Before: `{name: 'col1', type: 'number'}` Now: `{name: 'col1', type: 'number', isUniqueValuesLoading: false}` Also, add the corresponding getter and mutation

rhuille force-pushed the f/change-isloading-state branch from 96ce88a to a2926c7 Compare April 20, 2020 15:08

adimascio reviewed Apr 20, 2020

View reviewed changes

adimascio approved these changes Apr 20, 2020

View reviewed changes

raphael and others added 4 commits April 21, 2020 09:17

Update ListUniqueValues component

0247ad4

- Add a spinner when colunm unique values are loading - Emit an event at click on "Load All Values" button

Fix setup of store in Storybook

afdaaef

It must register `vqb` module and not setup directly the store This fix allow us make the storybook of ListUniqueValues

Fix story dataviewer setup store and remove form rename step story

f003e7f

form-renam-step is removed because it was not working since a long time, implementation of this step has changed a lot

Update CHANGELOG

d24ac3c

rhuille force-pushed the f/change-isloading-state branch from a2926c7 to d24ac3c Compare April 21, 2020 07:17

rhuille merged commit 455b22f into master Apr 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a `isUniqueValuesLoading` key to columns in the vuex state #499

Add a `isUniqueValuesLoading` key to columns in the vuex state #499

zegonz commented Apr 7, 2020 •

edited by rhuille

Loading

codecov bot commented Apr 7, 2020 •

edited

Loading

zegonz commented Apr 7, 2020

davinov commented Apr 7, 2020

rhuille commented Apr 7, 2020

adimascio Apr 14, 2020

adimascio Apr 14, 2020

zegonz commented Apr 15, 2020

adimascio Apr 19, 2020 •

edited

Loading

adimascio Apr 20, 2020 •

edited

Loading

rhuille Apr 21, 2020

adimascio Apr 20, 2020

adimascio left a comment

	state.dataset.headers[state.dataset.headers.map(col => col.name).indexOf(column)]
	state.dataset.headers.find(hdr => hdr.name === column).isUniqueValuesLoading

Add a isUniqueValuesLoading key to columns in the vuex state #499

Add a isUniqueValuesLoading key to columns in the vuex state #499

Conversation

zegonz commented Apr 7, 2020 • edited by rhuille Loading

codecov bot commented Apr 7, 2020 • edited Loading

Codecov Report

zegonz commented Apr 7, 2020

davinov commented Apr 7, 2020

rhuille commented Apr 7, 2020

adimascio Apr 14, 2020

Choose a reason for hiding this comment

adimascio Apr 14, 2020

Choose a reason for hiding this comment

zegonz commented Apr 15, 2020

adimascio Apr 19, 2020 • edited Loading

Choose a reason for hiding this comment

adimascio Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

rhuille Apr 21, 2020

Choose a reason for hiding this comment

adimascio Apr 20, 2020

Choose a reason for hiding this comment

adimascio left a comment

Choose a reason for hiding this comment

Add a `isUniqueValuesLoading` key to columns in the vuex state #499

Add a `isUniqueValuesLoading` key to columns in the vuex state #499

zegonz commented Apr 7, 2020 •

edited by rhuille

Loading

codecov bot commented Apr 7, 2020 •

edited

Loading

adimascio Apr 19, 2020 •

edited

Loading

adimascio Apr 20, 2020 •

edited

Loading