Skip to content

Commit

Permalink
Messages by scope (#801)
Browse files Browse the repository at this point in the history
Messages by scope + Get offers with unread messages + Mark all as read
  • Loading branch information
patrixr authored Aug 12, 2019
1 parent 2133ea5 commit a3bd226
Show file tree
Hide file tree
Showing 17 changed files with 168 additions and 26 deletions.
6 changes: 4 additions & 2 deletions app/controllers/api/v1/api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,12 @@ def page
@page.zero? ? 1 : @page
end

# max limit is 25
# max limit is 50, default is 25
def per_page
@per_page = params['per_page'].to_i
(@per_page.zero? || @per_page > DEFAULT_SEARCH_COUNT) ? DEFAULT_SEARCH_COUNT : @per_page
return DEFAULT_SEARCH_COUNT if @per_page < 1
return MAX_SEARCH_COUNT if @per_page > MAX_SEARCH_COUNT
@per_page
end

def array_param(key)
Expand Down
41 changes: 40 additions & 1 deletion app/controllers/api/v1/messages_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module V1
class MessagesController < Api::V1::ApiController
load_and_authorize_resource :message, parent: false

ALLOWED_SCOPES = %w[offer item order].freeze

resource_description do
short "List, show, create and mark_read a message."
formats ["json"]
Expand Down Expand Up @@ -30,13 +32,15 @@ class MessagesController < Api::V1::ApiController
param :item_id, String, desc: "Return messages for item id."
param :order_id, String, desc: "Return messages for order id"
param :state, String, desc: "Message state (unread|read) to filter on"
param :scope, String, desc: "The type of record associated to the messages (order/offer/item)"
def index
@messages = apply_scope(@messages, params[:scope]) if params[:scope].present?
@messages = @messages.where(id: params[:ids].split(",")) if params[:ids].present?
@messages = @messages.where(offer_id: params[:offer_id].split(",")) if params[:offer_id].present?
@messages = @messages.where(order_id: params[:order_id].split(",")) if params[:order_id].present?
@messages = @messages.where(item_id: params[:item_id].split(",")) if params[:item_id].present?
@messages = @messages.with_state_for_user(User.current_user, params[:state]) if params[:state].present?
render json: @messages, each_serializer: serializer
paginate_and_render(@messages)
end

api :GET, "/v1/messages/1", "Get a message"
Expand All @@ -58,8 +62,36 @@ def mark_read
render json: @message, serializer: serializer
end

api :PUT, "/v1/messages/mark_all_read", "Mark all messages as read"
def mark_all_read
@subscriptions = Subscription.unread.for_user(User.current_user.id)
if params[:scope].present?
@subscriptions = apply_scope(@subscriptions.joins(:message), params[:scope])
end
@subscriptions.update_all state: 'read'
render json: {}
end

private

def apply_scope(records, scope)
return records unless ALLOWED_SCOPES.include? scope
records.where("messages.#{scope}_id IS NOT NULL")
end

def paginate_and_render(records)
meta = {}
if params[:page].present?
records = records.page(page).per(per_page)
meta = {
total_pages: records.total_pages,
total_count: records.total_count
}
end
output = message_response(records)
render json: { meta: meta }.merge(output)
end

def order_id
params[:message][:designation_id].presence || params[:message][:order_id].presence
end
Expand All @@ -68,6 +100,13 @@ def serializer
Api::V1::MessageSerializer
end

def message_response(records)
ActiveModel::ArraySerializer.new(records,
each_serializer: serializer,
root: "messages"
).as_json
end

def message_params
params.require(:message).permit(
:body, :is_private,
Expand Down
17 changes: 10 additions & 7 deletions app/controllers/api/v1/offers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,11 @@ def index

api :GET, '/v1/offers/search?searchText=xyz', "Search for offers"
def search
records = @offers.search({ search_text: params['searchText'], states: array_param(:state) })
records = @offers
records = records.search({ search_text: params['searchText'], states: array_param(:state) }) if params['searchText'].present?
records = apply_filters(records)
records = records.page(params["page"]).per(params["per_page"] || params["recent_offer_count"] || DEFAULT_SEARCH_COUNT)
offers = offer_response(records.with_summary_eager_load)
offers = offer_summary_response(records.with_summary_eager_load)
render json: {meta: {total_pages: records.total_pages, search: params['searchText']}}.merge(offers)
end

Expand Down Expand Up @@ -161,10 +162,11 @@ def filter_created_by(offers)
end
end

def offer_response(records)
ActiveModel::ArraySerializer.new(
records,each_serializer: summary_serializer,
root: "offers"
def offer_summary_response(records)
ActiveModel::ArraySerializer.new(records,
each_serializer: summary_serializer,
root: "offers",
include_messages: params[:include_messages] == "true"
).as_json
end

Expand All @@ -175,7 +177,8 @@ def apply_filters(offers)
self_reviewer: bool_param(:selfReview, false),
recent_offers: bool_param(:recent_offers, false),
before: time_epoch_param(:before),
after: time_epoch_param(:after)
after: time_epoch_param(:after),
with_notifications: params[:with_notifications]
})
end

Expand Down
1 change: 1 addition & 0 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def message_abilities
end
end
can [:mark_read], Message, id: @user.subscriptions.pluck(:message_id)
can [:mark_all_read], Message
end

def offer_abilities
Expand Down
8 changes: 8 additions & 0 deletions app/models/concerns/offer_filtering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ module OfferFiltering
res = res.due_after(options[:after]) if options[:after].present?
res = res.due_before(options[:before]) if options[:before].present?
res = res.order("id DESC") if options[:recent_offers]
res = res.with_notifications(options[:with_notifications]) if options[:with_notifications].present?
res.distinct
end

Expand All @@ -42,6 +43,13 @@ def self.due_before(time)
where('schedules.scheduled_at <= (?)', time)
end

def self.with_notifications(state)
res = joins("LEFT OUTER JOIN subscriptions ON offers.id = subscriptions.offer_id")
res = res.where("subscriptions.user_id = (?)", User.current_user.id)
res = res.where("subscriptions.state = (?)", state) if %w[unread read].include?(state)
res
end

# Helpers
def self.last_6pm
now = Time.now.in_time_zone
Expand Down
8 changes: 4 additions & 4 deletions app/models/concerns/user_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ module UserSearch
}

