lexik / LexikPayboxBundle

LexikPayboxBundle eases the implementation of the Paybox payment system
MIT License
40 stars 47 forks source link

A parameter to set preproduction mode #35

Closed LouTerrailloune closed 9 years ago

LouTerrailloune commented 10 years ago

Setting the propuction/prepoduction environment with a parameter in the config should be great. This will allow us to use test mode for dev environment and production mode in the prod environment.

nykopol commented 10 years ago

I don't think that setting paybox environment with the value of the symfony dev or prod environment is a good idea. For example, a preprod website can be running in symfony prod environment but in preprod paybox environment.

However, this can easily be done without modification of the bundle.

Set the desired environment in parameter.yml :

parameters:
    paybox_environment: dev # or prod

And in your controller :

return $this->render(
    'AcmeMyBundle:Default:my_template.html.twig',
    array(
        'url'  => $paybox->getUrl( $this->container->getParameter('paybox_environment') ),
        'form' => $paybox->getForm()->createView(),
    )
);
gilles-g commented 10 years ago

+1 @nykopol :)

LouTerrailloune commented 10 years ago

I don't think that setting paybox environment with the value of the symfony dev or prod environment is a good idea.

I didn't say that.

Of course we can do it manually, but it would be better if the environment is in the config.

Olineuve commented 9 years ago

Changed in v1.1.0, you now have a configuration key to specify if the payment is in production or in test mode.

// config.yml lexik_paybox: parameters: production: true

soullivaneuh commented 9 years ago

@Olineuve You should add this notification on a Changelog file. This is BC break and broke all applications that was using v1.0.*.

Btw, a BC break functionnality always should be on a new major version (e.g. 2.0) instead of minor, according to semver rfc.

Regards.

soullivaneuh commented 9 years ago

Or, at least, this production parameter should be required. No value should throw error so we can see immediately what is wrong after the upgrade.