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.

Sign up for GitHub

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

Jump to bottom

Admin pricing view #1510

Merged
merged 11 commits into from
Nov 9, 2016
Merged

Admin pricing view #1510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
03306bd
Add for_master and for_variant scopes to price model
Oct 7, 2016
9eec41f
Partition prices for master and variants in prices controller
Oct 7, 2016
01b6f85
Expand price table view to show prices for master seperately
Oct 7, 2016
31f43b0
Add I18n variables for static text in price table
Oct 7, 2016
106f1e6
Add table view for option types and values to admin prices view
Oct 7, 2016
cd5f6f9
Update prices controller spec to reflect new assigns
Oct 7, 2016
edcd253
Update pricing feature spec to reflect new assigns
Oct 7, 2016
431971c
Update changelog
Oct 11, 2016
471979f
Add fieldset legends to sections of admin price page
Oct 11, 2016
a793e5e
Remove options table view from admin prices view
Nov 7, 2016
bcff2f0
Render variants only if some exist prices on admin prices page
Nov 7, 2016
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@

* The `lastname` field on `Address` is now optional. [#1369](https://github.com/solidusio/solidus/pull/1369)

* The admin prices listings page now shows master and variant prices
seperately. This changes `@prices` to `@master_prices` and `@variant_prices` in prices_controller

* Removals

* Removed deprecated method `Spree::TaxRate.adjust` (not to be confused with
Expand Down
8 changes: 7 additions & 1 deletion backend/app/controllers/spree/admin/prices_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ def index
params[:q] ||= {}

@search = @product.prices.accessible_by(current_ability, :index).ransack(params[:q])
@prices = @search.result
@master_prices = @search.result
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing instance variable names in controllers can be hazardous if people overrode those views. This has not been around for long at all, and I do not suspect many people have used this instance variable name, so I won't ask for a deprecation warning, but maybe a short note in the ChangeLog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, noted in the ChangeLog.

.currently_valid
.for_master
.order(:variant_id, :country_iso, :currency)
.page(params[:page]).per(Spree::Config.admin_variants_per_page)
@variant_prices = @search.result
.currently_valid
.for_variant
.order(:variant_id, :country_iso, :currency)
.page(params[:page]).per(Spree::Config.admin_variants_per_page)
end
Expand Down
41 changes: 41 additions & 0 deletions backend/app/views/spree/admin/prices/_master_variant_table.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<div class="row">
<div class="col-xs-12">
<fieldset class="no-border-bottom <%= "no-border-top" if !variants %>">
<% if variants %>
<legend align="center"><%= I18n.t(:master_variant, scope: :spree) %> <%= admin_hint I18n.t(:master_variant, scope: :spree), I18n.t(:master_variant, scope: [:spree, :hints, "spree/price"]) %></legend>
<% end %>
<table class="index master_prices">
<colgroup>
<col style="width: 30%">
<col style="width: 30%">
<col style="width: 20%">
<col style="width: 20%">
</colgroup>
<thead data-hook="master_prices_header">
<tr>
<th><%= Spree::Price.human_attribute_name(:country) %></th>
<th><%= Spree::Price.human_attribute_name(:currency) %></th>
<th><%= Spree::Price.human_attribute_name(:amount) %></th>
<th class="actions"></th>
</tr>
</thead>
<% master_prices.each do |price| %>
<tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.deleted? %> <%= cycle('odd', 'even')%>">
<td><%= price.display_country %></td>
<td><%= price.currency %></td>
<td class="align-right"><%= price.money.to_html %></td>
<td class="actions">
<% if can?(:update, price) %>
<%= link_to_edit(price, :no_text => true) unless price.deleted? %>
<% end %>
<% if can?(:destroy, price) %>
&nbsp;
<%= link_to_delete(price, :no_text => true) unless price.deleted? %>
<% end %>
</td>
</tr>
<% end %>
</table>
</fieldset>
</div>
</div>
68 changes: 35 additions & 33 deletions backend/app/views/spree/admin/prices/_table.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,35 +1,37 @@
<%= paginate prices, theme: "solidus_admin" %>
<%= paginate variant_prices, theme: "solidus_admin" %>

<table class="index prices">
<thead data-hook="prices_header">
<tr>
<th><%= Spree::Variant.model_name.human %> </th>
<th><%= Spree::Price.human_attribute_name(:country) %></th>
<th><%= Spree::Price.human_attribute_name(:currency) %></th>
<th><%= Spree::Price.human_attribute_name(:amount) %></th>
<th class="actions"></th>
</tr>
</thead>
<fieldset class="no-border-bottom">
<legend align="center"><%= I18n.t(:variant_pricing, scope: :spree) %></legend>
<table class="index prices">
<thead data-hook="prices_header">
<tr>
<th><%= Spree::Variant.model_name.human %> </th>
<th><%= Spree::Price.human_attribute_name(:country) %></th>
<th><%= Spree::Price.human_attribute_name(:currency) %></th>
<th><%= Spree::Price.human_attribute_name(:amount) %></th>
<th class="actions"></th>
</tr>
</thead>
<tbody>
<% variant_prices.each do |price| %>
<tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.deleted? %> <%= cycle('odd', 'even')%>">
<td><%= price.variant.descriptive_name %></td>
<td><%= price.display_country %></td>
<td><%= price.currency %></td>
<td class="align-right"><%= price.money.to_html %></td>
<td class="actions">
<% if can?(:update, price) %>
<%= link_to_edit(price, :no_text => true) unless price.deleted? %>
<% end %>
<% if can?(:destroy, price) %>
&nbsp;
<%= link_to_delete(price, :no_text => true) unless price.deleted? %>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>
</fieldset>

<tbody>
<% prices.each do |price| %>
<tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.deleted? %> <%= cycle('odd', 'even')%>">
<td><%= price.variant.descriptive_name %></td>
<td><%= price.display_country %>
<td><%= price.currency %></td>
<td class="align-right"><%= price.money.to_html %></td>
<td class="actions">
<% if can?(:update, price) %>
<%= link_to_edit(price, :no_text => true) unless price.deleted? %>
<% end %>
<% if can?(:destroy, price) %>
&nbsp;
<%= link_to_delete(price, :no_text => true) unless price.deleted? %>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>

<%= paginate prices, theme: "solidus_admin" %>
<%= paginate variant_prices, theme: "solidus_admin" %>
3 changes: 2 additions & 1 deletion backend/app/views/spree/admin/prices/index.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,5 @@
</div>
<% end %>

<%= render 'table', prices: @prices %>
<%= render 'master_variant_table', master_prices: @master_prices, variants: @product.variants.any? %>
<%= render 'table', variant_prices: @variant_prices if @product.variants.any? %>
8 changes: 5 additions & 3 deletions backend/spec/controllers/spree/admin/prices_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
it 'assigns usable instance variables' do
subject
expect(assigns(:search)).to be_a(Ransack::Search)
expect(assigns(:prices)).to eq(product.prices)
expect(assigns(:variant_prices)).to eq(product.prices.for_variant)
expect(assigns(:master_prices)).to eq(product.prices.for_master)
expect(assigns(:product)).to eq(product)
end
end
Expand All @@ -32,8 +33,9 @@
it 'assigns usable instance variables' do
subject
expect(assigns(:search)).to be_a(Ransack::Search)
expect(assigns(:prices)).to eq(product.prices)
expect(assigns(:prices)).to include(variant.default_price)
expect(assigns(:variant_prices)).to eq(product.prices.for_variant)
expect(assigns(:master_prices)).to eq(product.prices.for_master)
expect(assigns(:variant_prices)).to include(variant.default_price)
expect(assigns(:product)).to eq(product)
end
end
Expand Down
3 changes: 1 addition & 2 deletions backend/spec/features/admin/products/pricing_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,11 @@
expect(page).to have_content("Prices")
end

within('table.prices') do
within('table.master_prices') do
expect(page).to have_content("$19.99")
expect(page).to have_content("USD")
expect(page).to have_content("34.56 ₽")
expect(page).to have_content("RUB")
expect(page).to have_content("Master")
expect(page).to have_content("Any Country")
expect(page).to have_content("Germany")
end
Expand Down
2 changes: 2 additions & 0 deletions core/app/models/spree/price.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class Price < Spree::Base
validates :country, presence: true, unless: -> { for_any_country? }

scope :currently_valid, -> { order("country_iso IS NULL, updated_at DESC") }
scope :for_master, -> { joins(:variant).where(spree_variants: { is_master: true }) }
scope :for_variant, -> { joins(:variant).where(spree_variants: { is_master: false }) }
scope :for_any_country, -> { where(country: nil) }
scope :with_default_attributes, -> { where(Spree::Config.default_pricing_options.desired_attributes) }

Expand Down
4 changes: 4 additions & 0 deletions core/config/locales/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,8 @@ en:
hints:
spree/price:
country: "This determines in what country the price is valid.<br/>Default: Any Country"
master_variant: "Changing master variant prices will not change variant prices below, but will be used to populate all new variants"
options: "These options are used to create variants in the variants table. They can be changed in the variants tab"
spree/product:
promotionable: "This determines whether or not promotions can apply to this product.<br/>Default: Checked"
shipping_category: "This determines what kind of shipping this product requires.<br/> Default: Default"
Expand Down Expand Up @@ -1337,6 +1339,7 @@ en:
manual_intervention_required: Manual intervention required
manage_variants: Manage Variants
master_price: Master Price
master_variant: Master Variant
match_choices:
all: All
none: None
Expand Down Expand Up @@ -1992,6 +1995,7 @@ en:
value: Value
variant: Variant
variant_placeholder: Choose a variant
variant_pricing: Variant Pricing
variant_properties: Variant Properties
variant_search: Variant Search
variant_search_placeholder: "SKU or Option Value"
Expand Down