Closed MikeAlhayek closed 1 year ago
If you create a type with a CustomUserSettings
stereotype, it will be editable in the user screen, and the fields will be accessible from the user instance like we do for content items.
In your views you can then access these properties. You could even create a custom shape called UserDisplay
which would take the user as its model and render it.
I don't think we should do it by default. What could be done though is introduce a new shape for the admin and use it. Default implementation would be to use the username. Just in case one day we decide to change how users are rendered in the admin, or to make it customizable.
@sebastienros Thanks for your feedback. Yes I am aware of the CustomUserSettings
where I can add any fields to the user which is great. But still not sure I follow you on how to go about changing the username
to fields on the User object.
So looking at this screenshot, I want to change how the username appears from "malhayek" to "Mike Alhayek"
Currently, that value is rendered using ContentsMeta_SummaryAdmin which takes ContentItemViewModel so we just display the value of contentItem.Author
. At this point, we don't have a user object unless we query it in the view which is bad since that would cause the N+1 select case.
Ideally, we would query the content item with the User and pass the user to the view or something around these lines. if somehow we introduce a new shape on admin that providers the User
over, that should allow one to override the DisplayText on the user.
In other places like the UserPickerField I was able to replace the default implementation of IUserPickerResultProvider
by changing DisplayText.
This will slow down the Content Items list unless you create a cache with all the UserMetaData assuming that it won't contain any sensible data. Here, you should be able to get the "Owner" and display its MetaData too but it requires fetching the data from database for each content item which is why we set the Author on every ContentItemIndex row in the database.
@Skrypt I am aware of the problem. I am just looking for ways around it as displaying username isn't very acceptable is my case.
What if we add add an options to ContentOptionsViewModel
that would be IncludeAuthor
Then we check the settings to see how should we display the ownership "by default it would be the username" If the user want to show other data, then we set IncludeAuthor
to true which will execute the query with something like this .With<User>(x => ....)
which should not be too bad since we are not pulling many records by default in a single call. Now that the User
object is available, we can pass it to a service that would generate the display name if any defined
ContentOptionsViewModel is mostly about having filter options for the Content Items. Here, this is more about something that we want to display optionally and this is something that can already be done by simply overriding the Razor templates in a custom admin theme. I'm not sure if we should have every possible tenant setting in Orchard Core source code. What @sebastienros suggested is what should be done if anything.
I am okay with not making it part of the main code, but don't you think we should provide a way to change it when needed by others?
@sebastienros is suggesting that we add a new shape UserDisplay which sounds like what I need. But how would we pass the User object then while avoiding the N+1 select problem?
IMHO it should be kept as custom, as you can target contentsmeta shape with custom driver,
@ns8482e The problem is that in the driver you would have to fetch every user from the database "one at a time" since the display driver does not have info about all the users. So we would have to provide a way to prevent this "N+1 select problem"
@Skrypt Another way we may be able to do here is add a new feature just before contentItemSummaries
using something like this
HttpContext.Features.Set(new ContentListFeature(){ ContentItems = contentItemSummaries });
The feature would look something like this
public class ContentListFeature
{
public IEnumerable<ContentItem> ContentItems { get; set; }
}
and that's all that would go into OC.
Now, in custom private projects, we can customize the output using something like the following steps.
First create a service to catch the users retrieved from the user-store using something like this
public class UserForContentItemsProvider : IUserForContentItemsProvider
{
private readonly ISession _session;
private Dictionary<string, User> _users;
public UserForContentItemsProvider(ISession session)
{
_session = session;
}
public async Task LoadAsync(IEnumerable<string> userIds)
{
if (userIds == null || !userIds.Any())
{
return;
}
var existingIds = _users?.Select(x => x.Key) ?? new List<string>();
var missingIds = userIds.Where(y => !existingIds.Contains(y)).Distinct();
if (!missingIds.Any())
{
return;
}
await TryLoadingMissingUsersAsync(missingIds);
}
public async Task<User> UserAsync(IEnumerable<ContentItem> contentItems, string userId)
{
if (_users == null)
{
var userIds = contentItems.Select(x => x.Owner).Union(new List<string>() { userId }).Distinct();
await TryLoadingMissingUsersAsync(userIds);
}
if (!_users.ContainsKey(userId))
{
await TryLoadingMissingUsersAsync(new List<string>() { userId });
}
if (_users.TryGetValue(userId, out User user))
{
return user;
}
return null;
}
private async Task TryLoadingMissingUsersAsync(IEnumerable<string> missingIds)
{
var items = (await _session.Query<User, UserIndex>(x => x.UserId.IsIn(missingIds)).ListAsync()).ToDictionary(x => x.UserId, x => x);
if (_users == null)
{
_users = items;
return;
}
foreach (var item in items)
{
_users.TryAdd(item.Key, item.Value);
}
}
}
The service IUserForContentItemsProvider
would have to be registered using Scope "per request" otherwise it won't work correctly.
Finally create a new display driver using ContentDisplayDriver
. This display driver would require IUserForContentItemsProvider
service and would return something like this
public class UserContentsDriver : ContentDisplayDriver
{
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly IUserForContentItemsProvider _userForContentItemsProvider;
public UserContentsDriver(
IHttpContextAccessor httpContextAccessor,
IUserForContentItemsProvider userForContentItemsProvider)
{
_httpContextAccessor = httpContextAccessor;
_userForContentItemsProvider = userForContentItemsProvider;
}
public override Task<IDisplayResult> DisplayAsync(ContentItem contentItem, BuildDisplayContext context)
{
if (!context.DisplayType.Equals("SummaryAdmin", System.StringComparison.OrdinalIgnoreCase))
{
return null;
}
var feature = _httpContextAccessor.HttpContext.Features.Get<ContentListFeature>();
if (feature == null)
{
return null;
}
var result = Initialize<ContentItemWithUserViewModel>("ContentsMeta_SummaryAdmin_User", async viewModel =>
{
viewModel.ContentItem = contentItem;
viewModel.User = await _userForContentItemsProvider.UserAsync(feature.ContentItems, contentItem.Owner);
}).Location("SummaryAdmin", "Meta:25");
return Task.FromResult<IDisplayResult>(result);
}
}
you can subscribe to an event when query loads the items to read author ids, and then you load users from db. When summary shape builds - YesSql will return data from loaded version instead of querying the database again.
As you shared - you can also populate user query results in your feature that your driver can consume later.
@ns8482e I am not sure there is a way to listen for when all the content items in admin are loaded. Is there one that I am missing? We can't respond to each contentItem being loaded in this case otherwise we can't avoid the N+1 select problem.
I guess I could use the loaded event, and build some sort of queue to track the loaded items. But this could potentially lead to having to manage unnecessary changes in memory.
As far as I can see, best option is to add Feature in OrchardCore.Contents admin or fire a new event after all of the content items are loaded in admin.
I think adding a Feature to the request is a fair ask and we are not coupling OC with a specific implementation. All we are doing here is allowing a way to extend or make it more customizable.
@CrestApps IMHO there is no need to define new events just to identify n=0 and and n=length when you can achieve the the same with single variable within an event handler you define
There are loading and loaded events that you can use and track count of your items in handler
In loading you can collect usernames( you can even use this idea for loading any other referenced docs too) in your feature’s list, for all queried items.
And in loaded event. you can query all user objects all at once, using username list collected in your feature and keep object list in feature to be referred by driver later
In placement hide userid shape that you want to replace with display text
Create new driver for user display text, you read the user object for content item from feature’s list and target the display shape at contentmeta zone of summary admin
I published a module on nuget that provides a way to change how the user is displayed including avatar. The package is available https://www.nuget.org/packages/CrestApps.Components.Users
Is your feature request related to a problem? Please describe.
OC display the username field everywhere though the app like in content item list and the
UserPickerField
this may be great for performance, but IMO, it isn't great from user experience. Usernames are just not human friendly and should used for identification only.Describe the solution you'd like
One way to do this is by creating a "Display field" in the user screen that would allow the to type is a display name instead of a username. By default, we could save the username into the display name.
A better option may be to add First, Middle and Last name field in the user, then in the settings provide an option for the user how to display the user with the following possible options
Describe alternatives you've considered
For reporting and placed where I have control over, I use a the following contract to to get the display name. The implementation reads the settings to determine the format. I also attach a part on the
User
shape that would capture First, Middle, and Last name.Meanwhile on the content list/display views, I want to make the username as a clickable link so when the user clicks it a popover opens up with the user's info. Although this isn't ideal, this is the best I can do with the current OC setup.