rubengc / ct Goto Github PK

When you setup schema for a datetime field, you should be able to use CURRENT_TIMESTAMP as a value.

https://dev.mysql.com/doc/refman/5.6/en/data-type-defaults.html

Currently, this declaration gets wrapped in quote marks, and is deemed invalid by MySQL.

Obviously since we define it in a PHP array, it'll need to be initially parsed in quote marks to be valid in php, so this string will need to be caught when defining the schema and not considered a string and wrapped in quote marks when being formatted for SQL.

In ct_register_table(), entering the schema in as a string instead of an array does not correctly define the PRIMARY KEY.

For example, if I define the PRIMARY KEY as PRIMARY KEY (ID), it strips the value 'ID' and leaves it blank as PRIMARY KEY (), resulting in a SQL syntax error.

Eg.

"schema" => " ID BIGINT(20) NOT NULL AUTO_INCREMENT, post_id BIGINT(20) NOT NULL , user_id BIGINT(20) NOT NULL , role VARCHAR(60) NOT NULL DEFAULT 'member' , date_added DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP , PRIMARY KEY (ID)"

This will result in:

ID BIGINT(20) NOT NULL AUTO_INCREMENT, post_id BIGINT(20) NOT NULL , user_id BIGINT(20) NOT NULL , role VARCHAR(60) NOT NULL DEFAULT 'member' , date_added DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP , PRIMARY KEY ()

Hello,
I want to use this package in WP-Trait
How to Inlcude with PHP Composer?
Please Help

I wanted to test work of a plugin, require ct.php, example.php.
But it can't working with custom database table. I have created table myself and have filled one row, nothing changed. There is no items at the list page. Add new page is empty.

I followed all steps correctly but it was not working. List page was not showing anything.
After a lot of digging , was able to make the list page working by following this commit from another fork.
3d5b32a?diff=split
Now the Add new button does not work , it just redirects back to the same list page.
Also the edit page does not work. It shows blank page with just the box with save button.

https://prnt.sc/k56oQbsyLTJX

Any help appreciated

Hey @rubengc!

I believe I've found a bug, the source of it, and a quick fix.

Indexes are not being created with passing 'key' => true in as an option.

The code here is not actually doing anything, the $keys[] array is not being used.

As a first pass, simply moving it up to the __toString() function will fix the issue. Could be extracted into a function. Hope this helps.

public function __toString()
        {

            $fields_def = array();
            $keys = array();

            foreach ($this->fields as $field_id => $field_args) {
                $fields_def[] = $this->field_array_to_schema($field_id, $field_args);

                // KEY definition
                if ($field_args['key']) {
                    /*
                 * Indexes have a maximum size of 767 bytes. WordPress 4.2 was moved to utf8mb4, which uses 4 bytes per character.
                 * This means that an index which used to have room for floor(767/3) = 255 characters, now only has room for floor(767/4) = 191 characters.
                 */
                    $max_index_length = 191;

                    if ($field_args['length'] > $max_index_length) {
                        $keys[] = 'KEY ' . $field_id . ' (' . $field_id . '(' . $max_index_length . '))';
                    } else {
                        $keys[] = 'KEY ' . $field_id . ' (' . $field_id . ')';
                    }
                }
            }

            // Setup PRIMARY KEY definition
            $sql = implode(', ', $fields_def) . ', '
                . 'PRIMARY KEY  (' . $this->primary_key . ')'; // Add two spaces to avoid issues

            // Setup KEY definition
            if (!empty($keys)) {
                $sql .= ', ' . implode(', ', $keys);
            }

            return $sql;
        }

It passes the common sense test and I've tested it out on our fairly complex plugin and it works, but I have not run any CT specific tests.

The list table page is not created after I put example code, though it's able to create the database and entries with db->insert

Am I missing something?

WordPress 5.8.2
PHP 8.0

Howdy,

I think there's a tiny bug that's causing things to blow up when you try and create a custom table field with 'key' => true in some instances.

