MinnPost / object-sync-for-salesforce

WordPress plugin that maps and syncs data between Salesforce objects and WordPress objects.
https://wordpress.org/plugins/object-sync-for-salesforce/
GNU General Public License v2.0
93 stars 49 forks source link

Better log entries when modifying WordPress data #459

Closed jonathanstegall closed 2 years ago

jonathanstegall commented 2 years ago

Is your feature request related to a problem? Please describe. In the Object_Sync_Sf_WordPress class there's really inconsistent logging for errors that might happen with saving data in WordPress. A logging method is called on these methods:

This is both incomplete and inconsistent. The create and delete operations don't have any error log creation whatsoever. It also doesn't make sense that upsert is the only one with its own log method for each type of object.

Describe the solution you'd like Make log entries happen at the right moments so people can depend on logs when errors occur that are detectable.

jonathanstegall commented 2 years ago

This is the generic update log entry code:

if ( isset( $result['errors'] ) && ! empty( $result['errors'] ) ) {
    $status = 'error';
    if ( ! isset( $mapping_object['salesforce_id'] ) ) {
        $title = sprintf(
            // translators: 1) is log status, 2) is WordPress object type, 3) is WordPress id value.
            esc_html__( '%1$s: WordPress update for %2$s ID %3$s was unsuccessful with these errors:', 'object-sync-for-salesforce' ),
            ucfirst( esc_attr( $status ) ),
            esc_attr( $name ),
            esc_attr( $id )
        );
    } else {
        $title = sprintf(
            // translators: 1) is log status, 2) is WordPress object type, 3) is WordPress id value, 4) is Salesforce ID value.
            esc_html__( '%1$s: WordPress update for %2$s ID %3$s from Salesforce record %4$s was unsuccessful with these errors:', 'object-sync-for-salesforce' ),
            ucfirst( esc_attr( $status ) ),
            esc_attr( $name ),
            esc_attr( $id ),
            esc_attr( $mapping_object['salesforce_id'] )
        );
    }

    if ( isset( $this->logging ) ) {
        $logging = $this->logging;
    } elseif ( class_exists( 'Object_Sync_Sf_Logging' ) ) {
        $logging = new Object_Sync_Sf_Logging( $this->wpdb, $this->version );
    }

    $body  = '';
    $body .= '<h2>' . esc_html__( 'Errors from WordPress result:', 'object-sync-for-salesforce' ) . '</h2>';
    $body .= esc_html( print_r( $result['errors'], true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_print_r
    if ( is_array( $params ) && ! empty( $params ) ) {
        $body .= '<h2>' . esc_html__( 'Parameters sent to WordPress:', 'object-sync-for-salesforce' ) . '</h2>';
        $body .= esc_html( print_r( $params, true ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_print_r
    }

    $error_log = array(
        'title'   => $title,
        'message' => $body,
        'trigger' => 0,
        'parent'  => '',
        'status'  => $status,
    );
    $logging->setup( $error_log );
}

This is an example of the object-specific upsert code, which is identical on each object type and only runs if there is a missing ID:

// Create log entry for lack of a user id.
if ( isset( $this->logging ) ) {
    $logging = $this->logging;
} elseif ( class_exists( 'Object_Sync_Sf_Logging' ) ) {
    $logging = new Object_Sync_Sf_Logging( $this->wpdb, $this->version );
}

$status = 'error';
$title  = sprintf(
    // translators: placeholders are: 1) the log status.
    esc_html__( '%1$s: Users: Tried to run user_upsert, and ended up without a user id', 'object-sync-for-salesforce' ),
    ucfirst( esc_attr( $status ) )
);

$logging->setup(
    // todo: can we get any more specific about this?
    $title,
    '',
    0,
    0,
    $status
);
jonathanstegall commented 2 years ago

Comment also has this log:

$status = 'error';
// Create log entry for multiple matches.
if ( isset( $this->logging ) ) {
    $logging = $this->logging;
} elseif ( class_exists( 'Object_Sync_Sf_Logging' ) ) {
    $logging = new Object_Sync_Sf_Logging( $this->wpdb, $this->version );
}
$logging->setup(
    sprintf(
        // translators: %1$s is the log status. %2$s is a number. %3$s is a key. %4$s is the value of that key. %5$s is a var_export'd array of comments.
        esc_html__( '%1$s: Comments: there are %2$s comment matches for the Salesforce key %3$s with the value of %4$s. Here they are: %5$s', 'object-sync-for-salesforce' ),
        ucfirst( esc_attr( $status ) ),
        absint( count( $comments ) ),
        esc_html( $key ),
        esc_html( $value ),
        esc_html( var_export( $comments ) ) // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export
    ),
    '',
    0,
    0,
    $status
);
jonathanstegall commented 2 years ago

Remembering that the log distinctiveness here is because the more standard log events happen in salesforce pull operations.

jonathanstegall commented 2 years ago

What makes this really complicated is the combination of log entries on pull operations with the log entries on wordpress operations. They aren't the same because an error could happen during the process of pulling data from Salesforce and/or on the process of saving it in wordpress.

jonathanstegall commented 2 years ago

I think between #471, #470, and #465 I've done what I'd like to do with this for now.