com.drastikbydesign.stripe icon indicating copy to clipboard operation
com.drastikbydesign.stripe copied to clipboard

Published 20 hours ago • verified publisher icon drastik

add unit tests and api to help avoid regressions

Open jmcclelland opened this issue 7 years ago • 7 comments

jmcclelland avatar Jun 30 '17 15:06 jmcclelland

Wow this is exciting!

h-c-c avatar Jun 30 '17 15:06 h-c-c

This pull request is ready to go: https://github.com/drastik/com.drastikbydesign.stripe/pull/224

Would someone mind reviewing and merging it if it seems ok? It would be a good step toward stablizing the code and I'd like to continue refactoring things which will depend on good tests.

jmcclelland avatar Jul 14 '17 16:07 jmcclelland

Just a friendly bump - I imagine you are too busy to review now - but wanted to be sure it's not because you have any concerns with the commit.

I've been refreshing the pull requests with new commits because it's awkward to create new pull requests that depend on this one being committed.

jmcclelland avatar Jul 18 '17 14:07 jmcclelland

Hi again - just another friendly bump - I hope this extension isn't being abandoned!

jmcclelland avatar Aug 08 '17 15:08 jmcclelland

Hi @jmcclelland, sorry to take so long to get back to you. I've been swamped but I'd like look at the PRs soon!

Could you use API calls in favor of sql queries where possible? It's fair to say we've been making an effort to do it this way. I noticed your latest PR about the IDS exception has an sql query where there's an available API call.

Maybe something something like this?

$result = civicrm_api3('PaymentProcessor', 'get', array(
      'sequential' => 1,
      'return' => "id",
      'class_name' => "Payment_stripe",
      'is_active' => 1,
    ));

h-c-c avatar Aug 08 '17 19:08 h-c-c

Thanks for the response - glad to hear you are still plugging away at it.

Also, thanks for the feedback on #229 - I just made an update.

jmcclelland avatar Aug 09 '17 13:08 jmcclelland

Hi again, I removed one more DAO call (maybe that is the one you were referring to?). And I rebased against 4.7-dev. I know we missed the last release, but any chance this could get merged? Having a testing framework in place would really help with ensuring that new changes don't break things.

jmcclelland avatar Oct 24 '17 17:10 jmcclelland