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

Add a store credit reasons Admin UI #2798

Merged
merged 9 commits into from
Jan 18, 2019
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
@@ -0,0 +1,8 @@
# frozen_string_literal: true

module Spree
module Admin
class StoreCreditReasonsController < ResourceController
end
end
end
22 changes: 11 additions & 11 deletions backend/app/controllers/spree/admin/store_credits_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ module Admin
class StoreCreditsController < ResourceController
belongs_to 'spree/user', model_class: Spree.user_class
before_action :load_categories, only: [:new]
before_action :load_update_reasons, only: [:edit_amount, :edit_validity]
before_action :ensure_update_reason, only: [:update_amount, :invalidate]
before_action :load_reasons, only: [:edit_amount, :edit_validity]
before_action :ensure_store_credit_reason, only: [:update_amount, :invalidate]

helper Spree::Admin::StoreCreditEventsHelper

Expand Down Expand Up @@ -50,7 +50,7 @@ def update
def update_amount
@store_credit = @user.store_credits.find(params[:id])
amount = params.require(:store_credit).require(:amount)
if @store_credit.update_amount(amount, @update_reason, try_spree_current_user)
if @store_credit.update_amount(amount, @store_credit_reason, try_spree_current_user)
flash[:success] = flash_message_for(@store_credit, :successfully_updated)
redirect_to admin_user_store_credit_path(@user, @store_credit)
else
Expand All @@ -60,7 +60,7 @@ def update_amount

def invalidate
@store_credit = @user.store_credits.find(params[:id])
if @store_credit.invalidate(@update_reason, try_spree_current_user)
if @store_credit.invalidate(@store_credit_reason, try_spree_current_user)
redirect_to admin_user_store_credit_path(@user, @store_credit)
else
render_edit_page
Expand All @@ -78,18 +78,18 @@ def collection
@collection = super.reverse_order
end

def load_update_reasons
@update_reasons = Spree::StoreCreditUpdateReason.all.order(:name)
def load_reasons
@store_credit_reasons = Spree::StoreCreditReason.active.order(:name)
end

def load_categories
@credit_categories = Spree::StoreCreditCategory.all.order(:name)
end

def ensure_update_reason
@update_reason = Spree::StoreCreditUpdateReason.find_by(id: params[:update_reason_id])
unless @update_reason
@store_credit.errors.add(:base, t('spree.admin.store_credits.errors.update_reason_required'))
def ensure_store_credit_reason
@store_credit_reason = Spree::StoreCreditReason.find_by(id: params[:store_credit_reason_id])
unless @store_credit_reason
@store_credit.errors.add(:base, t('spree.admin.store_credits.errors.store_credit_reason_required'))
render_edit_page
end
end
Expand All @@ -103,7 +103,7 @@ def render_edit_page
translation_key = 'invalidate'
end

load_update_reasons
load_reasons
flash[:error] = "#{t("spree.admin.store_credits.unable_to_#{translation_key}")}: #{@store_credit.errors.full_messages.join(', ')}"
render(template) && return
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
<% if can?(:display, Spree::AdjustmentReason) %>
<%= settings_tab_item plural_resource_name(Spree::AdjustmentReason), spree.admin_adjustment_reasons_path %>
<% end %>

<% if can?(:display, Spree::StoreCreditReason) %>
<%= settings_tab_item plural_resource_name(Spree::StoreCreditReason), spree.admin_store_credit_reasons_path %>
<% end %>
</ul>
</nav>
<% end %>
15 changes: 15 additions & 0 deletions backend/app/views/spree/admin/store_credit_reasons/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<%= render 'spree/admin/shared/settings_checkout_tabs' %>

<% admin_breadcrumb t('spree.settings') %>
<% admin_breadcrumb t('spree.admin.tab.checkout') %>
<% admin_breadcrumb link_to plural_resource_name(Spree::StoreCreditReason), spree.admin_store_credit_reasons_path %>
<% admin_breadcrumb @object.name %>