1. I'm pretty sure that the call to max should actually be min here: https://github.com/rubengc/CT/blob/master/includes/class-ct-database-schema.php#L236

2. I'm not 100% why, but it seems that a key created on DATETIME types will fail if trying to create a partial key. In the same way that you are checking $this->is_numeric, I also had to add a $this->is_datetime check to make it work.

I hope this is helpful!

Issue

Query updater does not work with multiple ALTER TABLE commands.

For example I added a few columns, removed a couple, renamed a couple and was met with an "almost silent" DB error on the first page refresh. There was a var_dumped db error at the top of the screen, and when I refreshed again it was gone and there were no further attempts by the query updater to reconcile the difference between the actual DB table and the schema definition I was feeding it.

More details after the proposed solution, at the bottom of this comment.

Proposed Solution
Execute each query one at a time in class-ct-database-schema-updater.php -> run()

Before

                // line 149 class-ct-database-schema-updater.php
                if( ! empty( $sql ) ) {
                    // Execute the SQL
                    $updated = $this->ct_db->db->query( $sql );

                    // Was anything updated?
                    return ! empty( $updated );
                }

After

                if( ! empty( $sql ) ) {
                    $multiple_sql_queries = explode(';', $sql);
                    // Remove the last element because it's an empty string ($sql will always end in ';')
                    array_pop($multiple_sql_queries); 
                    
                    // Execute each query individually
                    $results = array_map(function($one_sql_query) {
                        return $this->ct_db->db->query( $one_sql_query );
                    }, $multiple_sql_queries);

                    $updated = array_map(function($result) {
                        return ! empty($result);
                    }, $results);
 
                    return in_array(true, $updated);
                }

Here is the exact error string that was var_dumped:
WordPress database error: [You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ALTER TABLE wp_solid_affiliate_referrals ADD commission_amount float NOT NULL ; ' at line 1] ALTER TABLE wp_solid_affiliate_referrals DROP COLUMN amount; ALTER TABLE wp_solid_affiliate_referrals ADD commission_amount float NOT NULL ; ALTER TABLE wp_solid_affiliate_referrals ADD order_amount float NOT NULL ; ALTER TABLE wp_solid_affiliate_referrals ADD order_source varchar(255) NOT NULL ;

Here is the query but with formatting, to make it easier to read:

ALTER TABLE
  wp_solid_affiliate_referrals DROP COLUMN amount;
ALTER TABLE
  wp_solid_affiliate_referrals
ADD
  commission_amount float NOT NULL;
ALTER TABLE
  wp_solid_affiliate_referrals
ADD
  order_amount float NOT NULL;
ALTER TABLE
  wp_solid_affiliate_referrals
ADD
  order_source varchar(255) NOT NULL;

@rubengc I know you're busy, but unless I'm misunderstanding something this seems like a severe issue that could also affect your projects. Would really appreciate if you took a glance at this, and if you have any tests on other projects using CT that would verify edge cases in my proposed solution would be great if you ran them.

Tried putting it in the plugins folder and it doesn't show up, do you have basic install instructions? Is this working? Any docs?

We have a user_id and post_id column. For our purposes, there can never be more than one row for each user_id and post_id combination in the database.

The following UNIQUE_KEY declaration would ensure this:

UNIQUE KEY unique_user_post (user_id, post_id)

However, using the CT schema creator, this is not possible. Even if I manually enter the schema as a string, this line is stripped.

It seems like CT is hooking into ALL screen settings updates without checking which are valid for CT or which should be ignored.

I fixed this error by changing the following:

public function set_screen_settings( $value_to_set, $option, $value ) {

        global $ct_table, $ct_list_table;

        //INSERTED THE BELOW:
        if (is_null($ct_list_table)) return $value;

Would have been much more easier to be included in projects using autoloader and namespaces instead of wp lib loader hackish approach. Also you could consider not having any functions outside the classes like the ones from the /includes/functions.php.

Love your work with this class, makes it so easier to use WP_List_Table, but probably it's still a work in progress.