#1635 image urls encode correctly. #1810
Conversation
|
Hey @crcommons, @rjmackay It looks like this brave person signed our Contributor License Agreement. 👍 Always at your service, clabot |
| } else { | ||
| return URL::site(Media::uri($this->get_relative_path() . $value), Request::current()); | ||
| return URL::site((Media::uri($this->get_relative_path() . $value)), Request::current()); |
There was a problem hiding this comment.
It'd be worth adding a comment above this to explain that either URL::site or Media::uri is already encoding the url - so that the next person to look at this knows why we're only encoding in 1 case, not the other.
| @@ -80,4 +80,18 @@ public function disableDataImport() | |||
| $config = \Kohana::$config->load('features'); | |||
| $config->set('data-import.enabled', false); | |||
| } | |||
|
|
|||
| /** @BeforeScenario @cdnEnabled */ | |||
| public function enableCdn() | |||
There was a problem hiding this comment.
You should remove these before this lands, since they didn't work anyway.
Given we don't have a convenient way to enable/disable the CDN during test runs, we should possibly just run with the CDN enabled for all tests since thats the production state. @willdoran what do you think?
There was a problem hiding this comment.
@willdoran Let me know if I should remove or not. Thanks!
There was a problem hiding this comment.
@rjmackay @crcommons If it doesn't work then dropping it seems fine. Out of curiosity why doesn't it work?
There was a problem hiding this comment.
I can't recall why it didn't work; only that it didn't. @rjmackay may be able to remember and explain.
There was a problem hiding this comment.
I removed this and now the checks have failed. @rjmackay Can we touch base on this?
There was a problem hiding this comment.
I think the only failure was linting.
There was a problem hiding this comment.
If it doesn't work then dropping it seems fine. Out of curiosity why doesn't it work?
@willdoran CDN config doesn't load from the DB, so any changes we make via behat don't stick.
| @@ -20,6 +20,7 @@ class Ushahidi_Formatter_API implements Formatter | |||
| // Formatter | |||
| public function __invoke($entity) | |||
| { | |||
| Kohana::$log->add(Log::ERROR, print_r($entity, true)); | |||
There was a problem hiding this comment.
Can you remove the log statement before this lands?
crcommons
force-pushed
the
1635-encode-images
branch
from
d772563 to
f0b013c
Compare
.travis.yml
Outdated
| @@ -39,7 +39,7 @@ before_script: | |||
| - mysql -e 'SET @@GLOBAL.wait_timeout=1800' | |||
| script: | |||
| - if [ "${coverage}" == "" ]; then composer test; fi | |||
| - if [ "${coverage}" != "" ]; then composer run coverage --timeout=0; fi | |||
| - if [ "${coverage}" != "" ]; then composer run coverage --timeout=2000; fi | |||
There was a problem hiding this comment.
So timeout=0 was supposed to remove the timeout entirely. Maybe that stopped working?
rjmackay
force-pushed
the
1635-encode-images
branch
from
104bb61 to
3173a94
Compare
|
Changes Unknown when pulling 3173a94 on 1635-encode-images into ** on develop**. |
This pull request makes the following changes:
-correctly encodes image filenames so that there is no whitespace in the url.
Test checklist:
Fixes #1635 .
Ping @ushahidi/platform