<%= render partial: 'spree/shared/error_messages', locals: { target: @object } %>

<%= form_for [:admin, @object] do |f| %>
<fieldset class='no-border-top' data-hook='edit_store_credit_reason'>
<%= render partial: 'spree/admin/store_credit_reasons/shared/form', locals: { f: f } %>
<%= render partial: 'spree/admin/shared/edit_resource_links' %>
</fieldset>
<% end %>
56 changes: 56 additions & 0 deletions backend/app/views/spree/admin/store_credit_reasons/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<%= render 'spree/admin/shared/settings_checkout_tabs' %>

<% admin_breadcrumb(t('spree.settings')) %>
<% admin_breadcrumb(t('spree.admin.tab.checkout')) %>
<% admin_breadcrumb(plural_resource_name(Spree::StoreCreditReason)) %>

<% content_for :page_actions do %>
<ul class='actions inline-menu'>
<li>
<%= link_to t('spree.new_adjustment_reason'), new_object_url, id: 'admin_new_named_type', class: 'btn btn-primary' %>
</li>
</ul>
<% end %>

<% if @collection.any? %>
<table class='index' id='listing_store_credit_reasons'>
<colgroup>
<col style='width: 65%' />
<col style='width: 20%' />
<col style='width: 15%' />
</colgroup>
<thead>
<tr data-hook='store_credit_reasons_header'>
<th><%= Spree::StoreCreditReason.human_attribute_name(:name) %></th>
<th><%= Spree::StoreCreditReason.human_attribute_name(:state) %></th>
<th class='actions'></th>
</tr>
</thead>
<tbody>
<% @collection.each do |store_credit_reason| %>
<tr id='<%= spree_dom_id store_credit_reason %>' data-hook='store_credit_reason_row'>
<td>
<%= store_credit_reason.name %>
</td>
<td>
<span class="pill pill-<%= store_credit_reason.active? ? 'active' : 'inactive' %>">
<%= t(store_credit_reason.active? ? :active : :inactive, scope: 'spree') %>
</span>
</td>
<td class='actions'>
<%= link_to_edit store_credit_reason, no_text: true %>
<% if can?(:destroy, store_credit_reason) %>
<%= link_to_delete store_credit_reason, no_text: true %>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>
<% else %>
<div class='no-objects-found'>
<%= render 'spree/admin/shared/no_objects_found',
resource: Spree::StoreCreditReason,
new_resource_url: new_object_url %>
</div>
<% end %>
18 changes: 18 additions & 0 deletions backend/app/views/spree/admin/store_credit_reasons/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<%= render 'spree/admin/shared/settings_checkout_tabs' %>

<% admin_breadcrumb(t('spree.settings')) %>
<% admin_breadcrumb(t('spree.admin.tab.checkout')) %>
<% admin_breadcrumb(link_to plural_resource_name(Spree::StoreCreditReason), spree.admin_store_credit_reasons_path) %>
<% admin_breadcrumb(t('spree.new_store_credit_reason')) %>

<% content_for :page_actions do %>
<% end %>

<%= render partial: 'spree/shared/error_messages', locals: { target: @object } %>

<%= form_for [:admin, @object] do |f| %>
<fieldset class='no-border-top'>
<%= render partial: 'spree/admin/store_credit_reasons/shared/form', locals: { f: f } %>
<%= render partial: 'spree/admin/shared/new_resource_links' %>
</fieldset>
<% end %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<div data-hook='admin_named_type_form_fields' class='row'>
<div class='col-3'>
<%= f.field_container :name do %>
<%= f.label :name, class: 'required' %><br />
<%= f.text_field :name, class: 'fullwidth' %>
<% end %>

