fix(gridService): addItem/updatedItemById must pass an array to setSelectedRows

[ljacques]

replace setSelectedRows to setSelectedRow

[codecov-io]

Codecov Report

Merging #308 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #308   +/-   ##
=======================================
  Coverage   97.28%   97.28%           
=======================================
  Files         135      135           
  Lines        7644     7644           
  Branches     2593     2594    +1     
=======================================
  Hits         7436     7436           
  Misses        208      208

Impacted Files	Coverage Δ
...modules/angular-slickgrid/services/grid.service.ts	100% <100%> (ø)	⬆️

---

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 2e90077...a2ddab2. Read the comment docs.

[ghiscoding]

Wow good catch, I never noticed the bug, setSelectedRows is the only method that SlickGrid (core) provides and yeah we always need to make sure to pass an array.

I think we need to modify some unit tests to make sure setSelectedRows is called with an array in both of the methods you fixed... Would you be able to modify the tests? or I can take a look if you want, just let me know. Thanks

[ljacques]

ok I will watch to change the tests as soon as I have some time ;-)

[ghiscoding]

I took a quick look at the tests I have and I think I forgot to add a test for the selected row when using the singular addItem but I have a test for the pluralized addItems with setSelectedRows. You can recopy the addItems plural test here

Below is basically what is missing in the test (there is more code than that, it's just to show how it works).

const selectSpy = jest.spyOn(gridStub, 'setSelectedRows');
service.addItem(mockItem, { selectRow: true });
expect(selectSpy).toHaveBeenCalledWith([0]);

[ljacques]

I d'ont know why but my last changes seem to affect a cypress test call
"should change "% Complete" filter range by using the slider left handle (min value) to make it a higher min value and expect all rows to be within new range" :(

[ljacques]

and now it's ok .... ^^

[ghiscoding]

I think that's the test that sometime fails (1/10) and re-running them works after. Why does your commit removes all spaces in objects? It's really hard to see what changed, I use VSCode, are you using another editor? I can hide white space changes in GitHub but even then, It changed format of the entire file, it's really hard to see what changed since the entire test file is touched lol.

[ljacques]

oups i'am using itellij and I mechanically apply automatic formatting.
wait i'am trying to rollback this.

[ghiscoding]

ahh that is much better, thanks 😃
I'm finishing up on Export to Excel new feature and I'll publish a new version in coming days, stay tuned

[ghiscoding]

@ljacques
C'est maintenant disponible sous la nouvelle version 2.12.0. Il y a une modification importante à faire pour le Prod Build, alors fait attention et lit au complet.

Merci pour les contributions 😃

[ljacques]

héhé merci c'est ok j'ai récupéré la release le Prod Build passe bien 😃
j'ai repéré quelques soucis sur les filtres basé sur des composants angular, je vais travailler la dessus 👍