-
Notifications
You must be signed in to change notification settings - Fork 20
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 lat/long to Request model, rework ID implementation #221
Conversation
@@ -28,6 +28,9 @@ exports.systemEmail = '[email protected]'; | |||
// of the navigation for the admin section. | |||
exports.signupEnabled = false; | |||
|
|||
// Configure Geocoding API API key | |||
exports.googleMapsApiKey = 'AIzaSyB9r-02V9oy8iluVuLweHx6IFi_VuUMXVg'; |
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.
Hey @kwhinnery, I see that you removed this in 3dd6ca0. Are you okay with having this in the history or does this mean you need to change your key?
@kwhinnery, thank you! This looks great. I don't know if you talked to @OhMcGoo about the sequential public ids vs. hashed public ids, but I think we want to preserve the sequential public ids since they're seen by humans. I'm definitely glad you've fixed the infinite loop problem (#159)! We might add a counter or something to avoid infinity while preserving the sequential ids. |
Ping @cecilia-donnelly When reviewing this, I'm not sure how the information is getting to allReady. Once I understand that link, I can rebase this off master and take another look. It also looks like we're still using hashed ids (?), or if that commit was replaced by corrected code. |
@frankduncan I think the question of how the information gets to AllReady is separate from this change. This change is about ensuring that the Smoke Alarm Portal has the lat/long. Later we can take a look at the API by which we communicate with allReady (or other systems) and see what it takes to get the information across. |
@frankduncan Re the hashed IDs, I agree with @cecilia-donnelly that we need human-friendly sequential public IDs. |
@kfogel Do we have an google maps API key to use for OTS? |
@frankduncan We have a Google Maps API key for DCSOps, but I think we should get a separate one for Smoke Alarm. I'll call you to work out details. |
Okay, the new Google Maps API Key has been communicated via private channel to @frankduncan. |
This PR has been replaced by #271, which is a branch local to the repository that I can manipulate. |
This PR addresses #206 and revises the way a request is saved.
Let me know what you think...