def self.search_user(role_name, search_query, search_text)
joins(:roles)
.where('roles.name = ?', role_name)
.where(search_query, search_text: "%#{search_text}%")
.distinct
res = joins(:roles)
res = res.where('roles.name = ?', role_name) if role_name.present?
res = res.where(search_query, search_text: "%#{search_text}%") if search_text.present?
res.distinct
end
end
end
1 change: 1 addition & 0 deletions app/models/subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ class Subscription < ActiveRecord::Base
after_create :send_new_message_notification # PushUpdatesForSubscription

scope :unread, -> { where(state: 'unread') }
scope :for_user, ->(user_id) { where(user_id: user_id) }
end
10 changes: 10 additions & 0 deletions app/serializers/api/v1/offer_summary_serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ class OfferSummarySerializer < ApplicationSerializer
has_one :reviewed_by, serializer: UserSummarySerializer, root: :user
has_one :received_by, serializer: UserSummarySerializer, root: :user
has_one :delivery, serializer: DeliverySerializer, root: :delivery
has_many :messages, serializer: MessageSerializer

def include_messages?
return false unless goodcity_user?
@options[:include_messages] == true
end

def display_image_cloudinary_id
object.images.first.try(:cloudinary_id)
Expand Down Expand Up @@ -72,5 +78,9 @@ def received_packages_count__sql
"(SELECT COUNT(*) FROM packages LEFT JOIN items ON items.id = packages.item_id LEFT JOIN offers o ON o.id = items.offer_id WHERE offers.id = o.id AND packages.state = 'received')"
end

