See title. I already have a working version on my laptop, will provide a patch later.

Comments

BenK’s picture

Subscribing.... cool!

berdir’s picture

StatusFileSize
new3.76 KB

And 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.

berdir’s picture

Status: Active » Needs review
BenK’s picture

Any 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

berdir’s picture

Everything 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).

guybrush’s picture

The 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?

berdir’s picture

Those 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.

berdir’s picture

New patch that declares entity properties and removes the data hook for rules.

Please test if rules integration still works as it did before.

berdir’s picture

StatusFileSize
new5.5 KB

And now with patch ;)

guybrush’s picture

To 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)?

berdir’s picture

Ah yes, that probably makes sense, especially because that would allow to just clear the cache of the the current transaction.

guybrush’s picture

One 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.

berdir’s picture

StatusFileSize
new17.61 KB

I 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.

berdir’s picture

Status: Needs review » Fixed

Well, let's move forward with this, we can deal with bugs later on.

Commited and pushed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.