chore: structure refactoring

vladsolntsev commented 5 months ago

Since your project starts to get bigger and bigger, maybe it's time to reorganize its structure a little bit? If your goal is really to stay as minimalistic as possible, ignore and close this of course, but I think it would be a bummer to not let it evolve !

This is an example of a structure that you project could adopt, respecting some of the good practices of software craftsmanship. It's just an idea and of course files could be renames/moved. I was trying to stay as closer as possible to the original, to maintain the same logic, just moving code around and using all the new things PHP has to offer. Proposed changes, will only work on PHP 8.2. I did not bother with multiple commits as almost all files are new, and this PR should be re-tested to check all functionality if we decide to keep it up.

Why reorganize?

simplifies reading of a huge single file
separates concerns
makes testing easier (if one day you are planning on adding unit testing)
harmonizes the code, reinforces re-usability

Proposal:

an MVC like structure, more frameworky. Still minimalistic with as less classes as possible while maintaining the same logic. Makes code more testable, isolating concerns, with namespace structure.
typing functions return types
using a date object to avoid errors and be more defensive
refactoring common parts of rss and sitemap generation
AuthorDTO, Post entity. As arrays become bigger, it's time for objects instead.
as your blog will grow, we will be able to extend more easily, a validator for each service, etc.
custom autoloader to stay minimalistic, but we could use composer later.

Possible evolutions:

ideally the config should not be static but it would be too far from your original index.php setup
separate config properties into multiple objects (GeneralSettings, DisplaySettings?)
create a PostIterator, where the validation logic could be stored with filters (like a filter interface with accept method)

Do you plan on implementing unit testing, and functional for the routes? If it's preferable to stay mimalistic without Composer but you wish to enjoy the world of tests, we can always use .phar files for phpunit or other utilities. Later you could run a CI here to automate this quality checking process, this way no one will break your code (or at least less :)).

coignard commented 5 months ago

Hey! Thank you for your contribution to the project development :-) I will try to test the changes soon and, if everything OK, I will merge it

coignard commented 5 months ago

@vladsolntsev

Unfortunately, I cannot accept this MR. I see that there are at least a few issues at present:

If you follow the link to the post (e.g., /example-post/), there won’t be a new lines before its title, although it is present in the original version.
If I try to use the configuration that I used previously, an error appears:

[Weblog]
author_name = "René Coignard"
author_email = "contact@renecoignard.com"
domain = "renecoignard.com"
about_text = "Political activist, human rights defender, and artist from Russia. Emigrated to Germany due to politically motivated persecution by the Russian government. Previously known as Mikhail Podivilov.\n\nThis weblog is mainly a space where I share my thoughts and broadcast today's agenda. From time to time, I also write about my personal projects and musical compositions, as well as share snippets from my latest travels."
show_powered_by = Off
show_urls = Off
show_separator = Off

PHP Fatal error: Uncaught InvalidArgumentException: Cannot cast value to string. in /<...>/src/Config.php:136

The following structure is used in the Config.php file:

public string $url = 'https://localhost',

It's unclear why https is used for localhost.

I quickly glanced through the code and these are the main issues I noticed. If you can align the functionality of the code with what is currently in the main branch, I would be happy to merge your MR. Have a great day!

coignard commented 5 months ago

Additionally, I noticed that the functionality for displaying posts by dates is broken. For instance, accessing /2023/ won't display posts for the entire year 2023 but will show an error instead.

vladsolntsev commented 5 months ago

Thanks for taking the time to test it! I wasn't expecting that everything would work straight away, but since you like the idea I 'll take a look to try to fix these issues in the coming days.

coignard commented 5 months ago

@vladsolntsev

Just in case you need a dump of all my posts and settings to check the functionality of the refactor, feel free to email me at contact@renecoignard.com

vladsolntsev commented 5 months ago

I managed to fix the default values bug in the Config, some line breaks and the posts by dates route. But I think there is some other minor line breaks missing or bugs that I may not have seen, I'll continue looking just in case.

coignard commented 5 months ago

@vladsolntsev

Thank you for the bug fixes. I see that the line breaks are still not working as intended:


Projects                         Weblog                    23 April 2024

   This weekend caught me by surprise: usually, after a workweek, I
   dedicate my free time to hanging out with friends or taking trips to
   new cities in Germany via Deutsche Bahn trains.  However, this time
   the weather turned sour for traveling, and my friends already had
   their own plans, so I decided to occupy myself with something
   interesting.  I chose to work on a project I had been contemplating
   for a while - a plain text-based blog engine on PHP.
   As you might have guessed, I succeeded (after all, you are most
   likely reading this text rendered by my engine).  I have long been a
   proponent of minimalism, and running a blog in such a style is
   thrilling for me because it requires neither a database, nor HTML,
   nor CSS - nothing!  Everything is stored in text files, which are
   rendered in a special way when the site is visited.  I was inspired
   by the RFC format, so the blog looks accordingly.
   Currently, the blog engine supports generating sitemap.xml, RSS,
   displaying posts by unique links, categories, and date.  As a bonus,
   you can find a cat on the 404 page if you try hard enough (give it a
   shot).  The blog engine's source code is available on GitHub.
   https://github.com/coignard/weblog

                 Copyright (c) 2024 example@example.com

In addition to this, I discovered that browsing posts by categories is also broken (/meditations/):

404 Not Found

                   Copyright (c)  example@example.com

coignard commented 5 months ago

Formatting is also broken on mobile devices. chore/oop-refacto version:

telegram-cloud-photo-size-4-5855136045291651505-y

Main version:

telegram-cloud-photo-size-4-5855136045291651508-y

coignard commented 5 months ago

Please first resolve the conflicts that are preventing the merge, and then click the "Ready for review" button when the code has been brought to the appropriate state. Thank you!

coignard commented 5 months ago

@vladsolntsev

Great job, thank you for your contribution to the project! The only thing I would do is disable the customization of 404 errors and stick to the standard messages — 404 Not Found and 404 Cat Found. I think it's unnecessary to give users details about what exactly went wrong. After all, I also return a 404 error if a user tries to access a resource they don't have permission for.

vladsolntsev commented 5 months ago

@coignard Thanks for your feedback! I'll take a look at this in the next week and keep you updated.

vladsolntsev commented 4 months ago

@coignard I removed the custom 404 error handling and resolved conflicts on this PR, it should be all good now. Don't hesitate to tell me if you see any more issues.

coignard commented 4 months ago

I can confirm that everything is now working as it should. Merci!

coignard / weblog

chore: structure refactoring #30