def goodcity_user?
User.current_user.present?
end

end
end
1 change: 1 addition & 0 deletions config/initializers/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
START_DAYS_COUNT = 0
CROSSROADS_TRUCK_COST = 600
DEFAULT_SEARCH_COUNT = 25
MAX_SEARCH_COUNT = 50
DONOR_APP = "app".freeze
ADMIN_APP = "admin".freeze
STOCK_APP = "stock".freeze
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

resources :messages, only: [:create, :update, :index, :show] do
put :mark_read, on: :member
put :mark_all_read, on: :collection
end

resources :offers, only: [:create, :update, :index, :show, :destroy] do
Expand Down
6 changes: 3 additions & 3 deletions spec/controllers/api/v1/api_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@ def index
it { expect(subject).to eql(20) }
end

context "30 per_page (limit is 25)" do
let(:per_page) { '30' }
it { expect(subject).to eql(25) }
context "30 per_page (limit is 50)" do
let(:per_page) { '60' }
it { expect(subject).to eql(50) }
end

context "nil per_page" do
Expand Down
42 changes: 42 additions & 0 deletions spec/controllers/api/v1/messages_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@
expect(subject['messages'].length).to eq(2)
end

it "supports pagination", :show_in_doc do
8.times { create :message, item: item }
get :index, page: 1, per_page: 6
expect(response.status).to eq(200)
expect(subject['meta']['total_pages']).to eq(2)
expect(subject['meta']['total_count']).to eq(8)
expect(subject['messages'].length).to eq(6)
end

describe 'filtering messages' do
before { 2.times { create :message } }

Expand Down Expand Up @@ -81,6 +90,18 @@
get :index, offer_id: "#{offer.id},#{offer2.id}", state: 'unread'
expect(subject['messages'].length).to eq(6)
end

it "for a certain type of associated record" do
1.times { create :message, offer: offer }
1.times { create :message, order: order }
4.times { create :message, item: item }

get :index, scope: 'item'
expect(subject['messages'].length).to eq(4)
subject['messages'].each do |m|
expect(m['item_id']).not_to be_nil
end
end
end
end

Expand All @@ -105,6 +126,27 @@
end
end

describe "PUT messages/mark_all_read" do
before { generate_and_set_token(user) }

let!(:subscriptions) { create_list(:subscription, 2, state: 'unread', user_id: user.id) }
let!(:order_subscriptions) { create_list(:order_subscription, 3, state: 'unread', user_id: user.id) }
let(:subscription_states) { subscriptions.map { |s| s.reload.state } }
let(:order_subscription_states) { order_subscriptions.map { |s| s.reload.state } }

it "mark all messages as read" do
put :mark_all_read
expect(subscription_states).to all(eq('read'))
expect(order_subscription_states).to all(eq('read'))
end

it "mark all messages of a certain scope as read" do
put :mark_all_read, scope: 'order'
expect(subscription_states).to all(eq('unread'))
expect(order_subscription_states).to all(eq('read'))
end
end

