attached patch moves the sql query in it's own ecard_load_card function

this would make it a lot easier to pass details of the message to the theming of the content itself (think of parameters to a flash object) so the message can be embedded

perhaps it would also be interesting to set $node->content['ecard_sender_name'] and $node->content['ecard_message'] and stuff so retheming this would be a breeze

Comments

kars-t’s picture

Status: Needs review » Needs work
StatusFileSize
new3.55 KB

Great idea! Sadly I couldn't apply your patch directly. I did extend this idea a bit. And made a follow up patch. Please take a look at it.

I added ecard_load and ecard_save so its more CRUD. So we still need update and delete. And I added alteration hooks. Maybe we should even add hook_ecard which is quiet easy.

So this is currently halve done but please share your thoughts.

seutje’s picture

You seem to have used ecard_load_card in one instance and ecard_load in another

+    $ecard = ecard_load_card($arg1);
+function ecard_load($random) {

and wouldn't ecard_load be misinterpreted as a implementation of hook_load? this was the main reason I dubbed it ecard_load_card

kars-t’s picture

and wouldn't ecard_load be misinterpreted as a implementation of hook_load? this was the main reason I dubbed it ecard_load_card

Sure didn't think about this. Do you want to tackle this issue? I think I might patch this Sunday night earliest? Feel free to assign this to you ;)

seutje’s picture

I don't think I'll be able to do much before sunday, weekend promises to be rather hectic, and since plans have changed, I don't need this to be split up. We decided to go with a node-based custom e-card sending service with a phplist backing it up

I'm also kinda prioritizing some D7 core issues that really need some love

but I'll see what I can do :)

kars-t’s picture

Status: Needs work » Needs review
StatusFileSize
new4.05 KB

New version with delete and update.

kars-t’s picture

Status: Needs review » Closed (won't fix)

#642292 by Kars-T, seutje: Added Split off loading in a separate api call.

I committed this. But please be aware that it is currently untested except some handish tests by me!