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 plugin for the custom post type for Projects #131

Merged
merged 7 commits into from
May 20, 2020
Merged

Conversation

benlk
Copy link
Collaborator

@benlk benlk commented May 18, 2020

Changes

This pull request adds a plugin with the following registrations

  • Adds custom post type projects
  • Adds "Project Status", "Project Organization Type" and "Project Category" custom taxonomies
  • Registers "Contact name", "Contact email", "Organization" and "Video" post meta fields, but doesn't yet finish implementing the metaboxes for them

Screen Shot 2020-05-18 at 18 26 48

Why

For #119.

Built as a plugin to enhance future portability of this database, and to keep this concern separate from the child theme if they ever decide to swap away from their current theme.

Testing/Questions

Features that this PR affects:

  • adds a new plugin

Questions that need to be answered before merging:

  • Is this PR targeting the correct branch in this repository?
  • These taxonomies are designated "hierarchical" in order to get a list of terms in the Gutenberg editor, rather than a new-tag tag box.
  • Do we want to try to finish the custom meta box for this, or is the standard "Custom Fields" editor enough for admin users? The Custom Fields editor doesn't load all registered custom fields, and so does not provide an easy way for editors to know or set these fields if they are not already set on the post:
    Screen Shot 2020-05-18 at 18 26 59

Steps to test this PR:

  1. Enable the "Current.org Local That Works Projects" plugin.

@benlk benlk added this to the INOV-001 milestone May 18, 2020
@benlk benlk requested a review from joshdarby May 18, 2020 22:34
@benlk benlk self-assigned this May 18, 2020
@benlk
Copy link
Collaborator Author

benlk commented May 18, 2020

As I'm working through the import process testing, I realized that this is missing one post meta field, now added to #119 (comment): The project link.

@benlk
Copy link
Collaborator Author

benlk commented May 18, 2020

Here's an updated screenshot from a test import, showing what the Custom Fields editor looks like when populated from an import or from the submission form:

Screen Shot 2020-05-18 at 18 59 17

@joshdarby
Copy link

joshdarby commented May 19, 2020

@benlk Since this is a pretty large project, maybe we should merge this (and every other PR for the project) into staging instead of master so that we're able to still merge and deploy master if something comes up.

@joshdarby
Copy link

joshdarby commented May 19, 2020

Do we want to try to finish the custom meta box for this, or is the standard "Custom Fields" editor enough for admin users? The Custom Fields editor doesn't load all registered custom fields, and so does not provide an easy way for editors to know or set these fields if they are not already set on the post:

I would prefer if we finish creating the custom metaboxes. Leaving the custom fields editor open for anyone to use will probably cause us lots of headaches in the future.

For example, I can see someone going to add in a value for an accepted field that's not yet included in the current project, for example project-contact-email, but they misspell it as project-contact-emial and then it doesn't display on the frontend and we have to spend time troubleshooting why.

@MirandaEcho any thoughts?

Copy link

@joshdarby joshdarby left a comment

Choose a reason for hiding this comment

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

  • form submission to project post type works well
  • no weird errors
  • taxonomies function as expected
  • REST endpoint for projects works
  • REST endpoints for taxonomies works

My only concern is the custom metabox question, and I also don't have a good answer (yet) about whether or not we want to use auth_callback.

@benlk benlk changed the base branch from master to staging May 19, 2020 16:36
@benlk
Copy link
Collaborator Author

benlk commented May 19, 2020

maybe we should merge this (and every other PR for the project) into staging instead of master so that we're able to still merge and deploy master if something comes up.

I've updated staging to the latest master and swapped this PR to be to staging.

@benlk
Copy link
Collaborator Author

benlk commented May 20, 2020

Issue created to capture the outstanding metabox work: #133

@benlk benlk merged commit ac2e999 into staging May 20, 2020
@benlk benlk deleted the 119-post-type branch May 20, 2020 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants