From fd5643bc9da4fda475df1e54144eef196d0f648f Mon Sep 17 00:00:00 2001 From: rhymes Date: Thu, 6 Feb 2020 13:36:56 +0100 Subject: [PATCH] Improve API controllers efficiency by selecting only columns needed for serialization (#5805) [deploy] --- app/controllers/api/v0/articles_controller.rb | 30 +++- .../api/v0/chat_channels_controller.rb | 8 +- .../api/v0/classified_listings_controller.rb | 18 +- app/controllers/api/v0/comments_controller.rb | 10 +- .../api/v0/followers_controller.rb | 5 + .../api/v0/followings_controller.rb | 50 +++--- .../api/v0/github_repos_controller.rb | 27 +-- .../api/v0/reactions_controller.rb | 10 +- app/controllers/api/v0/tags_controller.rb | 10 +- app/controllers/api/v0/users_controller.rb | 37 ++-- app/controllers/api/v0/webhooks_controller.rb | 17 +- app/models/chat_channel.rb | 17 +- app/services/article_api_index_service.rb | 32 ++-- app/services/suggester/users/recent.rb | 28 +++- app/services/suggester/users/sidebar.rb | 3 +- .../api/v0/articles/_article.json.jbuilder | 20 +++ app/views/api/v0/articles/index.json.jbuilder | 30 +--- app/views/api/v0/articles/show.json.jbuilder | 32 +--- .../_listing.json.jbuilder | 1 - .../api/v0/followers/_follows.json.jbuilder | 5 + .../api/v0/followings/tags.json.jbuilder | 15 +- app/views/api/v0/users/me.json.jbuilder | 11 -- config/routes.rb | 20 +-- spec/requests/api/v0/chat_channels_spec.rb | 108 ++++++++++-- .../api/v0/classified_listings_spec.rb | 46 +++-- spec/requests/api/v0/followers_spec.rb | 72 ++++++-- spec/requests/api/v0/followings_spec.rb | 158 +++++++++++++++--- spec/requests/api/v0/follows_spec.rb | 3 +- spec/requests/api/v0/github_repos_spec.rb | 21 ++- spec/requests/api/v0/reactions_spec.rb | 31 ++++ spec/requests/api/v0/users_spec.rb | 105 +++++++++++- spec/requests/api/v0/webhooks_spec.rb | 63 +++++-- spec/requests/reactions_api_spec.rb | 29 ---- spec/requests/users_api_spec.rb | 26 --- spec/services/suggester/users/sidebar_spec.rb | 2 +- 35 files changed, 778 insertions(+), 322 deletions(-) create mode 100644 app/views/api/v0/articles/_article.json.jbuilder create mode 100644 app/views/api/v0/followers/_follows.json.jbuilder delete mode 100644 app/views/api/v0/users/me.json.jbuilder create mode 100644 spec/requests/api/v0/reactions_spec.rb delete mode 100644 spec/requests/reactions_api_spec.rb delete mode 100644 spec/requests/users_api_spec.rb diff --git a/app/controllers/api/v0/articles_controller.rb b/app/controllers/api/v0/articles_controller.rb index a5dc6ee64..10c9ac2e3 100644 --- a/app/controllers/api/v0/articles_controller.rb +++ b/app/controllers/api/v0/articles_controller.rb @@ -17,12 +17,17 @@ module Api def index @articles = ArticleApiIndexService.new(params).get + @articles = @articles.select(INDEX_ATTRIBUTES_FOR_SERIALIZATION).decorate set_surrogate_key_header Article.table_key, @articles.map(&:record_key) end def show - @article = Article.published.includes(:user).find(params[:id]).decorate + @article = Article.published. + includes(:user). + select(SHOW_ATTRIBUTES_FOR_SERIALIZATION). + find(params[:id]). + decorate set_surrogate_key_header @article.record_key end @@ -62,12 +67,35 @@ module Api @articles = @articles. includes(:organization). + select(ME_ATTRIBUTES_FOR_SERIALIZATION). order(published_at: :desc, created_at: :desc). page(params[:page]). per(num). decorate end + INDEX_ATTRIBUTES_FOR_SERIALIZATION = %i[ + id user_id organization_id collection_id + title description main_image published_at crossposted_at social_image + cached_tag_list slug path canonical_url comments_count + positive_reactions_count created_at edited_at last_comment_at published + updated_at + ].freeze + private_constant :INDEX_ATTRIBUTES_FOR_SERIALIZATION + + SHOW_ATTRIBUTES_FOR_SERIALIZATION = [ + *INDEX_ATTRIBUTES_FOR_SERIALIZATION, :body_markdown, :processed_html + ].freeze + private_constant :SHOW_ATTRIBUTES_FOR_SERIALIZATION + + ME_ATTRIBUTES_FOR_SERIALIZATION = %i[ + id user_id organization_id + title description main_image published published_at cached_tag_list + slug path canonical_url comments_count positive_reactions_count + page_views_count crossposted_at body_markdown updated_at + ].freeze + private_constant :ME_ATTRIBUTES_FOR_SERIALIZATION + private def article_params diff --git a/app/controllers/api/v0/chat_channels_controller.rb b/app/controllers/api/v0/chat_channels_controller.rb index 7278e1a55..ca807bdad 100644 --- a/app/controllers/api/v0/chat_channels_controller.rb +++ b/app/controllers/api/v0/chat_channels_controller.rb @@ -2,9 +2,15 @@ module Api module V0 class ChatChannelsController < ApiController def show - @chat_channel = ChatChannel.find(params[:id]) + @chat_channel = ChatChannel. + select(SHOW_ATTRIBUTES_FOR_SERIALIZATION). + find(params[:id]) + error_not_found unless @chat_channel.has_member?(current_user) end + + SHOW_ATTRIBUTES_FOR_SERIALIZATION = %i[id description channel_name].freeze + private_constant :SHOW_ATTRIBUTES_FOR_SERIALIZATION end end end diff --git a/app/controllers/api/v0/classified_listings_controller.rb b/app/controllers/api/v0/classified_listings_controller.rb index 083da4d55..fec6adc82 100644 --- a/app/controllers/api/v0/classified_listings_controller.rb +++ b/app/controllers/api/v0/classified_listings_controller.rb @@ -2,9 +2,10 @@ module Api module V0 class ClassifiedListingsController < ApiController include ClassifiedListingsToolkit + respond_to :json - before_action :set_classified_listing, only: %i[show update] + before_action :set_classified_listing, only: %i[update] before_action :authenticate_with_api_key_or_current_user!, only: %i[create update] # skip CSRF checks for create and update @@ -12,11 +13,13 @@ module Api def index @classified_listings = ClassifiedListing.published. - order("bumped_at DESC"). + select(ATTRIBUTES_FOR_SERIALIZATION). includes(:user, :organization, :taggings) @classified_listings = @classified_listings.where(category: params[:category]) if params[:category].present? + @classified_listings = @classified_listings.order(bumped_at: :desc) + per_page = (params[:per_page] || 30).to_i num = [per_page, 100].min page = params[:page] || 1 @@ -26,7 +29,10 @@ module Api end def show - # rendering with json builder + @classified_listing = ClassifiedListing. + select(ATTRIBUTES_FOR_SERIALIZATION). + includes(:user, :organization). + find(params[:id]) end def create @@ -37,6 +43,12 @@ module Api super end + ATTRIBUTES_FOR_SERIALIZATION = %i[ + id user_id organization_id title slug body_markdown + cached_tag_list category processed_html published + ].freeze + private_constant :ATTRIBUTES_FOR_SERIALIZATION + private attr_accessor :user diff --git a/app/controllers/api/v0/comments_controller.rb b/app/controllers/api/v0/comments_controller.rb index 7a948fd3c..e672226de 100644 --- a/app/controllers/api/v0/comments_controller.rb +++ b/app/controllers/api/v0/comments_controller.rb @@ -8,7 +8,10 @@ module Api def index article = Article.find(params[:a_id]) - @comments = article.comments.includes(:user).select(%i[id processed_html user_id ancestry]).arrange + @comments = article.comments. + includes(:user). + select(ATTRIBUTES_FOR_SERIALIZATION). + arrange set_surrogate_key_header article.record_key, Comment.table_key, edge_cache_keys(@comments) end @@ -16,7 +19,7 @@ module Api def show tree_with_root_comment = Comment.subtree_of(params[:id].to_i(26)). includes(:user). - select(%i[id processed_html user_id ancestry]). + select(ATTRIBUTES_FOR_SERIALIZATION). arrange # being only one tree we know that the root comment is the first (and only) key @@ -26,6 +29,9 @@ module Api set_surrogate_key_header Comment.table_key, edge_cache_keys(tree_with_root_comment) end + ATTRIBUTES_FOR_SERIALIZATION = %i[id processed_html user_id ancestry].freeze + private_constant :ATTRIBUTES_FOR_SERIALIZATION + private # ancestry wraps a single or multiple trees of comments into a single hash, diff --git a/app/controllers/api/v0/followers_controller.rb b/app/controllers/api/v0/followers_controller.rb index 43da21cad..a80f5d8f4 100644 --- a/app/controllers/api/v0/followers_controller.rb +++ b/app/controllers/api/v0/followers_controller.rb @@ -8,6 +8,7 @@ module Api @follows = Follow. where(followable_id: @user.organization_id, followable_type: "Organization"). includes(:follower). + select(ATTRIBUTES_FOR_SERIALIZATION). order("created_at DESC"). page(params[:page]). per(@follows_limit) @@ -17,11 +18,15 @@ module Api @follows = Follow. where(followable_id: @user.id, followable_type: "User"). includes(:follower). + select(ATTRIBUTES_FOR_SERIALIZATION). order("created_at DESC"). page(params[:page]). per(@follows_limit) end + ATTRIBUTES_FOR_SERIALIZATION = %i[id follower_id follower_type].freeze + private_constant :ATTRIBUTES_FOR_SERIALIZATION + private def limit_per_page(default:, max:) diff --git a/app/controllers/api/v0/followings_controller.rb b/app/controllers/api/v0/followings_controller.rb index c49a4de7e..26f85084d 100644 --- a/app/controllers/api/v0/followings_controller.rb +++ b/app/controllers/api/v0/followings_controller.rb @@ -5,47 +5,49 @@ module Api before_action -> { limit_per_page(default: 80, max: 1000) } def users - @follows = current_user. - follows_by_type("User"). - order("created_at DESC"). - includes(:followable). - page(params[:page]). - per(@follows_limit) + relation = current_user.follows_by_type("User"). + select(ATTRIBUTES_FOR_SERIALIZATION). + order(created_at: :desc) + @follows = load_follows_and_paginate(relation) end def tags - @followed_tags = current_user. - follows_by_type("ActsAsTaggableOn::Tag"). - order("points DESC"). - includes(:followable). - page(params[:page]). - per(@follows_limit) + relation = current_user.follows_by_type("ActsAsTaggableOn::Tag"). + select(TAGS_ATTRIBUTES_FOR_SERIALIZATION). + order(points: :desc) + @followed_tags = load_follows_and_paginate(relation) end def organizations - @followed_organizations = current_user. - follows_by_type("Organization"). - order("created_at DESC"). - includes(:followable). - page(params[:page]). - per(@follows_limit) + relation = current_user.follows_by_type("Organization"). + select(ATTRIBUTES_FOR_SERIALIZATION). + order(created_at: :desc) + @followed_organizations = load_follows_and_paginate(relation) end def podcasts - @followed_podcasts = current_user. - follows_by_type("Podcast"). - order("created_at DESC"). - includes(:followable). - page(params[:page]). - per(@follows_limit) + relation = current_user.follows_by_type("Podcast"). + select(ATTRIBUTES_FOR_SERIALIZATION). + order(created_at: :desc) + @followed_podcasts = load_follows_and_paginate(relation) end + ATTRIBUTES_FOR_SERIALIZATION = %i[id followable_id followable_type].freeze + private_constant :ATTRIBUTES_FOR_SERIALIZATION + + TAGS_ATTRIBUTES_FOR_SERIALIZATION = [*ATTRIBUTES_FOR_SERIALIZATION, :points].freeze + private_constant :TAGS_ATTRIBUTES_FOR_SERIALIZATION + private def limit_per_page(default:, max:) per_page = (params[:per_page] || default).to_i @follows_limit = [per_page, max].min end + + def load_follows_and_paginate(relation) + relation.includes(:followable).page(params[:page]).per(@follows_limit) + end end end end diff --git a/app/controllers/api/v0/github_repos_controller.rb b/app/controllers/api/v0/github_repos_controller.rb index d9e9a5e27..c3af04a3b 100644 --- a/app/controllers/api/v0/github_repos_controller.rb +++ b/app/controllers/api/v0/github_repos_controller.rb @@ -3,9 +3,10 @@ module Api class GithubReposController < ApiController def index client = create_octokit_client - existing_user_repos = GithubRepo. - where(user_id: current_user.id, featured: true).pluck(:github_id_code) #=> [1,2,3] - existing_user_repos = Set.new(existing_user_repos) + + existing_user_repos = current_user.github_repos.where(featured: true). + distinct.pluck(:github_id_code) + @repos = client.repositories.map do |repo| repo.selected = existing_user_repos.include?(repo.id) repo @@ -15,19 +16,22 @@ module Api end def update_or_create - @client = create_octokit_client - fetched_repo = fetch_repo + client = create_octokit_client + + fetched_repo = fetch_repo(client) unless fetched_repo render json: "error: Could not find Github repo", status: :not_found return end - @repo = GithubRepo.find_or_create(fetched_repo_params(fetched_repo)) + repo = GithubRepo.find_or_create(fetched_repo_params(fetched_repo)) + current_user.touch(:github_repos_updated_at) - if @repo.valid? - render json: { featured: @repo.featured } + + if repo.valid? + render json: { featured: repo.featured } else - render json: "error: #{@repo.errors.full_messages}" + render json: "error: #{repo.errors.full_messages}" end end @@ -55,9 +59,10 @@ module Api } end - def fetch_repo + def fetch_repo(client) params[:github_repo] = JSON.parse(params[:github_repo]) - @client.repositories.detect do |repo| + + client.repositories.detect do |repo| repo.id == permitted_attributes(GithubRepo)[:github_id_code].to_i end end diff --git a/app/controllers/api/v0/reactions_controller.rb b/app/controllers/api/v0/reactions_controller.rb index c6b5570d8..fdf4e8286 100644 --- a/app/controllers/api/v0/reactions_controller.rb +++ b/app/controllers/api/v0/reactions_controller.rb @@ -2,21 +2,27 @@ module Api module V0 class ReactionsController < ApplicationController skip_before_action :verify_authenticity_token + def create @user = valid_user + unless @user render json: { message: "invalid_user" }, status: :unprocessable_entity return end + Rails.cache.delete "count_for_reactable-#{params[:reactable_type]}-#{params[:reactable_id]}" + @reaction = Reaction.create( user_id: @user.id, reactable_id: params[:reactable_id], reactable_type: params[:reactable_type], category: params[:category] || "like", ) + Notification.send_reaction_notification(@reaction, @reaction.reactable.user) - Notification.send_reaction_notification(@reaction, @reaction.reactable.organization) if organization_article?(@reaction) + Notification.send_reaction_notification(@reaction, @reaction.reactable.organization) if org_article?(@reaction) + render json: { reaction: @reaction.to_json } end @@ -36,7 +42,7 @@ module Api user end - def organization_article?(reaction) + def org_article?(reaction) reaction.reactable_type == "Article" && reaction.reactable.organization_id end end diff --git a/app/controllers/api/v0/tags_controller.rb b/app/controllers/api/v0/tags_controller.rb index bdb4bd132..0be6cb39e 100644 --- a/app/controllers/api/v0/tags_controller.rb +++ b/app/controllers/api/v0/tags_controller.rb @@ -10,16 +10,22 @@ module Api per_page = (params[:per_page] || 10).to_i num = [per_page, 1000].min - @tags = Tag.order(taggings_count: :desc).page(page).per(num) + @tags = Tag.select(ATTRIBUTES_FOR_SERIALIZATION). + order(taggings_count: :desc). + page(page).per(num) set_surrogate_key_header Tag.table_key, @tags.map(&:record_key) end def onboarding - @tags = Tag.where(name: SiteConfig.suggested_tags) + @tags = Tag.where(name: SiteConfig.suggested_tags). + select(ATTRIBUTES_FOR_SERIALIZATION) set_surrogate_key_header Tag.table_key, @tags.map(&:record_key) end + + ATTRIBUTES_FOR_SERIALIZATION = %i[id name bg_color_hex text_color_hex].freeze + private_constant :ATTRIBUTES_FOR_SERIALIZATION end end end diff --git a/app/controllers/api/v0/users_controller.rb b/app/controllers/api/v0/users_controller.rb index 2c6406f14..1147ea0e4 100644 --- a/app/controllers/api/v0/users_controller.rb +++ b/app/controllers/api/v0/users_controller.rb @@ -11,25 +11,40 @@ module Api return end - if params[:state] == "follow_suggestions" - @users = Suggester::Users::Recent.new(current_user).suggest - elsif params[:state] == "sidebar_suggestions" - given_tag = params[:tag] - @users = Suggester::Users::Sidebar.new(current_user, given_tag).suggest.sample(3) - else - @users = User.none - end + @users = if params[:state] == "follow_suggestions" + Suggester::Users::Recent.new( + current_user, + attributes_to_select: INDEX_ATTRIBUTES_FOR_SERIALIZATION, + ).suggest + elsif params[:state] == "sidebar_suggestions" + Suggester::Users::Sidebar.new(current_user, params[:tag]).suggest.sample(3) + else + User.none + end end def show + relation = User.select(SHOW_ATTRIBUTES_FOR_SERIALIZATION) + @user = if params[:id] == "by_username" - User.find_by(username: params[:url]) + relation.find_by!(username: params[:url]) else - User.find(params[:id]) + relation.find(params[:id]) end end - def me; end + def me + render :show + end + + INDEX_ATTRIBUTES_FOR_SERIALIZATION = %i[id name username summary].freeze + private_constant :INDEX_ATTRIBUTES_FOR_SERIALIZATION + + SHOW_ATTRIBUTES_FOR_SERIALIZATION = %i[ + id username name summary twitter_username github_username website_url + location created_at + ].freeze + private_constant :SHOW_ATTRIBUTES_FOR_SERIALIZATION private diff --git a/app/controllers/api/v0/webhooks_controller.rb b/app/controllers/api/v0/webhooks_controller.rb index 5b801d975..e972ce83a 100644 --- a/app/controllers/api/v0/webhooks_controller.rb +++ b/app/controllers/api/v0/webhooks_controller.rb @@ -14,6 +14,7 @@ module Api @webhook = @user.webhook_endpoints.new(webhook_params) @webhook.oauth_application_id = doorkeeper_token.application_id if doorkeeper_token @webhook.save! + render "show", status: :created end @@ -24,17 +25,23 @@ module Api def destroy webhook = webhooks_scope.find(params[:id]) webhook.destroy! + head :no_content end + ATTRIBUTES_FOR_SERIALIZATION = %i[id user_id source target_url events created_at].freeze + private_constant :ATTRIBUTES_FOR_SERIALIZATION + private def webhooks_scope - if doorkeeper_token - @user.webhook_endpoints.for_app(doorkeeper_token.application_id) - else - @user.webhook_endpoints - end + relation = if doorkeeper_token + @user.webhook_endpoints.for_app(doorkeeper_token.application_id) + else + @user.webhook_endpoints + end + + relation.select(ATTRIBUTES_FOR_SERIALIZATION) end def webhook_params diff --git a/app/models/chat_channel.rb b/app/models/chat_channel.rb index 1ccca2103..289cbbbcd 100644 --- a/app/models/chat_channel.rb +++ b/app/models/chat_channel.rb @@ -149,10 +149,13 @@ class ChatChannel < ApplicationRecord def channel_users # Purely for algolia indexing obj = {} - active_memberships. - order("last_opened_at DESC").includes(:user).each do |membership| + + relation = active_memberships.includes(:user).select(:id, :user_id, :last_opened_at) + + relation.order(last_opened_at: :desc).each do |membership| obj[membership.user.username] = user_obj(membership) end + obj end @@ -160,6 +163,12 @@ class ChatChannel < ApplicationRecord mod_users.pluck(:id) end + def pending_users_select_fields + pending_users.select(:id, :username, :name, :updated_at) + end + + private + def user_obj(membership) { profile_image: ProfileImage.new(membership.user).get(90), @@ -170,8 +179,4 @@ class ChatChannel < ApplicationRecord id: membership.user_id } end - - def pending_users_select_fields - pending_users.select(:id, :username, :name, :updated_at) - end end diff --git a/app/services/article_api_index_service.rb b/app/services/article_api_index_service.rb index 12d91f7e0..5ae93671d 100644 --- a/app/services/article_api_index_service.rb +++ b/app/services/article_api_index_service.rb @@ -1,6 +1,4 @@ class ArticleApiIndexService - attr_accessor :tag, :username, :page, :state, :top, :collection_id, :per_page - DEFAULT_PER_PAGE = 30 MAX_PER_PAGE = 1000 @@ -15,25 +13,25 @@ class ArticleApiIndexService end def get - articles = if tag.present? - tag_articles - elsif username.present? - username_articles - elsif state.present? - state_articles(state) - elsif top.present? - top_articles - elsif collection_id.present? - collection_articles(collection_id) - else - base_articles - end - - articles&.decorate + if tag.present? + tag_articles + elsif username.present? + username_articles + elsif state.present? + state_articles(state) + elsif top.present? + top_articles + elsif collection_id.present? + collection_articles(collection_id) + else + base_articles + end end private + attr_reader :tag, :username, :page, :state, :top, :collection_id, :per_page + def username_articles num = if @state == "all" MAX_PER_PAGE diff --git a/app/services/suggester/users/recent.rb b/app/services/suggester/users/recent.rb index a7b570b0e..deb071822 100644 --- a/app/services/suggester/users/recent.rb +++ b/app/services/suggester/users/recent.rb @@ -1,9 +1,9 @@ module Suggester module Users class Recent - attr_accessor :user - def initialize(user) + def initialize(user, attributes_to_select: []) @user = user + @attributes_to_select = attributes_to_select end def suggest @@ -19,6 +19,8 @@ module Suggester private + attr_reader :user, :attributes_to_select + def tagged_article_user_ids(num_weeks = 1) Article.published. tagged_with(user.decorate.cached_followed_tag_names, any: true). @@ -30,17 +32,27 @@ module Suggester end def recent_producers(num_weeks = 1) - User.where(id: tagged_article_user_ids(num_weeks)).order("updated_at DESC").limit(80).to_a + relation = User.where(id: tagged_article_user_ids(num_weeks)) + relation = relation.select(attributes_to_select) if attributes_to_select + + relation.order("updated_at DESC").limit(80).to_a end def recent_top_producers - User.where("articles_count > ? AND comments_count > ?", - established_user_article_count, established_user_comment_count). - order("updated_at DESC").limit(50).to_a + relation = User.where( + "articles_count > ? AND comments_count > ?", + established_user_article_count, established_user_comment_count + ) + relation = relation.select(attributes_to_select) if attributes_to_select + + relation.order("updated_at DESC").limit(50).to_a end - def recent_commenters(num_coumments = 2, limit = 8) - User.where("comments_count > ?", num_coumments).order("updated_at DESC").limit(limit).to_a + def recent_commenters(num_comments = 2, limit = 8) + relation = User.where("comments_count > ?", num_comments) + relation = relation.select(attributes_to_select) if attributes_to_select + + relation.order("updated_at DESC").limit(limit).to_a end def established_user_article_count diff --git a/app/services/suggester/users/sidebar.rb b/app/services/suggester/users/sidebar.rb index 038c163b8..576022fb3 100644 --- a/app/services/suggester/users/sidebar.rb +++ b/app/services/suggester/users/sidebar.rb @@ -1,7 +1,6 @@ module Suggester module Users class Sidebar - attr_accessor :user, :given_tag def initialize(user, given_tag) @user = user @given_tag = given_tag @@ -28,6 +27,8 @@ module Suggester private + attr_reader :user, :given_tag + def generate_cache_name "tag-#{given_tag}_user-#{user.id}-#{user.last_followed_at}/tag-follow-sugggestions" end diff --git a/app/views/api/v0/articles/_article.json.jbuilder b/app/views/api/v0/articles/_article.json.jbuilder new file mode 100644 index 000000000..668e266b2 --- /dev/null +++ b/app/views/api/v0/articles/_article.json.jbuilder @@ -0,0 +1,20 @@ +json.type_of "article" +json.id article.id +json.title article.title +json.description article.description +json.cover_image cloud_cover_url(article.main_image) +json.readable_publish_date article.readable_publish_date +json.social_image article_social_image_url(article) +json.slug article.slug +json.path article.path +json.url article.url +json.canonical_url article.processed_canonical_url +json.comments_count article.comments_count +json.positive_reactions_count article.positive_reactions_count +json.collection_id article.collection_id +json.created_at article.created_at.utc.iso8601 +json.edited_at article.edited_at&.utc&.iso8601 +json.crossposted_at article.crossposted_at&.utc&.iso8601 +json.published_at article.published_at&.utc&.iso8601 +json.last_comment_at article.last_comment_at&.utc&.iso8601 +json.published_timestamp article.published_timestamp diff --git a/app/views/api/v0/articles/index.json.jbuilder b/app/views/api/v0/articles/index.json.jbuilder index 565f4a8a1..a720ce9c0 100644 --- a/app/views/api/v0/articles/index.json.jbuilder +++ b/app/views/api/v0/articles/index.json.jbuilder @@ -1,26 +1,12 @@ json.array! @articles do |article| - json.type_of "article" - json.id article.id - json.title article.title - json.description article.description - json.cover_image cloud_cover_url(article.main_image) - json.readable_publish_date article.readable_publish_date - json.social_image article_social_image_url(article) - json.tag_list article.cached_tag_list_array - json.tags article.cached_tag_list - json.slug article.slug - json.path article.path - json.url article.url - json.canonical_url article.processed_canonical_url - json.comments_count article.comments_count - json.positive_reactions_count article.positive_reactions_count - json.collection_id article.collection_id - json.created_at article.created_at.utc.iso8601 - json.edited_at article.edited_at&.utc&.iso8601 - json.crossposted_at article.crossposted_at&.utc&.iso8601 - json.published_at article.published_at&.utc&.iso8601 - json.last_comment_at article.last_comment_at&.utc&.iso8601 - json.published_timestamp article.published_timestamp + json.partial! "article", article: article + + # /api/articles and /api/articles/:id have opposite representations + # of `tag_list` and `tags and we can't align them without breaking the API, + # this is fully documented in the API docs + # see for more details + json.tag_list article.cached_tag_list_array + json.tags article.cached_tag_list json.partial! "api/v0/shared/user", user: article.user diff --git a/app/views/api/v0/articles/show.json.jbuilder b/app/views/api/v0/articles/show.json.jbuilder index db4ef7aa1..faaf7f743 100644 --- a/app/views/api/v0/articles/show.json.jbuilder +++ b/app/views/api/v0/articles/show.json.jbuilder @@ -1,27 +1,13 @@ -json.type_of "article" -json.id @article.id -json.title @article.title -json.description @article.description -json.cover_image cloud_cover_url @article.main_image -json.readable_publish_date @article.readable_publish_date -json.social_image article_social_image_url(@article) -json.tag_list @article.cached_tag_list -json.tags @article.cached_tag_list_array -json.slug @article.slug -json.path @article.path -json.url @article.url -json.canonical_url @article.processed_canonical_url -json.comments_count @article.comments_count -json.positive_reactions_count @article.positive_reactions_count -json.collection_id @article.collection_id -json.created_at @article.created_at.utc.iso8601 -json.edited_at @article.edited_at&.utc&.iso8601 -json.crossposted_at @article.crossposted_at&.utc&.iso8601 -json.published_at @article.published_at&.utc&.iso8601 -json.last_comment_at @article.last_comment_at&.utc&.iso8601 -json.published_timestamp @article.published_timestamp +json.partial! "article", article: @article -json.body_html @article.processed_html +# /api/articles and /api/articles/:id have opposite representations +# of `tag_list` and `tags and we can't align them without breaking the API, +# this is fully documented in the API docs +# see for more details +json.tag_list @article.cached_tag_list +json.tags @article.cached_tag_list_array + +json.body_html @article.processed_html json.body_markdown @article.body_markdown json.partial! "api/v0/shared/user", user: @article.user diff --git a/app/views/api/v0/classified_listings/_listing.json.jbuilder b/app/views/api/v0/classified_listings/_listing.json.jbuilder index 9da42a95f..89c10fa43 100644 --- a/app/views/api/v0/classified_listings/_listing.json.jbuilder +++ b/app/views/api/v0/classified_listings/_listing.json.jbuilder @@ -8,4 +8,3 @@ json.tags listing.tag_list json.category listing.category json.processed_html listing.processed_html json.published listing.published -json.slug listing.slug diff --git a/app/views/api/v0/followers/_follows.json.jbuilder b/app/views/api/v0/followers/_follows.json.jbuilder new file mode 100644 index 000000000..ef2aa3ad7 --- /dev/null +++ b/app/views/api/v0/followers/_follows.json.jbuilder @@ -0,0 +1,5 @@ +json.array! follows do |follow| + json.type_of type_of + json.id follow.id + json.partial! "api/v0/shared/follows", user: follow.follower +end diff --git a/app/views/api/v0/followings/tags.json.jbuilder b/app/views/api/v0/followings/tags.json.jbuilder index 8144fd2aa..5b0e53c6a 100644 --- a/app/views/api/v0/followings/tags.json.jbuilder +++ b/app/views/api/v0/followings/tags.json.jbuilder @@ -1,8 +1,11 @@ json.array! @followed_tags do |follow| - json.type_of "tag_following" - json.id follow.id - json.name follow.followable.name - json.points follow.points - json.token form_authenticity_token - json.color HexComparer.new([follow.followable.bg_color_hex || "#0000000", follow.followable.text_color_hex || "#ffffff"]).brightness(0.8) + followable = follow.followable + colors = [followable.bg_color_hex || "#000000", followable.text_color_hex || "#ffffff"] + + json.type_of "tag_following" + json.id follow.id + json.name followable.name + json.points follow.points + json.token form_authenticity_token + json.color HexComparer.new(colors).brightness(0.8) end diff --git a/app/views/api/v0/users/me.json.jbuilder b/app/views/api/v0/users/me.json.jbuilder deleted file mode 100644 index 7dfe48eb1..000000000 --- a/app/views/api/v0/users/me.json.jbuilder +++ /dev/null @@ -1,11 +0,0 @@ -json.type_of "user" -json.id @user.id -json.username @user.username -json.name @user.name -json.summary @user.summary -json.twitter_username @user.twitter_username -json.github_username @user.github_username -json.website_url @user.website_url -json.location @user.location -json.joined_at @user.created_at.strftime("%b %e, %Y") -json.profile_image ProfileImage.new(@user).get(320) diff --git a/config/routes.rb b/config/routes.rb index 96e34afdd..1dfafdbf0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -119,19 +119,15 @@ Rails.application.routes.draw do end end resources :follows, only: [:create] - resources :followers do - collection do - get :users - get :organizations - end + namespace :followers do + get :users + get :organizations end - resources :followings do - collection do - get :users - get :tags - get :organizations - get :podcasts - end + namespace :followings do + get :users + get :tags + get :organizations + get :podcasts end resources :github_repos, only: [:index] do collection do diff --git a/spec/requests/api/v0/chat_channels_spec.rb b/spec/requests/api/v0/chat_channels_spec.rb index 286f41688..ca201e73a 100644 --- a/spec/requests/api/v0/chat_channels_spec.rb +++ b/spec/requests/api/v0/chat_channels_spec.rb @@ -1,27 +1,107 @@ require "rails_helper" RSpec.describe "Api::V0::ChatChannels", type: :request do - let(:user) { create(:user) } let(:chat_channel) { create(:chat_channel) } - let(:invite_channel) { create(:chat_channel, channel_type: "invite_only") } - - before { sign_in user } describe "GET /api/chat_channels/:id" do - it "returns 200 if user is a member of the channel" do - chat_channel.add_users([user]) - get "/api/chat_channels/#{chat_channel.id}" - expect(response).to have_http_status(:ok) + let(:user) { create(:user) } + + context "when there is no user signed in" do + before do + chat_channel.add_users([user]) + end + + it "returns not found" do + get api_chat_channel_path(chat_channel.id) + + expect(response).to have_http_status(:not_found) + end end - it "returns a 404 if user is not a memeber of the channel" do - get "/api/chat_channels/#{invite_channel.id}" - expect(response.status).to eq(404) + context "when the current user is a member of the channel" do + before do + chat_channel.add_users([user]) + + sign_in user + end + + it "returns ok if user is a member of the channel" do + get api_chat_channel_path(chat_channel.id) + + expect(response).to have_http_status(:ok) + end + + it "returns not found if channel id does not exist" do + get api_chat_channel_path("foobar") + + expect(response).to have_http_status(:not_found) + end + + it "returns chat channel with the correct json representation", :aggregate_failures do + get api_chat_channel_path(chat_channel.id) + + response_channel = response.parsed_body + expected_keys = %w[ + type_of id description channel_name username channel_users channel_mod_ids pending_users_select_fields + ] + expect(response_channel.keys).to match_array(expected_keys) + + %w[id description channel_name channel_mod_ids].each do |attr| + expect(response_channel[attr]).to eq(chat_channel.public_send(attr)) + end + + expect(response_channel["username"]).to eq(chat_channel.channel_name) + expect(response_channel["pending_users_select_fields"]).to be_empty + end + + it "returns the correct channel users representation" do + get api_chat_channel_path(chat_channel.id) + + response_channel = response.parsed_body + response_channel_users = response_channel["channel_users"] + + membership = user.chat_channel_memberships.last + expected_last_opened_at = Time.zone.parse(response_channel_users[user.username]["last_opened_at"]).to_i + response_user = response_channel_users[user.username] + + expect(response_user["profile_image"]).to eq(ProfileImage.new(user).get(90)) + expect(response_user["darker_color"]).to eq(user.decorate.darker_color) + expect(response_user["name"]).to eq(user.name) + expect(expected_last_opened_at).to eq(membership.last_opened_at.to_i) + expect(response_user["username"]).to eq(user.username) + expect(response_user["id"]).to eq(user.id) + end + + it "returns the correct pending users select fields representation" do + # add another user's pending membership + pending_user = create(:user) + chat_channel.add_users(pending_user) + pending_user.chat_channel_memberships.last.update(status: :pending) + + get api_chat_channel_path(chat_channel.id) + + response_channel = response.parsed_body + response_pending_user_select_fields = response_channel["pending_users_select_fields"].first + + expected_updated_at = Time.zone.parse(response_pending_user_select_fields["updated_at"]).to_i + + expect(response_pending_user_select_fields["id"]).to eq(pending_user.id) + expect(response_pending_user_select_fields["name"]).to eq(pending_user.name) + expect(expected_updated_at).to eq(pending_user.updated_at.to_i) + expect(response_pending_user_select_fields["username"]).to eq(pending_user.username) + end end - it "returns a 404 if channel is not found" do - get "/api/chat_channels/#{invite_channel.id + 100}" - expect(response.status).to eq(404) + context "when the current user is not a member of the channel" do + before do + sign_in user + end + + it "returns not found if user is not a member" do + get api_chat_channel_path(chat_channel.id) + + expect(response).to have_http_status(:not_found) + end end end end diff --git a/spec/requests/api/v0/classified_listings_spec.rb b/spec/requests/api/v0/classified_listings_spec.rb index 19477ef69..93002aa1d 100644 --- a/spec/requests/api/v0/classified_listings_spec.rb +++ b/spec/requests/api/v0/classified_listings_spec.rb @@ -45,10 +45,6 @@ RSpec.describe "Api::V0::Listings" do end end - def json_response - JSON.parse(response.body) - end - describe "GET /api/listings" do include_context "with 7 listings and 2 user" @@ -60,18 +56,18 @@ RSpec.describe "Api::V0::Listings" do it "returns listings created" do get api_classified_listings_path - expect(json_response.size).to eq(7) - expect(json_response.first["type_of"]).to eq("classified_listing") - expect(json_response.first["slug"]).to eq(ClassifiedListing.last.slug) - expect(json_response.first["user"]).to include("username") - expect(json_response.first["user"]["username"]).not_to be_empty + expect(response.parsed_body.size).to eq(7) + expect(response.parsed_body.first["type_of"]).to eq("classified_listing") + expect(response.parsed_body.first["slug"]).to eq(ClassifiedListing.last.slug) + expect(response.parsed_body.first["user"]).to include("username") + expect(response.parsed_body.first["user"]["username"]).not_to be_empty end it "supports pagination" do get api_classified_listings_path, params: { page: 2, per_page: 2 } - expect(json_response.length).to eq(2) + expect(response.parsed_body.length).to eq(2) get api_classified_listings_path, params: { page: 4, per_page: 2 } - expect(json_response.length).to eq(1) + expect(response.parsed_body.length).to eq(1) end end @@ -81,7 +77,7 @@ RSpec.describe "Api::V0::Listings" do it "displays only listings from the cfp category" do get api_classified_listings_category_path("cfp") expect(response).to have_http_status(:ok) - expect(json_response.size).to eq(3) + expect(response.parsed_body.size).to eq(3) end end @@ -91,11 +87,13 @@ RSpec.describe "Api::V0::Listings" do it "displays a unique listing" do get api_classified_listing_path(listing.id) + expect(response).to have_http_status(:ok) - expect(json_response["type_of"]).to eq("classified_listing") - expect(json_response["slug"]).to eq(listing.slug) - expect(json_response["user"]).to include("username") - expect(json_response["user"]["username"]).not_to be_empty + + expect(response.parsed_body["type_of"]).to eq("classified_listing") + expect(response.parsed_body["slug"]).to eq(listing.slug) + expect(response.parsed_body["user"]).to include("username") + expect(response.parsed_body["user"]["username"]).not_to be_empty end end @@ -215,7 +213,7 @@ RSpec.describe "Api::V0::Listings" do false end post_classified_listing(draft_params) - expect(json_response["errors"]["base"]).to eq(["is invalid"]) + expect(response.parsed_body["errors"]["base"]).to eq(["is invalid"]) expect(response).to have_http_status(:unprocessable_entity) end @@ -240,7 +238,7 @@ RSpec.describe "Api::V0::Listings" do post_classified_listing(listing_params) expect(response).to have_http_status(:created) end.to change(ClassifiedListing, :count).by(1) - expect(ClassifiedListing.find(json_response["id"]).user).to eq(user) + expect(ClassifiedListing.find(response.parsed_body["id"]).user).to eq(user) end it "creates a classified listing with a title, a body markdown, a category" do @@ -248,9 +246,9 @@ RSpec.describe "Api::V0::Listings" do post_classified_listing(listing_params) expect(response).to have_http_status(:created) end.to change(ClassifiedListing, :count).by(1) - expect(ClassifiedListing.find(json_response["id"]).title).to eq("Title") - expect(ClassifiedListing.find(json_response["id"]).body_markdown).to eq("Markdown text") - expect(ClassifiedListing.find(json_response["id"]).category).to eq("cfp") + expect(ClassifiedListing.find(response.parsed_body["id"]).title).to eq("Title") + expect(ClassifiedListing.find(response.parsed_body["id"]).body_markdown).to eq("Markdown text") + expect(ClassifiedListing.find(response.parsed_body["id"]).category).to eq("cfp") end it "creates a classified listing with a location" do @@ -259,7 +257,7 @@ RSpec.describe "Api::V0::Listings" do post_classified_listing(params) expect(response).to have_http_status(:created) end.to change(ClassifiedListing, :count).by(1) - expect(ClassifiedListing.find(json_response["id"]).location).to eq("Frejus") + expect(ClassifiedListing.find(response.parsed_body["id"]).location).to eq("Frejus") end it "creates a classified listing with a list of tags and a contact" do @@ -268,8 +266,8 @@ RSpec.describe "Api::V0::Listings" do post_classified_listing(params) expect(response).to have_http_status(:created) end.to change(ClassifiedListing, :count).by(1) - expect(ClassifiedListing.find(json_response["id"]).cached_tag_list).to eq("discuss, javascript") - expect(ClassifiedListing.find(json_response["id"]).contact_via_connect).to eq(true) + expect(ClassifiedListing.find(response.parsed_body["id"]).cached_tag_list).to eq("discuss, javascript") + expect(ClassifiedListing.find(response.parsed_body["id"]).contact_via_connect).to eq(true) end end diff --git a/spec/requests/api/v0/followers_spec.rb b/spec/requests/api/v0/followers_spec.rb index 68288a751..c46552586 100644 --- a/spec/requests/api/v0/followers_spec.rb +++ b/spec/requests/api/v0/followers_spec.rb @@ -1,27 +1,79 @@ require "rails_helper" RSpec.describe "Api::V0::FollowersController", type: :request do - let(:user) { create(:user) } let(:follower) { create(:user) } - describe "GET /api/followers" do + describe "GET /api/followers/organizations" do + let(:org) { create(:organization) } + let(:user) { create(:user, organization_id: org.id) } + before do - follower.follow user - user.reload + follower.follow(org) + + org.reload end context "when user is unauthorized" do - it "does not return user followers list" do - get users_api_followers_path - expect(response.body).not_to include follower.name + it "returns unauthorized" do + get api_followers_organizations_path + + expect(response).to have_http_status(:unauthorized) end end context "when user is authorized" do - it "returns user followers list" do + before do sign_in user - get users_api_followers_path - expect(response.body).to include follower.name + end + + it "returns user's followers list with the correct format" do + get api_followers_organizations_path + expect(response).to have_http_status(:ok) + + response_follower = response.parsed_body.first + expect(response_follower["type_of"]).to eq("organization_follower") + expect(response_follower["id"]).to eq(follower.follows.last.id) + expect(response_follower["name"]).to eq(follower.name) + expect(response_follower["path"]).to eq(follower.path) + expect(response_follower["username"]).to eq(follower.username) + expect(response_follower["profile_image"]).to eq(ProfileImage.new(follower).get(60)) + end + end + end + + describe "GET /api/followers/users" do + let(:user) { create(:user) } + + before do + follower.follow(user) + + user.reload + end + + context "when user is unauthorized" do + it "returns unauthorized" do + get api_followers_users_path + + expect(response).to have_http_status(:unauthorized) + end + end + + context "when user is authorized" do + before do + sign_in user + end + + it "returns user's followers list with the correct format" do + get api_followers_users_path + expect(response).to have_http_status(:ok) + + response_follower = response.parsed_body.first + expect(response_follower["type_of"]).to eq("user_follower") + expect(response_follower["id"]).to eq(follower.follows.last.id) + expect(response_follower["name"]).to eq(follower.name) + expect(response_follower["path"]).to eq(follower.path) + expect(response_follower["username"]).to eq(follower.username) + expect(response_follower["profile_image"]).to eq(ProfileImage.new(follower).get(60)) end end end diff --git a/spec/requests/api/v0/followings_spec.rb b/spec/requests/api/v0/followings_spec.rb index 40e82fe4d..52f701eac 100644 --- a/spec/requests/api/v0/followings_spec.rb +++ b/spec/requests/api/v0/followings_spec.rb @@ -2,40 +2,154 @@ require "rails_helper" RSpec.describe "Api::V0::FollowingsController", type: :request do let(:user) { create(:user) } - let(:user2) { create(:user) } - describe "GET /api/followings" do - let(:tag) { create(:tag) } - let(:podcast) { create(:podcast) } - let(:organization) { create(:organization) } + describe "GET /api/followings/users" do + let(:followed) { create(:user) } before do - sign_in user - user.follow user2 - user.follow tag - user.follow organization - user.follow podcast + user.follow(followed) + user.reload end - it "returns following users list" do - get "/api/followings/users" - expect(response.body).to include user2.name + context "when user is unauthorized" do + it "returns unauthorized" do + get api_followings_users_path + + expect(response).to have_http_status(:unauthorized) + end end - it "returns following tag list" do - get "/api/followings/tags" - expect(response.body).to include tag.name + context "when user is authorized" do + before do + sign_in user + end + + it "returns user's followings list with the correct format" do + get api_followings_users_path + expect(response).to have_http_status(:ok) + + response_following = response.parsed_body.first + expect(response_following["type_of"]).to eq("user_following") + expect(response_following["id"]).to eq(user.follows.last.id) + expect(response_following["name"]).to eq(followed.name) + expect(response_following["path"]).to eq(followed.path) + expect(response_following["username"]).to eq(followed.username) + expect(response_following["profile_image"]).to eq(ProfileImage.new(followed).get(60)) + end + end + end + + describe "GET /api/followings/tags" do + let(:followed) { create(:tag) } + + before do + user.follow(followed) + + user.reload end - it "returns following organization list" do - get "/api/followings/organizations" - expect(response.body).to include organization.name + context "when user is unauthorized" do + it "returns unauthorized" do + get api_followings_tags_path + + expect(response).to have_http_status(:unauthorized) + end end - it "returns following podcast list" do - get "/api/followings/podcasts" - expect(response.body).to include podcast.name + context "when user is authorized" do + before do + sign_in user + end + + it "returns user's followings list with the correct format" do + get api_followings_tags_path + expect(response).to have_http_status(:ok) + + follow = user.follows.last + response_following = response.parsed_body.first + + expect(response_following["type_of"]).to eq("tag_following") + expect(response_following["id"]).to eq(follow.id) + expect(response_following["name"]).to eq(followed.name) + expect(response_following["points"]).to eq(follow.points) + expect(response_following["token"]).to be_present + expect(response_following["color"]).to eq("#000000") + end + end + end + + describe "GET /api/followings/organizations" do + let(:followed) { create(:organization) } + + before do + user.follow(followed) + + user.reload + end + + context "when user is unauthorized" do + it "returns unauthorized" do + get api_followings_organizations_path + + expect(response).to have_http_status(:unauthorized) + end + end + + context "when user is authorized" do + before do + sign_in user + end + + it "returns user's followings list with the correct format" do + get api_followings_organizations_path + expect(response).to have_http_status(:ok) + + response_following = response.parsed_body.first + expect(response_following["type_of"]).to eq("organization_following") + expect(response_following["id"]).to eq(user.follows.last.id) + expect(response_following["name"]).to eq(followed.name) + expect(response_following["path"]).to eq(followed.path) + expect(response_following["username"]).to eq(followed.username) + expect(response_following["profile_image"]).to eq(ProfileImage.new(followed).get(60)) + end + end + end + + describe "GET /api/followings/podcasts" do + let(:followed) { create(:podcast) } + + before do + user.follow(followed) + + user.reload + end + + context "when user is unauthorized" do + it "returns unauthorized" do + get api_followings_podcasts_path + + expect(response).to have_http_status(:unauthorized) + end + end + + context "when user is authorized" do + before do + sign_in user + end + + it "returns user's followings list with the correct format" do + get api_followings_podcasts_path + expect(response).to have_http_status(:ok) + + response_following = response.parsed_body.first + expect(response_following["type_of"]).to eq("podcast_following") + expect(response_following["id"]).to eq(user.follows.last.id) + expect(response_following["name"]).to eq(followed.name) + expect(response_following["path"]).to eq("/#{followed.path}") + expect(response_following["username"]).to eq(followed.name) + expect(response_following["profile_image"]).to eq(ProfileImage.new(followed).get(60)) + end end end end diff --git a/spec/requests/api/v0/follows_spec.rb b/spec/requests/api/v0/follows_spec.rb index 9fbbab78c..a2c42b731 100644 --- a/spec/requests/api/v0/follows_spec.rb +++ b/spec/requests/api/v0/follows_spec.rb @@ -17,8 +17,7 @@ RSpec.describe "Api::V0::FollowsController", type: :request do it "returns the number of followed users" do post "/api/follows", params: { users: users_hash } - json_response = JSON.parse(response.body) - expect(json_response["outcome"]).to include("#{users_hash.size} users") + expect(response.parsed_body["outcome"]).to include("#{users_hash.size} users") end it "creates follows" do diff --git a/spec/requests/api/v0/github_repos_spec.rb b/spec/requests/api/v0/github_repos_spec.rb index 5783efb77..09396658e 100644 --- a/spec/requests/api/v0/github_repos_spec.rb +++ b/spec/requests/api/v0/github_repos_spec.rb @@ -16,22 +16,34 @@ RSpec.describe "Api::V0::GithubRepos", type: :request do describe "GET /api/v0/github_repos" do it "returns 200 on success" do - get "/api/github_repos" + get api_github_repos_path + expect(response).to have_http_status(:ok) end it "returns 401 if github raises an unauthorized error" do allow(Octokit::Client).to receive(:new).and_raise(Octokit::Unauthorized) - get "/api/github_repos" + + get api_github_repos_path expect(response).to have_http_status(:unauthorized) expect(response.parsed_body["error"]).to include("Github Unauthorized") end + + it "returns repos with the correct json representation" do + get api_github_repos_path + + response_repo = response.parsed_body.first + expect(response_repo["name"]).to eq(repo.name) + expect(response_repo["fork"]).to eq(repo.fork) + expect(response_repo["selected"]).to be(false) + end end describe "POST /api/v0/github_repos/update_or_create" do it "returns 200 and json response on success" do param = stubbed_github_repos.first.to_h.to_json - post "/api/github_repos/update_or_create", params: { github_repo: param } + post update_or_create_api_github_repos_path(github_repo: param) + expect(response).to have_http_status(:ok) expect(response.content_type).to eq("application/json") end @@ -39,7 +51,8 @@ RSpec.describe "Api::V0::GithubRepos", type: :request do it "returns 404 and json response on error" do allow(Octokit::Client).to receive(:new).and_return(my_ocktokit_client) allow(my_ocktokit_client).to receive(:repositories).and_return([]) - post "/api/github_repos/update_or_create", params: { github_repo: "{}" } + + post update_or_create_api_github_repos_path(github_repo: "{}") expect(response).to have_http_status(:not_found) expect(response.body).to include("Could not find Github repo") end diff --git a/spec/requests/api/v0/reactions_spec.rb b/spec/requests/api/v0/reactions_spec.rb new file mode 100644 index 000000000..31b794133 --- /dev/null +++ b/spec/requests/api/v0/reactions_spec.rb @@ -0,0 +1,31 @@ +require "rails_helper" + +RSpec.describe "Api::V0::Reactions", type: :request do + let(:user) { create(:user, secret: "TEST_SECRET") } + let(:article) { create(:article) } + + describe "POST /api/reactions" do + it "creates a new reaction for super admin users" do + user.add_role(:super_admin) + sign_in user + + expect do + post api_reactions_path( + reactable_id: article.id, reactable_type: "Article", + category: "like", key: user.secret + ) + expect(response).to have_http_status(:ok) + end.to change(article.reactions, :count).by(1) + end + + it "rejects non-authorized users" do + sign_in user + + post api_reactions_path( + reactable_id: article.id, reactable_type: "Article", + category: "like", key: user.secret + ) + expect(response).to have_http_status(:unprocessable_entity) + end + end +end diff --git a/spec/requests/api/v0/users_spec.rb b/spec/requests/api/v0/users_spec.rb index 8971e1f10..3ef4dd7e2 100644 --- a/spec/requests/api/v0/users_spec.rb +++ b/spec/requests/api/v0/users_spec.rb @@ -1,15 +1,90 @@ require "rails_helper" RSpec.describe "Api::V0::Users", type: :request do - def json_response - JSON.parse(response.body) - end - describe "GET /api/users" do it "returns no users if state is not present" do - get api_users_path, params: {} - expect(response).to have_http_status(:success) - expect(json_response).to be_empty + get api_users_path + + expect(response).to have_http_status(:ok) + + expect(response.parsed_body).to be_empty + end + + it "returns users from the original core team if they are present" do + user = create(:user, username: "ben", summary: "Something something") + + get api_users_path + + expect(response).to have_http_status(:ok) + + response_user = response.parsed_body.first + expect(response_user["id"]).to eq(user.id) + expect(response_user["name"]).to eq(user.name) + expect(response_user["username"]).to eq(user.username) + expect(response_user["summary"]).to eq(user.summary) + expect(response_user["profile_image_url"]).to eq(ProfileImage.new(user).get(90)) + expect(response_user["following"]).to be(false) + end + + it "returns follow suggestions for an authenticated user" do + user = create(:user) + tag = create(:tag) + user.follow(tag) + + other_user = create(:user) + create(:article, user: other_user, tags: [tag.name]) + + sign_in user + + get api_users_path(state: "follow_suggestions") + + response_user = response.parsed_body.first + expect(response_user["id"]).to eq(other_user.id) + end + + it "returns no sidebar suggestions for an authenticated user" do + sign_in create(:user) + + get api_users_path(state: "sidebar_suggestions") + + expect(response.parsed_body).to be_empty + end + end + + describe "GET /api/users/:id" do + let(:user) { create(:user, summary: "Something something", profile_image: "") } + + it "returns 404 if the user id is not found" do + get api_user_path("invalid-id") + + expect(response).to have_http_status(:not_found) + end + + it "returns 404 if the user username is not found" do + get api_user_path("by_username"), params: { url: "invalid-username" } + expect(response).to have_http_status(:not_found) + end + + it "returns 200 if the user username is found" do + get api_user_path("by_username"), params: { url: user.username } + expect(response).to have_http_status(:ok) + end + + it "returns the correct json representation of the user", :aggregate_failures do + get api_user_path(user.id) + + response_user = response.parsed_body + + expect(response_user["type_of"]).to eq("user") + + %w[ + id username name summary twitter_username github_username website_url location + ].each do |attr| + expect(response_user[attr]).to eq(user.public_send(attr)) + end + + expect(response_user["joined_at"]).to eq(user.created_at.strftime("%b %e, %Y")) + expect(response_user["profile_image"]).to eq(ProfileImage.new(user).get(320)) end end @@ -23,9 +98,21 @@ RSpec.describe "Api::V0::Users", type: :request do let_it_be(:user) { create(:user) } let_it_be(:access_token) { create(:doorkeeper_access_token, resource_owner: user, scopes: "public") } - it "return user's information" do + it "returns the correct json representation of the user", :aggregate_failures do get me_api_users_path, params: { access_token: access_token.token } - expect(json_response["username"]).to eq(user.username) + + response_user = response.parsed_body + + expect(response_user["type_of"]).to eq("user") + + %w[ + id username name summary twitter_username github_username website_url location + ].each do |attr| + expect(response_user[attr]).to eq(user.public_send(attr)) + end + + expect(response_user["joined_at"]).to eq(user.created_at.strftime("%b %e, %Y")) + expect(response_user["profile_image"]).to eq(ProfileImage.new(user).get(320)) end end end diff --git a/spec/requests/api/v0/webhooks_spec.rb b/spec/requests/api/v0/webhooks_spec.rb index 38661a7ce..fcb4b22fd 100644 --- a/spec/requests/api/v0/webhooks_spec.rb +++ b/spec/requests/api/v0/webhooks_spec.rb @@ -1,11 +1,15 @@ require "rails_helper" RSpec.describe "Api::V0::Webhooks", type: :request do - let(:user) { create(:user) } + let_it_be_changeable(:user) { create(:user) } + let_it_be_changeable(:webhook) do + create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") + end describe "GET /api/v0/webhooks" do - let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") } - let!(:webhook2) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/webhook") } + let_it_be_readonly(:webhook2) do + create(:webhook_endpoint, user: user, target_url: "https://api.example.com/webhook") + end before do sign_in user @@ -19,42 +23,79 @@ RSpec.describe "Api::V0::Webhooks", type: :request do it "returns json on success" do get api_webhooks_path - json = JSON.parse(response.body) + + json = response.parsed_body ids = json.map { |item| item["id"] } urls = json.map { |item| item["target_url"] } expect(ids).to eq([webhook.id, webhook2.id]) expect(urls).to eq(%w[https://api.example.com/go https://api.example.com/webhook]) end + + it "returns the correct json representation" do + get api_webhooks_path + + response_webhook = response.parsed_body.first + + expect(response_webhook["type_of"]).to eq("webhook_endpoint") + expect(response_webhook["id"]).to eq(webhook.id) + expect(response_webhook["source"]).to eq(webhook.source) + expect(response_webhook["target_url"]).to eq(webhook.target_url) + expect(response_webhook["events"]).to eq(webhook.events) + expect(response_webhook["created_at"]).to eq(webhook.created_at.rfc3339) + end end describe "GET /api/v0/webhooks/:id" do - let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") } - before do sign_in user end it "returns 200 on success" do get api_webhook_path(webhook.id) + expect(response).to have_http_status(:ok) end it "returns 404 if the webhook does not exist" do get api_webhook_path(9999) + expect(response).to have_http_status(:not_found) end it "returns 404 if another user webhook is accessed" do other_webhook = create(:webhook_endpoint, user: create(:user)) + get api_webhook_path(other_webhook.id) + expect(response).to have_http_status(:not_found) end - it "returns json on success" do + it "returns the correct json representation" do get api_webhook_path(webhook.id) - json = JSON.parse(response.body) - expect(json["target_url"]).to eq(webhook.target_url) - expect(json["user"]["username"]).to eq(user.username) + + response_webhook = response.parsed_body + + expect(response_webhook["type_of"]).to eq("webhook_endpoint") + expect(response_webhook["id"]).to eq(webhook.id) + expect(response_webhook["source"]).to eq(webhook.source) + expect(response_webhook["target_url"]).to eq(webhook.target_url) + expect(response_webhook["events"]).to eq(webhook.events) + expect(response_webhook["created_at"]).to eq(webhook.created_at.rfc3339) + end + + it "returns the correct json representation for the webhook user" do + get api_webhook_path(webhook.id) + + response_webhook_user = response.parsed_body["user"] + user_profile_image = ProfileImage.new(webhook.user) + + expect(response_webhook_user["name"]).to eq(webhook.user.name) + expect(response_webhook_user["username"]).to eq(webhook.user.username) + expect(response_webhook_user["twitter_username"]).to eq(webhook.user.twitter_username) + expect(response_webhook_user["github_username"]).to eq(webhook.user.github_username) + expect(response_webhook_user["website_url"]).to eq(webhook.user.processed_website_url) + expect(response_webhook_user["profile_image"]).to eq(user_profile_image.get(640)) + expect(response_webhook_user["profile_image_90"]).to eq(user_profile_image.get(90)) end end @@ -96,8 +137,6 @@ RSpec.describe "Api::V0::Webhooks", type: :request do end describe "DELETE /api/v0/webhooks/:id" do - let!(:webhook) { create(:webhook_endpoint, user: user, target_url: "https://api.example.com/go") } - before do sign_in user end diff --git a/spec/requests/reactions_api_spec.rb b/spec/requests/reactions_api_spec.rb deleted file mode 100644 index 8d7d45f57..000000000 --- a/spec/requests/reactions_api_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# http://localhost:3000/api/comments?a_id=23 -require "rails_helper" - -RSpec.describe "ArticlesApi", type: :request do - let(:user) { create(:user, :super_admin) } - let(:article) { create(:article) } - - before do - user.update(secret: "TEST_SECRET") - sign_in user - end - - describe "POST /api/reactions" do - it "creates a new reactions" do - post "/api/reactions", params: { - reactable_id: article.id, reactable_type: "Article", category: "like", key: user.secret - } - expect(Reaction.last.reactable_id).to eq(article.id) - end - - it "rejects non-authorized users" do - user.remove_role(:super_admin) - post "/api/reactions", params: { - reactable_id: article.id, reactable_type: "Article", category: "like", key: user.secret - } - expect(response).to have_http_status(:unprocessable_entity) - end - end -end diff --git a/spec/requests/users_api_spec.rb b/spec/requests/users_api_spec.rb deleted file mode 100644 index 5596fb223..000000000 --- a/spec/requests/users_api_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -require "rails_helper" - -RSpec.describe "Api::V0::Users", type: :request do - let(:user) { create(:user) } - let(:tag) { create(:tag) } - - describe "GET /api/users" do - it "returns user objects" do - user.follow(tag) - - other_user = create(:user) - create(:article, user: other_user, tags: [tag.name]) - - sign_in user - get "/api/users?state=follow_suggestions" - expect(response.body).to include(other_user.name) - end - end - - describe "GET /api/users/:id" do - it "returns user show object" do - get "/api/users/#{user.id}" - expect(response.body).to include(user.name) - end - end -end diff --git a/spec/services/suggester/users/sidebar_spec.rb b/spec/services/suggester/users/sidebar_spec.rb index 954895408..149e15baf 100644 --- a/spec/services/suggester/users/sidebar_spec.rb +++ b/spec/services/suggester/users/sidebar_spec.rb @@ -1,5 +1,5 @@ require "rails_helper" -# TODO: improve this test + RSpec.describe Suggester::Users::Sidebar, type: :service do let(:user) { create(:user) }