-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add /bbl route #438
Add /bbl route #438
Conversation
import bblDemux from '../utils/bbl-demux'; | ||
|
||
export default Ember.Route.extend({ | ||
model(params) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@allthesignals is this the right hook to use? I tried beforeModel
but it appeared that the params weren't fully loaded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pichot yes - beforeModel doesn't have the params object in the signature directly, you'd have to access it differently.
Rebased and ready to merge please :) |
|
||
export default Ember.Route.extend({ | ||
model(params) { | ||
const bbl = bblDemux(params.bbl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might also have said:
export default Ember.Route.extend({
model({ bbl }) {
const { boro, block, lot } = bblDemux(bbl);
this.transitionTo('lot', boro, block, lot);
}
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooh, better. Should have waited for your code review
Simple commit that adds a
/bbl
route. Uses already implementedbbl-demux
to parse bbl and redirect to/lot
route.Unit and acceptance tests included.