RopeWiki / app

Infrastructure for automated RopeWiki site creation
Apache License 2.0
4 stars 3 forks source link

Add Google Maps API key #58

Closed llamafilm closed 4 months ago

llamafilm commented 1 year ago

Fixes #47

BenjaminPelletier commented 1 year ago

Is the API key a secret? If not, could it go in the site config rather than the required environment variables?

llamafilm commented 1 year ago

The key is linked to my credit card so I'd prefer to keep it secret. The risk is low because it can only be used from *.ropewiki.com but it might be possible for some hacker to do some DNS spoofing trick. As long as we stay below 40K requests per month there is no cost.

hcooper commented 1 year ago

Thanks for trying to fix this issue. (I opened it, but forgot to update once I found new info).

Existing Google Maps API keys are already set here: https://github.com/RopeWiki/commonjs/blob/master/commonjs/lib/constants.js#L27 . The background to this other repo is that all the javascript (which powers a lot of features) gets merged together, and then copied to the main mediawiki javascript file here: http://ropewiki.ak/index.php?title=MediaWiki:Common.js

All the mapping on the rest of the site uses these keys. I've prefer we don't set two places which do the same thing. And actually the comments from @catware2 say "/Location" was disabled on purpose years ago.

So I think we've two options:

  1. Modify "/Location" to use the javascript keys like everywhere else on the site.
  2. Just delete "/Location" (The only link I can find to it is on this page: https://ropewiki.com/Beta).

I think 1. will be time consuming without a lot of benefit. I prefer 2 :-)

As an aside, and something which would be really helpful... I have a proposal for managing these custom constants better. Rather than defining them in the main js file, the js code should attempt to first read them from a file called "/custom_constants.js". This file is not checked into the main repo, and is only used when running dev versions of the site. It then falls back to using the default production codes if this file doesn't exist. We can then delete all the extra keys from the main file, and don't have to do the whole mess of checking in, building js, and copying over to run a new dev site.

llamafilm commented 1 year ago

I agree the utility of the search feature is fairly low. But it seemed like an easy fix, and free if we stay below the $200 monthly credit. Either way, this was a fun way for me to get familiar with the code 😄 .

If you want to disable it instead, we would also have to change https://ropewiki.com/World which is the Map link in the main sidebar. Try to search for anything on that page and it shows ugly php errors.

catware2 commented 1 year ago

It goes without saying that we already have Google Maps API keys that enable the site. As Coops posted the link to my comments above, I made the decision not to enable this feature. I could easily enable it, as I wrote if this Location search looked useful. We don't need to add additional Google Maps API keys added to the site.

Until a month ago when Coops joined, I was the only person modifying the Javascript for the site for the past 4 years, so the only two keys we have are the prod key and the key for my dev laptop, both of which are managed by me. Coop's suggestion to handle more developers by having the site look for your local version of the API key might work if it doesn't slow down the site while it tries to load the key, or I could just enable the dev key for the two of you, if you will be doing Javascript development.

hcooper commented 1 year ago

My main priority is to avoid duplicating API keys, making the site harder to manage. (The whole mediawiki commonjs setup isn't particularly nice, but at least it's all in one place!).

also have to change https://ropewiki.com/World which is the Map link...

Removing the Search option can be done in the Region template, here:

image

I'd suggest just starting with some <!-- --> comments! (Welcome to managing code via mediawiki - it, umhh, really sucks!).

if it doesn't slow down the site

My theory is that directly fetching a <1K static text file will be an order of magnitude faster than the rest of commonjs. commonjs is a quarter of a megabyte of text fetched from Mysql by PHP, that's then shipped over the internet to your browser, parsed and executed. :-)

I've made a new issue for this proposal (https://github.com/RopeWiki/app/issues/59), so we can keep this discussion related to the maps issue.