Skip to content

Adds final touches to custom fields across the network#99

Merged
matt-bernhardt merged 1 commit into
masterfrom
lm147
May 3, 2023
Merged

Adds final touches to custom fields across the network#99
matt-bernhardt merged 1 commit into
masterfrom
lm147

Conversation

@matt-bernhardt

@matt-bernhardt matt-bernhardt commented May 1, 2023

Copy link
Copy Markdown
Member

Why are these changes being introduced:

The AKDC site on our network has a set of custom fields added to its Post content type, which allow this content type to function as an event.

Relevant ticket(s):

https://mitlibraries.atlassian.net/browse/lm-147

How does this address that need:

This defines a site-specific extension for the Mitlib Post plugin family, which will be enabled only on the AKDC site. This is the same approach which we've previously adopted for the Parent and News sites when those sites needed to extend the built-in post types.

Document any side effects to this change:

This doubles down on using the Post content type for this content, when we might really need to define an Events post type - but that will need to be a coordinated change after we launch the rebuilt site.

Recommended verification for this PR

  • Log into the AKDC site, in the Custom Fields area of the admin dashboard, at /akdc/wp-admin/edit.php?post_type=acf-field-group
  • Note that the only set of custom fields listed is shown under a "Sync available" tab
  • Load the "Add New Post" form at /akdc/wp-admin/post-new.php and note that there is a "News and Events" field group visible
  • You can also see the these fields used to render existing content by editing an existing post that has event information, like Post 2194: /akdc/wp-admin/post.php?post=2194&action=edit

Developer

Secrets

  • No secrets are affected

Documentation

  • No documentation changes are needed

Accessibility

  • ANDI or Wave has been run in accordance to
    our guide and
    all issues introduced by these changes have been resolved or opened as new
    issues (link to those issues in the Pull Request details above)

Stakeholder approval

  • Stakeholder approval is not needed

Dependencies

YES dependencies are updated

Code Reviewer

  • The commit message is clear and follows our guidelines
    (not just this pull request message)
  • The changes have been verified
  • The documentation has been updated or is unnecessary
  • New dependencies are appropriate or there were no changes

@matt-bernhardt matt-bernhardt changed the title Adds final touchets to custom fields across the network Adds final touches to custom fields across the network May 2, 2023
** Why are these changes being introduced:

* The AKDC site on our network has a some custom fields added to its
  Post content type, which allow this content type to function as an
  event.

** Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/lm-147

** How does this address that need:

* This defines a site-specific extension for the Mitlib Post plugin
  family, which will be enabled only on the AKDC site. This is the same
  approach which we've previously adopted for the Parent and News sites
  when those sites needed to extend the built-in post types.

** Document any side effects to this change:

* This doubles down on using the Post content type when we might
  really need to use the Events post type - but I'm not going to fight
  that battle right now.

@JPrevost JPrevost left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks to do what it intends to do, and I agree we can revisit whether extending Posts or creating new types is better later.

Question regarding migration: did the existing test migrations for akdc bring in event data properly and this just allows us to add/edit/display that data or do we need to confirm that migrations will pull this data in now that we have support for it?

@matt-bernhardt

Copy link
Copy Markdown
Member Author

One of the quirks here is that the custom fields and field groups are also included in the content export - so the fact that I missed these fields in code initially didn't mean that these fields were left out of the first import. They were just entirely in the content side of things.

With this PR, we have the option to remove those items from the content XML (although this isn't necessary). I intend to do this for the final import, FWIW.

@JPrevost

JPrevost commented May 3, 2023

Copy link
Copy Markdown
Member

@matt-bernhardt lets chat about what the effect of your last comment means in slack or zoom to make sure I understand it. I'm not blocking merging as this is working as intended, but I'd like to ensure I understand what risk we are assuming in the akdc site migrating differently than in the final migration than our last test migration if nothing else. Thanks!

@matt-bernhardt matt-bernhardt merged commit 6f69792 into master May 3, 2023
@matt-bernhardt matt-bernhardt deleted the lm147 branch May 3, 2023 19:44
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