Show error messages from server when aspect membership changes fail

closes #7132

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)

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.listenToOnce(this.person.contact.aspectMemberships, "error", this._displayError);
-      this._displayError('aspect_dropdown.error');
-    });
 
     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
+    this.dropdown.closest(".aspect_membership_dropdown").removeClass("open"); // close the dropdown
-
+    app.flashMessages.error(resp.responseText);
-    var msg = Diaspora.I18n.t(msg_id, { 'name': this._name() });
-    app.flashMessages.error(msg);
   },
 
   // 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.listenToOnce(membership, "error", this._displayError);
-      this._displayError("aspect_dropdown.error_remove");
-    });
 
     return membership.destroy();
   },

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_to do |format|
-      respond_with 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")
+      respond_to do |format|
-      render nothing: true, status: 409
+        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
 

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

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(
+      expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message");
-        Diaspora.I18n.t("aspect_dropdown.error", {name: this.personName})
       );
     });
   });
@@ -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(
+      expect(spec.content().find(".flash-message")).toBeErrorFlashMessage("error message");
-        Diaspora.I18n.t("aspect_dropdown.error_remove", {name: this.personName})
-      );
     });
   });
 });