Allow custom populate in ctx.state.user

[juandl]

Description of what you did:

By default ctx.state.user populates only 'role', if there is a custom relation with another table it is not possible to modify ctx.state.user to show the populate of x relation.

This commit lets the user change this rule by overwriting the file /extensions/users-permissions/services/User.js using the function fetch

My PR is a:

• 💥 Breaking change
• 🐛 Bug fix
• 💅 Enhancement
• 🚀 New feature

Main update on the:

• Admin
• Documentation
• Framework
• Plugin

Manual testing is done on the following databases:

• Not applicable
• MongoDB
• MySQL
• Postgres
• SQLite

[juandl]

@derrickmehaffy as mention in PR 4540

[juandl]

I change to this way..

  fetch(params, populate) {
    let defaultPopulate = ['role'];
    if (populate) _.merge(defaultPopulate, populate);
    return strapi.query('user', 'users-permissions').findOne(params, defaultPopulate);
  },

to make it more understandable but to write less code I think doing only,
fetch(params, populate = ['role']), will work too.

I would like to know if it is ok as I left it or the short way form is better?

[codecov]

Codecov Report

Merging #6770 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6770   +/-   ##
=======================================
  Coverage   20.05%   20.05%           
=======================================
  Files         858      858           
  Lines       12049    12049           
  Branches     1950     1950           
=======================================
  Hits         2416     2416           
  Misses       8063     8063           
  Partials     1570     1570           

Flag	Coverage Δ
#front	14.67% <ø> (ø)
#unit	42.45% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd3c769...00d36f9. Read the comment docs.

[alexandrebodin]

LGTM, Thanks !

[rokasaitenta]

As I understand, the purpose of reducing the number of populated fields for the User object is to improve performance of some internal calls. However, this change has a side effect visible externally - when querying a single user by id, the REST API no longer populates the relational fields of the User object. This is not good, as it creates confusion, because the shape of data returned by /users endpoint using different HTTP methods differ (find vs. findOne) and it requires customization just to make it consistent again. Could we somehow not reduce the populated fields by default, but instead reduce them explicitly in places where it is required, i.e. to improve performace, etc.?

[juandl]

@rokasaitenta By default is already populated role so i guess the performance will not be affected, I exposed the query if another person need to add more population in That query for x reason, so i don't think is affecting performance if you are not customizing this query.

Aso this query is for populated that in header state user.

[rokasaitenta]

I am referring to 4540, as I guess your PR builds upon it. If I understand it correctly, it was meant to improve the performance by only populating the 'role' field and skipping all the other custom relations. That change the shape of data returned by the REST API when querying user by id, as it no longer populates custom relational fields.

Can I populate other fields without customizing the plugin, i.e. via query parameters, etc.?

[juandl]

@rokasaitenta okay, let me explain to you.

This query, I customised to allow the developers to add more population fields. But by default, I populate the field 'role', so that performance would stil be good. Again, this change is if you need a high customisation for some reason in header. In short terms, I kept the performance and customisation in the query.

[rokasaitenta]

@juandl I understand what you did, but I am talking about the 4540. I guess I'll comment there. Sorry for bothering you and thanks for your time and efforts to explain this to me. 👍

[juandl]

@rokasaitenta no worries, but remember this query is only for header, when a user is authenticated, so is not affecting any performance at the moment because only is populated "role" even the another PR.

[rokasaitenta]

It is affecting the data returned when requesting a single user via REST API, because now it does not populate any other custom relational field except the 'role'. See my issue 7197.

User controller does not pass populate parameter to the service, so the service uses your default ['role'] value. This causes other custom relation fields to not be populated.

[juandl]

@rokasaitenta that i think is no a issue, this query don't use "findOne or find" is using a custom service call "fetch", so it's nothing to do with the standard services "findOne and etc" if u want to populate you can add in the service.

[rokasaitenta]

User controller uses this service's fetch method, so when you request a single user via REST API, it returns just ids in relational fields, because it skips population, it only populates the 'role' field.

[juandl]

@rokasaitenta I see what you mean, yes you are right the controller is using fetch service, i guess is better to keep that in specific service to avoid this issue, i will do the changes and push it, for now you can add the customization and add the population you need, till strapi release one more time, will do changes now 👌🏻

[rokasaitenta]

@juandl It's good that we finally understood each other. Thanks again for your efforts! I will be waiting for the new version. :)

[juandl]

@rokasaitenta please feel free to comment here #7204 :)

[giacomocerquone]

@juandl quick question, could we have an example of how to customize the User service in order to show all the relationship's data?

[juandl]

@juandl quick question, could we have an example of how to customize the User service in order to show all the relationship's data?

Override the file:

/extensions/users-permissions/services/User.js

Adding

/**
   * Promise to fetch a/an user.
   * @return {Promise}
   */
  fetch(
    params,
    populate = [
      "role",
      "here_population",
    ]
  ) {
    return strapi.query("user", "users-permissions").findOne(params, populate);
  }

[giacomocerquone]

@juandl that's awesome, honestly I was searching through the strapi source and I wasn't finding the User.me service to override, so it's fetch

[juandl]

@juandl that's awesome, honestly I was searching through the strapi source and I wasn't finding the User.me service to override, so it's fetch

Yes but this is only working with 3.0.6+

Also in next release will have improve this part.

I know how is that hahaha i was looking before for something like this, is very helpful when you are doing something deeply and require to have more information in ctx.state.user

😅

[giacomocerquone]

Yeah I should be fine then being on 3.1.1