Cmb2: Merge CMB2 Attached Posts Field into core

[jtsternberg]

https://github.com/CMB2/cmb2-attached-posts
The idea here is to:

• [ ] Clean up/improve the UI. Would love contribution/help from @anhskohbo (and others, if willing/able)
  
  
  
  • Paying attention to the UI options already available https://github.com/CMB2/cmb2-attached-posts/blob/master/example-field-setup.php#L52-L53
  
  
• [ ] Add support for other object types (connected to and from) Users, Terms, Post-Types of course.
• [ ] Unit tests for field type (preferably with each supported object type)
• [ ] Make Scrutinizer happy. (Smaller methods, etc)
• [ ] Proper docblocks
• [ ] Ensure field works in repeatable groups. See https://github.com/CMB2/cmb2-attached-posts/issues/51
• [ ] Make sure field functionality is accessible!

[jtsternberg]

Copying @daggerhart who is interested in helping

[anhskohbo]

So, where's we can start the work?

[daggerhart]

I have a morning full or errands to run, and then plan to work on it this afternoon (~ 4 hours from now).

@anhskohbo, maybe wait for me to get a simple version into CMB2, then work from my fork? Alternatively, you could start working on the UI by forking cmb2-attached-post plugin, then copy your changes into my fork at a later time?

Those are just some random ideas. I'm open to anything.

[anhskohbo]

OK, I'll wait for your fork

[jtsternberg]

You bring up a good point. I will create a new branch and stub out the merge, then you can have something to build off. I should have done that yesterday. :)

[jtsternberg]

OK, so new branch is up and I quickly stubbed out what a merge would look like, though I haven't tested it _at all_: https://github.com/CMB2/CMB2/tree/associated-objects-field

Please do your PRs against that branch.

@daggerhart would LOVE some help with you with testing and getting it to work initially and any of the todos you want to tackle: https://github.com/CMB2/CMB2/blob/d632d759a655057403cf51da2070ac10ce6e753b/includes/types/CMB2_Type_Associated_Object.php#L5-L10

@anhskohbo Until it's actually working, as @daggerhart mentioned, it's probably best to work in the plugin first: https://github.com/CMB2/cmb2-attached-posts, and we can get the updates folded in.

[jtsternberg]

Ok, did a bit more work.

