diff --git a/Changelog.md b/Changelog.md index d91522834..ceb8c0cac 100644 --- a/Changelog.md +++ b/Changelog.md @@ -34,7 +34,8 @@ Note: Although this is a minor release, the configuration file changed because t * Clicking photos on mobile should no longer cause 404s [#7071](https://github.com/diaspora/diaspora/pull/7071) * Fix avatar size on mobile privacy page for ignored people [#7148](https://github.com/diaspora/diaspora/pull/7148) * Don't display tag following button when logged out [#7155](https://github.com/diaspora/diaspora/pull/7155) -* Fix message modal on profile page[#7137](https://github.com/diaspora/diaspora/pull/7137) +* Fix message modal on profile page [#7137](https://github.com/diaspora/diaspora/pull/7137) +* Display error message when aspect membership changes fail [#7132](https://github.com/diaspora/diaspora/pull/7132) ## Features * Deleted comments will be removed when loading more comments [#7045](https://github.com/diaspora/diaspora/pull/7045) diff --git a/app/assets/javascripts/app/views/aspect_membership_view.js b/app/assets/javascripts/app/views/aspect_membership_view.js index 30529837e..238349e1d 100644 --- a/app/assets/javascripts/app/views/aspect_membership_view.js +++ b/app/assets/javascripts/app/views/aspect_membership_view.js @@ -96,9 +96,7 @@ app.views.AspectMembership = app.views.Base.extend({ } this.listenToOnce(this.person.contact.aspectMemberships, "sync", this._successSaveCb); - this.listenToOnce(this.person.contact.aspectMemberships, "error", function() { - this._displayError('aspect_dropdown.error'); - }); + this.listenToOnce(this.person.contact.aspectMemberships, "error", this._displayError); return this.person.contact.aspectMemberships.create({"aspect_id": aspectId, "person_id": this._personId()}); }, @@ -124,21 +122,17 @@ app.views.AspectMembership = app.views.Base.extend({ }, // show an error flash msg - _displayError: function(msg_id) { + _displayError: function(model, resp) { this._done(); - this.dropdown.closest('.aspect_membership_dropdown').removeClass('open'); // close the dropdown - - var msg = Diaspora.I18n.t(msg_id, { 'name': this._name() }); - app.flashMessages.error(msg); + this.dropdown.closest(".aspect_membership_dropdown").removeClass("open"); // close the dropdown + app.flashMessages.error(resp.responseText); }, // remove the membership with the given id removeMembership: function(membershipId) { var membership = this.person.contact.aspectMemberships.get(membershipId); this.listenToOnce(membership, "sync", this._successDestroyCb); - this.listenToOnce(membership, "error", function() { - this._displayError("aspect_dropdown.error_remove"); - }); + this.listenToOnce(membership, "error", this._displayError); return membership.destroy(); }, diff --git a/app/controllers/aspect_memberships_controller.rb b/app/controllers/aspect_memberships_controller.rb index e31b90a78..68215bc37 100644 --- a/app/controllers/aspect_memberships_controller.rb +++ b/app/controllers/aspect_memberships_controller.rb @@ -25,12 +25,6 @@ class AspectMembershipsController < ApplicationController success = membership.destroy # set the flash message - if success - flash.now[:notice] = I18n.t "aspect_memberships.destroy.success" - else - flash.now[:error] = I18n.t "aspect_memberships.destroy.failure" - end - respond_to do |format| format.json do if success @@ -40,7 +34,14 @@ class AspectMembershipsController < ApplicationController end end - format.all { redirect_to :back } + format.all do + if success + flash.now[:notice] = I18n.t "aspect_memberships.destroy.success" + else + flash.now[:error] = I18n.t "aspect_memberships.destroy.failure" + end + redirect_to :back + end end end @@ -51,19 +52,29 @@ class AspectMembershipsController < ApplicationController @contact = current_user.share_with(@person, @aspect) if @contact.present? - flash.now[:notice] = I18n.t("aspects.add_to_aspect.success") - respond_with do |format| + respond_to do |format| format.json do render json: AspectMembershipPresenter.new( AspectMembership.where(contact_id: @contact.id, aspect_id: @aspect.id).first) .base_hash end - format.all { redirect_to :back } + format.all do + flash.now[:notice] = I18n.t("aspects.add_to_aspect.success") + redirect_to :back + end end else - flash.now[:error] = I18n.t("aspects.add_to_aspect.failure") - render nothing: true, status: 409 + respond_to do |format| + format.json do + render text: I18n.t("aspects.add_to_aspect.failure"), status: 409 + end + + format.all do + flash.now[:error] = I18n.t("aspects.add_to_aspect.failure") + render nothing: true, status: 409 + end + end end end diff --git a/spec/controllers/aspect_memberships_controller_spec.rb b/spec/controllers/aspect_memberships_controller_spec.rb index 78c0adfd6..a0a418061 100644 --- a/spec/controllers/aspect_memberships_controller_spec.rb +++ b/spec/controllers/aspect_memberships_controller_spec.rb @@ -48,7 +48,7 @@ describe AspectMembershipsController, type: :controller do it "failure flashes error" do expect(alice).to receive(:share_with).and_return(nil) - post :create, format: :json, person_id: @person.id, aspect_id: @aspect0.id + post :create, format: :mobile, person_id: @person.id, aspect_id: @aspect0.id expect(flash[:error]).not_to be_blank end @@ -57,7 +57,7 @@ describe AspectMembershipsController, type: :controller do post :create, params post :create, params expect(response.status).to eq(400) - expect(response.body).to include(I18n.t("aspect_memberships.destroy.invalid_statement")) + expect(response.body).to eq(I18n.t("aspect_memberships.destroy.invalid_statement")) end context "json" do @@ -67,6 +67,13 @@ describe AspectMembershipsController, type: :controller do contact = @controller.current_user.contact_for(@person) expect(response.body).to eq(AspectMembershipPresenter.new(contact.aspect_memberships.first).base_hash.to_json) end + + it "responds with an error message when the request failed" do + expect(alice).to receive(:share_with).and_return(nil) + post :create, format: :json, person_id: @person.id, aspect_id: @aspect0.id + expect(response.status).to eq(409) + expect(response.body).to eq(I18n.t("aspects.add_to_aspect.failure")) + end end end @@ -90,7 +97,7 @@ describe AspectMembershipsController, type: :controller do it "aspect membership does not exist" do delete :destroy, format: :json, id: 123 expect(response).not_to be_success - expect(response.body).to include I18n.t("aspect_memberships.destroy.no_membership") + expect(response.body).to eq(I18n.t("aspect_memberships.destroy.no_membership")) end end end diff --git a/spec/javascripts/app/views/aspect_membership_view_spec.js b/spec/javascripts/app/views/aspect_membership_view_spec.js index 7dabc075b..23766c7ab 100644 --- a/spec/javascripts/app/views/aspect_membership_view_spec.js +++ b/spec/javascripts/app/views/aspect_membership_view_spec.js @@ -1,6 +1,6 @@ describe("app.views.AspectMembership", function(){ var success = {status: 200, responseText: "{}"}; - var resp_fail = {status: 400}; + var resp_fail = {status: 400, responseText: "error message"}; beforeEach(function() { var contact = factory.contact(); @@ -58,8 +58,7 @@ describe("app.views.AspectMembership", function(){ this.newAspect.trigger('click'); jasmine.Ajax.requests.mostRecent().respondWith(resp_fail); - expect(spec.content().find(".flash-message")).toBeErrorFlashMessage( - Diaspora.I18n.t("aspect_dropdown.error", {name: this.personName}) + expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); ); }); }); @@ -100,9 +99,7 @@ describe("app.views.AspectMembership", function(){ this.oldAspect.trigger('click'); jasmine.Ajax.requests.mostRecent().respondWith(resp_fail); - expect(spec.content().find(".flash-message")).toBeErrorFlashMessage( - Diaspora.I18n.t("aspect_dropdown.error_remove", {name: this.personName}) - ); + expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message"); }); }); });