Skip to content

Commit

Permalink
fix missing configuration placeholder variable
Browse files Browse the repository at this point in the history
  • Loading branch information
wellingguzman committed Jan 18, 2017
1 parent e229d0c commit 1dc7a51
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
5 changes: 5 additions & 0 deletions api/core/Directus/Util/Installation/InstallerUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ protected static function createConfigurationFile($data, $path)
$data['default_language'] = 'en';
}

$data = ArrayUtils::defaults([
'default_language' => 'en',
'feedback_login' => true
], $data);

$configurationStub = file_get_contents(__DIR__ . '/stubs/configuration.stub');
$configurationStub = static::replacePlaceholderValues($configurationStub, $data);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ return [

'feedback' => [
'token' => '{{feedback_token}}',
'login' => '{{feedback_login}}',
'login' => {{feedback_login}}
],

// These tables will not be loaded in the directus schema
Expand Down

5 comments on commit 1dc7a51

@jcmendez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @wellingguzman - I see on commit e229d0c a fix for #1332 was merged, but then on this commit was reverted. I see feedback_login should be true, but when I install a fresh install I get error messages like the below. If you don't include the quote it looks the placeholder variable doesn't get replaced

01/Feb/2017:17:33:22 +0000 [ERROR 0 /index.php] PHP message: PHP Parse error:  syntax error, unexpected '{' in /var/www/directus/api/configuration.php on line 72

@wellingguzman
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcmendez Thanks pointing this out, I reverted it as it needs to be a boolean, I tested it before and it worked, I will take a look at this and fix it as soon as possible.

@jcmendez
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will send a pull request with the change. It looks the travis test failed after you merged e229d0c, but maybe we need to review the tests. On clean install no quotes gets a broken build

@wellingguzman
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it failed on that commit e229d0c. Are you using the build branch or the master branch?

@jcmendez
Copy link

@jcmendez jcmendez commented on 1dc7a51 Feb 1, 2017 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.