WordPoints / hooks-api

A basic API for hooking into user actions https://github.com/WordPoints/wordpoints/issues/321
GNU General Public License v2.0
0 stars 0 forks source link

Hit log primary_arg_guid should be null if no primary arg #88

Closed JDGrimes closed 8 years ago

JDGrimes commented 8 years ago

When there is no primary arg for the event related to a hit, we save the primary_arg_guid as an empty string. However, it would make more sense to just save the value as null instead, and that would also save memory.

When there is a primary arg but it has no GUID (which shouldn't really every happen, but...), the value saved will be null, but it will be JSON encoded, so 'null' will be saved in the field. This should be fixed too.

While we're add it we could add some unit tests for the wordpoints_hooks_get_event_primary_arg_guid_json().

JDGrimes commented 8 years ago

The problem with this is that we can do = '' in an SQL query but we can't do = null. We have to do IS NULL, which means we have to have an entirely separate query when we want the value to be null.

JDGrimes commented 8 years ago

I guess maybe the best way to handle this would be to introduce a custom query class for the hit logs, which could handle this stuff internally.

JDGrimes commented 8 years ago

null might actually be bad for performance though (micro-optimization). At the moment I'm thinking that maybe it would be best just to leave it as an empty string. But we'd still need to fix the fact that 'null' is sometimes being saved in the field.

JDGrimes commented 8 years ago

Since this doesn't seem to make much difference, in the interest of time, let's just leave this the way that it is, with the empty string.