diff --git a/app/controllers/api/v1/contacts_controller.rb b/app/controllers/api/v1/contacts_controller.rb new file mode 100644 index 000000000..3fc4dcf2e --- /dev/null +++ b/app/controllers/api/v1/contacts_controller.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Api + module V1 + class ContactsController < Api::V1::BaseController + before_action only: %i[index] do + require_access_token %w[read] + end + + before_action only: %i[create destroy] do + require_access_token %w[read write] + end + + rescue_from ActiveRecord::RecordNotFound do + render json: I18n.t("api.endpoint_errors.aspects.not_found"), status: :not_found + end + + def index + contacts = aspects_membership_service.contacts_in_aspect(params[:aspect_id]) + render json: contacts.map {|c| ContactPresenter.new(c, current_user).as_api_json_without_contact } + end + + def create + aspect_id = params[:aspect_id] + person = Person.find_by(guid: params[:person_guid]) + aspect_membership = aspects_membership_service.create(aspect_id, person.id) if person.present? + if aspect_membership + head :no_content + else + render json: I18n.t("api.endpoint_errors.contacts.cant_create"), status: :unprocessable_entity + end + rescue ActiveRecord::RecordNotUnique + render json: I18n.t("api.endpoint_errors.contacts.cant_create"), status: :unprocessable_entity + end + + def destroy + aspect_id = params[:aspect_id] + person = Person.find_by(guid: params[:id]) + result = aspects_membership_service.destroy_by_ids(aspect_id, person.id) if person.present? + if result && result[:success] + head :no_content + else + render json: I18n.t("api.endpoint_errors.contacts.cant_delete"), status: :unprocessable_entity + end + rescue ActiveRecord::RecordNotFound + render json: I18n.t("api.endpoint_errors.contacts.not_found"), status: :not_found + end + + def aspects_membership_service + AspectsMembershipService.new(current_user) + end + end + end +end diff --git a/app/controllers/aspect_memberships_controller.rb b/app/controllers/aspect_memberships_controller.rb index 69fb4098f..e85f374be 100644 --- a/app/controllers/aspect_memberships_controller.rb +++ b/app/controllers/aspect_memberships_controller.rb @@ -11,20 +11,9 @@ class AspectMembershipsController < ApplicationController respond_to :json def destroy - aspect = current_user.aspects.joins(:aspect_memberships).where(aspect_memberships: {id: params[:id]}).first - contact = current_user.contacts.joins(:aspect_memberships).where(aspect_memberships: {id: params[:id]}).first - - raise ActiveRecord::RecordNotFound unless aspect.present? && contact.present? - - raise Diaspora::NotMine unless current_user.mine?(aspect) && - current_user.mine?(contact) - - membership = contact.aspect_memberships.where(aspect_id: aspect.id).first - - raise ActiveRecord::RecordNotFound unless membership.present? - - # do it! - success = membership.destroy + delete_results = AspectsMembershipService.new(current_user).destroy_by_membership_id(params[:id]) + success = delete_results[:success] + membership = delete_results[:membership] # set the flash message respond_to do |format| @@ -39,17 +28,12 @@ class AspectMembershipsController < ApplicationController end def create - @person = Person.find(params[:person_id]) - @aspect = current_user.aspects.where(id: params[:aspect_id]).first + aspect_membership = AspectsMembershipService.new(current_user).create(params[:aspect_id], params[:person_id]) - @contact = current_user.share_with(@person, @aspect) - - if @contact.present? + if aspect_membership respond_to do |format| format.json do - render json: AspectMembershipPresenter.new( - AspectMembership.where(contact_id: @contact.id, aspect_id: @aspect.id).first) - .base_hash + render json: AspectMembershipPresenter.new(aspect_membership).base_hash end end else @@ -59,6 +43,12 @@ class AspectMembershipsController < ApplicationController end end end + rescue RuntimeError + respond_to do |format| + format.json do + render plain: I18n.t("aspects.add_to_aspect.failure"), status: :conflict + end + end end rescue_from ActiveRecord::StatementInvalid do diff --git a/app/presenters/contact_presenter.rb b/app/presenters/contact_presenter.rb index dba67bf20..d3cf1e545 100644 --- a/app/presenters/contact_presenter.rb +++ b/app/presenters/contact_presenter.rb @@ -17,6 +17,9 @@ class ContactPresenter < BasePresenter full_hash.merge(person: person_without_contact) end + def as_api_json_without_contact + PersonPresenter.new(person, current_user).as_api_json + end private def person_without_contact diff --git a/app/services/aspects_membership_service.rb b/app/services/aspects_membership_service.rb new file mode 100644 index 000000000..02b0f07d4 --- /dev/null +++ b/app/services/aspects_membership_service.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +class AspectsMembershipService + def initialize(user=nil) + @user = user + end + + def create(aspect_id, person_id) + person = Person.find(person_id) + aspect = @user.aspects.where(id: aspect_id).first + raise ActiveRecord::RecordNotFound unless person.present? && aspect.present? + + contact = @user.share_with(person, aspect) + raise I18n.t("aspects.add_to_aspect.failure") if contact.blank? + + AspectMembership.where(contact_id: contact.id, aspect_id: aspect.id).first + end + + def destroy_by_ids(aspect_id, contact_id) + aspect = @user.aspects.where(id: aspect_id).first + contact = @user.contacts.where(person_id: contact_id).first + destroy(aspect, contact) + end + + def destroy_by_membership_id(membership_id) + aspect = @user.aspects.joins(:aspect_memberships).where(aspect_memberships: {id: membership_id}).first + contact = @user.contacts.joins(:aspect_memberships).where(aspect_memberships: {id: membership_id}).first + destroy(aspect, contact) + end + + def contacts_in_aspect(aspect_id) + order = ["contact_id IS NOT NULL DESC", "profiles.first_name ASC", "profiles.last_name ASC", + "profiles.diaspora_handle ASC"] + @user.aspects.find(aspect_id) # to provide better error code if aspect isn't correct + contacts = @user.contacts.arel_table + aspect_memberships = AspectMembership.arel_table + @user.contacts.joins( + contacts.join(aspect_memberships).on( + aspect_memberships[:aspect_id].eq(aspect_id).and( + aspect_memberships[:contact_id].eq(contacts[:id]) + ) + ).join_sources + ).includes(person: :profile).order(order) + end + + private + + def destroy(aspect, contact) + raise ActiveRecord::RecordNotFound unless aspect.present? && contact.present? + raise Diaspora::NotMine unless @user.mine?(aspect) && @user.mine?(contact) + + membership = contact.aspect_memberships.where(aspect_id: aspect.id).first + raise ActiveRecord::RecordNotFound if membership.blank? + + success = membership.destroy + + {success: success, membership: membership} + end +end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index b0468c537..7e0efd561 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -964,6 +964,10 @@ en: not_allowed: "User is not allowed to comment" no_delete: "User not allowed to delete the comment" duplicate_report: "This item already has been reported by this user" + contacts: + not_found: "Aspect or contact on aspect not found" + cant_create: "Failed to add user to aspect" + cant_delete: "Failed to remove user from aspect" likes: user_not_allowed_to_like: "User is not allowed to like" like_exists: "Like already exists" diff --git a/config/routes.rb b/config/routes.rb index 8f98bd050..b5545ea37 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -222,7 +222,9 @@ Rails.application.routes.draw do get "podmin", to: "home#podmin" api_version(module: "Api::V1", path: {value: "api/v1"}) do - resources :aspects, only: %i[show index create destroy update] + resources :aspects, only: %i[show index create destroy update] do + resources :contacts, only: %i[index create destroy] + end resources :posts, only: %i[show create destroy] do resources :comments, only: %i[create index destroy] do post "report" => "comments#report" diff --git a/spec/integration/api/contacts_controller_spec.rb b/spec/integration/api/contacts_controller_spec.rb new file mode 100644 index 000000000..02f300cbe --- /dev/null +++ b/spec/integration/api/contacts_controller_spec.rb @@ -0,0 +1,233 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe Api::V1::ContactsController do + let(:auth) { FactoryGirl.create(:auth_with_read_and_write) } + let(:auth_read_only) { FactoryGirl.create(:auth_with_read) } + let!(:access_token) { auth.create_access_token.to_s } + let!(:access_token_read_only) { auth_read_only.create_access_token.to_s } + + before do + @aspect1 = auth.user.aspects.where(name: "generic").first + @aspect2 = auth.user.aspects.create(name: "another aspect") + @eve_aspect = eve.aspects.first + end + + describe "#show" do + before do + aspects_membership_service.create(@aspect2.id, alice.person.id) + end + + context "for valid aspect" do + it "lists members" do + get( + api_v1_aspect_contacts_path(@aspect2.id), + params: {access_token: access_token} + ) + expect(response.status).to eq(200) + contacts = JSON.parse(response.body) + expect(contacts.length).to eq(1) + confirm_person_format(contacts[0], alice) + + get( + api_v1_aspect_contacts_path(@aspect1.id), + params: {access_token: access_token} + ) + expect(response.status).to eq(200) + contacts = JSON.parse(response.body) + expect(contacts.length).to eq(@aspect1.contacts.length) + end + end + + context "for invalid aspect" do + it "fails for non-existant Aspect ID" do + get( + api_v1_aspect_contacts_path(-1), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.aspects.not_found")) + end + + it "fails for other user's Aspect ID" do + get( + api_v1_aspect_contacts_path(@eve_aspect.id), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.aspects.not_found")) + end + end + + context "improper credentials" do + it "fails when not logged in" do + get( + api_v1_aspect_contacts_path(@aspect2.id), + params: {access_token: "999_999_999"} + ) + expect(response.status).to eq(401) + end + end + end + + describe "#create" do + context "with valid person GUID and aspect" do + it "succeeds adding" do + post( + api_v1_aspect_contacts_path(@aspect2.id), + params: {person_guid: alice.guid, access_token: access_token} + ) + expect(response.status).to eq(204) + expect(@aspect2.contacts.length).to eq(1) + end + + it "fails if re-adding" do + aspects_membership_service.create(@aspect2.id, alice.person.id) + post( + api_v1_aspect_contacts_path(@aspect2.id), + params: {person_guid: alice.guid, access_token: access_token} + ) + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.contacts.cant_create")) + end + end + + context "with invalid Aspect" do + it "fails for non-existant Aspect ID" do + post( + api_v1_aspect_contacts_path(-1), + params: {person_guid: alice.guid, access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.aspects.not_found")) + end + + it "fails for other user's Aspect ID" do + post( + api_v1_aspect_contacts_path(@eve_aspect.id), + params: {person_guid: alice.guid, access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.aspects.not_found")) + end + end + + context "with invalid person GUID" do + it "fails to add" do + post( + api_v1_aspect_contacts_path(@aspect2.id), + params: {person_guid: "999_999_999", access_token: access_token} + ) + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.contacts.cant_create")) + end + end + + context "improper credentials" do + it "fails when not logged in" do + post( + api_v1_aspect_contacts_path(@aspect2.id), + params: {person_guid: alice.guid, access_token: "999_999_999"} + ) + expect(response.status).to eq(401) + end + + it "fails when only read only token" do + post( + api_v1_aspect_contacts_path(@aspect2.id), + params: {person_guid: alice.guid, access_token: access_token_read_only} + ) + expect(response.status).to eq(403) + end + end + end + + describe "#destroy" do + before do + aspects_membership_service.create(@aspect2.id, alice.person.id) + end + + context "with valid person GUID and aspect" do + it "succeeds deleting" do + delete( + api_v1_aspect_contact_path(@aspect2.id, alice.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(204) + expect(@aspect2.contacts.length).to eq(0) + end + + it "fails if not in aspect" do + delete( + api_v1_aspect_contact_path(@aspect2.id, eve.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.contacts.not_found")) + end + end + + context "with invalid Aspect" do + it "fails for non-existant Aspect ID" do + delete( + api_v1_aspect_contact_path(-1, eve.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.contacts.not_found")) + end + + it "fails for other user's Aspect ID" do + delete( + api_v1_aspect_contact_path(@eve_aspect.id, eve.guid), + params: {access_token: access_token} + ) + expect(response.status).to eq(404) + expect(response.body).to eq(I18n.t("api.endpoint_errors.contacts.not_found")) + end + end + + context "with invalid person GUID" do + it "fails to delete " do + delete( + api_v1_aspect_contact_path(@aspect2.id, "999_999_999"), + params: {access_token: access_token} + ) + expect(response.status).to eq(422) + expect(response.body).to eq(I18n.t("api.endpoint_errors.contacts.cant_delete")) + end + end + + context "improper credentials" do + it "fails when not logged in" do + delete( + api_v1_aspect_contact_path(@aspect2.id, alice.guid), + params: {access_token: "999_999_999"} + ) + expect(response.status).to eq(401) + end + + it "fails when only read only token" do + delete( + api_v1_aspect_contact_path(@aspect2.id, alice.guid), + params: {access_token: access_token_read_only} + ) + expect(response.status).to eq(403) + end + end + end + + def aspects_membership_service(user=auth.user) + AspectsMembershipService.new(user) + end + + # rubocop:disable Metrics/AbcSize + def confirm_person_format(post_person, user) + expect(post_person["guid"]).to eq(user.guid) + expect(post_person["diaspora_id"]).to eq(user.diaspora_handle) + expect(post_person["name"]).to eq(user.name) + expect(post_person["avatar"]).to eq(user.profile.image_url) + end + # rubocop:enable Metrics/AbcSize +end diff --git a/spec/integration/api/likes_controller_spec.rb b/spec/integration/api/likes_controller_spec.rb index 02c4c6cf4..8870fef22 100644 --- a/spec/integration/api/likes_controller_spec.rb +++ b/spec/integration/api/likes_controller_spec.rb @@ -1,4 +1,4 @@ -# frozen_sTring_literal: true +# frozen_string_literal: true require "spec_helper" diff --git a/spec/integration/api/reshares_controller_spec.rb b/spec/integration/api/reshares_controller_spec.rb index e833075fa..1c3c54adc 100644 --- a/spec/integration/api/reshares_controller_spec.rb +++ b/spec/integration/api/reshares_controller_spec.rb @@ -145,7 +145,6 @@ describe Api::V1::ResharesController do params: {access_token: access_token} ) - puts(response.body) expect(response.status).to eq(422) expect(response.body).to eq(I18n.t("reshares.create.error")) end diff --git a/spec/services/aspects_membership_service_spec.rb b/spec/services/aspects_membership_service_spec.rb new file mode 100644 index 000000000..9657af0e9 --- /dev/null +++ b/spec/services/aspects_membership_service_spec.rb @@ -0,0 +1,137 @@ +# frozen_string_literal: true + +describe AspectsMembershipService do + before do + @alice_aspect1 = alice.aspects.first + @alice_aspect2 = alice.aspects.create(name: "another aspect") + @bob_aspect1 = bob.aspects.first + end + + describe "#create" do + context "with valid IDs" do + it "succeeds" do + membership = aspects_membership_service.create(@alice_aspect2.id, bob.person.id) + expect(membership[:aspect_id]).to eq(@alice_aspect2.id) + expect(@alice_aspect2.contacts.find_by(person_id: bob.person.id)).not_to be_nil + end + + it "fails if already in aspect" do + aspects_membership_service.create(@alice_aspect2.id, bob.person.id) + expect { + aspects_membership_service.create(@alice_aspect2.id, bob.person.id) + }.to raise_error ActiveRecord::RecordNotUnique + end + end + + context "with invalid IDs" do + it "fails with invalid User ID" do + expect { + aspects_membership_service.create(@alice_aspect2.id, -1) + }.to raise_error ActiveRecord::RecordNotFound + end + + it "fails with invalid Aspect ID" do + expect { + aspects_membership_service.create(-1, bob.person.id) + }.to raise_error ActiveRecord::RecordNotFound + end + + it "fails with aspect ID that isn't user's" do + expect { + aspects_membership_service.create(@bob_aspect1.id, eve.person.id) + }.to raise_error ActiveRecord::RecordNotFound + end + end + end + + describe "#destroy" do + before do + @membership = aspects_membership_service.create(@alice_aspect2.id, bob.person.id) + end + + context "with aspect/user valid IDs" do + it "succeeds if in aspect" do + aspects_membership_service.destroy_by_ids(@alice_aspect2.id, bob.person.id) + expect(@alice_aspect2.contacts.find_by(person_id: bob.person.id)).to be_nil + end + + it "fails if not in aspect" do + expect { + aspects_membership_service.destroy_by_ids(@alice_aspect2.id, eve.person.id) + }.to raise_error ActiveRecord::RecordNotFound + end + end + + context "with a membership ID" do + it "succeeds if their membership" do + aspects_membership_service.destroy_by_membership_id(@membership.id) + expect(@alice_aspect2.contacts.find_by(person_id: bob.person.id)).to be_nil + end + + it "fails if not their membership" do + expect { + aspects_membership_service(eve).destroy_by_membership_id(@membership.id) + }.to raise_error ActiveRecord::RecordNotFound + end + + it "fails if invalid membership ID" do + expect { + aspects_membership_service(eve).destroy_by_membership_id(-1) + }.to raise_error ActiveRecord::RecordNotFound + end + end + + context "with invalid IDs" do + it "fails with invalid User ID" do + expect { + aspects_membership_service.destroy_by_ids(@alice_aspect2.id, -1) + }.to raise_error ActiveRecord::RecordNotFound + end + + it "fails with invalid Aspect ID" do + expect { + aspects_membership_service.destroy_by_ids(-1, eve.person.id) + }.to raise_error ActiveRecord::RecordNotFound + end + + it "fails with aspect ID that isn't user's" do + expect { + aspects_membership_service(eve).destroy_by_ids(@alice_aspect2.id, bob.person.id) + }.to raise_error ActiveRecord::RecordNotFound + end + end + end + + describe "#list" do + before do + aspects_membership_service.create(@alice_aspect2.id, bob.person.id) + aspects_membership_service.create(@alice_aspect2.id, eve.person.id) + @alice_aspect3 = alice.aspects.create(name: "empty aspect") + end + + context "with valid aspect ID" do + it "returns users in full aspect" do + contacts = aspects_membership_service.contacts_in_aspect(@alice_aspect2.id) + expect(contacts.length).to eq(2) + expect(contacts.map {|c| c.person.guid }.sort).to eq([bob.person.guid, eve.person.guid].sort) + end + + it "returns empty array in empty aspect" do + contacts = aspects_membership_service.contacts_in_aspect(@alice_aspect3.id) + expect(contacts.length).to eq(0) + end + end + + context "with invalid aspect ID" do + it "fails" do + expect { + aspects_membership_service.contacts_in_aspect(-1) + }.to raise_error ActiveRecord::RecordNotFound + end + end + end + + def aspects_membership_service(user=alice) + AspectsMembershipService.new(user) + end +end