<div class='checkbox'>
<label>
<%= f.check_box :active %>
<%= t('spree.active') %>
</label>
</div>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<%= f.field_container :reason do %>
<%= f.label :store_credit_reason, t('spree.reason'), class: 'required' %>
<%= select_tag :store_credit_reason_id, options_from_collection_for_select(@store_credit_reasons, :id, :name),
include_blank: t('spree.choose_reason'), class: 'custom-select fullwidth',
placeholder: t('spree.admin.store_credits.select_amount_store_credit_reason') %>
<%= f.error_message_on :store_credit_reason %>
<% end %>

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<% end %>
</div>
<div class="col-6">
<%= render 'update_reason_field', f: f %>
<%= render 'store_credit_reason_field', f: f %>
</div>
</div>
<%= render 'spree/admin/shared/edit_resource_links',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<legend align="center"><%= t('spree.admin.store_credits.invalidate_store_credit') %></legend>
<div data-hook="admin_store_credit_form_fields" class="row">
<div class="col-12">
<%= render partial: 'update_reason_field', locals: { f: f } %>
<%= render partial: 'store_credit_reason_field', locals: { f: f } %>
</div>
</div>
<div class="form-buttons filter-actions actions" data-hook="buttons">
Expand Down
4 changes: 2 additions & 2 deletions backend/app/views/spree/admin/store_credits/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
<th><%= t('spree.admin.store_credits.created_by') %></th>
<th><%= Spree::StoreCreditEvent.human_attribute_name(:user_total_amount) %></th>
<th><%= Spree::StoreCreditEvent.human_attribute_name(:amount_remaining) %></th>
<th><%= Spree::StoreCreditUpdateReason.human_attribute_name(:name) %></th>
<th><%= Spree::StoreCreditReason.human_attribute_name(:name) %></th>
</tr>
</thead>
<tbody>
Expand All @@ -98,7 +98,7 @@
<td><%= store_credit_event_originator_link(event) %></td>
<td><%= event.display_user_total_amount %></td>
<td><%= event.display_remaining_amount %></td>
<td><%= event.update_reason.try!(:name) %></td>
<td><%= event.store_credit_reason.try!(:name) %></td>
</tr>
<% end %>
</tbody>
Expand Down
1 change: 1 addition & 0 deletions backend/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@
resources :adjustment_reasons, except: [:show, :destroy]
resources :refund_reasons, except: [:show, :destroy]
resources :return_reasons, except: [:show, :destroy]
resources :store_credit_reasons, except: [:show]

resources :shipping_methods
resources :shipping_categories
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
require 'spec_helper'

shared_examples "update reason loader" do
it "sets the update_reasons variable to a list of categories sorted by category name " do
expect(assigns(:update_reasons)).to eq [update_reason]
it "sets the store_credit_reasons variable to a list of categories sorted by category name " do
expect(assigns(:store_credit_reasons)).to eq [store_credit_reason]
end
end

Expand All @@ -16,11 +16,11 @@

let!(:b_credit_category) { create(:store_credit_category, name: "B category") }
let!(:a_credit_category) { create(:store_credit_category, name: "A category") }
let!(:update_reason) { create(:store_credit_update_reason) }
let!(:store_credit_reason) { create(:store_credit_reason) }

describe "#show" do
let!(:store_credit) { create(:store_credit, user: user, category: a_credit_category) }
let!(:event) { create(:store_credit_auth_event, store_credit: store_credit, created_at: 5.days.ago) }
let!(:event) { create(:store_credit_auth_event, store_credit: store_credit, created_at: 5.days.ago) }

before { get :show, params: { user_id: user.id, id: store_credit.id } }

