-
-
Notifications
You must be signed in to change notification settings - Fork 76
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
Feature/search-engine-friendly-url #317
Feature/search-engine-friendly-url #317
Conversation
Thanks for providing the PR to tackle the URL / Slug stuff. I want to discuss a few points with you and get your input. I do find your approach interesting - avoiding some of the issues I had initially with the topic. My first approach was next to the page URL with a guid to allow the slug. So something like that: @page "/blogPost/{blogPostId}
@page "/blogPost/{slug} That said - here a few points I do think we might want to tackle:
What are your thoughts on that? |
Thanks for reviewing the PR. I agree with your first point but about the second one here are a few things we need to discuss: Imagine we make slugs immutable and for any reason, a user writes a wrong title for the blog post even after editing the title slug is still wrong even if we consider a new column in the database and ask the user to manually fill it maybe need to be changed unless we force the user to delete the current post and create another with a correct slug. So in my opinion we have two options:
What are your thoughts on that? |
That is a good point - and I don't know anymore what I did in my first draft of a "slug". One open point I don't have a conclusion on right now is whether or not the GUID should still be part of the URL. Currently, I am leaning toward your way of handling things. So the slug "is just a nice addition" without any impact. |
src/LinkDotNet.Blog.Infrastructure/Persistence/Sql/Mapping/BlogPostConfiguration.cs
Outdated
Show resolved
Hide resolved
src/LinkDotNet.Blog.Web/Features/ShowBlogPost/ShowBlogPostPage.razor
Outdated
Show resolved
Hide resolved
I would also remove the "URL-Friendly" link from the RSS part. Stackoverflow does the same: https://stackoverflow.com/feeds |
I have seen a lot of blogs that have both GUID and slug together not the least of which is www.searchenginejournal.com |
src/LinkDotNet.Blog.Web/Features/ShowBlogPost/ShowBlogPostPage.razor
Outdated
Show resolved
Hide resolved
I am currently a bit busy. Just ping me or request a review when I should have a second look! |
src/LinkDotNet.Blog.Web/Features/Components/ShortBlogPost.razor
Outdated
Show resolved
Hide resolved
Some of the tests seem like failing I guess we should modify them to cover our new changes also we need to add some new tests to cover our new features like slug generator. |
Absolutely. I do think we need to find edge cases. An extensive suite of tests would be great. |
Really great job @Kamyab7! |
Happy to hear that :) |
LGTM! If there is anything open, let me know - otherwise we can merge the changes. |
Let's merge it. |
This PR closes #202
Change Log