• [X] Started a PR (#1056) for more visible tracking of changes
• [X] Updated the css selectors to properly style the field. @anhskohbo this means you can probably start playing w/ cleaning up the UI on this branch rather than the plugin (if you haven't started)
• [X] Updated the field type. The type is associated_objects.

Along with checklist in issue description, still need to:

• [x] Fix the JS -- currently not working. Not opening the search modal

To test, you can use the example config: https://github.com/CMB2/cmb2-attached-posts/blob/master/example-field-setup.php#L20-L56
Just replace instances of custom_attached_posts with associated_objects

[jtsternberg]

Ok, got the JS enqueued and semi-working, though the search button isn't popping up the search modal.

[daggerhart]

Completed a first-pass of making the field type handle different query types.

Currently working (in simple tests):

• Posts
• Users
• Terms

I did this by creating a new set of classes that handle the query and object details work.

• abstract CMB2_Type_Associated_Objects_Query - Defines what is expected and required by other classes
• CMB2_Type_Associated_Objects_Query_Posts - Handles post objects.
• CMB2_Type_Associated_Objects_Query_Users - Handles user objects.
• CMB2_Type_Associated_Objects_Query_Terms - Handles term objects.

I'd like to do a bit more work here cleaning things up, but wanted to show progress.

Example usage:

$cmb = new_cmb2_box( array(
    'id' => 'my_metabox',
    'title' => esc_html__( 'CMB2 Contrib' ),
    'object_types'  => array( 'post' )
) );
$cmb->add_field([
    'id' => 'my_posts',
    'type' => 'associated_objects',
    'title' => 'Associated Posts',
    'options' => array(
        'query_args' => array(
            'posts_per_page' => 10,
            'post_type' => ['page','post'],
        ),
    ),
]);
$cmb->add_field([
    'id' => 'my_users',
    'type' => 'associated_objects',
    'title' => 'Associated Users',
    'options' => array(
        'query_object_type' => 'user',
        'query_args' => array(),
    ),
]);
$cmb->add_field([
    'id' => 'my_terms',
    'type' => 'associated_objects',
    'title' => 'Associated Terms',
    'options' => array(
        'query_object_type' => 'term',
        'query_args' => array(
            'taxonomy' => array( 'category', 'post_tag' ),
        ),
    ),
]);

[jtsternberg]

Woot! testing now.

[daggerhart]

Sorry, I missed one fairly important line of code, and now I'm doing that thing where i struggle with git trying to making 1 clean commit for 20 minutes.

[daggerhart]

Back on track, should be a single commit now.

[jtsternberg]

No worries about the single commit. I'm not super worried about having a perfect git history. If you don't believe me, just trawl through the history a bit. 😂

This is awesome, let me know if you'll get a chance to address some of the things in the review. If you won't be able to, that's fine too.

[daggerhart]

Not sure if I should have replied to each item as I worked through them, didn't want to accidentally blow up your email.

I've pushed an update to the PR (rebaseing again as 1 commit) with the following changes. Heck, I took the time to re-figure out how to rebase, so i ran with it :D

• Fixed the various coding standards issues.
• Renamed the classes
• Removed local $field property
• Made the $query a property on the field type object. No more passing it around, or calling for it 3 times.

For the Object Post Labels, I think it makes more sense for the query handler objects to return then as raw data and let the render() method format them how it wants. As of the current PR the only two methods in all these classes that deal with HTML are render() and get_list_item().

• Removed output buffer according to @TODO item
• Refactored the get_retrieved() and get_attached() methods to return rendered list items instead of echoing the rendered list items.

I'm sure there will be more to do once the search ajax gets going, but this should be a pretty good head start. I can see easily allowing for Comments, Blogs, Links (is that still a thing?) and and other object types by just expanding on the query handler classes.

[jtsternberg]

Awesome, thank you! Will review later. And no worries about blowing up email.

[jtsternberg]

I have begun making headway on updating the Ajax response for other object types (starting with users): https://github.com/CMB2/CMB2/blob/associated-objects-field/includes/CMB2_Associated_Objects_Search.php

[jtsternberg]

@anhskohbo Curious if you've had any time to look at this?

[anhskohbo]

@jtsternberg I busy in last few days. This will be done in next few days.

[daggerhart]

Just a heads up while I'm thinking about it. Last place I stopped I found a bug with multiple associated objects fields on the same form.

When adding a new item to a field, it always gets added to the last field on the form and not necessarily the field you're working with.

I think the issue is around here, but i'm not positive.
https://github.com/CMB2/CMB2/blob/associated-objects-field/js/cmb2-associated-objects.js#L202

Things are starting to get holidays-crazy around here, but I'll try to jump back into the issue within the next week or so and do push some more things forward.

[anhskohbo]

@jtsternberg Could you check this pull request can be merge into the trunk https://github.com/CMB2/CMB2/pull/1007?

I almost done with that (need to improve the group field and rollback some class was deleted).

So I can focus that PR first, then with this.

[jtsternberg]

Looking to get this going again. @daggerhart were you able to look further into the issue you were talking about here: https://github.com/CMB2/CMB2/issues/1055#issuecomment-349745840?

[daggerhart]

I tested the issue from my comment above, and I couldn't replicate the problem any more.

It may have been resolved with some update to CMB2_Type_Associated_Objects.php, but I didn't spend too much time trying to figure out the exact moment it was fixed. Instead, I spent some time debugging cmb2-associated-objects.js.

Everything I saw looked good. As long as the field wrapper has the associated-objects-wrap css class, and none of the wrapper's parents have the same class, then the problem shouldn't show up anymore.

[slaFFik]

Note: attached_posts library has an issue (generated warnings/notices in error_log) when "Search" ajax request is sent (popup is opened), and when the field is inside the group.
This should be fixed inside \WDS_CMB2_Attached_Posts_Field::maybe_callback in the lib (haven't checked the branch and its code, will do that this week).

[RubenMartins]

Move this to 2.8.0 Milestone?