spree-contrib / spree_multi_currency

Provides UI to allow configuring multiple currencies in Spree.
http://guides.spreecommerce.org
BSD 3-Clause "New" or "Revised" License
41 stars 117 forks source link

Add admin settings UI #39

Closed mbajur closed 9 years ago

mbajur commented 9 years ago

I'm not sure if that PR even makes sense but i was not able to display/handle those settings anywhere when on spree master branch so i've created a settings section for multi_currency settings.

To be honest, i'm a total spree newbie and probably there is already such functionality implemented but i just don't know how to enable it - so please feel free to let me know that i'm an idiot :)

zrzut ekranu 2015-04-10 o 10 55 51

mbajur commented 9 years ago

Oh, i've forgot to run hound and rubocop

mbajur commented 9 years ago

Ok, improvements of code style are done. I have also added help blocks to make it all more user friendly

image

futhr commented 9 years ago

+1 Very nice and clean PR.

JDutil commented 9 years ago

Nice work. Trying to merge now but hitting error on 3-0-stable:

/Users/JDutil/.rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/bundler/gems/spree-56ae9ba1dc44/core/lib/spree/testing_support/controller_requests.rb:32:in `block in <module:ControllerRequests>': undefined method `routes' for #<Class:0x007f99d415d338> (NoMethodError)

I've seen this recently on a couple extensions so not related to your work, which runs fine on master branch it's just 3-0-stable throwing a fit for some reason.

JDutil commented 9 years ago

Fixed it the:

  config.include Spree::TestingSupport::ControllerRequests

was missing type controller:

  config.include Spree::TestingSupport::ControllerRequests, type: :controller
mbajur commented 9 years ago

Oh, i've just realised that it kind of sucks that i'm not using i18n in the views/overrides. I think i will end up with another PR soon. Also, those .help-block styles shouldn't be here but i need to send a PR to the spree core to be able to remove that from here (help-blocks without these styles just looks really bad).