Batch insert support

[mdmitry01]

Closes https://github.com/feathersjs-ecosystem/feathers-objection/issues/125

[dekelev]

Hi @mdmitry01 , Thanks for contributing and sorry for the delay. There was another PR that was merged since you opened this one and it created some code conflicts. Can you please resolve them and I'll review the PR?

[mdmitry01]

Hi @dekelev! The conflicts have been resolved.

[dekelev]

Thanks, I'll check it out

בתאריך יום א׳, 8 בנוב׳ 2020, 13:09, מאת Dmitry Merkulov ‏< notifications@github.com>:

Hi @dekelev https://github.com/dekelev! The conflicts have been resolved.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/feathersjs-ecosystem/feathers-objection/pull/127#issuecomment-723561921, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB5E3MPY6RIX25EZR42B5DSOZ37PANCNFSM4S4WFDPQ .

[dekelev]

PR looks very promising! See if you can add tests for the parts that are missing code-coverage: https://coveralls.io/builds/34819739/source?filename=src/index.js

[mdmitry01]

• Lines 638 and 654 were added only for backward compatibility with this line. As far as I understand, this line wasn't covered before. And I can't think of any test case to cover it. Is this line supposed to be covered?

• To cover lines 647, 648 and 693, I need PostgreSQL (or another DB with .returning() support). I can see some tests for PostgreSQL, but I can't see any PostgreSQL config for the tests.

• Line 700 is now covered

[dekelev]

Thank you!

I'm running these PostgreSQL tests with a local PostgreSQL Docker when there are code changes that might impact PostgreSQL.

בתאריך יום ב׳, 9 בנוב׳ 2020, 19:08, מאת Dmitry Merkulov ‏< notifications@github.com>:

-

Lines 638 https://coveralls.io/builds/34819739/source?filename=src/index.js#L638 and 654 https://coveralls.io/builds/34819739/source?filename=src/index.js#L654 were added only for backward compatibility with this line https://github.com/feathersjs-ecosystem/feathers-objection/blob/master/src/index.js#L659. As far as I understand, this line wasn't covered before. And I can't think of any test case to cover it. Is this line supposed to be covered?

To cover lines 647 https://coveralls.io/builds/34819739/source?filename=src/index.js#L647, 648 https://coveralls.io/builds/34819739/source?filename=src/index.js#L648 and 693 https://coveralls.io/builds/34819739/source?filename=src/index.js#L693, I need PostgreSQL (or another DB with .returning() support). I can see some tests for PostgreSQL, but I can't see any PostgreSQL config for the tests.

Line 700 https://coveralls.io/builds/34819739/source?filename=src/index.js#L700 is now covered

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/feathersjs-ecosystem/feathers-objection/pull/127#issuecomment-724147262, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB5E3NYM2I66ATD5VGLJOTSPAOYPANCNFSM4S4WFDPQ .

[mdmitry01]

Ok, I just ran the tests locally with PostgreSQL. Lines 647, 648 and 693 are covered when using PostgreSQL.

[dekelev]

Great work! I'll merge the PR tomorrow.

בתאריך יום ב׳, 9 בנוב׳ 2020, 22:00, מאת Dmitry Merkulov ‏< notifications@github.com>:

Ok, I just ran the tests locally with PostgreSQL. Lines 647, 648 and 693 are covered when using PostgreSQL.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/feathersjs-ecosystem/feathers-objection/pull/127#issuecomment-724245206, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABB5E3MJU5MCCQ6AMUBGUNTSPBC5HANCNFSM4S4WFDPQ .

[mdmitry01]

Thank you!

[mdmitry01]

Added a fix

[dekelev]

Released with v6.3.0