jbdemonte / mongoose-elasticsearch-xp

A mongoose plugin that indexes models into Elasticsearch 2 / 5 / 6 and 7
93 stars 34 forks source link

Added discriminators support #47

Closed borodayev closed 6 years ago

coveralls commented 6 years ago

Coverage Status

Coverage decreased (-0.3%) to 91.898% when pulling 36037c509ae59d4f14ba5df09f3f76b79ca09177 on FrankAst:master into ca0c7d9dfd52bbde9cfabb3594089e788aa94377 on jbdemonte:master.

borodayev commented 6 years ago

Ok, I will consider your notices, @Pavel nodkz@mail.ru , and will change it today's evening.

вс, 26 авг. 2018 г. в 21:50, Pavel Chertorogov notifications@github.com:

@nodkz requested changes on this pull request.

In lib/index.js https://github.com/jbdemonte/mongoose-elasticsearch-xp/pull/47#discussion_r212830483 :

/**

  • Retrieve model options to ElasticSearch
  • static function
  • returns {Object} */

Remove line. Docs should be defined without space.

In lib/index.js https://github.com/jbdemonte/mongoose-elasticsearch-xp/pull/47#discussion_r212830506 :

function esOptions() { if (!options.index) { options.index = this.collection.name; }

  • if (!options.type) {
  • const kind = this.constructor.modelName || this.modelName;

kind should be defined inside if

In lib/index.js https://github.com/jbdemonte/mongoose-elasticsearch-xp/pull/47#discussion_r212830543 :

function esOptions() { if (!options.index) { options.index = this.collection.name; }

  • if (!options.type) {
  • const kind = this.constructor.modelName || this.modelName;

May be generateType also should be inside your if?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jbdemonte/mongoose-elasticsearch-xp/pull/47#pullrequestreview-149542529, or mute the thread https://github.com/notifications/unsubscribe-auth/ASbLTbHzDlg85a3IFr86fYF4DHmnwRBWks5uUsPFgaJpZM4WM0GD .

-- Kind regards, Valeriy Borodayev

nodkz commented 6 years ago

@FrankAst How do you think, if we remove withDiscriminators option?

If we using discriminators then we need to provide type as a function. And I think it will be enough. Or I'm missing something?

nodkz commented 6 years ago

Awesome, thanks!

nodkz commented 6 years ago

@jbdemonte please update your NPM_TOKEN in TravisCI. All tokens was revoked in July due security incident with eslint package. 1) Login to npmjs.com and open following page https://www.npmjs.com/settings/jbdemonte/tokens 2) Generate a new token and copy it 3) Open settings in TravisCI https://travis-ci.org/jbdemonte/mongoose-elasticsearch-xp/settings 4) Remove old Environment Variable NPM_TOKEN and add a new one with the same name and new token. 5) And then re-run build for publishing a new package with discriminators support https://travis-ci.org/jbdemonte/mongoose-elasticsearch-xp/jobs/425205400

Thanks.

nodkz commented 6 years ago

@jbdemonte sorry for the noise. I think that I do not have access to change token on TravisCI. But I have. So I did all myself - update token and publish a new version of the package.

Thanks.