search by parameter 'q' does not work

[JSProto]

i have model and resource

const User = sequelize.define("User", {
      username: DataTypes.STRING
});

let userResource = epilogue.resource({
    model: db.User,
    endpoints: ['/users', '/users/:id']
});

I try to make a request GET /users?q=testuser to get one item. but I get the whole list.

think the mistake is here: https://github.com/dchester/epilogue/blob/master/lib/Controllers/list.js#L145

variable criteria has one key {[Symbol (or)]: [{username: [Object]}]} but Object.keys(criteria).length is 0 and the condition is not met and options.where is not written.

test example:

var s = Symbol.for('s'); 
var o = {}; o[s] = true;
console.log(Object.keys(o).length) // 0

[GabeAtWork]

I stumbled accross the same problem. It seems the whole search block is broken with Sequelize 4 (I'm on 4.13.4).

I've tracked the bug down to these lines: https://github.com/dchester/epilogue/blob/d46def3dfc7b9dd7b25303b124bb337499cc9f1f/lib/Controllers/list.js#L102-L105

It appears than nothing is returned by Sequelize.or.apply our Sequelize.and.apply anymore. I'm looking for ways to fix it.

Edit

(an hour later)

Alright, so @JSProto's assertion was right: symbols don't show up in object keys, meaning the Object.keys(criteria).length test in insufficient. I propose to test criteria presence like so :

Object.keys(criteria).length || (Object.getOwnPropertySymbols && Object.getOwnPropertySymbols(criteria).length)

I'll submit a PR.

[micksatana]

Similar fix may need to appy at this line? https://github.com/dchester/epilogue/blob/ce700a7b543fda7fa105643e3d6bae66b488c73d/lib/Controllers/read.js#L34

[JSProto]

and change this line. https://github.com/dchester/epilogue/blob/ce700a7b543fda7fa105643e3d6bae66b488c73d/lib/Controllers/read.js#L29

I think you need to add a method to util.keys as a replacement for Object.keys, which will return all the object keys as an array. to get the correct size of the object and to iterate these keys.

[GabeAtWork]

Would I just make a new util file in lib/ then? With only that function? I'm afraid the iteration won't work as expected, because of the nature of symbols. I'll test it.

[GabeAtWork]

I updated the PR. I replaced every call to Object.keys() with a call to lib/util getKeys(), which takes symbols into account. Tell me if I should roll that back and only use it on the few instances that were mentioned in the issue.

I ran tests/util.test.js on a system which knows Symbols, it would crash otherwise. The getKeys() function itself checks for the presence of Object.getOwnPropertySymbols though, so it should be safe on systems which don't support Symbols.

[JSProto]

You can use Reflect.ownKeys if exists to get all keys.

let obj = {
    enum: 1,
    'non-enum': 2,
    [Symbol('symbol_key')]: 3
};
Object.defineProperty(obj, 'non-enum', { enumerable: false });

Object.getOwnPropertyNames(obj); // ['enum', 'non-enum']
Object.getOwnPropertySymbols(obj); // [Symbol(symbol_key)]
Reflect.ownKeys(obj); //[ 'enum', 'non-enum', Symbol(symbol_key)]

you can use this example or similar method in lib/util.js or other place:

exports.keys = function(obj){
    if (typeof Reflect === 'object') {
        return Reflect.ownKeys(obj);
    }
    else {
        let getOwnExists = typeof Object.getOwnPropertySymbols === 'function' && typeof Object.getOwnPropertyNames === 'function';
        return getOwnExists ? Object.getOwnPropertyNames(obj).concat(Object.getOwnPropertySymbols(obj)) : Object.keys(obj);
    }
};

[JSProto]

sorry, I can check test in the evening when I get home.

[GabeAtWork]

I updated the PR with the use of Reflect

[GabeAtWork]

Okay, so we now have 2 PRs:

• The original fix is now here : #224
• A new version with symbols worked on by @michailpanagiotis, with a dependency bump to Sequelize 4, integrating the new Symbols. Some more work is required, tests have broken with Sequelize 4.

[tommybananas]

Hi everyone, I am maintaining a new project based off of Epilogue that has full test coverage for Sequelize 4.x.x. I have already merged this and other relevant pull requests from this project. Contributions welcome!

https://github.com/tommybananas/finale

[omidn]

@tommybananas Will you create a PR?

[tommybananas]

Not here, I am already maintaining a Sequelize 4.x compatible version with this fix implemented:

https://github.com/tommybananas/finale