laravel-currency icon indicating copy to clipboard operation
laravel-currency copied to clipboard

Implement 4 sources of exchange rates, contract and manager

Open semyonchetvertnyh opened this issue 6 years ago • 16 comments

  • Implementing source contract and 4 sources:
  1. https://exchangeratesapi.io
  2. https://openexchangerates.org
  3. https://currencylayer.com
  4. https://fixer.io
  • Adding clear exceptions

  • Adding env variables to config file

  • Switching from option to --source argument in Update command

php artisan currency:update --source=fixer
  • Adding Manager and Facade

  • Adding possibility to update exchange rates from everywhere

use Torann\Currency\Facades\SourceManager;

// Option 1
SourceManager::fetchFrom('fixer')->update();

// Option 2
SourceManager::source('fixer')->update();

// Option 2
$manager = app(\Torann\Currency\SourceManager::class);
$manager->fetchFrom(CustomImplementationOfSourceContract::class);
$manager->update();

// Option 4
SourceManager::extend('customApiService', function () {
    return new CustomApiService;
});

SourceManager::fetchFrom('customApiService')->update();

semyonchetvertnyh avatar Dec 09 '18 02:12 semyonchetvertnyh

@Torann take a look please

semyonchetvertnyh avatar Dec 10 '18 15:12 semyonchetvertnyh

This looks great! There's a lot here so I'm going to have to pull it down and play with it some before I approve.

Torann avatar Dec 12 '18 14:12 Torann

Ok, will waiting for your answer!

semyonchetvertnyh avatar Dec 12 '18 22:12 semyonchetvertnyh

Hey @Torann!

I just committed some changes to make the code a bit easier to read. Still waiting for you answer :)

Btw to see the difference, it's better to choose "Hide whitespace changes" in "Files changed" tab.

semyonchetvertnyh avatar Mar 14 '19 12:03 semyonchetvertnyh

Is this ready to be merged?

f-liva avatar Jun 18 '19 07:06 f-liva

In my opinion - yes, it is. But @Torann should make code review.

semyonchetvertnyh avatar Jun 19 '19 14:06 semyonchetvertnyh

Let's hope he takes care of it soon then

f-liva avatar Jun 20 '19 07:06 f-liva

At a glance there is a lot going on here. I will need to sit down and make sure this doesn't affect current installs of the package (backwards compatible), if it does we will need an upgrade doc and it will be given a new version. Great work!

Torann avatar Jun 21 '19 14:06 Torann

Any news here?

f-liva avatar Sep 03 '19 08:09 f-liva

There is a lot of changes here. I will need to sit down and test them all before I merge. The quick once over I did of the code though looks great!

Torann avatar Sep 04 '19 15:09 Torann

@semyonchetvertnyh Семьон, have you considered releasing own version? I really would like to use this patched version but it's not possible unless it's either merged or released as another package.

klimenttoshkov avatar May 14 '20 16:05 klimenttoshkov

@klimenttoshkov I don't have plans to create and maintain my own version of the package. If you want to use a patched version, you could fork this branch and use directly in your project.

Just add in your composer.json a path to your GitHub repository:

    "require": {
        "torann/currency": "dev-master",
    },
    "repositories": [
        {
            "url": "https://github.com/klimenttoshkov/laravel-currency.git",
            "type": "git"
        }
    ]

semyonchetvertnyh avatar May 15 '20 15:05 semyonchetvertnyh

Already did that, thank you for the advice! — Kliment Toshkov [email protected]

On 15 May 2020, at 18:27, Semyon Chetvertnyh [email protected] wrote:

@klimenttoshkov https://github.com/klimenttoshkov I don't have plans to create and maintain my own version of the package. If you want to use a patched version, you could fork this branch and use directly in your project.

Just add in your composer.json a path to your GitHub repository:

"require": {
    "torann/currency": "dev-master",
},
"repositories": [
    {
        "url": "https://github.com/klimenttoshkov/laravel-currency.git",
        "type": "git"
    }
]

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Torann/laravel-currency/pull/114#issuecomment-629307962, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOIQGVJSNH62X4YGNC6TJ6DRRVNMLANCNFSM4GJHUVTQ.

klimenttoshkov avatar May 15 '20 15:05 klimenttoshkov

@Torann any update on this please

tariqdev avatar Sep 07 '20 11:09 tariqdev

@semyonchetvertnyh can you please help with this issue on Laravel 8:

Undefined property: Torann\Currency\SourceManager::$app

Thank you!

klimenttoshkov avatar Jan 30 '21 16:01 klimenttoshkov

@klimenttoshkov I guess property $app was renamed to $container in abstract Manager class of Laravel 8.

Just change $this->app to $this->container in your SourceManager class. It should help.

semyonchetvertnyh avatar Jan 30 '21 16:01 semyonchetvertnyh