-
-
Notifications
You must be signed in to change notification settings - Fork 158
Routing does not respect custom pluralized resource name #805
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See review feedback. In general, these types are mixing a lot of concerns and are deeply independent on each other, which makes it hard to track what is going on. But this was already the case, I'm not asking you to clean that up.
I think this PR should address #807 as well. Docs are fully up-to-date and verified right now, so I don't see why it should be postponed. |
Another question: can you confirm that #800 (comment) still holds? |
No: it this has not been the case for a long while now. The mapping between resources/routes/controllers is handled by |
Rephrasing
fix example fix: bump appveyor build fix: undo c# 8 syntax for using statements fix: attempt to fix win dispose bug fix: bump build fix: appveyor bump fix: appveyor build fix final
18a3cf7
to
4262410
Compare
src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs
Outdated
Show resolved
Hide resolved
src/JsonApiDotNetCore/Configuration/ServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs
Outdated
Show resolved
Hide resolved
public class Startup | ||
{ | ||
private Action<MvcOptions> _postConfigureMvcOptions; | ||
|
||
public void ConfigureServices(IServiceCollection services) | ||
{ | ||
... | ||
|
||
var builder = services.AddMvcCore(); | ||
services.AddJsonApi<AppDbContext>( ... , mvcBuilder: builder); | ||
mvcCoreBuilder.AddMvcOptions(x => | ||
{ | ||
// execute the mvc configuration callback after the JsonApiDotNetCore callback as been executed. | ||
_postConfigureMvcOptions?.Invoke(x); | ||
}); | ||
|
||
... | ||
} | ||
|
||
public void Configure(IApplicationBuilder app, IWebHostEnvironment environment) | ||
{ | ||
|
||
... | ||
|
||
// Using a callback, we can defer to later (when service collection has become available). | ||
_postConfigureMvcOptions = mvcOptions => | ||
{ | ||
mvcOptions.Filters.Clear(); | ||
mvcOptions.Filters.Insert(0, app.ApplicationServices.GetService<CustomFilter>()); | ||
}; | ||
|
||
... | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You put the call to app.UseJsonApi()
in the wrong place. And didn't fix the white space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments. I've resolved the fixed items. What's still open is not yet addressed,
Closes #786
Closes #807
JsonApiRoutingConvention
now depends onIResourceGraph
rather thanResourceNameFormatter
. As a result:publicResourceName
argument inAddResource<T>( ... )
is now respected, see my comment in PluralizedTypeName option when adding resource doesn't work for routing #786.JsonApiRoutingConvention
, specifically)