The required 'type' parameter is missing from hook_entity_property_info(). This parameter allows Views to treat timestamps as actual dates and allows us to filter by a list of predefined statuses. It also makes sure one can never set a status not defined by the module.

See patch in next comment.

CommentFileSizeAuthor
#8 add_type_to_hook_entity_property_info-2559797-8.patch14.54 KBneograph734
#6 add_type_to_hook_entity_property_info-2559797-6.patch14.22 KBmvdve
#5 add_type_to_hook_entity_property_info-2559797-5.patch14.33 KBmvdve
#4 interdiff.txt849 bytesneograph734
#4 2559797-4-add_types_to_hook_entity_property_info.patch9.14 KBneograph734
#3 interdiff.txt1.96 KBneograph734
#3 2559797-3-add_types_to_hook_entity_property_info.patch9.13 KBneograph734
#2 2559797-2-add_types_to_hook_entity_property_info.patch7.9 KBneograph734

Comments

Neograph734 created an issue. See original summary.

neograph734’s picture

neograph734
he/him
Location Netherlands
commented
Status: Active » Needs review
StatusFileSize
new 2559797-2-add_types_to_hook_entity_property_info.patch7.9 KB
neograph734’s picture

neograph734
he/him
Location Netherlands
commented
StatusFileSize
new 2559797-3-add_types_to_hook_entity_property_info.patch9.13 KB
new interdiff.txt1.96 KB

I had to add a computed (pseudo) field to the list so we can show the amount in Euro's in the admin overview page.

neograph734’s picture

neograph734
he/him
Location Netherlands
commented
StatusFileSize
new 2559797-4-add_types_to_hook_entity_property_info.patch9.14 KB
new interdiff.txt849 bytes

Forgot about translating the statuses, this should be the final patch.

mvdve’s picture

mvdve commented
StatusFileSize
new add_type_to_hook_entity_property_info-2559797-5.patch14.33 KB

Thanks for the patch!

Two things:
- The purchase_id can also be defined by the user via hook_ideal_advanced_commerce_transaction_alter. It may be better to define it as a text field instead of integer to give them more freedom.
- The ideal statuses are already defined in the ideal connector wrapper. My suggestion would be to move the statuses to the ideal_advanced module file. This way they are more generic and can be used everywhere, without calling the whole wrapper class.

Please review the updated patch.

mvdve’s picture

mvdve commented
StatusFileSize
new add_type_to_hook_entity_property_info-2559797-6.patch14.22 KB

Introduced a bug. New patch with moved statuses and updated purchase_id type.

neograph734’s picture

neograph734
he/him
Location Netherlands
commented

Just read through the patch and is seemed ok. We might make it even better by changing the config_id type to the actual configuration entity type (I believe that did some entity relation magic). I'll give that a try this afternoon and let you know the outcome.

neograph734’s picture

neograph734
he/him
Location Netherlands
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new add_type_to_hook_entity_property_info-2559797-8.patch14.54 KB

Implemented the proposed change of #7.

By having the type of the transaction config_id set to `ideal_advanced_config` rather then `integer`, the configuration entity is now connected to the transaction.
This is visible as a Views relation and with Entity Metadata Wrappers, where both entities can be chained together.

$transaction_id = 1;
$wrapper = entity_metadata_wrapper('ideal_advanced_transaction', $transaction_id);
echo $wrapper->config_id->title->value();

Apart from that one little change, your patch has remained unchanged and is ready to be committed.

  • mvdve committed 37aa525 on 7.x-1.x authored by Neograph734 
    Issue #2559797 by Neograph734, mvdve: Add appropriate types to...
mvdve’s picture

mvdve commented
Status: Reviewed & tested by the community » Closed (fixed)

Excellent work! Thanks for the contribution. The patch is committed.