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

Application Rating & Review #186

Merged
merged 22 commits into from
Jan 16, 2017
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
b824a0b
Add App User Rating
x4d3 Nov 28, 2016
25328df
Merge pull request #159 from x4d3/feature/rating
x4d3 Nov 29, 2016
a22b92c
Implement comments management in Admin Panel
hedudelgado Nov 29, 2016
0748434
Ratings property is always displayed, even as an empry array
alexnoox Dec 5, 2016
cf3a82d
Merge pull request #165 from alexnoox/feature/app-rating
x4d3 Dec 5, 2016
f8073f2
Merge pull request #161 from hedudelgado/implement-comments-in-admin-…
x4d3 Dec 5, 2016
5a9e268
Rename model Rating to Review
x4d3 Dec 5, 2016
3d0e8ed
Merge pull request #167 from x4d3/feature/rating
x4d3 Dec 5, 2016
cb08e97
Add missing locales for reviews
Dec 6, 2016
c6fe257
Add route to expose comments for a single app
hedudelgado Dec 21, 2016
952c8a2
Add pagination
hedudelgado Dec 22, 2016
afe587f
Fix specs and refactoring
hedudelgado Dec 28, 2016
f76351f
Refactor App Review API
ouranos Dec 29, 2016
4665231
Merge pull request #177 from hedudelgado/comments-pagination
ouranos Dec 30, 2016
b8ec8fa
Add locales in mnoe
alexnoox Dec 30, 2016
9d4bf24
Merge pull request #179 from alexnoox/feature/app-rating
ouranos Dec 30, 2016
ca79ec9
Frogot to commit specs :s
ouranos Jan 2, 2017
cb86691
Merge pull request #180 from ouranos/comments-pagination
ouranos Jan 2, 2017
05fa0b8
Merge remote-tracking branch 'upstream/master' into feature/app-rating
ouranos Jan 6, 2017
6754b44
App Rating - Add feature flag
ouranos Jan 12, 2017
5a44cb5
Update locales
ouranos Jan 12, 2017
a5badc4
Style: Remove empty lines
ouranos Jan 16, 2017
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 @@ -11,6 +11,7 @@ angular.module('mnoEnterprise.configuration', [])
.constant('PRICING_CONFIG', <%= Hash(Settings.pricing).to_json %>)
.constant('DOCK_CONFIG', <%= Hash(Settings.dock).to_json %>)
.constant('DEVELOPER_SECTION_CONFIG', <%= Hash(Settings.developer).to_json %>)
.constant('REVIEWS_CONFIG', <%= Hash(Settings.reviews).to_json %>)
.constant('GOOGLE_TAG_CONTAINER_ID', <%= MnoEnterprise.google_tag_container.to_json %>)
.constant('INTERCOM_ID', <%= MnoEnterprise.intercom_app_id.to_json %>)
.constant('APP_NAME', <%= MnoEnterprise.app_name.to_json %>)
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
module MnoEnterprise
class Jpi::V1::Admin::AppReviewsController < Jpi::V1::Admin::BaseResourceController
# GET /mnoe/jpi/v1/admin/app_reviews
def index
@app_reviews = MnoEnterprise::AppReview
@app_reviews = @app_reviews.limit(params[:limit]) if params[:limit]
@app_reviews = @app_reviews.skip(params[:offset]) if params[:offset]
@app_reviews = @app_reviews.order_by(params[:order_by]) if params[:order_by]
@app_reviews = @app_reviews.where(params[:where]) if params[:where]
@app_reviews = @app_reviews.all.fetch
response.headers['X-Total-Count'] = @app_reviews.metadata[:pagination][:count]
end

# GET /mnoe/jpi/v1/admin/app_reviews/1
def show
@app_review = MnoEnterprise::AppReview.find(params[:id])
end


Copy link
Contributor

Choose a reason for hiding this comment

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

Remove an empty line.

# PATCH /mnoe/jpi/v1/admin/app_reviews/1
def update
@app_review = MnoEnterprise::AppReview.find(params[:id])
@app_review.update(app_review_params)
render :show
end

def app_review_params
params.require(:app_review).permit(:status)
end

end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
module MnoEnterprise
class Jpi::V1::AppReviewsController < Jpi::V1::BaseResourceController
# GET /mnoe/jpi/v1/marketplace/:id/app_reviews
def index
@app_reviews = MnoEnterprise::AppReview.approved.where(reviewable_id: params[:id])
@app_reviews = @app_reviews.limit(params[:limit]) if params[:limit]
@app_reviews = @app_reviews.skip(params[:offset]) if params[:offset]
@app_reviews = @app_reviews.order_by(params[:order_by]) if params[:order_by]
@app_reviews = @app_reviews.where(params[:where]) if params[:where]
@app_reviews = @app_reviews.all.fetch
response.headers['X-Total-Count'] = @app_reviews.metadata[:pagination][:count]
end

