-
Notifications
You must be signed in to change notification settings - Fork 16
Conversation
Conflicts: php/class-post-type.php
@PatelUtkarsh Please fix merge conflicts so I can review. Thanks! |
Todo:
|
php/class-post-type.php
Outdated
$this->split_snapshot_id = wp_insert_post( $new_post_arr ); | ||
$this->split_processing_post_id = $post->ID; | ||
$this->restore_kses(); | ||
$content = Customize_Snapshot_Manager::encode_json( $data ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be wrapped in wp_slash()
php/class-post-type.php
Outdated
if ( $should_allow_split ) { | ||
$split_text = __( 'Split', 'customize-snapshots' ); | ||
echo sprintf( '<button id="split-activate" class="button button-secondary" data-cancel-text="%s" data-original-text="%s">%s</button>', | ||
esc_html__( 'Cancel Split', 'customize-snapshots' ), esc_attr( $split_text ), esc_html( $split_text ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's reformat the sprintf()
here so that each of its args are on a separate line.
php/class-post-type.php
Outdated
$this->split_processing_post_id = $post->ID; | ||
$this->restore_kses(); | ||
$content = Customize_Snapshot_Manager::encode_json( $data ); | ||
add_action( 'post_updated', array( $this, 'redirect_split_post' ), 100 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should happen at shutdown instead?
php/class-post-type.php
Outdated
$this->suspend_kses(); | ||
// Remove key so it doesn't save recursively. | ||
unset( $_REQUEST[ $split_key ], $_POST[ $split_key ] ); | ||
$this->split_snapshot_id = wp_insert_post( $new_post_arr ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$new_post_arr
here needs to be wrapped in wp_slash()
); | ||
$theme = get_post_meta( $post->ID, '_snapshot_theme', true ); | ||
if ( ! empty( $theme ) ) { | ||
$new_post_arr['meta_input']['_snapshot_theme'] = $theme; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we include meta about the source changeset that it came from? Wouldn't this be like a fork?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add it, But where would we use it? If we are showing source from where changeset was split from that would work. Should i add that? @westonruter
Fixes #16