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 Api::CreditCardsController#update endpoint #208

Closed
Show file tree
Hide file tree
Changes from 2 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
19 changes: 18 additions & 1 deletion api/app/controllers/spree/api/credit_cards_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
module Spree
module Api
class CreditCardsController < Spree::Api::BaseController
before_action :user
before_action :user, only: [:index]
before_action :find_credit_card, only: [:update]

def index
@credit_cards = user
Expand All @@ -12,6 +13,14 @@ def index
respond_with(@credit_cards)
end

def update
if @credit_card.update_attributes(credit_card_params)
respond_with(@credit_card, default_template: :show)
else
invalid_resource!(@credit_card)
end
end

private

def user
Expand All @@ -20,6 +29,14 @@ def user
end
end

def find_credit_card
@credit_card = Spree::CreditCard.find(params[:id])
authorize! :update, @credit_card
end

def credit_card_params
params.require(:credit_card).permit(permitted_credit_card_attributes)
end
end
end
end
2 changes: 2 additions & 0 deletions api/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@
resources :credit_cards, only: [:index]
end

resources :credit_cards, only: [:update]

resources :properties
resources :stock_locations do
resources :stock_movements
Expand Down
136 changes: 84 additions & 52 deletions api/spec/controllers/spree/api/credit_cards_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,78 +2,110 @@

module Spree
describe Api::CreditCardsController, :type => :controller do
render_views
describe '#index' do
render_views

let!(:admin_user) do
user = Spree.user_class.new(:email => "spree@example.com", :id => 1)
user.generate_spree_api_key!
allow(user).to receive(:has_spree_role?).with('admin').and_return(true)
user
end

let!(:normal_user) do
user = Spree.user_class.new(:email => "spree2@example.com", :id => 2)
user.generate_spree_api_key!
user
end
let!(:admin_user) do
user = Spree.user_class.new(:email => "spree@example.com", :id => 1)
user.generate_spree_api_key!
allow(user).to receive(:has_spree_role?).with('admin').and_return(true)
user
end

let!(:card) { create(:credit_card, :user_id => admin_user.id, gateway_customer_profile_id: "random") }
let!(:normal_user) do
user = Spree.user_class.new(:email => "spree2@example.com", :id => 2)
user.generate_spree_api_key!
user
end

before do
stub_authentication!
end
let!(:card) { create(:credit_card, :user_id => admin_user.id, gateway_customer_profile_id: "random") }

it "the user id doesn't exist" do
api_get :index, user_id: 1000
expect(response.status).to eq(404)
end
before do
stub_authentication!
end

context "calling user is in admin role" do
let(:current_api_user) do
user = admin_user
user
it "the user id doesn't exist" do
api_get :index, user_id: 1000
expect(response.status).to eq(404)
end

it "no credit cards exist for user" do
api_get :index, user_id: normal_user.id
context "calling user is in admin role" do
let(:current_api_user) do
user = admin_user
user
end

expect(response.status).to eq(200)
expect(json_response["pages"]).to eq(0)
it "no credit cards exist for user" do
api_get :index, user_id: normal_user.id

expect(response.status).to eq(200)
expect(json_response["pages"]).to eq(0)
end

it "can view all credit cards for user" do
api_get :index, user_id: current_api_user.id

expect(response.status).to eq(200)
expect(json_response["pages"]).to eq(1)
expect(json_response["current_page"]).to eq(1)
expect(json_response["credit_cards"].length).to eq(1)
expect(json_response["credit_cards"].first["id"]).to eq(card.id)
end
end

it "can view all credit cards for user" do
api_get :index, user_id: current_api_user.id
context "calling user is not in admin role" do
let(:current_api_user) do
user = normal_user
user
end

let!(:card) { create(:credit_card, :user_id => normal_user.id, gateway_customer_profile_id: "random") }

it "can not view user" do
api_get :index, user_id: admin_user.id

expect(response.status).to eq(404)
end

expect(response.status).to eq(200)
expect(json_response["pages"]).to eq(1)
expect(json_response["current_page"]).to eq(1)
expect(json_response["credit_cards"].length).to eq(1)
expect(json_response["credit_cards"].first["id"]).to eq(card.id)
it "can view own credit cards" do
api_get :index, user_id: normal_user.id

expect(response.status).to eq(200)
expect(json_response["pages"]).to eq(1)
expect(json_response["current_page"]).to eq(1)
expect(json_response["credit_cards"].length).to eq(1)
expect(json_response["credit_cards"].first["id"]).to eq(card.id)
end
end
end

context "calling user is not in admin role" do
let(:current_api_user) do
user = normal_user
user
end
describe '#update' do
let(:credit_card) { create(:credit_card, name: 'Joe Shmoe', user: credit_card_user) }
let(:credit_card_user) { create(:user) }

let!(:card) { create(:credit_card, :user_id => normal_user.id, gateway_customer_profile_id: "random") }
before do
stub_authentication!
end

it "can not view user" do
api_get :index, user_id: admin_user.id
context 'when the user is authorized' do
let(:current_api_user) { credit_card_user }

expect(response.status).to eq(404)
it 'updates the credit card' do
expect {
api_put :update, id: credit_card.to_param, credit_card: {name: 'Jordan Brough'}
}.to change {
credit_card.reload.name
}.from('Joe Shmoe').to('Jordan Brough')
end
end

it "can view own credit cards" do
api_get :index, user_id: normal_user.id
context 'when the user is not authorized' do
let(:current_api_user) { create(:user) }

expect(response.status).to eq(200)
expect(json_response["pages"]).to eq(1)
expect(json_response["current_page"]).to eq(1)
expect(json_response["credit_cards"].length).to eq(1)
expect(json_response["credit_cards"].first["id"]).to eq(card.id)
it 'rejects the request' do
api_put :update, id: credit_card.to_param, credit_card: {name: 'Jordan Brough'}
expect(response.status).to eq(401)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def initialize(user)
can :create, ReturnAuthorization do |return_authorization|
return_authorization.order.user == user
end
can :display, CreditCard, user_id: user.id
can [:display, :update], CreditCard, user_id: user.id
can :display, Product
can :display, ProductProperty
can :display, Property
Expand Down
4 changes: 4 additions & 0 deletions core/lib/spree/core/controller_helpers/strong_parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ def permitted_attributes
to: :permitted_attributes,
prefix: :permitted

def permitted_credit_card_attributes
permitted_source_attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we use this for user facing authorization.

The user can change the following:

[:number,
 :month,
 :year,
 :expiry,
 :verification_value,
 :first_name,
 :last_name,
 :cc_type,
 :gateway_customer_profile_id,
 :gateway_payment_profile_id,
 :last_digits,
 :name,
 :encrypted_data]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Senjai if that's an issue then I don't think it's specific to this PR is it? Those attributes are already exposed via payments_attributes -> source_attributes on the order (when creating a payment). Perhaps we need a separate PR for that if it's an issue?

end

def permitted_payment_attributes
permitted_attributes.payment_attributes + [
source_attributes: permitted_source_attributes
Expand Down