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

feat: Dokan Data Store #2346

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
ea2a6e9
feat: Dokan Data Store
shohag121 Aug 21, 2024
1628844
Add base Model
mrabbani Oct 21, 2024
40e5ecb
Add vendor Model
mrabbani Oct 22, 2024
a216c96
Add Test case
mrabbani Oct 23, 2024
7e681f9
Add test case for datastore
mrabbani Oct 23, 2024
f583f2c
Add test case for CRUD
mrabbani Oct 24, 2024
73b289d
Cast Date object to mysql date formate
mrabbani Oct 24, 2024
c8b207c
Add developer docs
mrabbani Oct 25, 2024
aae536f
Add delete_by method
mrabbani Oct 25, 2024
9b5aa1c
Add update_by to base store
mrabbani Oct 29, 2024
80b6bee
Add general update query method in Data Sotre
mrabbani Nov 6, 2024
fc33dc8
Update vendor balance Model and Store to perform update operations
mrabbani Nov 6, 2024
a07efbf
Refactor dokan order sync for vendor balance
mrabbani Nov 6, 2024
afaeb93
Merge branch 'develop' into feat/data-store-and-models
mrabbani Nov 6, 2024
be45417
Merge with refactor/vendor-balance-raw-query
mrabbani Nov 6, 2024
a47f411
Add database mission assertion method for test case
mrabbani Nov 7, 2024
f57f2fa
Add read_meta method to BaseStore
mrabbani Nov 7, 2024
7d4b345
Add method to get vendor total balance
mrabbani Nov 7, 2024
d348e0c
Fetch vendor balance with the method of VendorBalance Method
mrabbani Nov 7, 2024
45965a3
Refactor/vendor balance raw query (#2430)
mrabbani Nov 8, 2024
a8ce67d
Rename method get_total_balance_by_vendor to test_get_total_earning_b…
mrabbani Nov 8, 2024
59ea07f
Update docs blocks
mrabbani Nov 8, 2024
d504acb
Fix merge conflict
mrabbani Nov 8, 2024
b950b9c
Bind model to Service provider
mrabbani Nov 12, 2024
774843d
Get model from the container to perform operation
mrabbani Nov 12, 2024
8417e49
Remove empty lines
mrabbani Nov 13, 2024
cb6b364
Add data provider for test case
mrabbani Nov 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions includes/Models/BaseModel.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php

namespace WeDevs\Dokan\Models;

use WC_Data;

abstract class BaseModel extends WC_Data {
/**
* Save should create or update based on object existence.
*
* @return int
*/
public function save() {
// wc_get_product()
if ( ! $this->data_store ) {
return $this->get_id();
}

/**
* Trigger action before saving to the DB. Allows you to adjust object props before save.
*
* @param WC_Data $this The object being saved.
* @param WC_Data_Store_WP $data_store THe data store persisting the data.
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct capitalization in parameter descriptions.

In the docblocks for $data_store at lines 23 and 37, "THe data store persisting the data." should be "The data store persisting the data."

Apply this diff to fix the typos:

-		 * @param WC_Data_Store_WP $data_store THe data store persisting the data.
+		 * @param WC_Data_Store_WP $data_store The data store persisting the data.

Also applies to: 37-37

*/
do_action( 'dokan_before_' . $this->object_type . '_object_save', $this, $this->data_store );

if ( $this->get_id() ) {
$this->data_store->update( $this );
} else {
$this->data_store->create( $this );
}

/**
* Trigger action after saving to the DB.
*
* @param WC_Data $this The object being saved.
* @param WC_Data_Store_WP $data_store THe data store persisting the data.
*/
do_action( 'dokan_after_' . $this->object_type . '_object_save', $this, $this->data_store );

return $this->get_id();
}

/**
* Delete an object, set the ID to 0, and return result.
*
* @param bool $force_delete Should the date be deleted permanently.
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix typo in parameter description of delete() method.

In the docblock for the delete() method, "Should the date be deleted permanently." should be "Should the data be deleted permanently."

Apply this diff to correct the typo:

-	 * @param  bool $force_delete Should the date be deleted permanently.
+	 * @param  bool $force_delete Should the data be deleted permanently.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* @param bool $force_delete Should the date be deleted permanently.
* @param bool $force_delete Should the data be deleted permanently.

* @return bool result
*/
public function delete( $force_delete = false ) {
/**
* Filters whether an object deletion should take place. Equivalent to `pre_delete_post`.
*
* @param mixed $check Whether to go ahead with deletion.
* @param Data $this The data object being deleted.
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure consistency in parameter types in docblocks.

At line 55, the parameter type for $this is listed as Data. To maintain consistency and accuracy, it should be WC_Data, matching the type used in other methods.

Apply this diff to correct the parameter type:

-		 * @param Data $this The data object being deleted.
+		 * @param WC_Data $this The data object being deleted.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* @param Data $this The data object being deleted.
* @param WC_Data $this The data object being deleted.

* @param bool $force_delete Whether to bypass the trash.
*
* @since 8.1.0.
*/
$check = apply_filters( "dokan_pre_delete_$this->object_type", null, $this, $force_delete );
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix variable interpolation in apply_filters call.

When interpolating object properties within double-quoted strings, you need to enclose the property in braces to ensure correct parsing. Without braces, PHP may not parse the variable correctly.

Apply this diff to fix the variable interpolation:

-		$check = apply_filters( "dokan_pre_delete_$this->object_type", null, $this, $force_delete );
+		$check = apply_filters( "dokan_pre_delete_{$this->object_type}", null, $this, $force_delete );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$check = apply_filters( "dokan_pre_delete_$this->object_type", null, $this, $force_delete );
$check = apply_filters( "dokan_pre_delete_{$this->object_type}", null, $this, $force_delete );


if ( null !== $check ) {
return $check;
}

if ( $this->data_store ) {
$this->data_store->delete( $this, array( 'force_delete' => $force_delete ) );
$this->set_id( 0 );
return true;
}

return false;
}

/**
* Prefix for action and filter hooks on data.
*
* @return string
*/
protected function get_hook_prefix() {
return 'dokan_' . $this->object_type . '_get_';
}
}
290 changes: 290 additions & 0 deletions includes/Models/DataStore/BaseDataStore.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,290 @@
<?php

namespace WeDevs\Dokan\Models\DataStore;

use Automattic\WooCommerce\Admin\API\Reports\SqlQuery;
use Automattic\WooCommerce\Pinterest\API\Base;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unused import

The Automattic\WooCommerce\Pinterest\API\Base import appears to be unused in this class and should be removed.

-use Automattic\WooCommerce\Pinterest\API\Base;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use Automattic\WooCommerce\Pinterest\API\Base;

use Exception;
use WeDevs\Dokan\Models\BaseModel;

abstract class BaseDataStore extends SqlQuery implements DataStoreInterface {
protected $selected_columns = [ '*' ];

/**
* Get the fields with format as an array where key is the db field name and value is the format.
*
* @return array
*/
abstract protected function get_fields_with_format(): array;

/**
* Get the table name with or without prefix
*
* @return string
*/
abstract public function get_table_name(): string;

/**
* Create a new record in the database using the provided model data.
*
* @param BaseModel $model The model object containing the data to be inserted.
*/
public function create( BaseModel &$model ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Remove unnecessary passing of objects by reference

In the methods create(), read(), update(), and delete(), the BaseModel object is passed by reference using &$model. In PHP, objects are already passed by reference by default, so the ampersand & is unnecessary and may cause confusion.

Apply this diff to remove the unnecessary reference operator:

-    public function create( BaseModel &$model ) {
+    public function create( BaseModel $model ) {

-    public function read( BaseModel &$model ) {
+    public function read( BaseModel $model ) {

-    public function update( BaseModel &$model ) {
+    public function update( BaseModel $model ) {

-    public function delete( BaseModel &$model, $args = array() ) {
+    public function delete( BaseModel $model, $args = array() ) {

Also applies to: 42-42, 87-87, 117-117

$data = $this->get_fields_data( $model );

$inserted_id = $this->insert( $data );

if ( $inserted_id ) {
$model->set_id( $inserted_id );
$model->apply_changes();
}

do_action( $this->get_hook_prefix() . 'created', $inserted_id, $data );

return $inserted_id;
}


/**
* Method to read a download permission from the database.
*
* @param BaseModel $model BaseModel object.
*
* @phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
*
* @throws Exception Throw exception if invalid entity is passed.
*/
public function read( BaseModel &$model ) {
global $wpdb;

if ( ! $model->get_id() ) {
$message = $this->get_hook_prefix() . ' : ' . __( 'Invalid entity item.', 'dokan-lite' );

throw new Exception( esc_html( $message ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid using esc_html() in exception messages

The esc_html() function is intended for escaping output in HTML contexts, not for internal exception messages. Using it here is unnecessary and may lead to confusion.

Remove esc_html() from the exception messages:

             if ( ! $model->get_id() ) {
                 $message = $this->get_hook_prefix() . ' : ' . __( 'Invalid entity item.', 'dokan-lite' );

-                throw new Exception( esc_html( $message ) );
+                throw new Exception( $message );
             }

---

             if ( ! $raw_item ) {
                 $message = $this->get_hook_prefix() . ' : ' . __( 'Invalid entity item.', 'dokan-lite' );

-                throw new Exception( esc_html( $message ) );
+                throw new Exception( $message );
             }

Exception messages should be plain strings since they are not rendered in a browser context.

Also applies to: 90-90

}

$model->set_defaults();

$id_field_name = $this->get_id_field_name();
$format = $this->get_id_field_format();

$this->clear_all_clauses();
$this->add_sql_clause( 'select', $this->get_selected_columns() );
$this->add_sql_clause( 'from', $this->get_table_name_with_prefix() );

$this->add_sql_clause(
'where',
$wpdb->prepare(
"{$id_field_name} = $format",

Check failure on line 78 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found interpolated variable {$id_field_name} at "{$id_field_name} = $format"

Check failure on line 78 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found interpolated variable $format at "{$id_field_name} = $format"

Check warning on line 78 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Replacement variables found, but no valid placeholders found in the query.
$model->get_id()
)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential SQL injection vulnerability in query preparation

The SQL query in the read() method uses interpolated variables within the query string, which can lead to SQL injection vulnerabilities. Specifically, the variables {$id_field_name} and $format are directly interpolated.

Apply this diff to properly prepare the SQL statement using placeholders:

             $this->add_sql_clause(
                 'where',
-                $wpdb->prepare(
-                    "{$id_field_name} = $format",
+                $wpdb->prepare(
+                    "{$id_field_name} = %s",
                     $model->get_id()
                 )
             );

Note: Since $id_field_name is a column name, ensure it is sanitized and originates from a trusted source. Consider using a whitelist of allowed field names.

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 63-63:
Use placeholders and $wpdb->prepare(); found interpolated variable {$id_field_name} at "{$id_field_name} = $format"


[failure] 63-63:
Use placeholders and $wpdb->prepare(); found interpolated variable $format at "{$id_field_name} = $format"


[warning] 63-63:
Replacement variables found, but no valid placeholders found in the query.


$raw_item = $wpdb->get_row(
$this->get_query_statement()

Check warning on line 84 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found $this

Check warning on line 84 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found get_query_statement
);

if ( ! $raw_item ) {
$message = $this->get_hook_prefix() . ' : ' . __( 'Invalid entity item.', 'dokan-lite' );

throw new Exception( esc_html( $message ) );
}

$model->set_props( $this->format_raw_data( $raw_item ) );
$model->set_object_read( true );

return $raw_item;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix SQL injection vulnerability in read() method

The SQL query construction in the read method is vulnerable to SQL injection. While the ID value is properly prepared, the field name and format are directly interpolated into the query string.

-            $wpdb->prepare(
-                "{$id_field_name} = $format",
-                $model->get_id()
-            )
+            $wpdb->prepare(
+                $wpdb->prepare( '%s = %d', $id_field_name, $model->get_id() )
+            )

Additionally, consider validating $id_field_name against a whitelist of allowed column names.

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 78-78:
Use placeholders and $wpdb->prepare(); found interpolated variable {$id_field_name} at "{$id_field_name} = $format"


[failure] 78-78:
Use placeholders and $wpdb->prepare(); found interpolated variable $format at "{$id_field_name} = $format"


[warning] 78-78:
Replacement variables found, but no valid placeholders found in the query.


[warning] 84-84:
Use placeholders and $wpdb->prepare(); found $this


[warning] 84-84:
Use placeholders and $wpdb->prepare(); found get_query_statement


/**
* Method to update a download in the database.
*
* @param BaseModel $model WC_Customer_Download object.
*/
public function update( BaseModel &$model ) {
global $wpdb;

$data = $this->get_fields_data( $model );

$format = $this->get_fields_format();

$result = $wpdb->update(
$this->get_table_name_with_prefix(),
$data,
array(
$this->get_id_field_name() => $model->get_id(),
),
$format
);

$model->apply_changes();

return $result;
}

/**
* Method to delete a download permission from the database.
*
* @param BaseModel $model BaseModel object.
* @param array $args Array of args to pass to the delete method.
*/
public function delete( BaseModel &$model, $args = array() ) {
$model_id = $model->get_id();
$this->delete_by_id( $model_id );

$model->set_id( 0 );
}

/**
* Method to delete a download permission from the database by ID.
*
* @param int $id permission_id of the download to be deleted.
*
* @phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
*/
public function delete_by_id( $id ) {
global $wpdb;

$table_name = $this->get_table_name_with_prefix();
$id_field_name = $this->get_id_field_name();
$format = $this->get_id_field_format();

$result = $wpdb->query(
$wpdb->prepare(
"DELETE FROM {$table_name}

Check failure on line 154 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found interpolated variable {$table_name} at "DELETE FROM {$table_name}
WHERE $id_field_name = $format",

Check failure on line 155 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found interpolated variable $id_field_name at WHERE $id_field_name = $format"

Check failure on line 155 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Use placeholders and $wpdb->prepare(); found interpolated variable $format at WHERE $id_field_name = $format"

Check warning on line 155 in includes/Models/DataStore/BaseDataStore.php

View workflow job for this annotation

GitHub Actions / Run PHPCS inspection

Replacement variables found, but no valid placeholders found in the query.
$id
)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential SQL injection vulnerability in deletion query

In the delete_by_id() method, variables are interpolated directly into the SQL string, which poses a security risk. The variables {$table_name}, $id_field_name, and $format should not be directly interpolated into the query.

Refactor the code to use $wpdb->delete() for safer query execution:

-            $result = $wpdb->query(
-                $wpdb->prepare(
-                    "DELETE FROM {$table_name}
-                    WHERE $id_field_name = $format",
-                    $id
-                )
-            );
+            $result = $wpdb->delete(
+                $table_name,
+                array( $id_field_name => $id ),
+                array( $format )
+            );

This approach ensures that the table name and where clause are handled safely, preventing SQL injection.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"DELETE FROM {$table_name}
WHERE $id_field_name = $format",
$id
)
$result = $wpdb->delete(
$table_name,
array( $id_field_name => $id ),
array( $format )
);
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 140-140:
Use placeholders and $wpdb->prepare(); found interpolated variable {$table_name} at "DELETE FROM {$table_name}


[failure] 141-141:
Use placeholders and $wpdb->prepare(); found interpolated variable $id_field_name at WHERE $id_field_name = $format"


[failure] 141-141:
Use placeholders and $wpdb->prepare(); found interpolated variable $format at WHERE $id_field_name = $format"


[warning] 141-141:
Replacement variables found, but no valid placeholders found in the query.

);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential SQL injection vulnerability in delete_by_id() method

In the delete_by_id() method, variables are interpolated directly into the SQL query. Injecting {$table_name}, $id_field_name, and $format into the query string can lead to SQL injection vulnerabilities.

Refactor the code to use $wpdb->delete() for safer query execution:

             $table_name = $this->get_table_name_with_prefix();
             $id_field_name = $this->get_id_field_name();
             $format = $this->get_id_field_format();

-            $result = $wpdb->query(
-                $wpdb->prepare(
-                    "DELETE FROM {$table_name}
-                    WHERE $id_field_name = $format",
-                    $id
-                )
-            );
+            $result = $wpdb->delete(
+                $table_name,
+                [ $id_field_name => $id ],
+                [ $format ]
+            );

Using $wpdb->delete() ensures that the table name and where clause are handled safely, preventing SQL injection vulnerabilities. Ensure that $table_name and $id_field_name are sanitized or come from trusted sources.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$result = $wpdb->query(
$wpdb->prepare(
"DELETE FROM {$table_name}
WHERE $id_field_name = $format",
$id
)
);
$result = $wpdb->delete(
$table_name,
[ $id_field_name => $id ],
[ $format ]
);
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 154-154:
Use placeholders and $wpdb->prepare(); found interpolated variable {$table_name} at "DELETE FROM {$table_name}


[failure] 155-155:
Use placeholders and $wpdb->prepare(); found interpolated variable $id_field_name at WHERE $id_field_name = $format"


[failure] 155-155:
Use placeholders and $wpdb->prepare(); found interpolated variable $format at WHERE $id_field_name = $format"


[warning] 155-155:
Replacement variables found, but no valid placeholders found in the query.

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential SQL injection vulnerability in delete_by_id() method

Interpolating variables directly into the SQL query can lead to SQL injection vulnerabilities. Specifically, using {$table_name}, $id_field_name, and $format in the query string is unsafe.

Refactor the code to use $wpdb->delete() for safer query execution:

-        $result = $wpdb->query(
-            $wpdb->prepare(
-                "DELETE FROM {$table_name}
-                WHERE $id_field_name = $format",
-                $id
-            )
-        );
+        $result = $wpdb->delete(
+            $table_name,
+            array( $id_field_name => $id ),
+            array( $format )
+        );

This approach ensures that the table name and where clause are handled safely, preventing SQL injection.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"DELETE FROM {$table_name}
WHERE $id_field_name = $format",
$id
)
);
$result = $wpdb->delete(
$table_name,
array( $id_field_name => $id ),
array( $format )
);
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection

[failure] 153-153:
Use placeholders and $wpdb->prepare(); found interpolated variable {$table_name} at "DELETE FROM {$table_name}


[failure] 154-154:
Use placeholders and $wpdb->prepare(); found interpolated variable $id_field_name at WHERE $id_field_name = $format"


[failure] 154-154:
Use placeholders and $wpdb->prepare(); found interpolated variable $format at WHERE $id_field_name = $format"


[warning] 154-154:
Replacement variables found, but no valid placeholders found in the query.


do_action( $this->get_hook_prefix() . 'deleted', $id, $result );

return $result;
}

/**
* Create download permission for a user, from an array of data.
* Assumes that all the keys in the passed data are valid.
*
* @param array $data Data to create the permission for.
* @return int The database id of the created permission, or false if the permission creation failed.
*/
protected function insert( array $data ) {
global $wpdb;

$format = $this->get_fields_format();

$table_name = $this->get_table_name_with_prefix();

$hook_prefix = $this->get_hook_prefix();

$result = $wpdb->insert(
$table_name,
apply_filters( $hook_prefix . 'insert_data', $data ),
apply_filters( $hook_prefix . 'insert_data_format', $format, $data )
);

do_action( $hook_prefix . 'after_insert', $result, $data );

return $result ? $wpdb->insert_id : 0;
}

protected function get_fields_data( BaseModel &$model ): array {
$data = array();

foreach ( $this->get_fields() as $db_field_name ) {
if ( method_exists( $this, 'get_' . $db_field_name ) ) {
$data[ $db_field_name ] = call_user_func( array( $this, 'get_' . $db_field_name ), $model, 'edit' );
} else {
$data[ $db_field_name ] = call_user_func( array( $model, 'get_' . $db_field_name ), 'edit' );
}
}

return $data;
}

/**
* Returns a list of columns selected by the query_args formatted as a comma separated string.
*
* @return string
*/
protected function get_selected_columns(): string {
$selections = apply_filters( $this->get_hook_prefix() . 'selected_columns', $this->selected_columns );

$selections = implode( ', ', $selections );

return $selections;
}


protected function format_raw_data( $raw_data ): array {
$data = array();

foreach ( $this->get_fields() as $db_field_name ) {
$data[ $db_field_name ] = $raw_data->{$db_field_name};
}

return apply_filters( $this->get_hook_prefix() . 'format_raw_data', $data, $raw_data );
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add property existence check in format_raw_data()

The method directly accesses object properties without checking their existence, which could lead to PHP notices or errors.

     protected function format_raw_data( $raw_data ): array {
         $data = array();
 
         foreach ( $this->get_fields() as $db_field_name ) {
-            $data[ $db_field_name ] = $raw_data->{$db_field_name};
+            $data[ $db_field_name ] = property_exists( $raw_data, $db_field_name ) 
+                ? $raw_data->{$db_field_name} 
+                : null;
         }
 
         return apply_filters( $this->get_hook_prefix() . 'format_raw_data', $data, $raw_data );
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
protected function format_raw_data( $raw_data ): array {
$data = array();
foreach ( $this->get_fields() as $db_field_name ) {
$data[ $db_field_name ] = $raw_data->{$db_field_name};
}
return apply_filters( $this->get_hook_prefix() . 'format_raw_data', $data, $raw_data );
}
protected function format_raw_data( $raw_data ): array {
$data = array();
foreach ( $this->get_fields() as $db_field_name ) {
$data[ $db_field_name ] = property_exists( $raw_data, $db_field_name )
? $raw_data->{$db_field_name}
: null;
}
return apply_filters( $this->get_hook_prefix() . 'format_raw_data', $data, $raw_data );
}


protected function get_hook_prefix(): string {
global $wpdb;

$table_name = $this->get_table_name();

$hook_prefix = str_replace( $wpdb->prefix, '', $table_name );

return "dokan_{$hook_prefix}_";
}

protected function get_table_name_with_prefix(): string {
global $wpdb;

$table_name = $this->get_table_name();

if ( ! str_starts_with( $table_name, $wpdb->prefix ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure PHP version compatibility when using str_starts_with()

The function str_starts_with() is available in PHP 8.0 and later. If the project needs to support earlier PHP versions, using this function will cause compatibility issues.

Replace str_starts_with() with an alternative compatible with older PHP versions:

         protected function get_table_name_with_prefix(): string {
             global $wpdb;

             $table_name = $this->get_table_name();

-            if ( ! str_starts_with( $table_name, $wpdb->prefix ) ) {
+            if ( strpos( $table_name, $wpdb->prefix ) !== 0 ) {
                 $table_name = $wpdb->prefix . $table_name;
             }

             return $table_name;
         }

This change maintains the same functionality while ensuring compatibility with PHP versions prior to 8.0.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ( ! str_starts_with( $table_name, $wpdb->prefix ) ) {
if ( strpos( $table_name, $wpdb->prefix ) !== 0 ) {

$table_name = $wpdb->prefix . $table_name;
}
Comment on lines +394 to +396
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure compatibility with PHP versions when using str_starts_with()

The function str_starts_with() is available from PHP 8.0 onwards. If the project needs to support earlier PHP versions, using this function will cause compatibility issues.

Replace str_starts_with() with an alternative that works with older PHP versions:

-        if ( ! str_starts_with( $table_name, $wpdb->prefix ) ) {
+        if ( strpos( $table_name, $wpdb->prefix ) !== 0 ) {

This change maintains the same functionality while ensuring compatibility with PHP versions prior to 8.0.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ( ! str_starts_with( $table_name, $wpdb->prefix ) ) {
$table_name = $wpdb->prefix . $table_name;
}
if ( strpos( $table_name, $wpdb->prefix ) !== 0 ) {
$table_name = $wpdb->prefix . $table_name;
}


return $table_name;
}
Comment on lines +389 to +399
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add table name validation

The table name should be validated to prevent potential security issues and ensure it follows WordPress naming conventions.

     protected function get_table_name_with_prefix(): string {
         global $wpdb;
 
         $table_name = $this->get_table_name();
 
+        // Validate table name format
+        if ( ! preg_match('/^[a-zA-Z0-9_]+$/', $table_name) ) {
+            throw new Exception( __( 'Invalid table name format', 'dokan-lite' ) );
+        }
+
         if ( ! str_starts_with( $table_name, $wpdb->prefix ) ) {
             $table_name = $wpdb->prefix . $table_name;
         }
 
         return $table_name;
     }

Committable suggestion was skipped due to low confidence.


protected function get_id_field_name(): string {
return apply_filters( $this->get_hook_prefix() . 'id_field_name', 'id' ); // 'id';
}


protected function get_id_field_format(): string {
return apply_filters( $this->get_hook_prefix() . 'id_field_format', '%d' ); // 'id';
}

/**
* Get the fields.
*
* @return array The filtered array of fields.
*/
protected function get_fields(): array {
$fields = array_keys( $this->get_fields_with_format() );

return apply_filters(
$this->get_hook_prefix() . 'fields',
$fields,
$this->get_fields_with_format()
);
}

/**
* Get the fields with format.
*
* @return array The filtered array of format of the fields.
*/
protected function get_fields_format(): array {
$format = array_values( $this->get_fields_with_format() );

return apply_filters(
$this->get_hook_prefix() . 'fields_format',
$format,
$this->get_fields_with_format()
);
}
}
Loading
Loading