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 menus #91

Merged
merged 19 commits into from
Nov 8, 2021
Merged

Add menus #91

merged 19 commits into from
Nov 8, 2021

Conversation

xjensen
Copy link
Contributor

@xjensen xjensen commented Oct 29, 2021

Per cagov/drought.ca.gov#126, this PR adds support for navigation menus to wordpress-to-github.

Before, we called the Wordpress menu API directly from the 11ty build. The problem with this approach is that changes to the menus would not trigger an 11ty build. We'd need to wait for something else to trigger another 11ty build before the menus would update.

This change to wordpress-to-github will now fetch menus and deposit them into the 11ty site repo as static files. Hence, builds will now get triggered upon menu changes, since we're dropping new/updated files into the repo.

Among everything else, one specific thing to confirm is the caching strategy for menus here. I wasn't sure how to test that locally.

Note: sample output is available in the automation-development-target repo.

@xjensen xjensen requested a review from carterm October 29, 2021 21:29
@xjensen
Copy link
Contributor Author

xjensen commented Oct 29, 2021

Also, I need to update the docs. Will do that now.

@xjensen xjensen requested a review from a team October 29, 2021 21:47
README.md Outdated Show resolved Hide resolved
@carterm
Copy link
Contributor

carterm commented Oct 30, 2021

Since the menus are a plug-in and a non standard Wordpress feature, I'm not sure if the naming should be in the schema. What if they use another menu plugin?

@carterm
Copy link
Contributor

carterm commented Oct 30, 2021

What happens if we use another menu plug-in? Would like to have a more declarative way to connect to custom endpoints.

@carterm
Copy link
Contributor

carterm commented Oct 30, 2021

Reminds me that we need to document that the module works with out-of-the-box WordPress and makes no assumptions about plug-in usage.

@xjensen
Copy link
Contributor Author

xjensen commented Oct 30, 2021

Since all we need to do here is hit an explicit menu API route, without any additional processing, we could just create a generic API downloader. This wouldn't be tied to any specific Wordpress plugin. It could be configured for each site in a plugin-agnostic way. Example:

"ApiDownloads": [
  {
    "Path": "wordpress/menus/header-menu.json",
    "Source": "/wp-json/menus/v1/menus/header-menu"
  }
]

Menu problem solved, schema stays clean from plugins.

If we did need to process the API calls before writing out to the repo, then perhaps we could create a plugin interface for wordpress-to-github itself. But that might be too much for now.

