Our beloved clients sometime ask for really fine tuning. In one of my use cases, i need to be able to alter the description sent to Stripe.

We need a patch to let the modules do that.

CommentFileSizeAuthor
#12 2545400-stripe-charge-alter-12.patch3.17 KBtorgospizza
#10 2545400-stripe-charge-alter-10.patch3.62 KBtorgospizza
#6 2545400-Commerce_stripe-Alter_charge_data-6.patch3.32 KBartusamak
#1 2545400-let_modules_alter_description-Commerce_stripe-1.patch2.72 KBartusamak

Comments

artusamak’s picture

artusamak
he/him
Primary language French
Location Bzh
commented
Status: Active » Needs review
StatusFileSize
new 2545400-let_modules_alter_description-Commerce_stripe-1.patch2.72 KB

Here is a patch for that.
We *could* send data in the metadata of the payment but it's not enough.

The other implementation option could be to let any module alter the Charge object (i have another use case (and patch coming) where we need to set the destination attribute) but we could let any module alter the amount of the Charge. I have mixed feelings about this, i think it's not a good idea to let anyone alter the whole object but on the other side it avoids to have multiple hooks to implement.

I'm opened for discussion about this.

aviindub’s picture

aviindub commented

seems reasonable. committing.

  • aviindub committed 0f34009 on 7.x-1.x authored by Artusamak 
    Issue #2545400 by Artusamak: Let modules alter the order description
aviindub’s picture

aviindub commented
Status: Needs review » Fixed
artusamak’s picture

artusamak
he/him
Primary language French
Location Bzh
commented

Cool, thanks! This should be reverted if the submodule is created though.

artusamak’s picture

artusamak
he/him
Primary language French
Location Bzh
commented
Title: Let modules alter the order description » Let modules alter the charge data
Status: Fixed » Needs review
StatusFileSize
new 2545400-Commerce_stripe-Alter_charge_data-6.patch3.32 KB

Actually, i believe that it should be wilder than that. We may want to add / alter other information to the charge object so instead of adding a hook for each attribute, we should let the modules alter the whole data.

alberto56’s picture

alberto56 commented

#2569597: Provide an API hook to make it possible to alter information sent to stripe marked as a duplicate of this one.

Remon’s picture

Remon commented

Patch in #6 works perfectly.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Status: Needs review » Reviewed & tested by the community

I'll move this to RTBC then!

And actually since commerce_stripe_add_metadata() was added in a -dev version we may even want to consider removing it in factor of putting everything into an alter hook. That would make it more standardized, although on the other hand, having a specific add_metadata() function might have advantages. We did technically add a new hook_commerce_stripe_metadata() which is more specific than the one for charge data, which is more generic. So the concept of moving the metadata actions into a more generic charge data alter hook gets a +1 from me.

That might be worth opening up a new issue, though, since the metadata hook is in -dev.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new 2545400-stripe-charge-alter-10.patch3.62 KB

Here is a re-roll of the patch in #6 against the latest dev as some lines have changed.

artusamak’s picture

artusamak
he/him
Primary language French
Location Bzh
commented
Status: Needs review » Needs work

Beware, there is an extra whitespace in the last line of your patch. After that it looks RTBC to me.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Status: Needs work » Needs review
StatusFileSize
new 2545400-stripe-charge-alter-12.patch3.17 KB

Whoops! Here's a re-reoll.

aviindub’s picture

aviindub commented

all looks good except for this line... what is this doing?

+  // Add application fees.
+  $charge['application_fee'] = rand(1, 1000);
torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Issue summary: View changes

That's just an example for the API documents, on how you might add an extra (hidden) fee. I probably wouldn't advocate for that, but it was a holdover from the first patch in the issue.

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented
Issue summary: View changes

Accidental edit fix

aviindub’s picture

aviindub commented

oh, right, obviously :) -- i was seeing it out of context.

  • aviindub committed 96c840b on 7.x-1.x authored by torgosPizza 
    Issue #2545400 by Artusamak, torgosPizza: Let modules alter the charge...
aviindub’s picture

aviindub commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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