FIX: simplify and improve choosing favorite badges (#13743)

* No need to return anything except a status code from the server

* Switch a badge state before sending a request and then switch it back in case of an error

app/assets/javascripts/discourse/app/models/user-badge.js

@@ -22,12 +22,14 @@ const UserBadge = EmberObject.extend({
   },
 
   favorite() {
-    return ajax(`/user_badges/${this.id}/toggle_favorite`, { type: "PUT" })
+    this.toggleProperty("is_favorite");
-      .then((json) => {
+    return ajax(`/user_badges/${this.id}/toggle_favorite`, {
-        this.set("is_favorite", json.user_badge.is_favorite);
+      type: "PUT",
-        return this;
+    }).catch((e) => {
-      })
+      // something went wrong, switch the UI back:
-      .catch(popupAjaxError);
+      this.toggleProperty("is_favorite");
+      popupAjaxError(e);
+    });
   },
 });
 

app/controllers/user_badges_controller.rb

@@ -109,14 +109,10 @@ class UserBadgesController < ApplicationController
       return render json: failed_json, status: 400
     end
 
-    new_is_favorite_value = !user_badge.is_favorite
     UserBadge
       .where(user_id: user_badge.user_id, badge_id: user_badge.badge_id)
-      .update_all(is_favorite: new_is_favorite_value)
+      .update_all(is_favorite: !user_badge.is_favorite)
     UserBadge.update_featured_ranks!(user_badge.user_id)
-
-    user_badge.is_favorite = new_is_favorite_value
-    render_serialized(user_badge, DetailedUserBadgeSerializer, root: :user_badge)
   end
 
   private

spec/requests/user_badges_controller_spec.rb

@@ -288,17 +288,14 @@ describe UserBadgesController do
       SiteSetting.max_favorite_badges = 3
 
       put "/user_badges/#{user_badge.id}/toggle_favorite.json"
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(204)
     end
 
     it "favorites a badge" do
       sign_in(user)
       put "/user_badges/#{user_badge.id}/toggle_favorite.json"
 
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(204)
-      parsed = response.parsed_body
-      expect(parsed["user_badge"]["is_favorite"]).to eq(true)
-
       user_badge = UserBadge.find_by(user: user, badge: badge)
       expect(user_badge.is_favorite).to eq(true)
     end
@@ -308,10 +305,7 @@ describe UserBadgesController do
       user_badge.toggle!(:is_favorite)
       put "/user_badges/#{user_badge.id}/toggle_favorite.json"
 
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(204)
-      parsed = response.parsed_body
-      expect(parsed["user_badge"]["is_favorite"]).to eq(false)
-
       user_badge = UserBadge.find_by(user: user, badge: badge)
       expect(user_badge.is_favorite).to eq(false)
     end
@@ -328,23 +322,17 @@ describe UserBadgesController do
       other_user_badge = UserBadge.create(badge: other_badge, user: user, granted_by: Discourse.system_user, granted_at: Time.now)
 
       put "/user_badges/#{user_badge.id}/toggle_favorite.json"
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(204)
-      parsed = response.parsed_body
-      expect(parsed["user_badge"]["is_favorite"]).to eq(false)
       expect(user_badge.reload.is_favorite).to eq(false)
       expect(user_badge2.reload.is_favorite).to eq(false)
 
       put "/user_badges/#{user_badge.id}/toggle_favorite.json"
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(204)
-      parsed = response.parsed_body
-      expect(parsed["user_badge"]["is_favorite"]).to eq(true)
       expect(user_badge.reload.is_favorite).to eq(true)
       expect(user_badge2.reload.is_favorite).to eq(true)
 
       put "/user_badges/#{other_user_badge.id}/toggle_favorite.json"
-      expect(response.status).to eq(200)
+      expect(response.status).to eq(204)
-      parsed = response.parsed_body
-      expect(parsed["user_badge"]["is_favorite"]).to eq(true)
       expect(other_user_badge.reload.is_favorite).to eq(true)
     end
   end