Giter VIP home page Giter VIP logo

Comments (22)

fretje avatar fretje commented on July 19, 2024 1

Got it to work... the not supported part was for classic asp.net... it does work in aspnetcore apparently... it was me who made a stupid mistake :-s

see fullstackhero/dotnet-webapi-starter-kit#319

which ends up nicely under the role name:
image

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

@fretje We can use this space for discussing instead of the closed PR.

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

So, instead of dataAnnotations validator we can make use of the local FluentValidator and remove the validation dependency with NSWAG's generated code. or maybe even use both?

What do you mean with "validation dependency with nswag's generated code"?

There's not really a dependency. NSwag does generate dataannotation attributes on the generated dto's according to the openapi spec of the api.

This is simply using the default built-in blazor (data annotation) validation... This can probably be augmented with fluent validators for sure... but I've yet to see an example of that in this code base :)

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

I think there's also a difference between standard blazor validation and the validation that's used inside mudblazor? I haven't really researched that yet... But from what I've seen they aren't working with <EditForm>?

Would be nice to see a working sample, so we can see what the differences are...

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

Hmmm, I started adding TCreateRequest and TUpdateRequest type parameters, but I run into the problem that the EditFormContent RenderFragment is typed and can then of course only be one of those types, or I have to provide 2 fragments EditContent and UpdateContent, but then we have to copy the same thing twice again where we have to use them :-s

We're probably gonna have to resort to one extra type "TAddEditRequest" and then in the updateFunc we'll have to do the mapping manually, or in the createFunc, depending on which request type you want to use for both in the EditFormContent block.

Which is a pitty because then we have the validation problem again as it's only checking the rules for the createrequest then. grmbl :-s

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

image

Added fluentvalidation rules to Nswag using the zymlabs package. It's able to take in the validation rules we defined onto the APIClient code. and Login component seems to working now as required,

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

For Brands and Products, I think the issue is that we are trying to validate the Dto class instead of the Create*****Request classes where the actual fluent validators are attached.

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

yet indeed, that's why I'm adding the extra type parameter... but then it's still only one (the same for create and update)...

Also: the login already worked for me with the previous code... we should better find out why it didn't work for you?

But very nice btw with fluent validation... are the rules converted to dataannotations? or is the actial validater somehow generated in the client dode?

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

I have seen that the brand and product services return not a problemdetails with the 400 when the validation is wrong, but our own ErrorResult... Any reason why we're not following the standard here? We don't have enough information this way to add the validation errors to the controls on the client (propertyname is missing).

I've added a ValidationErrors property to the ValidationException CustomException and put the errors in there similar like with probolemdetails, but then I run into the problem on the client that the propertynames don't have the same case because the jsonserializer changes the properties to camelcase??

I think we really should consolidate everything to one style of validation... it gets really compicated like this...

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

It converts the FluentValidations to equivalent DataAnnotations in the nswag generated code, which is sweet.

I see that in the API there are few places where we are still using DataAnnotations. I think it's better to switch everything to Fluent Validation to maintain consistency. I will push those changes in some time.

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

Yeah, can you make the 400 returns uniform throughout? It's better to follow one standard.

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

But i guess i already added these to the API Conventions. Wont that be enough?

[ProducesDefaultResponseType(typeof(ErrorResult<string>))]
    [ProducesResponseType(400, Type = typeof(HttpValidationProblemDetails))]

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

One thing is the difference between dataannotations and fluentvalidations...

It shouldn't be a problem to mix those up... they should in the end give the same behavior, which is returning a 400 with a problemdetails... which they don't now.

The problem is with the exceptionmiddleware, but I don't understand why we even need that. Especially as we're using "Results" as well...

And all the controllers return IActionResults while they never do anything else then "return OK"... that's another abstraction which shouldn't be there? There seems to be quite some different patterns at work here and I'm not sure how (or even if) they work together...

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024
  • Or your services always return results, which in the controller at the end are translated to specific actionresults (and the result isn't sent to the client, but the actual data only)
  • Or your services throw exceptions, which get translated to actionresults in the exceptionmiddleware...

Not sure if it's wise to combine those 2... especially if the exceptionmiddleware sends back another type of error than the default validation...

So I'm a bit at a loss here and not sure how to start...

from blazor-wasm-boilerplate.

iammukeshm avatar iammukeshm commented on July 19, 2024

the idea with ExceptionMiddleware was for catching all exceptions at one place and returning a uniform response for all errors.

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

Hmmm... It's kind of a big design decision, but I think the ExceptionMiddleware should only handle "Unhandled" exceptions and we should never explicitly throw exceptions from our services...

I'm not seeing the forest through the tree's right now... there's too many things customized and not working nicely together with the things that are not customized :-s

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

I am merging in your changes...

Like it is now, we can not put

[ProducesResponseType(400, Type = typeof(HttpValidationProblemDetails))]

on all the actionmethods in FSHApiConventions as sometimes a 400 is sent back with an ErrorResult in stead... and there's no way to differentiate between the two... and the client even throws an exception I think... Anyway I'll leave them in for now and search for a scenario where that can be a problem...

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

I've made some pull requests to show my changes:

#62
fullstackhero/dotnet-webapi-starter-kit#314

Hmm still have some issues with your latest changes... let me check that and get back to you ;-)

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

It's ok, but now that you added the fluentvalidation schemaprocessor, I don't get there anymore :-s
Better first to implement server validation properly before doing the client validation ;-)

There will still be a need for that for validation rules that can only be run on the server for instance (e.g. checking something in the database)... maybe we should introduce one of those to be able to test this...

Anyways, there's still a problem now on the Roles AddEditModal. There a 400 is returned which is a errorresult, but gets parsed as a problemdetails (due to the 400 you added on all the FshApiConventions)... so it throws:
image

Thats the exception I was talking about a few comments ago ;-)

Also when I comment out the fluentvalidationschemaprocessor on the server and regenerate the client, I have the same exception on the products addeditform, as there as well a 400 ErrorResult is returned... so you can't simply add [ProducesResponseType(400, Type = typeof(HttpValidationProblemDetails))] everywhere... Unless we can make that work somehow that a 400 will always be a HttpValidationProblemDetails...

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

I think we shouldn't throw our own validationexception in our customvalidator... If I comment out those lines, validation errors are returned as HttpValidationProblemDetails, and everything works the same (at least with regards to validation...)

Then I can remove that code again where the validationerrors are converted and also the camelcased jsonserializer...

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

It feels much better without the customvalidator, but I still get a problem with the roles...

Thats because in RolesService.RegisterRoleAsync this is thrown:

throw new IdentityException(_localizer["Name is required"], statusCode: HttpStatusCode.BadRequest);

which results in a 400 ErrorResult again...

Aha, maybe that's the perfect canditate to make a server side only validationrule with ("can't have role with same name")...

Hmm you seem to be checking for similar names when creating a new role, but not when updating one...

_roleManager.UpdateAsync(existingRole) actually returns a failed result with "name already exists", but that's not looked at...

from blazor-wasm-boilerplate.

fretje avatar fretje commented on July 19, 2024

Ok, most issues seem to be fixed now...

I was just trying to add a validation rule for RoleRequest, something like:

        RuleFor(p => p.Name)
            .NotEmpty()
            .MustAsync(async (name, _) => !await _roleService.RoleExistsAsync(name));

but apparently async validation is not supported in the aspnetcore pipeline :-s (https://docs.fluentvalidation.net/en/latest/async.html).

from blazor-wasm-boilerplate.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.