Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

list for currency selection in general settings #329

Merged
merged 7 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ defmodule AdminAppWeb.ShippingPolicyController do

alias Snitch.Core.Tools.MultiTenancy.Repo
alias Snitch.Data.Schema.ShippingCategory
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel
alias Snitch.Data.Model.ShippingCategory, as: ScModel

@defaults Application.get_env(:snitch_core, :defaults_module)
Expand Down Expand Up @@ -66,7 +67,7 @@ defmodule AdminAppWeb.ShippingPolicyController do
end

def set_shipping_amount(rules) do
{:ok, currency} = @defaults.fetch(:currency)
currency = GCModel.fetch_currency()

Enum.map(rules, fn rule ->
amount = rule["shipping_cost"] || "0.00"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
<div class = "list-container" >
<h4 class="p-3 m-0">GENERAL SETTINGS</h4>
<div class = "card col-12" >
<%= form_for @changeset, @action, [as: :settings, id: "settings-form", method: @method, multipart: true], fn f -> %>

<%= form_for @changeset, @action, [as: :settings, id: "settings-form", method: @method, multipart: true], fn f -> %>
<div class="form-group row ">
<%= input f, :name, nil, is_horizontal: true, description: "Name for the store." %>
</div>
Expand All @@ -22,7 +21,15 @@
<%= input f, :hosted_payment_url, nil, is_horizontal: true, description: "Payment URL for the store." %>
</div>
<div class="form-group row ">
<%= input f, :currency, nil, is_horizontal: true, description: "Default currency for the store." %>
<label class="col-sm-3 col-form-label">
<div class="label">
Currency
</div>
<div class="label-txt">Default currency of the store</div>
</label>
<div class="col-sm-9 input-group">
<%= select f, :currency, get_currency(), value: f.data.currency, class: "form-control" %>
</div>
</div>
<%= if @changeset.data.image != nil do %>
<div class="img-wrap col-3 p-1">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
<div class="col-sm-9 input-group">
<%= text_input :selling_price, :amount, value: get_amount(f.data.selling_price), class: "form-control", name: "product[selling_price][amount]" %>
<div class="input-group-append">
<%= select :selling_price, :currency, get_currency(),value: get_currency_value(f.data.selling_price), class: "form-control", name: "product[selling_price][currency]" %>
Copy link
Member

Choose a reason for hiding this comment

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

@jyotigautam
As we will be using currency from GeneralSetting. Does showing currency selection to user makes sense?

Currency for product prices can be set in changeset.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here the currency selected only shows one value i.e, from the general settings or "usd" if no general setting is set. So, the user can't choose from this list.

Copy link
Member

Choose a reason for hiding this comment

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

@jyotigautam @SagarKarwande I would say we remove the select altogether and just show a label of the currency. Just like it is done with etsy store. Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, there is no point of showing it as select input when we have just one option.

<%= text_input :selling_price, :currency, value: get_currency_value(), class: "form-control", name: "product[selling_price][currency]", readonly: true %>
</div>
</div>
</div>
Expand All @@ -66,7 +66,7 @@
<div class="col-sm-9 input-group">
<%= text_input :max_retail_price, :amount, value: get_amount(f.data.max_retail_price), class: "form-control", name: "product[max_retail_price][amount]" %>
<div class="input-group-append">
<%= select :max_retail_price, :currency, get_currency(),value: get_currency_value(f.data.max_retail_price), class: "form-control", name: "product[max_retail_price][currency]" %>
<%= text_input :max_retail_price, :currency, value: get_currency_value(), class: "form-control", name: "product[max_retail_price][currency]", readonly: true%>
</div>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
defmodule AdminAppWeb.GeneralSettingsView do
use AdminAppWeb, :view
alias Snitch.Data.Model.ProductBrand
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel

def get_image_url(image, general_settings) do
ProductBrand.image_url(image.name, general_settings)
end

def get_currency() do
GCModel.get_currency_list()
end
end
10 changes: 7 additions & 3 deletions apps/admin_app/lib/admin_app_web/views/layout_view.ex
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
defmodule AdminAppWeb.LayoutView do
use AdminAppWeb, :view
alias AdminAppWeb.Guardian
alias Snitch.Data.Schema.{GeneralConfiguration, Taxonomy}
alias Snitch.Data.Schema.GeneralConfiguration, as: GC
alias Snitch.Data.Schema.Taxonomy
alias Snitch.Core.Tools.MultiTenancy.Repo
alias AdminAppWeb.Helpers

Expand Down Expand Up @@ -35,8 +36,11 @@ defmodule AdminAppWeb.LayoutView do
|> Enum.at(0)
end

def check_general_settings() do
Repo.all(GeneralConfiguration) |> List.first()
def check_general_settings do
case Repo.one(GC) do
nil -> false
_ -> true
end
end

def get_default_taxonomy() do
Expand Down
13 changes: 4 additions & 9 deletions apps/admin_app/lib/admin_app_web/views/product_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ defmodule AdminAppWeb.ProductView do
alias Snitch.Core.Tools.MultiTenancy.Repo
alias Snitch.Domain.Taxonomy

alias Snitch.Data.Model.{Product, ProductProperty, Property, StockItem}
alias Snitch.Data.Model.{GeneralConfiguration, Product, ProductProperty, Property, StockItem}
alias Snitch.Data.Schema
import Ecto.Query

@currencies ["USD", "INR"]
@dummy_image_url "/images/empty-img.png"
@search_keys ["rummage", "search", "state", "search_term"]
@sort_field_keys ["rummage", "sort", "field"]
Expand Down Expand Up @@ -76,17 +75,13 @@ defmodule AdminAppWeb.ProductView do
conn.params["taxon"]
end

def get_currency_value(nil) do
@currencies |> List.first()
end

def get_currency_value(money) do
money.currency
def get_currency_value() do
GeneralConfiguration.fetch_currency()
end

# TODO This needs to fetched from config
def get_currency() do
@currencies
[GeneralConfiguration.fetch_currency()]
end

def get_image_url(image, product) do
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
defmodule SnitchApiWeb.HostedPaymentController do
use SnitchApiWeb, :controller
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel
alias SnitchApi.Payment.HostedPayment
alias SnitchPayments
alias SnitchPayments.Gateway.{PayuBiz, RazorPay, Stripe}
Expand Down Expand Up @@ -39,7 +40,8 @@ defmodule SnitchApiWeb.HostedPaymentController do

def stripe_purchase(conn, params) do
## TODO get the currency set for store here and use that.
amount = Money.new!(:USD, params["amount"])
currency = GCModel.fetch_currency()
amount = Money.new!(currency, params["amount"])
preferences = HostedPayment.get_payment_preferences(params["payment_method_id"])
secret = preferences[:credentials]["secret_key"]
request_params = stripe_params_setup(params)
Expand Down
15 changes: 15 additions & 0 deletions apps/snitch_core/lib/core/data/model/general_configuration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,21 @@ defmodule Snitch.Data.Model.GeneralConfiguration do
alias Snitch.Tools.Helper.ImageUploader
alias Ecto.Multi

@currency_list ["USD", "INR", "GDP", "EUR"]

@spec fetch_currency() :: String.t()
def fetch_currency do
case Repo.one(GC) do
nil -> "USD"
gc -> gc.currency
end
end

@spec get_currency_list() :: List.t()
def get_currency_list do
@currency_list
end

Copy link
Member

Choose a reason for hiding this comment

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

  def fetch_currency do
    case Repo.one(GC)
      nil -> "USD"
      gc -> gc.currency
    end
  end

Copy link
Member

Choose a reason for hiding this comment

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

This is looking better.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is more succinct way.

@spec build_general_configuration(map) :: Ecto.Changeset.t()
def build_general_configuration(attrs \\ %{}) do
%GC{} |> GC.create_changeset(attrs)
Expand Down
12 changes: 6 additions & 6 deletions apps/snitch_core/lib/core/domain/order/order.ex
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ defmodule Snitch.Domain.Order do
import Ecto.Query
alias Snitch.Data.Schema.{Order, Package, Payment}
alias Snitch.Data.Model.Product
alias Snitch.Tools.Defaults
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel

@spec validate_change(Ecto.Changeset.t()) :: Ecto.Changeset.t()
def validate_change(%{valid?: false} = changeset), do: changeset
Expand All @@ -38,7 +38,7 @@ defmodule Snitch.Domain.Order do
"""
@spec payments_total(Order.t(), String.t()) :: Money.t()
def payments_total(order, payment_state) do
{:ok, currency} = Defaults.fetch(:currency)
currency = GCModel.fetch_currency()
Copy link
Member

Choose a reason for hiding this comment

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

I would really prefer to have {:ok, currency} or {:error, msg} rather than returning just as currency. this way matching becomes easier. @SagarKarwande thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

We can use just the return value since we will be using a Repo.one query, which either gives a value or nil.

Copy link
Member

Choose a reason for hiding this comment

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

Yes returning tuples with success and error will help.

@ashish173 As per the definition of GeneralConfiguration.fetch_currency() we would never be able to get the error case.

Copy link
Member

Choose a reason for hiding this comment

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

That's okay @SagarKarwande for now. Since we always start/provision a store with a currency. I think now I agree with what @pkrawat1 is saying.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But there would be no {:error, msg} scenario, it will always return a string.


query =
from(
Expand Down Expand Up @@ -76,7 +76,7 @@ defmodule Snitch.Domain.Order do

def total_amount(%Order{} = order) do
order = Repo.preload(order, [:line_items, packages: :items])
{:ok, currency} = Defaults.fetch(:currency)
currency = GCModel.fetch_currency()

total =
Money.add!(
Expand All @@ -88,7 +88,7 @@ defmodule Snitch.Domain.Order do
end

def line_item_total(order) do
{:ok, currency} = Defaults.fetch(:currency)
currency = GCModel.fetch_currency()

order.line_items
|> Enum.reduce(Money.new(currency, 0), fn line_item, acc ->
Expand Down Expand Up @@ -130,7 +130,7 @@ defmodule Snitch.Domain.Order do
end

def total_tax(packages) do
{:ok, currency} = Defaults.fetch(:currency)
currency = GCModel.fetch_currency()

packages
|> Enum.reduce(Money.new(currency, 0), fn %{
Expand All @@ -146,7 +146,7 @@ defmodule Snitch.Domain.Order do
end

def shipping_total(packages) do
{:ok, currency} = Defaults.fetch(:currency)
currency = GCModel.fetch_currency()

packages
|> Enum.reduce(Money.new(currency, 0), fn %{cost: cost}, acc ->
Expand Down
5 changes: 4 additions & 1 deletion apps/snitch_core/lib/core/domain/package.ex
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ defmodule Snitch.Domain.Package do
alias Snitch.Domain.ShippingCalculator
alias Snitch.Tools.Money, as: MoneyTools
alias Snitch.Data.Model.StockItem
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel

@doc """
Saves
Expand All @@ -23,8 +24,10 @@ defmodule Snitch.Domain.Package do
# if we can't find the selected shipping method, we must force the
# Packge.update to fail
# Eventually replace with some nice API contract/validator.
currency = GCModel.fetch_currency()

shipping_method =
Enum.find(package.shipping_methods, %{cost: Money.zero(:INR), id: nil}, fn %{id: id} ->
Enum.find(package.shipping_methods, %{cost: Money.zero(currency), id: nil}, fn %{id: id} ->
id == shipping_method_id
end)

Expand Down
5 changes: 2 additions & 3 deletions apps/snitch_core/lib/core/domain/shipping_calculator.ex
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ defmodule Snitch.Domain.ShippingCalculator do

alias Snitch.Core.Tools.MultiTenancy.Repo
alias Snitch.Domain.Order, as: OrderDomain
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel
alias Snitch.Data.Schema.{Order, Package}

@defaults Application.get_env(:snitch_core, :defaults_module)

@doc """
Returns the `shipping_cost` for a `package`.

Expand Down Expand Up @@ -52,7 +51,7 @@ defmodule Snitch.Domain.ShippingCalculator do

active_rules = get_category_active_rules(package.shipping_category)

{:ok, currency_code} = @defaults.fetch(:currency)
currency_code = GCModel.fetch_currency()
cost = Money.new!(currency_code, 0)

# The piping here is in the order of priority,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ defmodule Snitch.Tools.OrderEmail do
user_email = order.user.email

logo = if general_config.image != nil, do: general_config.image.name, else: nil

mail_template =
order_email(%{
order: order,
Expand All @@ -40,6 +41,7 @@ defmodule Snitch.Tools.OrderEmail do
})

store_name = general_config.name

email =
new_email()
|> to(user_email)
Expand Down
18 changes: 6 additions & 12 deletions apps/snitch_core/lib/core/tools/money.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ defmodule Snitch.Tools.Money do
@moduledoc """
Some (weak) helpers to work with zeroes and `Money.t`.
"""

@defaults Application.get_env(:snitch_core, :defaults_module)
alias Snitch.Data.Model.GeneralConfiguration, as: GCModel

@doc """
Returns the zero `Money.t` with `currency`.
Expand All @@ -20,10 +19,8 @@ defmodule Snitch.Tools.Money do
def zero(currency \\ nil)

def zero(nil) do
case @defaults.fetch(:currency) do
{:ok, default_currency} -> Money.zero(default_currency)
error -> error
end
currency = GCModel.fetch_currency()
Money.zero(currency)
end

def zero(currency) when is_atom(currency) or is_binary(currency) do
Expand All @@ -34,8 +31,7 @@ defmodule Snitch.Tools.Money do
Returns the zero `Money.t` with `currency`.

If `currency` is not passed,
* attempts to fetch default currency from application config.
* if default currency is not set, raises a `Money.UnkownCurrencyError`
* attempts to fetch default currency from the general settings.

## Note
Makes use of `Money.new!/2` and this function can `raise`.
Expand All @@ -44,10 +40,8 @@ defmodule Snitch.Tools.Money do
def zero!(currency \\ nil)

def zero!(nil) do
case @defaults.fetch(:currency) do
{:ok, default_currency} -> Money.new!(0, default_currency)
{:error, msg} -> raise(RuntimeError, msg)
end
currency = GCModel.fetch_currency()
Money.zero(currency)
end

def zero!(currency) when is_atom(currency) or is_binary(currency) do
Expand Down
9 changes: 9 additions & 0 deletions apps/snitch_core/priv/repo/seed/general_configuration.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
defmodule Snitch.Seed.GeneralConfiguration do
alias Snitch.Data.Schema.GeneralConfiguration, as: GCSchema
alias Snitch.Core.Tools.MultiTenancy.Repo

def seed!() do
Repo.delete_all(GCSchema)
%GCSchema{currency: "USD"} |> Repo.insert()
end
end
2 changes: 0 additions & 2 deletions apps/snitch_core/priv/repo/seed/option_types.ex
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ defmodule Snitch.Seed.OptionType do
alias Snitch.Core.Tools.MultiTenancy.Repo
alias Snitch.Data.Schema.OptionType

@base_path Application.app_dir(:snitch_core, "priv/repo/demo/demo_data")

def seed!() do
create_option_type("size", "Size")
create_option_type("color", "Color")
Expand Down
4 changes: 3 additions & 1 deletion apps/snitch_core/priv/repo/seed/seeds.exs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
alias Snitch.Core.Tools.MultiTenancy.Repo

alias Snitch.Seed.{
GeneralConfiguration,
CountryState,
PaymentMethods,
OptionType,
Expand All @@ -33,9 +34,10 @@ alias Snitch.Tools.Helper.Taxonomy, as: TaxonomyHelper

variant_count = 9

# seeds general settings for the store.
GeneralConfiguration.seed!()
# seeds the taxonomy
# Taxonomy.seed()

# seeds countries and states entity
Repo.transaction(fn ->
CountryState.seed_countries!()
Expand Down
Loading