Expand Down Expand Up @@ -173,12 +173,12 @@
describe "#update_amount" do
let(:original_amount) { 100.0 }
let!(:store_credit) { create(:store_credit, user: user, amount: original_amount) }
let!(:update_reason) { create(:store_credit_update_reason) }
let!(:store_credit_reason) { create(:store_credit_reason) }
let(:parameters) do
{
user_id: user.id,
id: store_credit.id,
update_reason_id: update_reason.id,
store_credit_reason_id: store_credit_reason.id,
store_credit: {
amount: updated_amount
}
Expand Down Expand Up @@ -271,7 +271,7 @@
{
user_id: user.id,
id: store_credit.id,
update_reason_id: update_reason.id
store_credit_reason_id: store_credit_reason.id
}
end

Expand Down
4 changes: 2 additions & 2 deletions backend/spec/features/admin/store_credits_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@

describe "updating store credit" do
let(:updated_amount) { "99.0" }
let!(:update_reason) { create(:store_credit_update_reason) }
let!(:store_credit_reason) { create(:store_credit_reason) }

before do
visit spree.admin_path
Expand All @@ -74,7 +74,7 @@
click_link "Change amount"
expect(page).to have_content 'Editing store credit amount'
page.fill_in 'store_credit_amount', with: updated_amount
page.select update_reason.name, from: 'update_reason_id'
page.select store_credit_reason.name, from: 'store_credit_reason_id'
click_button "Update"
expect(page.find('#sc-detail-table')).to have_content "$99.00"
expect(store_credit.reload.amount.to_f).to eq updated_amount.to_f
Expand Down
4 changes: 2 additions & 2 deletions backend/spec/helpers/admin/store_credit_events_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@
end

context "originator is not specifically handled" do
let(:originator) { create(:store_credit_update_reason) }
let(:originator) { create(:store_credit) }

it "raises an error" do
expect { subject }.to raise_error(RuntimeError, "Unexpected originator type Spree::StoreCreditUpdateReason")
expect { subject }.to raise_error(RuntimeError, "Unexpected originator type Spree::StoreCredit")
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions core/app/models/spree/store_credit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Spree::StoreCredit < Spree::PaymentSource
before_destroy :validate_no_amount_used
validate :validate_no_amount_used, if: :discarded?

attr_accessor :action, :action_amount, :action_originator, :action_authorization_code, :update_reason
attr_accessor :action, :action_amount, :action_originator, :action_authorization_code, :store_credit_reason
Copy link
Member

Choose a reason for hiding this comment

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

I don't think store_credit_reason here is needed anymore


extend Spree::DisplayMoney
money_methods :amount, :amount_used, :amount_authorized
Expand Down Expand Up @@ -176,15 +176,15 @@ def update_amount(amount, reason, user_performing_update)
self.amount = amount
self.action_amount = self.amount - previous_amount
self.action = ADJUSTMENT_ACTION
self.update_reason = reason
self.store_credit_reason = reason
self.action_originator = user_performing_update
save
end

def invalidate(reason, user_performing_invalidation)
if invalidateable?
self.action = INVALIDATE_ACTION
self.update_reason = reason
self.store_credit_reason = reason
self.action_originator = user_performing_invalidation
self.invalidated_at = Time.current
save
Expand Down Expand Up @@ -241,7 +241,7 @@ def store_event
amount_remaining: amount_remaining,
user_total_amount: user.available_store_credit_total(currency: currency),
originator: action_originator,
update_reason: update_reason
store_credit_reason: store_credit_reason
})
end

Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/store_credit_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ class StoreCreditEvent < Spree::Base

belongs_to :store_credit
belongs_to :originator, polymorphic: true
belongs_to :update_reason, class_name: "Spree::StoreCreditUpdateReason"
belongs_to :store_credit_reason, class_name: 'Spree::StoreCreditReason', inverse_of: :store_credit_events

validates_presence_of :update_reason, if: :action_requires_reason?
validates_presence_of :store_credit_reason, if: :action_requires_reason?

NON_EXPOSED_ACTIONS = [Spree::StoreCredit::ELIGIBLE_ACTION, Spree::StoreCredit::AUTHORIZE_ACTION]

Expand Down
11 changes: 11 additions & 0 deletions core/app/models/spree/store_credit_reason.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

class Spree::StoreCreditReason < Spree::Base
include Spree::NamedType

has_many :store_credit_events, inverse_of: :store_credit_reason

validates :name, presence: true, uniqueness: { case_sensitive: false, allow_blank: true }

scope :active, -> { where(active: true) }
end
4 changes: 0 additions & 4 deletions core/app/models/spree/store_credit_update_reason.rb

This file was deleted.

Loading