search

ColFusion / ColfusionWeb

Other
1 stars 2 forks source link

Publish story as public (and probably other options) redirect to localhost #9

Open EvgenyKarataev opened 9 years ago

EvgenyKarataev commented 9 years ago

After filling the form to submit new data and clicking on Publish as Public, was redirected to http://localhost/Colfusion/story.php?title=3

ghost commented 9 years ago

This is fixed in 3a9ac739eab36c49e12707503a080125b86b94d6. The link used a variable in global.js, so I changed the link to use a relative path (relative to domain, but absolute path with respect to that domain). There was also another similar link I saw was broken when clicking a link within the published data, so I fixed that too. However, it still appears that there is code referencing variables in global.js. I was going to change "localhost" in there to window.location.hostname (and a possible port too), but it looks like some of the other servers may not be running on the same host as the web page, so I refrained (although it would probably work for just ColFusionDomain). I'm not sure if ColFusionAppName is still relevant. It seems like the site does not run behind the path /Colfusion.

EvgenyKarataev commented 9 years ago

Thanks for fixing it, I wrote some comments on the commit. It's been a while, but I think the reason we introduced the global.js file is to be the one place where we could configure some kind of global constants. I am not sure if we still need all of them or not, but as I mentioned in my questions to your commit, will your approach work if colfusion is hosted e.g. as mydomain.com/colfusion?

ghost commented 9 years ago

I reverted back to the globals.js approach, so that this will work when the site is behind /colfusion, assuming that is updated in the code (or an addition to have it auto-populated). An alternative is to use relative paths (e.g., ../story.php..), but that could also be problematic when changing structure, etc..

ghost commented 9 years ago

If we switch to running behind /colfusion in production, then we should also setup our local environments the same way to keep the code simpler.

EvgenyKarataev commented 9 years ago

The main reason of having this global.js file is to make the system configurable and as the result it make it easy to switch between dev, staging, production because we don't have many separate machine/[sub]domains/etc.

The global.js file could be easily auto-populated by dev/staging/production scripts.

ghost commented 9 years ago

Gotcha.

Also, I saw some places in the code (didn't make a note of where though) that hard-coded domains and paths (http://localhost...), so there may be a few more instances that direct the user to localhost or the wrong address (if that code is still used). Grepping for example, "localhost", reveals a bunch of occurrences, but I'll refrain from updating code that I'm not sure yet what it does or if it's still being used...

EvgenyKarataev commented 9 years ago

Yeah, I know there are more places like that. I think all those places need to be fixed, but you are right, I don't know myself as well if that is still being used or not.

One of things that I want to do is to actually do some cleanup/reorganization/refactoring, but first of all I need to have some version of the system running by Sep 20th...