From ef7f84b59b1bab6ae85b38538c189fbb746f7185 Mon Sep 17 00:00:00 2001 From: Maja Komel <email@example.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
We should write tests for this too
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.
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 looking up primary group name
Why though? What’s wrong with having 2 properties:
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