xklusive / laravel-battlenet-api

Laravel 5 client for Battle.net API
MIT License
8 stars 3 forks source link

configuration not read with laravel 5.5 #22

Closed ola-mattsson closed 6 years ago

ola-mattsson commented 6 years ago

Using Laravel 5.5. Published configuration is not read. adding the filename to the tag helps private function getApiEndPoint() { return config('battlenet-api.battlenet-api.domain'); }

atraides commented 6 years ago

Let me check it why this is the case.

sdousley commented 6 years ago

This is because the battlenet-api.php config file has a battlenet-api index in the array. remove this index, and it works perfectly fine.

Link to my working battlenet-api.php file: https://gist.github.com/sdousley/af4aa0ee6a151c559345eb667dcba75d

Masked my API key for obvious reasons

ola-mattsson commented 6 years ago

Hi,

Cool, makes sense. Thanks!

Ola

On 5 Dec 2017, at 14:25, sdousley notifications@github.com wrote:

This is because the battlenet-api.php config file has a battlenet-api index in the array. remove this index, and it works perfectly fine.

Link to my working battlenet-api.php file: https://gist.github.com/sdousley/af4aa0ee6a151c559345eb667dcba75d https://gist.github.com/sdousley/af4aa0ee6a151c559345eb667dcba75d Masked my API key for obvious reasons

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/xklusive/laravel-battlenet-api/issues/22#issuecomment-349303019, or mute the thread https://github.com/notifications/unsubscribe-auth/AJqOUro31Uw8CgYXnDy1q0wFB5UaOFoVks5s9URAgaJpZM4Q1CiU.

atraides commented 6 years ago

Yeah looks like it might be the reason. Unfortunately I still dont have a working laravel environment to test this :)

I will finish the setup today and apply the change to the repo and push a new version.

PS: Do not store your key in the configuration file. I know it is currently the only option if you don't change the config manually. I will fix that so you can store the key in the .env file as you should.

ola-mattsson commented 6 years ago

Didn't work, here is a bit of analysis. This is my findings for this specific issue. Happy to hear if somebody has more complete explanation removing the tag, i.e. config/battlenet-api.php contains: return [ // no tag here [ 'api_key' => 'the key', 'locale' => 'the locale', 'domain' => 'https://eu.api.battle.net', 'cache' => true, ], ]; calling config('battlenet-api') will return an indexed array with 2 elements [0] being the default one in the 'vendor' directory and [1] will return my overrides in my config folder. So to get the value from my example from my initial post needs to be config('battlenet-api')[1]['domain]; So, the your-config.php needs to be an associative array and thus contain tags for all entries. Original format is fine but querying the contents needs to contain the filename, i.e. config('filenamewithoutextention.tag.tag'); config('battlenet-api.battlenet-api.domain'); filename needs to be in the request.

This is done in Laravel 5.5, php 7.2. Hope it helps

Ola

On 5 Dec 2017, at 22:04, Dániel Hagyárossy notifications@github.com wrote:

Yeah looks like it might be the reason. Unfortunately I still dont have a working laravel environment to test this :)

I will finish the setup today and apply the change to the repo and push a new version.

PS: Do not store your key in the configuration file. I know it is currently the only option if you don't change the config manually. I will fix that so you can store the key in the .env file as you should.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/xklusive/laravel-battlenet-api/issues/22#issuecomment-349440878, or mute the thread https://github.com/notifications/unsubscribe-auth/AJqOUiXk2sJtGSMsKAWNCyslC2RWhKF7ks5s9a_hgaJpZM4Q1CiU.

sdousley commented 6 years ago

Hi Ola

My apologies, I probably was a little unclear in what I said compared to what I meant!

It's not just a case of removing the array index tag, you need to remove the whole top level of the array in that file.

What you need (no comments) is:

return [ 'api_key' => 'the key', 'locale' .... ];

This will allow you to access using config('battlenet-api.api_key') etc

sdousley commented 6 years ago

I've put up a pull request from my own fork of this to fix this

ola-mattsson commented 6 years ago

That’s it. nice and clean!

Ola

On 6 Dec 2017, at 09:39, sdousley notifications@github.com wrote:

Hi Ola

My apologies, I probably was a little unclear in what I said compared to what I meant!

It's not just a case of removing the array index tag, you need to remove the whole top level of the array in that file.

What you need (no comments) is:

return [ 'api_key' => 'the key', 'locale' .... ];

This will allow you to access using config('battlenet-api.api_key') etc

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/xklusive/laravel-battlenet-api/issues/22#issuecomment-349571636, or mute the thread https://github.com/notifications/unsubscribe-auth/AJqOUvGTLLpKs1dbFvukFgVtXZe2NooSks5s9lLFgaJpZM4Q1CiU.

atraides commented 6 years ago

I created a new release v1.0.1 with the fixes.

Thanks for the help