venusinterns / blogtemplate Goto Github PK

Should be using time of creation instead of last modified time

Our blog should only allow one person to register as a user, that way everyone else that visits the blog is a guest and cannot edit/write posts.

Currently not working

Methods that retrieve posts from the DataStore do not account for the tags

• CollectPostInfo
• GetPost
• IteratePosts
• GetAllPosts

Repro:
Edit title of an existing post

Expected:
Old slug will still work as a link

Actual:
New slug is generated and replaces old slug, therefore breaking any external links to the post

Possible solution:
Warn user that external links could break, allow them to keep old slug?

Currently this is the only MVC controller in the app: https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Controllers/AccountController.cs

It should probably be replaced with a Razor Page for consistency with the rest of the app.

The slug generator currently replaces only a space ( ) with a dash (-). It is more typical for a slug generator to have a limited set of allowed characters (e.g. alpha chars, numbers, etc.) and replace all other chars with a replacement char (dash is fine).

See https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Services/SlugGenerator.cs#L20

Repro:

1. Create a new post
2. Write a post with multiple paragraphs
3. Publish and view post

Expected:
Paragraph breaks are preserved when rendered.

Actual:
It's just a single glob of text.

We should not be posting the drafts with the actual posts in the index page. Those should go in the drafts page.

Currently not working

This could allow invalid data to be submitted and stored.

I wonder if XML would be perceived as "old." As an author on an OLD blog engine, I can say folks give us crap regularly about our XML.

E.g. see https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Services/SlugGenerator.cs#L11

This code:

        private BlogDataStore _dataStore;

Should be:

        private readonly BlogDataStore _dataStore;

This ensures that the _dataStore field can't be overwritten anywhere else. The readonly modifier indicates that the field can be set from only the constructor.

I recommend checking elsewhere in the code in case this same recommendation applies anywhere else.

When user creates a comment, make sure that required fields are filled out

When there are existing drafts need to remove "This is where drafts go" line

Show an alert message, as specified by the spec.

Take a look at @xoofx's writable xplat filesystem abstraction: https://github.com/xoofx/zio/blob/master/src/Zio/IFileSystem.cs

In Startup.cs a singleton 'Blog' service is registered:

https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Startup.cs#L48

But the implementation of the Blog class is clearly not thread-safe for multiple writers:

https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Models/Blog.cs#L11-L13

That is, if two threads (e.g. two requests) both call Blog.Comments.Add(...) at the same time, something "bad" will happen.

For example, see this code here: https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Pages/New.cshtml.cs#L70

Viewers of the blog should not be able to read the email of the people who have commented.

Never save dates in a "culture specific" format. Always use ISO8601 as a culture invariant. You deserailize the date from that format and THEN output it specific to the user's machine in the view.

Need to validate email addresses in the comments form

Cannot write a new post. When clicked it says "An error occurred during the compilation of a resource required to process this request. Please review the following specific error details and modify your source code appropriately."

The test for SlugGenerator about removing invalid characters in the title is failing.

The SlugGenerator is new'ed up wherever it's used, e.g.:

1. BlogTemplate/BlogTemplate/Pages/New.cshtml.cs

   Line 60 in 007603b

   SlugGenerator slugGenerator = new SlugGenerator(_dataStore);

2. BlogTemplate/BlogTemplate/Pages/Edit.cshtml.cs

   Line 78 in fb4d281

   SlugGenerator slugGenerator = new SlugGenerator(_dataStore);

It would be more typical to register the SlugGenerator as an actual service in Startup.cs's ConfigureService method, and then consume it from the DI container wherever it's used.

If you haven't used this pattern before, your friendly local Ryan (e.g. @rynowak) or Taylor (@NTaylorMullen) can help you do this in Razor Pages.

Posts also need to have their required fields validated

• Title - Required
• Post - Required
• Tags is optional
• Excerpt is optional

See dotnet/templating#1191 for more info.

Ultimately the code here is just derived from the original template code, but we need to clean it up here as well.

See https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Pages/New.cshtml#L25

There's an invalid </> closing tag in there.

Might be a good idea to check the whole site for HTML5 compliance.

Display PubDate for when the comment was actually published. PubDates for comments are currently being automatically set to Jan 01, 0001

Repro:
Enter the following form fields:
Title: punctuation in slugs?
Your post...: huh
Excerpt: huh
Tags: idk

Expected:
Special characters are removed from the slug (in the slug generator method).

Actual:
Throw an Exception User-Unhandled

System.ArgumentException: 'Illegal characters in path.'