describe "PUT messages/:id/mark_read" do
before { generate_and_set_token(user) }
it "donor will read a message and automatically marked Read" do
Expand Down
34 changes: 29 additions & 5 deletions spec/controllers/api/v1/offers_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -394,14 +394,14 @@
let(:submitted_offer2) { create :offer, :submitted, notes: 'Test' }
let(:reviewing_offer) { create :offer, :under_review, notes: 'Tester' }
let(:receiving_offer) { create :offer, :receiving, notes: 'Tester', reviewed_by_id: reviewer.id }
let(:scheduled_offer) { create :offer, :scheduled, notes: 'Test' }
let(:scheduled_offer1) { create :offer, :scheduled, notes: 'Test for before' }
let(:scheduled_offer) { create :offer, state: 'scheduled', notes: 'Test for before' }
let(:scheduled_offer1) { create :offer, state: 'scheduled', notes: 'Test for after' }
let(:priority_reviewed_offer) { create :offer, :reviewed, notes: 'Tester', review_completed_at: Time.now - 3.days }
let(:priority_reviewing_offer) { create :offer, :under_review, notes: 'Tester', reviewed_at: Time.now - 2.days }
let(:schedule) { create :schedule, scheduled_at: Time.now - 3.days }
let(:schedule1) { create :schedule, scheduled_at: Time.now + 1.days }
let(:delivery) { create :delivery, offer_id: scheduled_offer.id, schedule_id: schedule.id }
let(:delivery1) { create :delivery, offer_id: scheduled_offer1.id, schedule_id: schedule1.id }
let(:delivery) { create :delivery, offer: scheduled_offer, schedule: schedule }
let(:delivery1) { create :delivery, offer: scheduled_offer1, schedule: schedule1 }
before(:each) { generate_and_set_token(reviewer) }
subject { JSON.parse(response.body) }

Expand Down Expand Up @@ -455,7 +455,7 @@ def epoch_ms(time)
end

it 'can return offers scheduled after a certain time' do
after = epoch_ms(Time.zone.now - 3.day)
after = epoch_ms(Time.zone.now - 4.day)
get :search, searchText: 'Test', after: after
expect(response.status).to eq(200)
expect(subject['offers'].size).to eq(2)
Expand Down Expand Up @@ -506,6 +506,10 @@ def epoch_ms(time)
receiving_offer
scheduled_offer
User.current_user = reviewer

# Create notifications for offers
3.times { create(:subscription, offer_id: receiving_offer.id, user_id: reviewer.id, state: 'unread') }
3.times { create(:subscription, offer_id: reviewing_offer.id, user_id: reviewer.id, state: 'read') }
}

it "returns offers created by logged in user if selfReview is present in params" do
Expand All @@ -519,6 +523,26 @@ def epoch_ms(time)
expect(response.status).to eq(200)
expect(subject['offers'].size).to eq(3)
end

it "returns offers with notifications" do
get :search, with_notifications: 'all'
expect(response.status).to eq(200)
expect(subject['offers'].size).to eq(2)
end

it "returns offers with unread notifications" do
get :search, with_notifications: 'unread'
expect(response.status).to eq(200)
expect(subject['offers'].size).to eq(1)
expect(subject['offers'][0]['id']).to eq(receiving_offer.id)
end

it "returns offers with read notifications" do
get :search, with_notifications: 'read'
expect(response.status).to eq(200)
expect(subject['offers'].size).to eq(1)
expect(subject['offers'][0]['id']).to eq(reviewing_offer.id)
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/controllers/api/v1/orders_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def create_order_with_transport(state, opts = {})
expect(parsed_body['designations'].count).to eq(Order.where.not(state: 'draft').count)
expect(parsed_body['designations'].map { |it| it['state'] }).to_not include('draft')
end

# Test turned off as currently hardcoded to 150
# it 'returns the number of items specified for the page' do
# 5.times { FactoryBot.create :order, :with_state_submitted } # There are now 7 no-draft orders in total
Expand Down
12 changes: 12 additions & 0 deletions spec/controllers/api/v1/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@
get :index, searchText: charity_users.first.first_name, role_name: "Charity"
expect(User.find(parsed_body["users"].first["id"]).roles.pluck(:name)).to include("Charity")
end

it "does not return searched user if the specified role is different" do
get :index, searchText: charity_users.first.first_name, role_name: "Supervisor"
expect(response.status).to eq(200)
expect(parsed_body['users'].count).to eq(0)
end

it "returns searched user if role isn't specified" do
get :index, searchText: charity_users.first.first_name
expect(response.status).to eq(200)
expect(parsed_body['users'].count).to eq(1)
end
end

describe "PUT user/1" do
Expand Down
Loading

0 comments on commit a3bd226

Please sign in to comment.