Check if a property exists in the original object before injecting its hashed field

[TissuePowder]

Uses hasOwnProperty() for checking, not in.

[JaneJeon]

Mind adding the tests for your usecase here? thanks

[TissuePowder]

I didn't test it extensively before, apologies. Found some major problems after I used your jest testing file. Doing the .hasOwnProperty(field) check tries to match field names explicitly, so some cases like algolia, which uses ObjectID as field name, or for compund primary keys that have no original id field, but will get one after hashed, doesn't work. Maybe the static get hashIdField() function needs some additional check?

[TissuePowder]

Can we just remove the id key from json if it's empty? Probably not the best solution, but easier.

[JaneJeon]

Sorry, I don't think I understand the problem, but here's my understanding of it and please let me know if I'm misunderstanding it:

so some cases like algolia, which uses ObjectID as field name, or for compound primary keys that have no original id field, but will get one after hashed, doesn't work

You can specify your own hashIdField - in the first case, as 'ObjectID', and in the latter case, as the array of primary keys that do specify its primary key (e.g. ['a', 'b']). Are you referring to that?

Also, when you say it doesn't work, how exactly does it not work (could you perhaps push the failing test case to the PR so I can see what's going on)? Does it not work in the serialization ($formatJson) or the deserialization ($parseJson)? How does it "fail" (i.e. what's the setup, what's the expected outcome, what's the real outcome)?

Can we just remove the id key from json if it's empty?

The same thing as above - I'm not sure what you're referring to here. Which JSON (the original JSON, or the serialized JSON)? In serialization or deserialization? What exactly constitutes "empty" (an empty string? null? undefined?)

Thanks

[TissuePowder]

In last test I used the jest test file index.test.js you provided, (with some minor change as I don't use objection-visibility or things like that, but that isn't the issue). You can reach the same pass/fail test outcomes as me using your testing file. I kind of gave a vague/crappy explanation since I was using your test file, sorry, I should have been more precise.

Anyway, I'll try to explain it better. For the first failed case with algolia, (https://github.com/JaneJeon/objection-hashid/blob/master/index.test.js#L124), value of idColumn() is id, but we are writing the hashed value on a new field defined in hashIdField(), namely ObjectID.

Now the obj.hasOwnProperty() in the commit checks it like this: "does ObjecID field exist in the original object? (No it doesn't, because the original field is id). And hence, it doesn't inject the hash into the serialized json.

Exactly same happens with composite primary key (https://github.com/JaneJeon/objection-hashid/blob/master/index.test.js#L156). The specified primary key can be [a, b] as you said, where the hash would be written in a new field called id. But as the id field doesn't exist in the original object, it doesn't pass the hasOwnProperty check and so the hash doesn't get written.

Can we just remove the id key from json if it's empty?

I am talking about the serialized json ($formatJson), yes, because all this concern is about how we see the fields in output, in something like res.status(200).json(data).

Let me know if I need to explain more.

[JaneJeon]

Sorry I haven't been able to reply sooner (I'm at the airport), but I promise to get back to this soon. If I don't, just @ me

[TissuePowder]

@JaneJeon