# POST /mnoe/jpi/v1/marketplace/:id/app_reviews
def create
@app = MnoEnterprise::App.find(params[:id])
return render json: "could not find App #{params[:id]}", status: :not_found unless @app

# TODO: use the has_many associations -> @app.reviews.build
@app_review = MnoEnterprise::AppReview.new(review_params(@app.id))
if @app_review.save
@average_rating = @app.reload.average_rating
render :show
else
render json: @app_review.errors, status: :bad_request
end
end

def review_params(app_id)
params.require(:app_review).permit(:rating, :description, :organization_id)
.merge(app_id: app_id, user_id: current_user.id)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
json.id app_review.id
json.rating app_review.rating
json.description app_review.description
json.status app_review.status
json.app_id app_review.app_id
json.app_name app_review.app_name
json.user_id app_review.user_id
json.user_name app_review.user_name
json.organization_id app_review.organization_id
json.organization_name app_review.organization_name
json.created_at app_review.created_at
json.updated_at app_review.updated_at
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
json.app_reviews @app_reviews, partial: 'app_review', as: :app_review
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
json.app_review do
json.partial! 'app_review', app_review: @app_review
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
json.id app_review[:id]
json.rating app_review[:rating]
json.description app_review[:description]
json.status app_review[:status]
json.user_id app_review[:user_id]
json.user_name app_review[:user_name]
json.organization_id app_review[:organization_id]
json.organization_name app_review[:organization_name]
json.app_id app_review[:app_id]
json.app_name app_review[:app_name]
json.created_at app_review[:created_at]
json.updated_at app_review[:updated_at]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
json.app_reviews do
json.array! @app_reviews do |app_review|
json.partial! 'resource', app_review: app_review
end
end

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
json.app_review do
json.partial! 'resource', app_review: @app_review
end
json.average_rating @average_rating
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ json.is_coming_soon app.coming_soon?
json.single_billing app.single_billing?
json.multi_instantiable app.multi_instantiable
json.subcategories app.subcategories
json.average_rating app.average_rating

if app.logo
json.logo app.logo.to_s
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
json.cache! ['v1', 'marketplace'], expires_in: 20.minutes do
json.categories @categories
json.apps @apps, partial: 'app', as: :app
end
json.categories @categories
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll do some perf analysis to see the impact of removing the cache

json.apps @apps, partial: 'app', as: :app
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
json.app do
json.partial! 'app', app: @app
end
end
7 changes: 6 additions & 1 deletion api/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@
#============================================================
namespace :jpi do
namespace :v1 do
resources :marketplace, only: [:index, :show]
resources :marketplace, only: [:index, :show] do
member do
resources :app_reviews, only: [:index, :create]
end
end
resource :current_user, only: [:show, :update] do
put :update_password
put :register_developer
Expand Down Expand Up @@ -150,6 +154,7 @@
namespace :admin, defaults: {format: 'json'} do
resources :audit_events, only: [:index]
resources :app_instances, only: [:destroy], shallow: true
resources :app_reviews, only: [:index, :show, :update]
resources :users, only: [:index, :show, :destroy, :update, :create] do
collection do
get :count
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
require 'rails_helper'

module MnoEnterprise
include MnoEnterprise::TestingSupport::SharedExamples::JpiV1Admin

describe Jpi::V1::Admin::AppReviewsController, type: :controller do
render_views
routes { MnoEnterprise::Engine.routes }
before { request.env['HTTP_ACCEPT'] = 'application/json' }
let(:user) { build(:user, :admin, :with_organizations) }
let(:app_review) { build(:app_review, user: user) }
before do
api_stub_for(get: '/app_reviews', response: from_api([app_review]))
api_stub_for(get: "/app_reviews/#{app_review.id}", response: from_api(app_review))
api_stub_for(get: "/users/#{user.id}", response: from_api(user))
sign_in user
end

def partial_hash_for_app_review(app_review)
{
'id' => app_review.id,
'rating' => app_review.rating,
'description' => app_review.description,
'status' => app_review.status,
'app_id' => app_review.app_id,
'app_name' => app_review.app_name,
'user_id' => app_review.user_id,
'user_name' => app_review.user_name,
'organization_id' => app_review.organization_id,
'organization_name' => app_review.organization_name,
'created_at' => app_review.created_at,
'updated_at' => app_review.updated_at,
}
end

def hash_for_app_review(app_review)
{
'app_review' => partial_hash_for_app_review(app_review)
}
end

def hash_for_app_reviews(app_reviews)
{
'app_reviews' => app_reviews.map { |o| partial_hash_for_app_review(o) }
}
end


