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

Introduce Posts and Post Carousel block deprecations #1638

Merged
merged 3 commits into from
Sep 3, 2020

Conversation

AnthonyLedesma
Copy link
Member

@AnthonyLedesma AnthonyLedesma commented Aug 21, 2020

Description

Closes #1632
Following the lead of the Gutenberg team, this PR introduces measures that will allow proper migration/deprecation from previously configured and published blocks into a working state using modern interface and features.

In this case, the core/latest-posts block had a number of breaking changes related to the implementation of a tokenized category selector. The new selector requires new logic and a new attribute schema.

This change is inspired by this PR in Gutenberg. This PR resolves a bug with migration from old-style coblocks/posts and coblocks/post-carousel attributes.

This can be replicated using old markup:

<!-- wp:coblocks/post-carousel {“columns”:3,“categories”:“1”} /-->
OR
<!-- wp:coblocks/post-carousel {"categories":"4"} /-->

<!-- wp:coblocks/posts {“className”:“is-style-stacked”,“categories”:“3”} /-->

Please note This fix will successfully migrate the chosen category from a previously configured block into a category token within the new interface. While the token exists and works as expected as a filter, the token is missing contextual language. This is a known issue with migration.
image

Types of changes

JavaScript changes and PHP changes.

How has this been tested?

Tested manually migrating from 5.4.2 -> 5.5
Tested with and without the Gutenberg plugin.
Tested migration from CoBlocks 1.23.0 -> 2.2.2

Checklist:

  • My code is tested
  • My code follows accessibility standards
  • My code has proper inline documentation
  • I've included any necessary tests
  • I've included developer documentation
  • I've added proper labels to this pull request

@AnthonyLedesma AnthonyLedesma added [Type] Bug Something that is not working as expected [Priority] High This issue/pull request needs resolving before the next release [Status] Needs Review Tracking pull requests that need another set of eyes labels Aug 21, 2020
@AnthonyLedesma AnthonyLedesma self-assigned this Aug 21, 2020
@jrtashjian jrtashjian removed the [Status] Needs Review Tracking pull requests that need another set of eyes label Aug 21, 2020
@cypress
Copy link

cypress bot commented Aug 21, 2020



Test summary

14 0 0 0


Run details

Project CoBlocks
Status Passed
Commit 67f84b3
Started Aug 21, 2020 3:49 PM
Ended Aug 21, 2020 3:50 PM
Duration 01:42 💡
OS Linux Debian - 10.4
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@jrtashjian jrtashjian added the [Status] Needs Review Tracking pull requests that need another set of eyes label Aug 21, 2020
Copy link
Member

@jrtashjian jrtashjian left a comment

Choose a reason for hiding this comment

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

This is great! On the issue with the blank token in the field, we can actually fix this during the attribute migration.

The functionality I'm seeing in the block using it new is the REST API response data is saved for each category in the attribute. Personally this seems way too verbose but maybe there is a reason. Regardless, we can perform an API request to get this additional data for the category ID's we find. Here's an example I've used in my own plugins:

import apiFetch from '@wordpress/api-fetch';
import { addQueryArgs } from '@wordpress/url';
import { decodeEntities } from '@wordpress/html-entities';

apiFetch( {
        path: addQueryArgs(
            '/wp/v2/categories',
            {
                per_page: 100, // should be set to the number of categories we are querying.
                _fields: 'id,name',
                include: categoryIDs.join( ',' ),
            }
        )
    } )
    .then(
        ( entities ) => entities.map(
            ( entity ) => (
                {
                    id: entity.id,
                    name: decodeEntities( entity.name ),
                }
            )
        )
    )

So this PR would migrate to this:

<!-- wp:coblocks/post-carousel {"columns":3,"categories":[{"id":3}]} /-->

but we need at least this:

<!-- wp:coblocks/post-carousel {"columns":3,"categories":[{"id":3,"name": "Category 2"}]} /-->

src/blocks/post-carousel/index.php Show resolved Hide resolved
src/blocks/posts/deprecated.js Show resolved Hide resolved
src/blocks/posts/index.php Show resolved Hide resolved
@jrtashjian jrtashjian removed the [Status] Needs Review Tracking pull requests that need another set of eyes label Aug 28, 2020
@jrtashjian jrtashjian added this to the Next Release milestone Aug 28, 2020
@jrtashjian jrtashjian merged commit f4350bf into master Sep 3, 2020
@jrtashjian jrtashjian deleted the fix/post-blocks branch September 3, 2020 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Priority] High This issue/pull request needs resolving before the next release [Type] Bug Something that is not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Post blog category filter not working
2 participants