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

API docs #86

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Draft

API docs #86

wants to merge 1 commit into from

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Oct 25, 2024

Rendered

WIP

@psrpinto psrpinto marked this pull request as draft October 25, 2024 14:34

```
{
type: 'header' | 'footer' | 'blog-post' | ...;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure of how things like header/footer/nav type would have a sourceUrl to begin with, since they may or may not change with different pages. Mostly won't change (should be our first consideration), but can change (should be our second consideration).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, type gets used in the endpoint url, so the minimal request body just looks like:

{
	sourceUrl: string;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just acknowledge that some subject types won't have a source url.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, the fact that sourceUrl does not apply to header/footer/navigation is a sign that we should think of those in a different way than we do other subjects.

In reality, they are different, because there's only one (or a few) header/footer in a site, they are "global", or in other words part of the "layout" (aka theme) of the site, they aren't data in the same sense blog posts or products are.

My suggestion would be, as mentioned in another comment, to not tackle header/footer for now, and make it so that all subjects have a source url, because all "data subjects" need a source url.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, but then how do we handle navmenu?

API.md Show resolved Hide resolved
Comment on lines +6 to +25
### Header

```
{
type: 'header';
id: number;
sourceUrl: string; // URL of the original page from which the header was created.
// More fields TBD.
}
```
### Footer

```
{
type: 'footer';
id: number;
sourceUrl: string; // URL of the original page from which the footer was created.
// More fields TBD.
}
```
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about Header & Footer, what really do we want to liberate there? At most menu? Then just treating menu as one of the navigation menus as we originally were targeting takes away the complexity of header and footer.

I like the idea of liberating as much as we can, even though we don't know where some of it would go, but can be decided later on.

So applying KISS principle, I think we just liberate as many nav menus we find on the source site, save them as navigation menus, even if we just use 2 of them in playground preview - one in header and one in footer. The user can always operated on WP admin to adjust if desired.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about header, footer and navigation in general, the less things are clear for me :)

If we consider that it's acceptable for the user to modify the header/footer directly through WordPress, my suggestion would be to double-down on the KISS principle you mention, and don't deal with header/footer at all for now. The user will need to go into WordPress to author header/footer, for now.

Instead, we could first tackle Pages, because very likely the navigation will reference Pages, so having those done before tackling navigation would make sense to me.

We can also consider to never handle navigation. If we consider it to be acceptable for the user to author navigation directly in WordPress, having try-wordpress import navigation will be of limited usefulness.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree for the most part, but I think we should handle navigation at the bare minimum. Considering it's a universal thing, every source site is going to have it, our WP theme in PG preview would have it, so seems like a low-hanging fruit to make the end result look good.

The only thing that's icky about is, it doesn't fit our current mental model of subjects, but considering every subject has its type, we can easily handle every subject type differently (this being the exception as its not pure data, but rather meta data about the site).

As far as handling pages go, I think handling simple text pages is quite simple with what we have already built (exactly as posts), but think of other kind of pages such as landing pages with different kind of sections making up the page. Here, I think we would be building some kind of parser to recognize stack of sections that make up the page and transform them to blocks and/or patterns to really liberate them, otherwise all context is gone.


```
{
type: 'header' | 'footer' | 'blog-post' | ...;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just acknowledge that some subject types won't have a source url.


```
{
id: number;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The id of any resource should just be part of the url itself, so it would be a GET request to /whatever/blogpost/{id}.


```
{
sourceUrl: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we are creating a resource, I don't think we should use POST at all. Following REST CRUD conventions, this would just be a parameter to the list endpoint.

So /whatever/blogpost?sourceurl=xxxxx

@ashfame
Copy link
Member

ashfame commented Nov 5, 2024

@psrpinto Would also be great to define a list of subject types that we would be handling in our MVP build. That is likely to uncover more stuff to consider.

So far, the list is:

  • blogpost
  • page (this would be the most complicated IMO)
  • navmenu
  • blogpost comment
  • product

There is also small pieces of content like stuff in sidebar, header and footer, which I think we should ignore for now, since they are High-effort Low-reward items.

sourceUrl: string; // URL of the source page from which this blog post was created.
date: {
type: 'date';
original: string; // original HTML
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should rename original to raw. Original indicates a valid previous value, but Raw holds the essence correctly i.e. its raw and requires processing.

API.md Show resolved Hide resolved
Comment on lines +39 to +48
title: {
type: 'text';
original: string; // original HTML
parsed: string; // plain text
},
content: {
type: 'html';
original: string; // original HTML
parsed: string; // block markup
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's best to keep 2 representation objects to keep things simple. This one being the representation frontend works with and a flatten object representation for api interaction. We would continue to have a function that converts objects in both directions.

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.

2 participants