Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

6.0.1 update breaks route-restful option #80

Closed
gdibble opened this issue May 13, 2016 · 6 comments
Closed

6.0.1 update breaks route-restful option #80

gdibble opened this issue May 13, 2016 · 6 comments
Assignees
Labels
bug
Milestone

Comments

@gdibble
Copy link

@gdibble gdibble commented May 13, 2016

An || got change to an &&:

image

@stongo

This comment has been minimized.

Copy link
Contributor

@stongo stongo commented May 19, 2016

isn't the && correct? the route setting should precede the plugin setting in the check and both conditions should pass.

@gdibble

This comment has been minimized.

Copy link
Author

@gdibble gdibble commented May 19, 2016

I thought so too, but in application my routes which specify...

    config: {
        plugins: {
            crumb: {
                restful: false
            }
        }
    }

...stopped working (whereas these send a form-field-value instead of req. header) when I upgraded to v6.0.1

Sorry if I've misintrepreted anything, I just couldn't get it to work w/o changing that && => ||


btw sorry I haven't done more; trying to prep for nodebots classes for 100 students so i have a lot of code & curriculum to write at the moment; after i hope to do more investigation & check the tests/etc

@aef-

This comment has been minimized.

Copy link
Contributor

@aef- aef- commented May 25, 2016

We need to add a condition that explicitly checks for false as the value and to short-circuit the plugin/settings config condition.

I think this should do the trick (test ln 659 should expect 200 instead of 403, this causes it to fail appropriately)

const routeIsRestful = request.route.settings.plugins._crumb ? 
                       request.route.settings.plugins._crumb.restful : undefined;
 if (routeIsRestful === false ||
     (routeIsRestful !== true && settings.restful === false))
stongo added a commit that referenced this issue Jun 29, 2016
@stongo stongo added the bug label Jun 29, 2016
@stongo stongo added this to the 6.0.2 milestone Jun 29, 2016
@stongo stongo self-assigned this Jun 29, 2016
@stongo

This comment has been minimized.

Copy link
Contributor

@stongo stongo commented Jun 29, 2016

The pull request of branch route-config should fix this. Can anyone confirm?

stongo added a commit that referenced this issue Jun 30, 2016
#80: fixes crumb route settings bug
@stongo

This comment has been minimized.

Copy link
Contributor

@stongo stongo commented Jun 30, 2016

Fixed by release 6.0.2

@stongo stongo closed this Jun 30, 2016
@gdibble

This comment has been minimized.

Copy link
Author

@gdibble gdibble commented Jun 30, 2016

Many thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.