Skip to content
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

Add a package that provides a publications feature #685

Merged
merged 4,940 commits into from
May 14, 2023

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Nov 23, 2022

About

Publication pages adds an easy way to create custom no-code page types, with support using a custom front matter schema and Blade templates. This feature is in development at the HydePHP develop monorepository, #685, and will be published as an official HydePHP extension tied to HydePHP v1.0.

Thank you to @rgasch for suggesting and implementing this feature in #667!

Tasks before merge:

General

  • Add documentation for the publications feature (@rgasch do you want to handle this?)
  • Ensure 100% test coverage for publications code
  • Resolve code review comments (20)

Pull Requests

Issues

Comments

  • Generate list and detail pages when creating publication types (comment)
  • Generated pubtype field names should be lower/kebab case (comment)
  • Move logic to SourceFileParser class (comment)
  • Create class for the tag collection (comment)

Ideas/Bugs/Other

  • Add visual regression tests for generated pages? (skipping for now)
  • Replace validation message placeholders like validation.required
  • Maybe we want to validate the schema files when running the validate command?
  • Store __createdAt publications field as string instead of Unix timestamp (unable to repro atm)
  • Normalize command names

Updates to the make:publication command #786

  • When using php hyde make:publication foo and there are no publications at all, the current message "Error: Unable to locate any publication types. Did you create any?" might not be entirely relevant.
  • Fix integers are serialized as strings
  • Make sure that the proper type is selected when asked (fixed in 2be5959 / Refactor publication page creation internals #793)

Narrow scope of PR

Design questions to confer with rgasch

  • Should pagination be enabled by default? (Disabled in c4df021, 6acde59) (probably yes, as it's more future proof as eventually pagination will likely be desired)
  • Composer attribution
  • Tags as yml? (sure (or maybe both?))
  • Navigation links to list pages? (yes)
  • Confirm if list/detail is best or if we should go with show/index. Or single/list (I feel detail is a bit confusing)
  • Are the default pagination templates named well? And are they satisfactory?

Documentation (draft)

Publications

Introduction

Publication pages adds an easy way to create custom no-code page types, with support using a custom front matter schema and Blade templates.

Creating your first publication type

Overview

Publication types provide the rules and structure for the publication pages within. At their core, a publication type is just a JSON schema file in a top-level directory of your HydePHP project. When adding publication pages, you put the Markdown files in that directory, along with any [#templates] you want to use for compiling them.

Getting started

It could not be simpler to get started! All you need to do is enter the php hyde make:publicationType command in your terminal, and you will be asked to fill in details in the interactive prompt.

[img]

Once done, the command will generate a new directory based on the selected name, as well as a few files:

schema.json - This file defines information for Hyde on how to render your publications, as well as the front matter types used by the pages.
list.blade.php - This file will be used when rendering the index page (or pages if you enabled pagination)
detail.blade.php - This file is used to render a single publication page

Creating publication pages

Overview

Publication pages are nothing more than Markdown files in your publication’s directory. They function very similarly to [#blog posts] and [#Markdown pages] and can even be scaffolded the same way

Features

Pagination…
Validation…

@codecov
Copy link

codecov bot commented Nov 23, 2022

Codecov Report

Merging #685 (9896fc2) into master (e11f339) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##             master     #685     +/-   ##
===========================================
  Coverage     99.97%   99.97%             
- Complexity     2678     3388    +710     
===========================================
  Files           304      360     +56     
  Lines          6978     9164   +2186     
===========================================
+ Hits           6976     9162   +2186     
  Misses            2        2             
Impacted Files Coverage Δ
packages/publications/src/Actions/CreateAction.php 100.00% <100.00%> (ø)
...ications/src/Actions/CreatesNewPublicationPage.php 100.00% <100.00%> (ø)
...ications/src/Actions/CreatesNewPublicationType.php 100.00% <100.00%> (ø)
...tions/src/Actions/GeneratesPublicationTagPages.php 100.00% <100.00%> (ø)
...ications/src/Actions/PublicationFieldValidator.php 100.00% <100.00%> (ø)
...blications/src/Actions/PublicationPageCompiler.php 100.00% <100.00%> (ø)
...lications/src/Actions/PublicationPageValidator.php 100.00% <100.00%> (ø)
...cations/src/Actions/PublicationSchemaValidator.php 100.00% <100.00%> (ø)
...publications/src/Actions/SeedsPublicationFiles.php 100.00% <100.00%> (ø)
...ations/src/Commands/Helpers/InputStreamHandler.php 100.00% <100.00%> (ø)
... and 18 more

... and 28 files with indirect coverage changes

@caendesilva caendesilva marked this pull request as ready for review November 23, 2022 18:13
@caendesilva caendesilva marked this pull request as draft November 23, 2022 18:14
@caendesilva caendesilva marked this pull request as ready for review November 23, 2022 18:28
@caendesilva caendesilva marked this pull request as draft November 23, 2022 18:28
@caendesilva
Copy link
Member Author

caendesilva commented Nov 26, 2022

Are list and detail the best names for templates? Maybe we should match Laravel and use show and index?

Edit: List and detail are probably best for now.

@caendesilva
Copy link
Member Author

From Robert RE: Generate list and detail pages when creating publication types

That is a good idea, it could just be a simple/stupid template which does

{{ $fieldName}}: {{ $fieldValue}

Maybe something a bit more involved, but some sort of basic generic template which displays the data

@caendesilva
Copy link
Member Author

caendesilva commented Nov 27, 2022

Generated pubtype field names should be lower/kebab case

Done: they are converted via Str::slug()

config/app.php Outdated Show resolved Hide resolved
@caendesilva
Copy link
Member Author

caendesilva commented Dec 3, 2022

Some notes for me to circle back on later(tm)

1. I think the default values used in the make pubtype command can be set as default values in the constructor as it is extremely verbose, making it bulky to test. 2. Consider putting the tag collection in a tag collection class to move all that logic to one place. Maybe, as it might be overengineering.

Update: All resolved

caendesilva added a commit that referenced this pull request Dec 4, 2022
Generated PublicationFieldType field names should be lower/kebab case to match the constructor normalization made in the PublicationFieldType class

_Originally posted by @caendesilva in #685 (comment)
@caendesilva
Copy link
Member Author

GitHub Copilot gave me an idea. What if instead of a schema.json for publications, we actually generate a PHP class? That would ensure direct type safety and reduce parsing. Something to experiment with maybe?

@caendesilva
Copy link
Member Author

I also want to make the publications source root configurable, for example by having them all in a publications/ directory.

@rgasch
Copy link
Contributor

rgasch commented Dec 7, 2022

So:

  1. Generate a PHP Class instread of json: could be done, I'm not sure if it's a good idea, because quite frankly more people will know how to edit a json than how to edit a PHP class. It's not a bad idea, I just wonder what the 'cost' of doing it that way would be (in terms of accessability & adoption).

  2. Configurable publication dir: If we assume that all content would be under the /content directory and that we wouldn't have the hardcoded blogs thing anymore, then isn't that just the same as having a configurable "content" directory?

@caendesilva
Copy link
Member Author

So:

  1. Generate a PHP Class instread of json: could be done, I'm not sure if it's a good idea, because quite frankly more people will know how to edit a json than how to edit a PHP class. It's not a bad idea, I just wonder what the 'cost' of doing it that way would be (in terms of accessability & adoption).
  2. Configurable publication dir: If we assume that all content would be under the /content directory and that we wouldn't have the hardcoded blogs thing anymore, then isn't that just the same as having a configurable "content" directory?

1: Thanks for your thoughts, it was just a random idea I had, but you're right, JSON is probably easier.

2: Right, my sleepy brain forgot about that!

@caendesilva
Copy link
Member Author

Oh sorting out some composer merge conflicts, I'm reminded, we probably should move illuminate/validation to the framework package... (*Note to later self)

@caendesilva
Copy link
Member Author

caendesilva commented Dec 14, 2022

RE: f87dac1 We might want to show index (list) pages in navigation. Idea for later...

Edit: Could also be something to put in the schema

@caendesilva
Copy link
Member Author

The range validations add a very large amount of complexity. I'm starting to think about how much we really need them. Sure length validation for article descriptions might be good, but do we really need to validate date ranges and integer values? Maybe it would be better to allow arbitrary Laravel validation rules to be specified in the schema file instead of its min/max values.

Originally posted by @caendesilva in #765 (comment)

@caendesilva caendesilva temporarily deployed to pr-documentation-685 May 7, 2023 19:13 — with GitHub Actions Inactive
@caendesilva
Copy link
Member Author

While the package is not done, I think I will soon merge this as the size of this PR is so massive, it will help the polishing phase to be more granular.

@caendesilva caendesilva marked this pull request as ready for review May 14, 2023 13:33
@caendesilva caendesilva merged commit 5e9e4ec into master May 14, 2023
@caendesilva caendesilva deleted the publications-feature branch November 8, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants