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

Move get helper behind labs flag #6041

Merged
merged 1 commit into from
Nov 4, 2015
Merged

Move get helper behind labs flag #6041

merged 1 commit into from
Nov 4, 2015

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Nov 3, 2015

This PR has a couple of key parts, but I think it all makes sense together.

First, I split the code from middleware/auth.js out into utils/labs.js, so we have a one-size-fits-all utility for finding out if a labs flag is set.

Secondly, I had to do a bit of a weird dance in the get helper to be able to depend on this flag in a way that live updates.

The get helper is wrapped in a function that checks the labs flag, if the flag is set, then the get helper is called using .call to bind to the original this.

If the flag is not set, doing nothing results in 'undefined' being printed out to the page - not ideal. With normal unregistered helpers you get a 500 error printed on screen in the browser and usually also in the server console. In this case, that behaviour is somewhat undesirable.

Therefore, I did a little extra work, setup a 'nice' error message and made it so that this would be output on both the client and server console:

The intention is to be informative and not destructive, in this case. I think that is sane?

issue #5976

  • break out the labs check into a utility
  • wrap the get helper in a labs check, so it only works if the checkbox is checked
  • make the get helper output an error to both the server and browser console if used when not enabled

issue TryGhost#5976

- break out the labs check into a utility
- wrap the get helper in a labs check, so it only works if the checkbox is checked
- make the get helper output an error to both the server and browser console if used when not enabled
sebgie added a commit that referenced this pull request Nov 4, 2015
Move get helper behind labs flag
@sebgie sebgie merged commit 63d353d into TryGhost:master Nov 4, 2015
@ErisDS ErisDS deleted the get-labs branch November 23, 2015 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants