Tests/tests for new changes #8
Conversation
| @@ -44,7 +44,7 @@ def scope | |||
| end | |||
|
|
|||
| def resource | |||
| scope.find_by(slug: params[:id]) || scope.find(params[:id]) | |||
| scope.friendly.find(params[:id]) | |||
There was a problem hiding this comment.
does this work when we pass slug instead of id?
There was a problem hiding this comment.
yes it works for both slug and id
|
|
||
| # PUT/PATCH /api/v2/vendor/vendors/:vendor_id/questions/:id | ||
| def update | ||
| question = @vendor.product_questions.find(params[:id]) |
There was a problem hiding this comment.
can we please ensure user have proper access to update?
I am afraid, user might be able to update product question from another vendor
also please add test
we have to ensure this for destroy as well
app/models/spree/vendor.rb
Outdated
| Spree::ProductQuestion.joins(product: :vendor).where(spree_products: { vendor_id: id }) | ||
| end | ||
|
|
||
| def reviews |
There was a problem hiding this comment.
yo ta review ra product question ko lib install garena bhane fail huncha..
yo ta vendor lai extend garera respective extension ma lekhnu parne
| let!(:vendor_user) { create(:vendor_user, vendor: vendor, user: user) } | ||
|
|
||
| before do | ||
| create(:vendor_user, vendor: create(:active_vendor, name: 'Test Vendor'), user: user) |
There was a problem hiding this comment.
name should be from factory,
unless we want to specifically test string override..
| end | ||
|
|
||
| describe 'vendors#create' do | ||
| let(:vendor_params) { { vendor: { name: 'Vendor test' } } } |
There was a problem hiding this comment.
why do we have let inside describe block and not at top of file?
| let(:new_user) { create(:user) } | ||
|
|
||
| it 'creates a new vendor user' do | ||
| post "/api/v2/vendor/vendors/#{vendor.id}/users", params: { vendor_user: { email: new_user.email } } |
There was a problem hiding this comment.
only vendor admin should be able to add new vendor users
There was a problem hiding this comment.
@poudelprakash dai, should I add a new attribute admin in the vendor users to indicate admin or create a function to pick up the first create vendor user as admin.
There was a problem hiding this comment.
Creating user as admin will make user admin of whole app and not just vendor..
It would be better to manage this role via vendor users
| describe '#destroy' do | ||
| let(:new_vendor_user) { create(:vendor_user, vendor: vendor, user: create(:user)) } | ||
|
|
||
| it 'deletes the vendor user' do |
There was a problem hiding this comment.
we also need to validate authorizaion
No description provided.