codetuner / Arebis.MvcDashboards

ASP.NET MVC Dashboard projects
MIT License
2 stars 1 forks source link

Are you happy to receive Pull Requests? #1

Closed daleholborow closed 3 weeks ago

daleholborow commented 4 months ago

Hi, I am the same guy that messaged you on LinkedIn. Love your library, its been a great help. That said, I am experiencing a few issues and am making fixes, would you like them as pull requests?

Currently, issues include: 1) custom tag helpers do not get recognized/parsed, I have to make the first two lines to replace the second two lines, for example

image

2) The DeepL translater explicitly creates a HttpClient instead of using the newer .NET CORE style of resolving from HttpClientFactory 3) DeepL translator api config documentation vs key values seems mismatched

4) I cannot get the validation regex-type logic to work for RazorPages, only MVC. Where you have builder.Services.AddControllers(config => { config.Filters.Add<ModelStateLocalizationFilter>(); });

To that end, I wrote a new class that WILL work for RazorPages, based on your initial version, register it in Program.cs as follows:

builder.Services .AddRazorPages() .AddMvcOptions(options => { // Performs localizations on the RazorPage ModelState.Error collection, specifically options.Filters.Add<RazorPagesLocalizationAsyncPageFilter>(); });

and the class definition is:

`///

/// Perform localization of the ModelState Error messages for RazorPages. /// Implementation inspired by MVC logic from https://github.com/codetuner/Arebis.MvcDashboards /// /// /// public class RazorPagesLocalizationAsyncPageFilter : IAsyncPageFilter { private readonly ILogger _logger; private readonly ModelStateLocalizationMapping _mapping; private readonly IStringLocalizer _localizer;

public RazorPagesLocalizationAsyncPageFilter(
    ILogger<RazorPagesLocalizationAsyncPageFilter> logger,
    ModelStateLocalizationMapping mapping,
    IStringLocalizer localizer
)
{
    _logger = logger;
    _mapping = mapping;
    _localizer = localizer;
}

/// <summary>
/// No additional logic required, is simply a pass-through method
/// </summary>
/// <param name="context"></param>
/// <returns></returns>
public Task OnPageHandlerSelectionAsync(PageHandlerSelectedContext context)
{
    // No custom processing related to Localization required in this method
    return Task.CompletedTask;
}

/// <summary>
/// Post-execution, update ModelState errors with localized content
/// </summary>
/// <param name="context"></param>
/// <param name="next"></param>
/// <returns></returns>
public async Task OnPageHandlerExecutionAsync(
    PageHandlerExecutingContext context,
    PageHandlerExecutionDelegate next
)
{
    if (_mapping.Mapping.Count == 0)
    {
        return;
    }

    foreach (KeyValuePair<string, ModelStateEntry> keyValuePair in context.ModelState)
    {
        List<ModelError> list = [.. keyValuePair.Value.Errors];
        keyValuePair.Value.Errors.Clear();
        foreach (ModelError modelError in list)
        {
            bool flag = false;
            foreach (ModelStateLocalizationMapping.Map map in _mapping.Mapping)
            {
                System.Text.RegularExpressions.Match match = map.Pattern.Match(modelError.ErrorMessage);
                if (!match.Success)
                {
                    continue;
                }
                if (map.ArgLocalization != null && map.ArgLocalization.Length != 0)
                {
                    object[] objArray = new object[map.ArgLocalization.Length];
                    for (int index = 0; index < objArray.Length; ++index)
                    {
                        string name = match.Groups[index + 1].Value;
                        if (map.ArgLocalization[index])
                        {
                            name = _localizer.GetString(name);
                        }
                        objArray[index] = name;
                    }
                    keyValuePair.Value.Errors.Add(_localizer.GetString(map.LocalizationKey, objArray));
                }
                else
                {
                    keyValuePair.Value.Errors.Add(_localizer.GetString(map.LocalizationKey));
                }
                flag = true;
                break;
            }
            if (!flag)
            {
                keyValuePair.Value.Errors.Add(modelError);
                _logger.LogWarning("No ModelStateLocalization pattern matched error \"{errorMessage}\" for key \"{key}\".", modelError.ErrorMessage, keyValuePair.Key);
            }
        }
    }

    await next.Invoke();
}

}`

daleholborow commented 4 months ago

I would add, the only thing I need to figure out now is, how to get the model validation text to write out to HTML forms in translated format based on the culture, during initial page render. Currently, when I render a page with, for example, EmailAddress field type, the validation message to say "email address requires an @ symbol" is always in English, regardless of what culture i have selected. On POST, the ModelState.Error parsing occurs, any other validations fire and the page is returned with all field content translated.. but on that initial load, I stil have issues. Any thoughts on whether this is possible/give up?

codetuner commented 4 months ago

Hi @daleholborow,

About your first point (custom tag helpers do not get recognized/parsed), the code of your code screenshot <OrderTableHeaderTagHelper> as if it was a tag. It isn't , it's a taghelper that adds attributes to the <th> tag but it isn't a tag on it's own. The OrdereableTableHeaderTagHelper is for instance used in: ...\Areas\MvcDashboardLocalize\Views\Key\Index.cshtml The code is something like:

Name

Where do you have the code of your screenshot from ?

About the DeepL service, you are right, some documentation was not updated. In addition, there is no default service URL (because DeepL has a different URL depending on the subscription plan type) but that is not well documented. I'll have a look at the HttpClientFactory as well.

And indeed, the ModelStateLocalizationFilter only works for MVC: it's an ActionFilter and AcitonFilters are a thing of MVC. RazorPages need PageFilters as the one you made. https://learn.microsoft.com/en-us/aspnet/core/fundamentals/middleware/index/_static/mvc-endpoint.svg

If you can contribute the class (and/or other changes) as pull request that would be much appreciated and you'll have the credits for the contribution.

Cheers!

codetuner commented 4 months ago

About your issue on validation messages on initial rendering.

I'm not sure why it would work on non-initial renderings and not on initial renderings. Validation messages (modelstate errors) are set on model binding time, so it depend if you do model binding on POST request but not on GET request while your initial request is a GET request and the later requests are POST requests, that could be an explanation...

Anyway, I tried some things out and according to me, the call to

    await next.Invoke();

in the OnPageHandlerExecutionAsync method of RazorPagesLocalizationAsyncPageFilter should be the very first command, not the very last one. Only then I got validation messages to work on GET and POST requests.

codetuner commented 4 months ago

If you want to contribute the class as a pull request, please do so on the codetuner/Arebis.Core repository, as that's where the class should come. More particularly in the "Arebis.Core.AspNet.Mvc.Localization" project. And yeah, I will rename the project into "Arebis.Core.AspNet.Localization" removing the "Mvc" part. I see no depency to specific Mvc nugets so I will be able to do so.

codetuner commented 4 months ago

Let me also add that I registered the RazorPagesLocalizationAsyncPageFilter class with the following code:

builder.Services.AddRazorPages()
    .AddMvcLocalization()
    .AddMvcOptions(options =>
    {
        options.Filters.Add<RazorPagesLocalizationAsyncPageFilter>();
    });
daleholborow commented 4 months ago

Hei, great, i will send some PRs. It might take some time, i am going away for a couple weeks but if i can do it before i leave, i will.

The tag helper in my screenshot, bottom two lines (crossed out in red) is how the localization nuget code was auto-generated into my project (ie. as SomeCustomTagHelper ) and because i am not too familiar with tag helpers, it took me some time to diagnose why things werent working. I had to manually edit the generated .cshtml files to use the tag helpers as attributes on th elements, for example, then they work fine.

I will check out your suggestions regarding the bind-time ordering etc.

PRs to come soon, thanks!

codetuner commented 3 weeks ago

Hi Dale, in follow up:

If you upgrade, don't forget to delete the ITranslationService and different implementations in the Localize folder, and add a package reference to Arebis.Core.Services.Interfaces, and - if you use translations - to Arebis.Core.Services.Translation.

daleholborow commented 3 weeks ago

thanks for update, i have been pretty slammed with various other work so not so involved in localization, but if/as i resume , i will take a look. Recommended your libs to a couple teams I know btw.,

codetuner commented 3 weeks ago

It's good to be busy, but hopefully not too much ;) Take your time anyway. Thanks for the recommendations!

I'll close this issue now. If something pops up, feel free to create a new one.