Add groups to current user serializer (#6748)

Add groups to current user serializer (#6748)
From ef7f84b59b1bab6ae85b38538c189fbb746f7185 Mon Sep 17 00:00:00 2001
From: Maja Komel <maja.komel@gmail.com>
Date: Mon, 10 Dec 2018 16:23:29 +0100
Subject: [PATCH] Add groups to current user serializer (#6748)


diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb
index d7c683d..27eb350 100644
--- a/app/serializers/current_user_serializer.rb
+++ b/app/serializers/current_user_serializer.rb
@@ -45,6 +45,8 @@ class CurrentUserSerializer < BasicUserSerializer
              :top_category_ids,
              :hide_profile_and_presence
 
+  has_many :groups, embed: :object, serializer: BasicGroupSerializer
+
   def link_posting_access
     scope.link_posting_access
   end

GitHub

We should write tests for this too :grin:

Sorry @majakomel marking this for followup not purely cause of the test, but more cause we need to “re-jig” this serializer (might as well add a test as well)

This is no longer needed:

And the client needs to be taught so, it can simply lookup the group now using group id, which is included per:

This will eliminate a query on the home page. It also cleans up the serializer so it no longer will send duplicate data.

1 Like

Do we need to serialize all the info about the groups? I thought names were enough?

I think we still need ids as well

Do you have an use case in mind?

Yep :slight_smile: looking up primary group name

Why though? What’s wrong with having 2 properties: primary_group and groups on the users?

I don’t see why we need to complicate the client-side code for this?

The big reason is cause we can eliminate one query from the home page, cause now we query the same table twice

We can workaround in the serializer but it seems just as simple to do this on the client and then hold proper refs to the group on the client, id is 100% stable name less so

1 Like

Good point :thumbsup: