funktionswerk / hapi-i18n

Translation module for hapi based on mashpie's i18n module
MIT License
39 stars 22 forks source link

I18n not add when JOI validation failed #2

Closed yodutouf closed 9 years ago

yodutouf commented 9 years ago

The i18n data is not added to request when using the "failAction" callback of the joi plugin.

Have you got any trick to achieve it ?

server.route({
      method: 'POST',
       path: '/publish_new',
       config: {
        auth: 'simple',
        handler: function (request, reply) {

        },
        validate: {
            payload: {
                title:Joi.string().required(),
                content:Joi.string().required(),
                published:Joi.string().required()
            },
            failAction: function (request, reply, source, error) {
               //i18n has not been added to request -> the jade template rendering failed
                reply.view('publish', {payload:request.payload, error: error})
            }
        }
    }
});
codeva commented 9 years ago

Hi yodutouf,

i18n is currently added in the 'onPreHandler' extension point. I guess that i18n is not available in the failAction handler since validation is done earlier in the request lifecycle. Did you already try to use another extension point, for example 'onPreAuth'?

Marsup commented 9 years ago

@codeva Shouldn't you be using server.decorate to avoid that kind of problem ?

codeva commented 9 years ago

Hi Marsup,

I'm not sure if server.decorate provides all the information we need to initialize i18n. You can check if this approach works for you and send me a pull request if you want.

Thanks

Marsup commented 9 years ago

I'm not myself a user of that module, just providing potentially useful advice to a fellow hapi developer :)

codeva commented 9 years ago

In that case: Thanks for your advice :)

yodutouf commented 9 years ago

@Marsup I look at the server.decorate, but it seems it can't do the job because the i18n needs information from the request object itself.

At the end I do the validation "manually" inside the handler. What you think about ?

handler: function (request, reply) {
            Joi.validate(request.payload, {
                title:Joi.string().required(),
                content:Joi.string().required(),
                published:Joi.string().required()
            }, function (err, value) {
                if(err === null){

                }else{
                    reply.view('publish', {payload:request.payload, error: err})
                }

            });

        }
Marsup commented 9 years ago

You have access to the request, it's the this. I don't think that's what @devinivy had in mind.

devinivy commented 9 years ago

I imagined adding a server extension and leaving the handler alone,

server.ext('onPreResponse', function(request, reply) {

    // if there was a bad request due to validation on a certain
    // route then reply.view(), otherwise reply.continue().
})
yodutouf commented 9 years ago

Thanks for your answers

@devinivy the problem was that the i18n variable was not added to the request when server.ext('onPreResponse') was called.

@Marsup I use the decorate as you suggest You could check my pullRequest https://github.com/codeva/hapi-i18n/pull/3

devinivy commented 9 years ago

The i18n variable is added during onPreHandler, so it should be there later in the lifecycle during onPreResponse unless it's removed somewhere.

yodutouf commented 9 years ago

The problem is that "onPreHandler" is only called with the "handler" function, but not called with the "failAction" function. So using a decorator is doing the trick.

There is no "onPreFailAction" event available and I just begin with Hapi. Do you think the server must emit a "onPreFailAction" ?

Marsup commented 9 years ago

No, it's the only correct way to add features to the request object.

codeva commented 9 years ago

Hi yodutouf,

Thanks for the pull request.

devinivy's comment describes the problem with server.decorate: We need access to the reply object to respond with 404 if the language code is invalid: https://github.com/codeva/hapi-i18n/blob/master/index.js#L27

I have reproduced your failAction problem in the unit tests on a new branch 'validation': https://github.com/codeva/hapi-i18n/blob/validation/test/test.js#L89

The test will fail with 'Cannot call method of undefined'. Does anybody have a solution to add i18n before failAction and reply with 404 if the requested language code is invalid?

codeva commented 9 years ago

Hi yodutouf,

I have changed the extension point to onPreAuth, this seems to work. Can you please check the validation branch?

Marsup commented 9 years ago

That's good the request object contains the response then. His PR is a better solution I think.

codeva commented 9 years ago

Hi yodutouf,

I have just published version 1.0.2 of the plugin which includes the above fix.

yodutouf commented 9 years ago

Thanks all for your answers and contributions

antl3x commented 6 years ago

Guys, this isnt working on the stable version ˆ2.0.0 ?

Marsup commented 6 years ago

this isn't working has never been a proper description of any problem. Also you should create your own issue and not resuscitate a 3 years old one.

kaywolter commented 6 years ago

Hi @aasmm7x, This should be covered with the tests: https://github.com/funktionswerk/hapi-i18n/blob/master/test/test.js#L317 Could you provide a sample that reproduces your problem? Thanks