Currently the drafts page doesn't include the number of comments on the post or the date of the post. If it has already been published and then saved to drafts we should outline the date. In other cases where the post has yet to be published there should be nothing in that space.

When the logged in user goes to drafts and selects a draft he/she is taken to the edit page of that post. However, there is a small cancel button at the bottom that when pressed just takes you back to the previous page. I think this button may not be necessary in this scenario since you could just use the arrows to go back.

Currently the Index page and Drafts page both have the same "model" behavior: the model retrieves all blog posts, regardless of their visibility state (public or draft).

Then the view pages have their own logic for filtering the posts: when they enumerate the full list of posts, they apply some (simple) filtering logic to determine which subset of posts to show.

Instead of that, the views should not know what kind of posts they are showing: they should be provided with an already-filtered list of posts, and then they can iterate over them. You want "smart" models and "dumb" views in general.

Here are the "offending" lines:

• Retrieving the data without filtering:
  • https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Pages/Index.cshtml.cs#L30
  • https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Pages/Drafts.cshtml.cs#L27
• Listing the data and applying filtering:
  • https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Pages/Index.cshtml#L6-L17
  • https://github.com/VenusInterns/BlogTemplate/blob/master/BlogTemplate/Pages/Drafts.cshtml#L11-L20

I recommend that instead of the models doing this:

            _blog.Posts = _dataStore.GetAllPosts();

That they do something like this:

            _blog.PublicPosts = _dataStore.GetAllPosts().Where(post => post.IsPublic);

And then the views get "dumber" and instead of this:

    foreach (var post in Model.Posts)
    {
        if (!post.IsPublic)
        {

Do something like this:

    foreach (var post in Model.PublicPosts)
    {

In the following files:

• New.cshtml
• Edit.cshtml

Might therefore need to change functionality slightly in New.cshtml.cs and Edit.cshtml.cs

When a comment is deleted the number displayed under the post in the homepage isn't updated.

To reproduce:
Write a new post. Enter the following fields:
Title: panda panda panda
Your post...: I got broads in Atlanta
Excerpt: I got broads in Atlanta
Tags: broads,atlanta

Expected:
Redirects to the Index and displays the new post at the top, showing Title, PubDate, Excerpt, and Tags.

Actual:
In New.cshtml.cs, line 66 throws an exception whenever commas are entered in the tags field.

Exception User-Unhandled
System.IndexOutOfRangeException: 'Index was outside the bounds of the array.'

The following characters alone or repeated 'break' the slug so that once you publish or save the draft, you cannot access it since the character title seems to break the slug. Those characters are: '#', '/', '', '|', '<', '>'

Similarly, if the title of the post is '%' then we get a BadRequest 400 error 'Invalid URL'.

Similar to #41 (well, the opposite actually). Since the ManageComments page doesn't actually render anything, a Controller may be more appropriate. Either way, we should be consistent about logic-only handlers.

Preview the post after editing it and before actually publishing it to the site.

Remove any references to tags or else the blog breaks when you save a post.

I made a post and clicked Save Draft. Nothing happened? Unclear from the UI if it's gone or not.

Only the user who has logged in (the owner) should be able to view drafts and edit them.
Need to add restrictions to this page.

I haven't actually tried it, but I'm certain it will happen:

1. Launch the site, but do not visit the / (or /Index) page!
2. Browse to the /New page and type in your awesome new blog post
3. Press the Save button (doesn't matter whether it's Public or Draft)

Result: I'm guessing that this line will throw a null ref exception:

Why? Because the only thing that seems to "load" the blog posts is the /Index page here:

While this bug is trivial to fix, I think there's a bit of a mismatch between how the singleton Blog service is used, and how the IDataStore is used. It isn't clear from the code what is the responsibility of each - in fact, it seems like they both share certain responsibilities, which is apparent in this code, where when a new post is added, two different data structures need to be modified. (This is also orthogonally related to #47.)

I'm happy to discuss this in person if anything here wasn't clear.

The use of DateTime is prevalent in the codebase:
https://github.com/VenusInterns/BlogTemplate/search?utf8=%E2%9C%93&q=datetime&type=

But, per MSDN, it is preferred to use DateTimeOffset when referring to actual times (i.e. the actual time a blog post was posted, or a comment was commented).

In fact, it goes so far as to say:

Note

These uses for DateTimeOffset values are much more common than those for DateTime values. As a result, DateTimeOffset should be considered the default date and time type for application development.

The change is probably straightforward: just search & replace all DateTime with DateTimeOffset.