Adding user attributes to the directory item serializer

#1

A client approached me with a request to create a card-based user directory.

As he’s hosted on discourse.org, I initially said this would not be possible (aside from any performance issues), as the attributes required to fill a user card are not available in the user object serialized with directory items.

However, after discussing it further, I determined that one approach could be to make a PR into core which adds user attributes to the directory item serializer via a site setting. The idea being that this functionality will be useful for various styles of user directory which require various mixtures of user attributes (example below).

Experimenting with this, I’ve come up with a relatively simple approach, which you can see on this diff:

Namely:

  1. Create a dedicated DirectoryItemUserSerializer (the current “UserSerializer” inside the DirectoryItemSerializer has always felt a bit strange to me).

  2. If the site admin has selected any user attributes to add:

    1. Create a serialized user to take advantage of the protections of that serializer, e.g. staff / private / untrusted attributes.

    2. Serialize any added attributes which pass the UserSerializer filter.

The goals here being to:

  1. Not affect the existing behviour in any way.

  2. Serialize only those attributes requested.

If you guys are on board with this approach / request, I’ll finish this off by:

  1. Adding a validation on the site setting (i.e. to ensure the string entered is a user attribute)

  2. Adding tests.

However I thought it best to run this by you guys first.

Example

As an example of the kind of functionality this would allow, the screenshot below from my local just using this change to core + a theme component that overrides the existing directory and uses a modified user-card component:

52%20PM
52%20PM2294×1368 486 KB

15 Likes
2019: The Year in Review
#2

@sam Reckon this approach is do-able in a PR?

1 Like
#3

I think @blake and @awesomerobot is working with another customer with similar requirements. I am kind of open for a site setting like we have always_include_topic_excerpts

Caveat is that I would like this tested and I would like the site setting to be hidden.

I know @david has been interested in the problem as well, I am a bit mixed here but maybe a longer term solution here that is cleaner is:

  1. Add hidden site settings for “allow plugins and themes to opt for extra serialized info” default off
  2. Then have a protocol in themes/plugins for “asking” for the extra info.

The advantage here is that some themes then will have the extra serialized info and some not.

I would like to wait on David a bit here before giving you a green light.

9 Likes
#4

How about making the user directory serializer accept a new parameter like users.json?include_profile=true

Then we can add a JS plugin API which allows themes to add this parameter:

api.userListIncludeProfile(true)

“Profile” would include all the things in the regular user serializer (being careful not to introduce n+1 queries). I don’t think specifying individual fields is going to make much difference to performance, so IMO we should keep it simple.

The same approach could work for topic lists as well ?include_excerpts=true

I also like this because it means the “per-theme serializer configuration” won’t get broken by the anon cache!

What do you think @sam?

#5

Close but this does not work cleanly cause when you head to the home page of the site or to the u path you don’t have the luxury of adding a param to the path, we need defaults changed :frowning:

4 Likes
#6

Oh yes :cry:

In that case, I think a hidden site setting is a good starting point. Would be nice to make it per-theme, but we need to build some infrastructure there first. I still think a boolean “user list includes profile” setting will be easier to reason with.

1 Like
#7

Would that include properties like featured_user_badges used in the user card?

1 Like
#8

Yes, I think so - everything that is included in the regular user serializer (As long as it can be done without introducing an n+1)

4 Likes
#9

Cool. Let us know if you guys consider this #pr-welcome and in what form, and we’d be happy to make one. On our end, @fzngagan is going to handle this from here.

3 Likes
#12

Let’s go ahead with a single, boolean, hidden site setting, which adds all the relevant information.

5 Likes
#13

Thanks for the heads up here @david. I’ll start looking into it monday onwards.

5 Likes
#14

One thing to clarify here is the protocol for requesting additional data. Can we rely on the site setting on the frontend to request the data or as you suggested, add another api in the plugin-api?

#15

Let’s keep it simple, disregard my post about parameters and JavaScript APIs.

If the site setting is enabled on the server, return the extra information. If the site setting is disabled, do not.

So at the moment, admins will have to enable the setting manually. We may look at allowing themes to automatically toggle this setting in future.

6 Likes
#16

Sure. The theme component should be consuming the additional fields conditionally for when the setting is enabled right?

Also, the server should send additional fields only when the setting is enabled…

1 Like
#18

@david Thanks David. We’ll work up the PR and get back to you.

5 Likes
#19

@david I’ve made a PR here

cc @angus

5 Likes
#20

Closing the loop here. We decided not to add any more attributes to the directory serializer. Instead, we made vast improvements to the performance of user serialization. We split user cards onto their own route, and added a new route which returns multiple cards in one go.

You can find an example of using this new route in this theme component:

4 Likes
#21

This topic was automatically closed after 3 hours. New replies are no longer allowed.