Skip to content

Commit

Permalink
DEV: remove assign_path and assign_icon from seralizers (#568)
Browse files Browse the repository at this point in the history
Both fields are unnecessary and should not be included in the serializer to reduce payload.
  • Loading branch information
lis2 authored May 6, 2024
1 parent c8f669d commit b796ae3
Show file tree
Hide file tree
Showing 10 changed files with 8 additions and 44 deletions.
10 changes: 1 addition & 9 deletions app/serializers/assigned_group_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
# frozen_string_literal: true

class AssignedGroupSerializer < ApplicationSerializer
attributes :id, :name, :assign_icon, :assign_path

def assign_icon
"group-plus"
end

def assign_path
"/g/#{object.name}/assigned/everyone"
end
attributes :id, :name
end
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ function initialize(api) {
} else {
assignedPath = `/t/${topic.id}`;
}
const icon = iconHTML(assignee.assign_icon);
const icon = iconHTML(assignee.username ? "user-plus" : "group-plus");
const name = assignee.username || assignee.name;
const tagName = params.tagName || "a";
const href =
Expand Down
10 changes: 1 addition & 9 deletions lib/discourse_assign/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,7 @@ module DiscourseAssign
module Helpers
def self.build_assigned_to_user(user, topic)
return if !user
{
username: user.username,
name: user.name,
avatar_template: user.avatar_template,
assign_icon: "user-plus",
assign_path: SiteSetting.assigns_user_url_path.gsub("{username}", user.username),
}
{ username: user.username, name: user.name, avatar_template: user.avatar_template }
end

def self.build_assigned_to_group(group, topic)
Expand All @@ -22,8 +16,6 @@ def self.build_assigned_to_group(group, topic)
flair_color: group.flair_color,
flair_icon: group.flair_icon,
flair_upload_id: group.flair_upload_id,
assign_icon: "group-plus",
assign_path: "/g/#{group.name}/assigned/everyone",
}
end

Expand Down
6 changes: 0 additions & 6 deletions plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -694,12 +694,6 @@ module ::DiscourseAssign
AssignedGroupSerializer.new(assigned_to, scope: scope, root: false).as_json
end

add_to_serializer(:basic_user, :assign_icon) { "user-plus" }
add_to_serializer(:basic_user, :assign_path) do
return if !object.is_a?(User)
SiteSetting.assigns_user_url_path.gsub("{username}", object.username)
end

# PostSerializer
add_to_serializer(
:post,
Expand Down
2 changes: 0 additions & 2 deletions spec/components/search_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@
expect(indirectly_assigned_to).to eq(
post5.id => {
assigned_to: {
assign_icon: "user-plus",
assign_path: "/u/#{user.username}/activity/assigned",
avatar_template: user.avatar_template,
name: user.name,
username: user.username,
Expand Down
2 changes: 0 additions & 2 deletions spec/requests/search_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@

expect(assigned_to_group_data["id"]).to eq(group.id)
expect(assigned_to_group_data["name"]).to eq(group.name)
expect(assigned_to_group_data["assign_icon"]).to eq("group-plus")
expect(assigned_to_group_data["assign_path"]).to eq("/g/#{group.name}/assigned/everyone")
end

it "does not result in N+1 queries when search returns multiple results" do
Expand Down
10 changes: 1 addition & 9 deletions spec/serializers/flagged_topic_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,7 @@
json = FlaggedTopicSerializer.new(topic, scope: guardian).as_json

expect(json[:flagged_topic][:assigned_to_user]).to match(
{
username: user.username,
name: user.name,
assign_icon: "user-plus",
avatar_template: /letter_avatar_proxy.*/,
assign_path: "/u/#{user.username}/activity/assigned",
},
{ username: user.username, name: user.name, avatar_template: /letter_avatar_proxy.*/ },
)
expect(json[:flagged_topic]).to_not have_key(:assigned_to_group)
end
Expand Down Expand Up @@ -78,8 +72,6 @@
flair_color: assign_allowed_group.flair_color,
flair_icon: assign_allowed_group.flair_icon,
flair_upload_id: assign_allowed_group.flair_upload_id,
assign_icon: "group-plus",
assign_path: "/g/#{assign_allowed_group.name}/assigned/everyone",
},
)
expect(json[:flagged_topic]).to_not have_key(:assigned_to_user)
Expand Down
2 changes: 0 additions & 2 deletions spec/serializers/post_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
serializer = PostSerializer.new(post, scope: guardian)
post = serializer.as_json[:post]
expect(post[:assigned_to_user][:id]).to eq(user.id)
expect(post[:assigned_to_user][:assign_icon]).to eq("user-plus")
expect(post[:assigned_to_group]).to be(nil)
end

Expand All @@ -30,7 +29,6 @@
serializer = PostSerializer.new(post, scope: guardian)
post = serializer.as_json[:post]
expect(post[:assigned_to_group][:id]).to eq(assign_allowed_group.id)
expect(post[:assigned_to_group][:assign_icon]).to eq("group-plus")
expect(post[:assigned_to_user]).to be(nil)
end

Expand Down
4 changes: 0 additions & 4 deletions spec/serializers/user_bookmark_base_serializer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
bookmark = serializer.as_json[:user_topic_bookmark]

expect(bookmark[:assigned_to_user][:id]).to eq(user.id)
expect(bookmark[:assigned_to_user][:assign_icon]).to eq("user-plus")
expect(bookmark[:assigned_to_group]).to be(nil)
end

Expand All @@ -34,7 +33,6 @@
bookmark = serializer.as_json[:user_topic_bookmark]

expect(bookmark[:assigned_to_group][:id]).to eq(assign_allowed_group.id)
expect(bookmark[:assigned_to_group][:assign_icon]).to eq("group-plus")
expect(bookmark[:assigned_to_user]).to be(nil)
end
end
Expand All @@ -47,7 +45,6 @@
bookmark = serializer.as_json[:user_post_bookmark]

expect(bookmark[:assigned_to_user][:id]).to eq(user.id)
expect(bookmark[:assigned_to_user][:assign_icon]).to eq("user-plus")
expect(bookmark[:assigned_to_group]).to be(nil)
end

Expand All @@ -57,7 +54,6 @@
bookmark = serializer.as_json[:user_post_bookmark]

expect(bookmark[:assigned_to_group][:id]).to eq(assign_allowed_group.id)
expect(bookmark[:assigned_to_group][:assign_icon]).to eq("group-plus")
expect(bookmark[:assigned_to_user]).to be(nil)
end
end
Expand Down
4 changes: 4 additions & 0 deletions test/javascripts/acceptance/assigned-topic-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ acceptance("Discourse Assign | Assigned topic", function (needs) {
"shows assignment and indirect assignments in the first post"
);
assert.dom("#post_1 .assigned-to svg.d-icon-user-plus").exists();
assert.dom("#post_1 .assigned-to a[href='/']").exists();
assert
.dom(".discourse-tags .assigned-to[href='/t/28830'] span")
.hasAttribute("title", "Shark Doododooo", "shows topic assign notes");
Expand Down Expand Up @@ -137,6 +138,9 @@ acceptance("Discourse Assign | Assigned topic", function (needs) {
"shows assignment in the first post"
);
assert.dom("#post_1 .assigned-to svg.d-icon-group-plus").exists();
assert
.dom("#post_1 .assigned-to a[href='/g/Developers/assigned/everyone']")
.exists();
assert
.dom("#topic-footer-dropdown-reassign")
.exists("shows reassign dropdown at the bottom of the topic");
Expand Down

0 comments on commit b796ae3

Please sign in to comment.