See title. I already have a working version on my laptop, will provide a patch later.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | userpoints_entity3.patch | 17.61 KB | berdir |
| #9 | userpoints_entity2.patch | 5.5 KB | berdir |
| #2 | userpoints_entity.patch | 3.76 KB | berdir |
See title. I already have a working version on my laptop, will provide a patch later.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | userpoints_entity3.patch | 17.61 KB | berdir |
| #9 | userpoints_entity2.patch | 5.5 KB | berdir |
| #2 | userpoints_entity.patch | 3.76 KB | berdir |
Comments
Comment #1
BenK commentedSubscribing.... cool!
Comment #2
berdirAnd here is the patch.
Not that this is just the absolute minimum, that is, implementing hook_entity_info() and implementing the load function so that it uses the EntityController.
ToDo:
- Implement hook_entity_properties() info (based on the existing data object function used for rules)
Not sure if we need anything more, maybe a few tests for the load function.
Everything else will probably follow later.
Comment #3
berdirComment #4
BenK commentedAny suggestions on how I should test this? I'm not quite sure what to do since this is the new 7.x-2.x branch.... what should work and what shouldn't?
--Ben
Comment #5
berdirEverything should work just as it did before. 7.x-2.x is currently still 100% the same as 7.x-1.0.
There isn't much to test with issues like this, as I'm just shuffling internal stuff around.
The only difference is that you will see a Userpoints transaction in places where you can select an entity, like for example in rules (but that part won't work yet) or in add userpoints screen, where you can select an entity to reference (which is currently not that useful since we have a separate column to reference another transaction -- Which we might be able to deprecate/remote after this).
Comment #6
guybrush commentedThe patch at #2 above contains 2 references to:
userpoints_transaction_load(NULL, TRUE);
These appear to be unnecessary?
Also, what about using the Entity module (http://drupal.org/project/entity) to provide easy CRUD capabilities, and integration with Rules?
Comment #7
berdirThose calls clear the cache. Might be possible to optimize them, but they are at least necessary when updating existing transactions.
I certainly want to integrate with entity.module at least to the point of declaring properties to be able to access them in rules (they are already declared as a data object, we just need to move things around a bit). Not sure about any more than that, because we don't have the typical CRUD operations. We have CREATE through a custom API function that will be heavily refactored (see the corresponding issue in the roadmap). And we obviously have read but we only have limited support for update (right now pretty good, but that will be removed to ensure a correct history - only for pending transactions will changes be allowed) and we therefore also don't have (nor want) delete.
Comment #8
berdirNew patch that declares entity properties and removes the data hook for rules.
Please test if rules integration still works as it did before.
Comment #9
berdirAnd now with patch ;)
Comment #10
guybrush commentedTo clear the cache, can't you use:
entity_get_controller('userpoints_transaction')->resetCache();
instead of:
userpoints_transaction_load(NULL, TRUE);
(as used by modules such as node, user and taxonomy)?
Comment #11
berdirAh yes, that probably makes sense, especially because that would allow to just clear the cache of the the current transaction.
Comment #12
guybrush commentedOne other small point regarding userpoints_entity2.patch. The comment in userpoints_entity_property_info() should refer to entities not nodes. That is:
// Add meta-data about the basic entity properties.
instead of:
// Add meta-data about the basic node properties.
Comment #13
berdirI am starting to like this :) The patch now removes more code than there are new lines without loosing any functionality.
Changes:
- Using resetCache() just when editing existing transaction and just clearing the changed transaction. Haven't run the tests yet, but there is no reason that it shouldn't pass (Never say never...)
- Fixed the comment mentioned in #12 and also updated some other comments
- Replaced usage of deprecated entity_metadata_* functions with entity_property_*
- Dropped a large hunk of the hook_tokens()/hook_token_info() implementations, we can rely on entity_token now. Note that I had to rename some properties/tokens (points-abs/time-stamp) to use an _ internally, but that shouldn't affect the visible name.
- There is also a uri callback defined now, which is also accessible through the url token/property.
- Dropped the currently useless/outcommented bundle information, that will be re-added later on.
Again, please make sure that rules integration still works as before.
Comment #14
berdirWell, let's move forward with this, we can deal with bugs later on.
Commited and pushed.