valdisiljuconoks / localization-provider-core

Database driven localization provider for .NET applications (with administrative management UI)
Apache License 2.0
118 stars 22 forks source link

Problem with resources for ViewModels #5

Closed qbus00 closed 6 years ago

qbus00 commented 6 years ago

Hello, I found that there's a problem with ViewModels localizations.

It looks that in properties of attriubutes you have to put Resource Keys instead of texts - ASP.NET Core looks these values (from Name or ErrorMessage) as keys to lookup resources.

In your example you are putting actual texts there but this does not work.

valdisiljuconoks commented 6 years ago

Hi,

How you are using metadata from the view models? Did you check sample project?

Here is a model: https://github.com/valdisiljuconoks/localization-provider-core/blob/master/tests/DbLocalizationProvider.Core.AspNetSample/Models/UserViewModel.cs

Usage: https://github.com/valdisiljuconoks/localization-provider-core/blob/master/tests/DbLocalizationProvider.Core.AspNetSample/Views/Home/Index.cshtml#L31

qbus00 commented 6 years ago

Yes, I have a standard CSHTML with asp validation-for attribute. I see that data-val-req.. attributes of HTML are not being populated correctly. The only way I managed to run this is to create an attributes on ViewModel like this:

[Display(Name = "namespace.ResourceClass." + nameof(ResourceClass.Email))]
[Required(ErrorMessage = "namespace.ResourceClass." + nameof(ResourceClass.EmailRequired))]
[EmailAddress(ErrorMessage = "namespace.ResourceClass." + nameof(ResourceClass.EmailValidation))]
public string Email { get; set; }

When I decorate ViewModel with [LocalizedModel] attribute I see that library properly creates resource keys in database (for example Email-Required) but the string inside "Name" or "ErrorMessage" is treated as resource key not a value. My workaround for now is to use structure above (then I do not have to add [LocalizedModel] attribute) but this is something that I think is against your idea of this plugin. This looks that this is a behavior that is expected to be standard ASP.NET Core one when using RESX resources - you put Errormessage or Display string as a key into RESX.

valdisiljuconoks commented 6 years ago

Interesting.. I just tested in sandbox sample project (with changed ErrorMessage property) and it seems to be working just fine.

image

Can you fork and see if you can repro in sandbox project? I'm speculating that maybe there is something extra on top in your project that triggers metadata provider replace?! Out of the box this model metadata provider is registered here: https://github.com/valdisiljuconoks/localization-provider-core/blob/master/src/DbLocalizationProvider.AspNetCore/IServiceCollectionExtensions.cs#L56

Can you check what providers you have in MvcOptions.ModelValidatorProviders collection?

qbus00 commented 6 years ago

here's how it looks on my side:

image image

Your case also works on my side but does changing entries in database for UserViewModel.Password or UserViewModel.Password-Required keys works? I've also debugged inside InMemoryCacheManager Get method and it looks that "key" argument for Model field is DbLocalizationProviderCache_Value inside ErrorMessage/Display string so in your example it would be DbLocalizationProviderCache_Password is kinda required :)

valdisiljuconoks commented 6 years ago

even more interesting.. there really seems to be something fishy going on.. here is entry from the sandbox project:

image

valdisiljuconoks commented 6 years ago

and model metadata provider collection seems to be about right.. sandbox has 16 detail providers tho (instead of 18), but guess that might not be the issue

qbus00 commented 6 years ago

Thank you for checking that, so probably there's something on my side. Could be an issue that I'm using ASP.NET Core 2.1?

valdisiljuconoks commented 6 years ago

might be. this is yet untested target on my side

qbus00 commented 6 years ago

OK, thank you once more. I'll try to spot what's going on my side.

valdisiljuconoks commented 6 years ago

hi,

quick update. just tested in empty core 2.1 webapp - seems to be working just fine there as well. metadata is filled in correctly.

qbus00 commented 6 years ago

Hi again, I found something but don't know how to proceed with that, maybe you're will be able to help me. On Javascript validations localization of viewmodel in my solution works as I described before - the string inside "Name" or "ErrorMessage" is treated as resource key not a value. The solution works as it should when validation when there's a post to server and server returns validation errors - then validation summary presents value using your method (for example with .Email-Required suffix). I'm not an expert in ASP.NET Core validation framework, maybe you can point me what else I should check?

valdisiljuconoks commented 6 years ago

hi,

do you have code snippet I could look at (client-side)?

qbus00 commented 6 years ago

Hi, Actually what I found is that when I removed ".AddDataAnnotationsLocalization();" from AddMvc it started to work as you described in your solution. I'm now doing more tests will let you know if it is 100% true.