describe '#index' do
subject { get :index }

it_behaves_like "a jpi v1 admin action"

context 'success' do
before { subject }

it 'returns a list of app_review' do
expect(response).to be_success
expect(JSON.parse(response.body)).to eq(JSON.parse(hash_for_app_reviews([app_review]).to_json))
end
end
end

describe 'GET #show' do
subject { get :show, id: app_review.id }

it_behaves_like "a jpi v1 admin action"

context 'success' do
before { subject }

it 'returns a complete description of the app_review' do
expect(response).to be_success
expect(JSON.parse(response.body)).to eq(JSON.parse(hash_for_app_review(app_review).to_json))
end
end
end


describe 'PUT #update' do
subject { put :update, id: app_review.id, app_review: {status: 'rejected'} }

before do
sign_in user
api_stub_for(put: "/app_reviews/#{app_review.id}", response: -> { app_review.status = 'rejected'; from_api(app_review) })
end

it_behaves_like "a jpi v1 admin action"

context 'success' do
before { subject }

it { expect(response).to be_success }

# Test that the app_review is updated by testing the api endpoint was called
it { expect(app_review.status).to eq('rejected') }
end
end


end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
require 'rails_helper'

module MnoEnterprise
describe Jpi::V1::AppReviewsController, type: :controller do
include MnoEnterprise::TestingSupport::JpiV1TestHelper
render_views
routes { MnoEnterprise::Engine.routes }
before { request.env["HTTP_ACCEPT"] = 'application/json' }


#===============================================
# Assignments
#===============================================
let(:user) { build(:user) }
before { api_stub_for(get: "/users/#{user.id}", response: from_api(user)) }
before { sign_in user }

let(:app) { build(:app) }
let(:app_review) { build(:app_review) }
let(:expected_hash_for_review) do
attrs = %w(id rating description status user_id user_name organization_id organization_name app_id app_name)
app_review.attributes.slice(*attrs).merge({'created_at' => app_review.created_at.as_json, 'updated_at' => app_review.updated_at.as_json})
end
let(:expected_hash_for_reviews) do
{
'app_reviews' => [expected_hash_for_review],
# 'metadata' => {'pagination' => {'count' => 1}}
}
end

before do
api_stub_for(get: "/apps/#{app.id}", response: from_api(app))
end

describe 'GET #index' do

before do
api_stub_for(get: "/app_reviews?filter[reviewable_id]=#{app.id}", response: from_api([app_review]))
end

subject { get :index, id: app.id }

it_behaves_like "jpi v1 protected action"

it_behaves_like "a paginated action"

it 'renders the list of reviews' do
subject
expect(JSON.parse(response.body)).to eq(expected_hash_for_reviews)
end
end

describe 'POST #create', focus: true do
let(:params) { {organization_id: 1, description: 'A Review', rating: 5, foo: 'bar'} }
let(:app_review) { build(:app_review) }

before do
api_stub_for(post: "/app_reviews", response: from_api(app_review))
api_stub_for(get: "/app_reviews/#{app_review.id}", response: from_api(app_review))
end

subject { post :create, id: app.id, app_review: params }

it_behaves_like "jpi v1 protected action"

it 'renders the new review' do
expect(JSON.parse(subject.body)).to include('app_review' => expected_hash_for_review)
end

it 'renders the new average rating' do
expect(JSON.parse(subject.body)).to include('average_rating' => app.average_rating)
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def partial_hash_for_app(app)
'rank' => app.rank,
'multi_instantiable' => app.multi_instantiable,
'subcategories' => app.subcategories,

'average_rating' => app.average_rating,
}
end

Expand Down Expand Up @@ -73,10 +73,7 @@ def hash_for_apps(apps)
)
end

it 'is successful' do
subject
expect(response).to be_success
end
it { is_expected.to be_success }

it 'returns the right response' do
subject
Expand All @@ -90,10 +87,7 @@ def hash_for_apps(apps)
api_stub_for(get: '/apps', response: from_api([app]))
end

it 'is successful' do
subject
expect(response).to be_success
end
it { is_expected.to be_success }

it 'returns the right response' do
subject
Expand All @@ -106,10 +100,7 @@ def hash_for_apps(apps)
before { api_stub_for(get: "/apps/#{app.id}", response: from_api(app)) }
subject { get :show, id: app.id }

it 'is successful' do
subject
expect(response).to be_success
end
it { is_expected.to be_success }

it 'returns the right response' do
subject
Expand All @@ -129,10 +120,7 @@ def hash_for_apps(apps)
api_stub_for(get: '/apps', response: from_api([app1,app2]))
end

it 'is successful' do
subject
expect(response).to be_success
end
it { is_expected.to be_success }

it 'returns the right response' do
subject
Expand Down
Loading