@xjensen xjensen marked this pull request as draft October 30, 2021 01:08
@chachasikes chachasikes self-requested a review November 2, 2021 00:51
@@ -9,6 +9,13 @@
"PostPath": "wordpress/posts",
"PagePath": "wordpress/pages",
"MediaPath": "wordpress/media",
"ApiRequests": [
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this.

@chachasikes
Copy link
Contributor

chachasikes commented Nov 2, 2021

Another option (to add to our general thinking) for handling third-party plugins:
An example from drought.ca.gov API

  • A consistent field name (in this case, it doesn't come from another endpoint)
  • The data. This data is formatted to a draft og_meta data spec that I designed for rendering og content for the .
  • A mini "meta" object that includes the plugin machine name, plugin label, platform, plugin version, editor_url
  • The version is included because the field structure may change. We could include a JSON Schema for this as well.

I share this because we will need to adapt the menu to other data sources. WordPress has its own specific data structure.

We don't need to find all the solutions right now (until we have real situation where we can use or prototype with another CMS), but we can think about CMS agnosticism, plugin changes, upgrades, platform updates & how a system that doesn't have a menu feature could use fields to recreate the menu?

For example, if using Contentful, there is not going to be a menu built in, and if there was a third-party service, it's guaranteed that the API structure won't be the same, but we still want the design system component to render the same output.

Questions for thought
How could we re-create the menu API structure from another system?
What would the 11ty renderer need to know?
How do we scope code by CMS?

{
    og_meta: {
        editor: {
            platform: "WordPress",
            plugin: "autodescription",
            plugin_name: "The SEO Framework",
            plugin_version: "4.1.5.1",
            editor_url: "https://live-drought-ca-gov.pantheonsite.io"
        },
        site_name: "California drought action",
        site_description: "Learn more about current conditions, the state’s response and informational resources available to the public.",
        site_url: "https://live-drought-ca-gov.pantheonsite.io",
        canonical_url: "https://live-drought-ca-gov.pantheonsite.io/dwr-offers-grants-for-urban-and-multibenefit-drought-relief/",
        page_title: "DWR Offers Grants for Urban and Multibenefit Drought Relief",
        page_description: "Learn more about current conditions, the state’s response and informational resources available to the public.",
        page_social_image_url: "https://live-drought-ca-gov.pantheonsite.io/wp-content/uploads/2021/07/cropped-2021_06_23_FL_0494_Folsom_Lake.jpg",
        page_social_image_width: 1200,
        page_social_image_height: 630,
        page_social_image_alt: "",
        meta_title: "DWR Offers Grants for Urban and Multibenefit Drought Relief",
        meta_description: "Learn more about current conditions, the state’s response and informational resources available to the public.",
        meta_canonical_url: "https://live-drought-ca-gov.pantheonsite.io/dwr-offers-grants-for-urban-and-multibenefit-drought-relief/",
        open_graph_title: "DWR Offers Grants for Urban and Multibenefit Drought Relief",
        open_graph_description: "Learn more about current conditions, the state’s response and informational resources available to the public.",
        twitter_title: "DWR Offers Grants for Urban and Multibenefit Drought Relief",
        twitter_description: "Learn more about current conditions, the state’s response and informational resources available to the public."
    }
}

EDIT: looking at this again, the API connector would benefit from its own WP plugin & its own plugin - in this case, there are two packages involve - a WordPress UI (third party, no API) & a cagov WP plugin that extends an monolith UI with an API. Can definitely see this pattern happening more often with other features, if UI's are mature but API integrations are not.

@xjensen
Copy link
Contributor Author

xjensen commented Nov 2, 2021

These are good questions. I was very tempted to go down this road before retreating to the much simpler (for now) ApiRequests approach.

So, ideation time. We could deploy a plugin system. Let's call it cms-to-github. We would have separate plugins for the menu, Contentful, etc. Even wordpress-to-github could become a plugin of this system. The cms-to-github program would execute the code within each plugin before dumping all the output to GitHub. This might behave similarly to Rollup. Sounds good to me. But as you point out, I think it would be difficult to effectively architect this system until we have more examples/scenarios where it'd be useful.

@xjensen xjensen marked this pull request as ready for review November 2, 2021 18:44
@xjensen
Copy link
Contributor Author

xjensen commented Nov 2, 2021

This is ready for another review. Here's the revised interface for wordpress-to-github.config.json files.

  "ApiRequests": [
    {
      "Destination": "wordpress/menus/header-menu.json",
      "Source": "/wp-json/menus/v1/menus/header-menu",
      "ExcludeProperties": ["description"]
    }
  ]

Some notes follow.

  • I've revised the debugger so we can run it multiple times, based on the args property in the .vscode/launch.json file. This provides a convenient way to test caching.
  • The ExcludeProperties key may not be so useful in this scenario, because it can only remove properties from the very top-most level of the API request object/json. Bad-cache-busting properties could be nested deeply in sub-objects, arrays, etc. The current implementation wouldn't remove those nested properties. I don't think it's going to be a problem for the immediate use case of menus. But something to note for future development.
  • I did not add the ability to fetch other external URLs. This only supports calls to the WordPress API at the moment. External URLs felt a bit out-of-scope, both for the immediate need and perhaps for this project in general. Open to discussion, of course.

Questions.

  • Is this package versioned yet? Should I bump the version number?
  • Should we create a changelog or other form of release notes?

@carterm
Copy link
Contributor

carterm commented Nov 5, 2021

This is good iteration as-is in my opinion. Would solve the immediate need for adding api endpoints. I like the cache testing debug option.

@carterm
Copy link
Contributor

carterm commented Nov 5, 2021

This is ready for another review. Here's the revised interface for wordpress-to-github.config.json files.

  "ApiRequests": [
    {
      "Destination": "wordpress/menus/header-menu.json",
      "Source": "/wp-json/menus/v1/menus/header-menu",
      "ExcludeProperties": ["description"]
    }
  ]

Some notes follow.

  • I've revised the debugger so we can run it multiple times, based on the args property in the .vscode/launch.json file. This provides a convenient way to test caching.
  • The ExcludeProperties key may not be so useful in this scenario, because it can only remove properties from the very top-most level of the API request object/json. Bad-cache-busting properties could be nested deeply in sub-objects, arrays, etc. The current implementation wouldn't remove those nested properties. I don't think it's going to be a problem for the immediate use case of menus. But something to note for future development.
  • I did not add the ability to fetch other external URLs. This only supports calls to the WordPress API at the moment. External URLs felt a bit out-of-scope, both for the immediate need and perhaps for this project in general. Open to discussion, of course.

Questions.

  • Is this package versioned yet? Should I bump the version number?
  • Should we create a changelog or other form of release notes?

ExcludeProperties Would benefit by having some sort of expression support.

@carterm
Copy link
Contributor

carterm commented Nov 5, 2021

Issue to add change-log - #105

@xjensen
Copy link
Contributor Author

xjensen commented Nov 5, 2021

ExcludeProperties Would benefit by having some sort of expression support.

Something like JSONata might work well here.

@carterm
Copy link
Contributor

carterm commented Nov 5, 2021

(I updated the readme files to match the new split)

@carterm carterm self-requested a review November 5, 2021 21:32
@carterm carterm merged commit 3724aa7 into main Nov 8, 2021
@carterm carterm deleted the add-menus branch November 8, 2021 18:16
@carterm carterm linked an issue Nov 8, 2021 that may be closed by this pull request
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.

Option to add custom endpoints
4 participants