valdisiljuconoks commented 6 years ago

order might be important. please note sample project source code or you can paste your relevant fragments of Startup.cs here to take a look.

qbus00 commented 6 years ago

Yes, order is correct, I double checked it. I know it's a bit chaotic but my current observations are:

  1. I double checkoud configuration in startup.cs and it looks OK
  2. I've added [LocalizedModel] attribute to my view model and put translation values into Message attributes - example: [Display(Name = "E-postadresse")] [Required(ErrorMessage = "E-postadresse er påkrevd.")] [EmailAddress(ErrorMessage = "E-postadresse må være en gyldig e-postadresse.")] public string Email { get; set; }

For [Display] attribute everything now works perfectly fine, changes in DB are visible on page as they shoudl - it means that label is being translated OK. <label asp-for="Email" for="email" </label>

For [Required] and [EmailAddress] we use only validation summary panel: <div asp-validation-summary="All" ></div>

Inside that panel when validation occures only on javascript messages are always taken from ErrorMessage property of these attributes.

When form is not validated by javascript but only server side then after loading page with validation errors inside that panel all strings are translated to their proper values from database. For example for Required for EmailAddress it is being taken from "namespace.ViewModel.Email-Required".

When I check HTML it looks that input field that has validation has invalid data-val-required attribute (this attribute contains text that should be displayed when javascript validation occures) - it is always original value from attribute (not database).

I'll download your sample project now and try to reproduce this issue.

qbus00 commented 6 years ago

Sorry for spamming - I managed to reproduce an issue on sample project. What I did:

  1. Because of problems with node I removed references to projects in AspNetSample project with NuGetPackages.
  2. I have added [LocalizedModel] attribute to RegisterViewModel class.
  3. I have added [Required(ErrorMessage = "Email field is required.")] to Email Property
  4. I ran project - in database I found out that "DbLocalizationProvider.Core.AspNetSample.Models.AccountViewModels.RegisterViewModel.Email-Required" has been successfully created.
  5. I checked that when clicking "Register" button having all field empty displays "Email field is required." message.
  6. Using AdminUI I modified key "DbLocalizationProvider.Core.AspNetSample.Models.AccountViewModels.RegisterViewModel.Email-Required" from "Email field is required." to something else ("12345").
  7. Just to make sure I've restarted application.
  8. When I enter Register page and click "Register" button I still see "Email field is required." in both summary validation and under input field (should be "12345" as this value in DB).
  9. On the same time when I modify "DbLocalizationProvider.Core.AspNetSample.Models.AccountViewModels.RegisterViewModel.Email" in AdminUI which is used as a caption for Email field I see that changes are being made immediately. image

image

I hope this helps to resolve an issue.

qbus00 commented 6 years ago

More info: I've checked your code. It looks that javascript validation (the one that is using data-val-* fields does not use LocalizedModelValidator. Instead of that it taking ErrorMessage property as it is and displays it.

I checked LocalizedDisplayMetadataProvider class and found following code: var displayAttribute = theAttributes.OfType<DisplayAttribute>().FirstOrDefault(); if(displayAttribute?.Description != null) modelMetadata.Description = () => ModelMetadataLocalizationHelper.GetTranslation(containerType, $"{propertyName}-Description");

then I tried to use similar approach with ValidationAttribute. Unfortunately there's a problem as this \ ErrorMessage is a normal property and not Func so it's impossible to do it in the same way.

I suspect that to make it properly it would require to create a custom validators that derrives from original ones and override FormatErrorMessage method which will find proper resource key model and display it. The other way would be to modify validation engine of ASP.NET MVC to work it in a same way as your LocalizedModelValidator.

As I've already proceed with my previous way of defining resource key in ErrorMessage properties of validator attributes, I've changed behavior of LocalizedModelValidator to use _attribute.ErrorMessage property as a key instead of ResourceKeyBuilder.BuildResourceKey method.

This way it's not working in your way but at least it's consistent in both validation scenarios.

valdisiljuconoks commented 6 years ago

ok, many thanks for the repo and insights, i'll take a look at mvc validation piepline. would love to see it unified across all scenarios. and shame that they are not using the same approach ;)

valdisiljuconoks commented 6 years ago

got first prototype of ClientValidator pipeline working ;) need to sandpaper rough edges and hope ready to go..

valdisiljuconoks commented 6 years ago

give it a try and let's see whether it blends

https://www.nuget.org/packages/LocalizationProvider.AspNetCore/4.3.0

qbus00 commented 6 years ago

Now it works, thank you for your support!