feat: Rollback product statuses on dokan pro deactivation (integration) #2476
Conversation
mralaminahamed
added
Needs: Testing
WalkthroughThis pull request introduces a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
includes/ProductStatusRollback.php (2)
86-86: Enhance exception message with product ID for better debuggingIncluding the product ID in the exception message can aid in debugging by specifying which product caused the error.
Apply this diff to improve the exception message:
-throw new Exception( 'Invalid product' ); +throw new Exception( 'Invalid product with ID ' . $product_id );
28-28: Clarify the@sinceannotations in the documentationThe
@since DOKAN_PRO_SINCEannotations use a placeholder constantDOKAN_PRO_SINCE, which may not be resolved properly in generated documentation. Consider replacingDOKAN_PRO_SINCEwith the actual version number or ensure that it will be correctly processed during documentation generation.Also applies to: 37-37, 48-48, 92-92, 102-102, 119-119
includes/DependencyManagement/Providers/CommonServiceProvider.php (1)
70-71: Adjust indentation to match the existing code styleThe indentation of the method chaining in lines 70-71 is misaligned by one space compared to the existing code. Consistent indentation enhances readability and maintainability.
Apply this diff to fix the alignment:
- $this->getContainer() - ->addShared( \WeDevs\Dokan\ProductStatusRollback::class, \WeDevs\Dokan\ProductStatusRollback::class ) - ->addTag( self::TAG ); + $this->getContainer() + ->addShared( \WeDevs\Dokan\ProductStatusRollback::class, \WeDevs\Dokan\ProductStatusRollback::class ) + ->addTag( self::TAG );🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 70-70:
Found precision alignment of 1 spaces.
[warning] 71-71:
Found precision alignment of 1 spaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
includes/DependencyManagement/Providers/CommonServiceProvider.php(1 hunks)includes/ProductStatusRollback.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/DependencyManagement/Providers/CommonServiceProvider.php
[warning] 70-70:
Found precision alignment of 1 spaces.
[warning] 71-71:
Found precision alignment of 1 spaces.
includes/ProductStatusRollback.php
[failure] 126-126:
Expected no spaces between the increment operator and $processed; 1 found
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
includes/ProductStatusRollback.php (4)
16-32: Consider making BATCH_SIZE configurableWhile 20 items per batch is a reasonable default, consider making this configurable through filters to allow adjustment based on server capacity and load conditions.
- private const BATCH_SIZE = 20; + /** + * Get batch size for processing + * + * @since DOKAN_PRO_SINCE + * + * @return int + */ + private function get_batch_size(): int { + return (int) apply_filters( 'dokan_product_status_rollback_batch_size', 20 ); + }
41-43: Document the hook priorityConsider adding a comment explaining why the default priority (10) was chosen for this hook, especially since it's part of a batch processing system.
65-76: Optimize database query performanceConsider adding an index hint to optimize the query performance, especially since this is running in batches.
$products = $wpdb->get_col( $wpdb->prepare( - "SELECT ID FROM $wpdb->posts + "SELECT /*+ INDEX(wp_posts post_type_status_date) */ ID FROM $wpdb->posts WHERE post_type = 'product' AND post_status = %s ORDER BY ID
55-169: Consider breaking down the large method into smaller, focused methodsThe
process_batch_reject_operationmethod is quite long and handles multiple responsibilities. Consider extracting some logic into separate methods:
- Product retrieval logic
- Individual product processing logic
- Batch scheduling logic
This would improve readability and testability.
Example refactor:
private function get_batch_products(int $batch): array { // Extract product retrieval logic } private function process_single_product(WC_Product $product): void { // Extract single product processing logic } private function schedule_next_batch(WC_Queue_Interface $queue, int $batch, int $total_batches): void { // Extract scheduling logic }🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 126-126:
Expected no spaces between the increment operator and $processed; 1 found
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
includes/ProductStatusRollback.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/ProductStatusRollback.php
[failure] 126-126:
Expected no spaces between the increment operator and $processed; 1 found
🔇 Additional comments (3)
includes/ProductStatusRollback.php (3)
1-10: LGTM! Well-organized namespace and imports.
The namespace declaration and class imports follow PSR standards and best practices.
126-126: Fix coding standard violation: remove space between increment operator and variable
The increment operator should not have a space between it and the variable.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 126-126:
Expected no spaces between the increment operator and $processed; 1 found
97-98: Verify status transition handling across the system
Let's verify that other parts of the system properly handle the status transition from 'reject' to 'draft'.
✅ Verification successful
Based on the search results, I can now generate the final response:
Status transition from 'reject' to 'draft' is properly handled
The codebase shows comprehensive handling of product status transitions:
- The
dokan_product_rollback_statusfilter is used consistently for status transitions - Product listing and dashboard components properly handle 'draft' status
- Status transitions are properly tracked and logged
- The system maintains proper status labels and UI elements for draft products
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other handlers of product status transitions
rg -A 5 "post_status.*reject|draft" --type php
# Search for status-related filters
ast-grep --pattern 'add_filter($_, "dokan_product_.*status.*")'
Length of output: 25775
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
includes/ProductStatusRollback.php (3)
1-24: LGTM! Consider enhancing constant documentation.The class structure, namespace, and imports are well-organized. The constants are appropriately defined with private scope.
Consider adding more detailed documentation for the constants, especially explaining why BATCH_SIZE is set to 20:
/** * Batch size for processing + * + * Maximum number of products to process in a single batch. + * Set to 20 to balance between performance and memory usage. * * @var int */
55-169: Consider breaking down the large method into smaller, focused methods.The
process_batch_reject_operationmethod is handling multiple responsibilities. Consider extracting the following into separate methods:
- Product retrieval logic (lines 65-76)
- Individual product processing (lines 82-137)
- Batch scheduling logic (lines 149-158)
This would improve readability and maintainability.
127-136: Consider more specific exception handling.The current catch block catches all
Throwableinstances. Consider catching specific exceptions that might occur during product processing (e.g.,WC_Data_Exceptionfor WooCommerce data operations).Example:
- } catch ( Throwable $e ) { + } catch ( WC_Data_Exception $e ) { + dokan_log( + sprintf( + '[Product Status Rollback] Data error rolling back product #%d: %s', + $product_id, + $e->getMessage() + ), + 'error' + ); + } catch ( Exception $e ) { dokan_log( sprintf( '[Product Status Rollback] Error rolling back product #%d: %s',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
includes/ProductStatusRollback.php(1 hunks)
🔇 Additional comments (2)
includes/ProductStatusRollback.php (2)
126-126: Fix coding standard violation: remove space between increment operator and variable
There should be no space between the increment operator and the variable.
89-97: LGTM! Well-documented filter hook.
The filter hook is well-documented with proper PHPDoc including @SInCE tag and parameter descriptions.
mralaminahamed
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
includes/ProductStatusRollback.php (1)
96-97: Consider using update_meta instead of add_meta_dataUsing
add_meta_datafollowed bysave_meta_datarequires two database operations. Consider usingupdate_metawhich is more efficient for single meta updates.- $product->add_meta_data( '_dokan_previous_status', $product->get_status(), true ); - $product->save_meta_data(); + update_post_meta( $product->get_id(), '_dokan_previous_status', $product->get_status() );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/ProductStatusRollback.php(1 hunks)
🔇 Additional comments (5)
includes/ProductStatusRollback.php (5)
1-8: LGTM: File structure and imports are well organized
The namespace and use statements follow PSR-4 standards and include all necessary dependencies.
27-28: Replace DOKAN_PRO_SINCE placeholder with actual version number
The @SInCE tags are using a placeholder value that needs to be replaced with the actual version number where this feature will be released.
Also applies to: 37-38
40-42: LGTM: Hook registration follows WordPress standards
The action hook registration is clean and follows WordPress coding standards.
56-62: Use $wpdb->posts instead of direct table reference
For better compatibility with custom WordPress table prefixes, use $wpdb->posts instead of directly referencing the table.
9-23: Verify batch size performance implications
The batch size of 10 items might need validation:
- Small batch sizes can lead to more database operations
- Large batch sizes can impact memory usage
Consider benchmarking different batch sizes to find the optimal value.
Additionally, consider making the queue group identifier more specific to avoid potential conflicts, e.g., 'dokan-product-status-rollback-operation'.
✅ Verification successful
Let me gather more context about these batch operations to better understand their implementation and performance implications.
Batch size of 10 is consistent with similar operations
The batch size of 10 is well-aligned with other batch processing operations in the codebase:
- It matches the batch size used in
Update_Product_CommissionandUpdate_Vendor_Commission - Only
Update_Category_Commissionuses a slightly larger batch size of 20
The queue group identifier 'dokan-product-status-rollback' is already sufficiently specific, as no other conflicting identifiers were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if batch size is configurable or hardcoded elsewhere
rg "BATCH_SIZE.*=.*\d+" --type php
# Check for other queue group identifiers to avoid conflicts
rg "QUEUE_GROUP.*=.*'dokan-" --type php
Length of output: 512
Script:
#!/bin/bash
# Check the implementation of batch processing in these files
ast-grep --pattern 'class $_ {
$$$
const BATCH_SIZE = $_;
$$$
}'
# Look for any performance-related comments or documentation
rg -B2 -A2 "BATCH_SIZE" --type php
Length of output: 4589
mrabbani
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
includes/ProductStatusRollback.php (5)
36-54: 🛠️ Refactor suggestionImplement Hookable interface as suggested.
As per previous review, implement the
WeDevs\Dokan\Contracts\Hookableinterface to maintain consistency with the codebase architecture.-class ProductStatusRollback { +use WeDevs\Dokan\Contracts\Hookable; + +class ProductStatusRollback implements Hookable {🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
68-74: 🛠️ Refactor suggestionUse proper table reference in SQL query.
Use
{$wpdb->posts}instead of direct table reference for better compatibility with custom table prefixes.🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
123-123:⚠️ Potential issueFix coding standard violation.
Remove space between increment operator and variable as per PHPCS.
-++ $processed; ++$processed;🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 123-123:
Expected no spaces between the increment operator and $processed; 1 found🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
66-78: 🛠️ Refactor suggestionAdd transaction handling for batch operations.
Wrap the batch operation in a database transaction to ensure data consistency in case of failures.
🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
138-140:⚠️ Potential issuePrevent infinite loop in queue scheduling.
Check if there are more products to process before scheduling the next batch.
- // Schedule next batch - WC()->queue()->add( 'dokan_rollback_product_status_reject_to_draft_schedule', array(), self::QUEUE_GROUP ); + // Check if there are more products to process + $more_products = $wpdb->get_var( + $wpdb->prepare( + "SELECT EXISTS (SELECT 1 FROM {$wpdb->posts} WHERE post_type = 'product' AND post_status = %s LIMIT 1)", + 'reject' + ) + ); + + if ( $more_products ) { + WC()->queue()->add( 'dokan_rollback_product_status_reject_to_draft_schedule', array(), self::QUEUE_GROUP ); + }🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
🧹 Nitpick comments (1)
includes/ProductStatusRollback.php (1)
22-34: Document rationale for BATCH_SIZE value.Consider adding a comment explaining why 10 was chosen as the batch size. This helps future maintainers understand if this value can be adjusted based on system resources or performance requirements.
🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
includes/ProductStatusRollback.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/ProductStatusRollback.php
[failure] 123-123:
Expected no spaces between the increment operator and $processed; 1 found
🪛 GitHub Actions: Inspections
includes/ProductStatusRollback.php
[error] Expected no spaces between the increment operator and $processed; 1 found
🔇 Additional comments (2)
includes/ProductStatusRollback.php (2)
1-8: LGTM! Well-organized namespace and imports.The file structure follows PHP best practices with proper namespace declaration and relevant use statements.
🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
9-21: Enhance class documentation with package information.While the documentation is good and includes use cases as requested, consider adding:
@packagetag to indicate the package/module this class belongs to- Replace
DOKAN_PRO_SINCEwith actual version number🧰 Tools
🪛 GitHub Actions: Inspections
[error] Expected no spaces between the increment operator and $processed; 1 found
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/ProductStatusRollback.php (1)
10-21: Enhance documentation to clarify timing of rollback operationsWhile the documentation clearly describes what the class does, it would be helpful to specify when these rollback operations occur (e.g., during plugin deactivation) and why products need to be rolled back from 'reject' to 'draft' status.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
includes/ProductStatusRollback.php(1 hunks)
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/ProductStatusRollback.php
[failure] 124-124:
Expected no spaces between the increment operator and $processed; 1 found
🔇 Additional comments (7)
includes/ProductStatusRollback.php (7)
40-40: Replace DOKAN_PRO_SINCE placeholder with actual version numberThe @SInCE tag is using a placeholder value that needs to be replaced with the actual version number where this feature will be released.
42-44: Constructor implementation looks goodWhile a past review suggested removing the constructor, I believe it's appropriate to keep it as it provides a clear initialization point for registering hooks when the class is instantiated. The implementation follows the Hookable interface contract correctly.
53-55: Hook registration looks goodThe hook registration is clean, follows WordPress naming conventions, and properly sets up the batch processing callback.
124-124: Fix coding standard violation: remove space between increment operator and variableAccording to the coding standards, there should be no space between the increment operator and the variable.
-++ $processed; ++$processed;🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 124-124:
Expected no spaces between the increment operator and $processed; 1 found
125-134: Error handling and logging implementation looks goodThe implementation uses proper error handling with nested try-catch blocks and appropriate logging at both individual product and batch levels.
Also applies to: 141-149
69-75: 🛠️ Refactor suggestionOptimize database query and add transaction handling
The database query could be improved in several ways:
- Use
{$wpdb->posts}for better compatibility with custom table prefixes- Add transaction handling for batch operations
- Use appropriate index hint for better performance
+ $wpdb->query('START TRANSACTION'); $products = $wpdb->get_col( $wpdb->prepare( - "SELECT ID FROM $wpdb->posts WHERE post_type = 'product' AND post_status = %s ORDER BY ID LIMIT %d", + "SELECT ID FROM {$wpdb->posts} USE INDEX (type_status_date) WHERE post_type = 'product' AND post_status = %s ORDER BY ID LIMIT %d", 'reject', self::BATCH_SIZE, ) );Likely invalid or redundant comment.
139-141:⚠️ Potential issuePrevent infinite loop in queue scheduling
The current implementation always schedules the next batch without checking if there are more products to process. This could lead to infinite scheduling if no products are found in the next batch.
- // Schedule next batch - WC()->queue()->add( 'dokan_rollback_product_status_reject_to_draft_schedule', array(), self::QUEUE_GROUP ); + // Check if there are more products to process before scheduling next batch + $more_products = $wpdb->get_var( + $wpdb->prepare( + "SELECT EXISTS (SELECT 1 FROM {$wpdb->posts} WHERE post_type = 'product' AND post_status = %s LIMIT 1)", + 'reject' + ) + ); + + if ( $more_products ) { + WC()->queue()->add( 'dokan_rollback_product_status_reject_to_draft_schedule', array(), self::QUEUE_GROUP ); + }Likely invalid or redundant comment.
mralaminahamed
force-pushed
the
feat/pending-product-rejection-integration
branch
from
e8ea01e to
c3d40f2
Compare
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🧹 Nitpick comments (20)
tests/pw/tests/e2e/storeAppearance.spec.ts (5)
1-3: Use consistent imports & remove commented-out imports if no longer needed
SinceCustomerPageis commented out, consider removing or uncommenting it if you plan to use it in the future. The new import forStoreAppearanceis fine; just ensure your imports reflect the intended usage.
13-13: Follow up on the TODO
A TODO about removing default Dokan store sidebar content is noted. If this is part of the PR scope, address it before merging to prevent stale tasks.
16-19: Check performance overhead of new browser contexts
Creating a new browser context for the customer is valid, but be mindful of performance if scaling tests. Ensure you close this context if you create more context instances in the suite.
27-29: Add assertions to verify the map is actually hidden
After disabling the store map, it might help to add explicit verification that the map element is indeed removed or hidden.
37-39: Revisit skipped test for vendor info
This test is skipped, possibly for future debugging or development. Unskip or remove it if no longer needed; otherwise, ensure it’s revisited so vendor info coverage isn’t overlooked.tests/pw/pages/storeAppearance.ts (2)
15-18: Consider adding an assertion after hiding the map
disableMapOnStoreSidebarensures the element is not visible, but adding a small assertion to confirm the side effect (e.g., an alternative selector is displayed or a user-facing message) can make the test more robust.
20-23: Use consistent navigation methods
Here you useawait this.goto, whereas indisableMapOnStoreSidebar, you useawait this.goIfNotThere. Consider standardizing the navigation approach across all methods for consistency.tests/pw/tests/e2e/privacyPolicy.spec.ts (2)
19-20: Remove or restore commented code
The calls to set the store contact form widget are commented out. If this is intentional, consider removing them or adding a clear comment explaining why they’re disabled, to avoid confusion.
44-44: Check thoroughness of new test
By no longer skipping this test, ensure it adequately verifies that the store contact form is indeed disabled on the storefront. If coverage is partial, consider adding additional checks.tests/pw/tests/e2e/catalogmode.spec.ts (1)
58-62: Use a try/finally block to guarantee cleanup
StoringpreviousSettingsand restoring it later is great. For added reliability, wrap the main test logic in a try/finally so your test always restores the previous state, even if the test fails.tests/pw/e2e.config.ts (1)
28-28: Shortened default expectation timeout.
Decreasing the expect timeout from 15 to 10 seconds speeds up failing tests, but could increase transient failures in resource-heavy or slower CI environments. Consider monitoring test stability.tests/pw/tests/e2e/euCompliance.spec.ts (1)
113-113: Check for synchronous fallback after resetting.
You're restoring the old settings withsetOptionValue. Consider handling potential errors or concurrency issues that might occur if multiple tests attempt to modify the same settings simultaneously.tests/pw/utils/dbData.ts (3)
233-234: Check map defaults.
store_map: 'on'andmap_api_source: 'google_maps'are now the defaults. This can increase dependency on Google Maps APIs. Validate you have the proper keys and fallback logic for users without a Google Maps setup.
243-243: Disable or enable store sidebar with caution.
enable_theme_store_sidebar: 'off'helps keep a minimal interface, but ensure you have coverage for scenarios relying on a store sidebar.
1424-1429: Store contact form widget consistency.
Renaming or reorganizing this widget might break legacy references. Confirm that existing tests or pages referencing the contact form still function.tests/pw/pages/selectors.ts (3)
1457-1460: Simplify selector paths for better maintainabilityThe following selectors have been updated to use more concise class-based selectors:
-upgradeToProText: 'div.modal-content p.upgrade-text', -upgradeToPro: 'div.modal-content a.upgrade-button', -proCard: 'div.modal-content div.promo-card', -alreadyUpdated: 'div.modal-content a.already-updated', +upgradeToProText: '.upgrade-text', +upgradeToPro: '.upgrade-button', +proCard: '.promo-card', +alreadyUpdated: '.already-updated',This change makes the selectors more maintainable by:
- Removing unnecessary parent element references
- Using simpler class-based selectors
- Reducing selector specificity
7506-7522: Fix inconsistent indentation and remove redundant commentsThe indentation is inconsistent in this section and there are some commented out lines that should be cleaned up:
- storeBanner: '.profile-info-img', - profileInfoHead: '.profile-info-head', - profileImage: '.profile-img.profile-img-circle', - storeName: '.profile-info-head .store-name', - verifiedIcon: '//div[@data-original-title="Verified"]', - verifiedIconByIcon: (icon: string) => `//div[@data-original-title="Verified"]//i[@class="${icon}"]`, - profileInfo: '.profile-info', - storeInfo: '.dokan-store-info', - storeAddress: '.dokan-store-address', - storePhone: '.dokan-store-phone', - storeEmail: '.dokan-store-email', - // storeRating: '.dokan-store-rating', - // storeOpenClose: '.dokan-store-open-close', - storeSocial: '.store-social',The code should be properly indented and commented out lines should be removed:
storeBanner: '.profile-info-img', profileInfoHead: '.profile-info-head', profileImage: '.profile-img.profile-img-circle', storeName: '.profile-info-head .store-name', verifiedIcon: '//div[@data-original-title="Verified"]', verifiedIconByIcon: (icon: string) => `//div[@data-original-title="Verified"]//i[@class="${icon}"]`, profileInfo: '.profile-info', storeInfo: '.dokan-store-info', storeAddress: '.dokan-store-address', storePhone: '.dokan-store-phone', storeEmail: '.dokan-store-email', storeSocial: '.store-social',
7662-7662: Fix trailing comma styleThe trailing comma is inconsistent with the rest of the codebase:
-storeMap: 'div#dokan-store-location', -storeOpenCloseTime: 'div.dokan-store-open-close', +storeMap: 'div#dokan-store-location', +storeOpenCloseTime: 'div.dokan-store-open-close', // Add trailing commaAdd a trailing comma to maintain consistency with the codebase style.
.github/workflows/e2e_api_tests.yml (2)
200-200: Avoid leaving commented-out steps for clarity.These lines for "Find PR comment by github-actions[bot]" are commented out. If they are no longer needed, consider removing them to avoid confusion.
- # - name: Find PR comment by github-actions[bot] - # uses: peter-evans/find-comment@v3 + # (Optional) Remove commented block if no longer required
211-211: Avoid leaving commented-out steps for clarity.Similarly, these lines for creating or updating PR comments are commented out. If unneeded, you may want to remove them, or if you intend to restore them in the future, note the reason with a comment.
🛑 Comments failed to post (1)
tests/pw/pages/storeAppearance.ts (1)
10-13: 🛠️ Refactor suggestion
Remove unnecessary constructor
The constructor is empty and flagged by the linter as unnecessary. Remove it to simplify the class definition, unless you plan to extend logic here soon.export class StoreAppearance extends BasePage { - constructor(page: Page) { - super(page); - } + // Simply rely on the parent class constructor if no special logic is needed }📝 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.export class StoreAppearance extends BasePage { // Simply rely on the parent class constructor if no special logic is needed }🧰 Tools
🪛 Biome (1.9.4)
[error] 11-13: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
mralaminahamed
force-pushed
the
feat/pending-product-rejection-integration
branch
from
c3d40f2 to
8ab1b11
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
includes/DependencyManagement/Providers/CommonServiceProvider.php (2)
69-71: Add a brief docblock explaining the purpose of the newly registered service.
It's helpful to maintain clarity on why theProductStatusRollbackservice is being introduced, especially for future contributors who will refer to this code.🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 70-70:
Found precision alignment of 1 spaces.
[warning] 71-71:
Found precision alignment of 1 spaces.
70-71: Align code formatting to address PHPCS warnings.
Adjust spacing to maintain consistent indentation.Apply this diff to fix indentation:
- ->addShared( \WeDevs\Dokan\ProductStatusRollback::class, \WeDevs\Dokan\ProductStatusRollback::class ) - ->addTag( self::TAG ); + ->addShared(\WeDevs\Dokan\ProductStatusRollback::class, \WeDevs\Dokan\ProductStatusRollback::class) + ->addTag(self::TAG);🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 70-70:
Found precision alignment of 1 spaces.
[warning] 71-71:
Found precision alignment of 1 spaces.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
includes/DependencyManagement/Providers/CommonServiceProvider.php(1 hunks)includes/ProductStatusRollback.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- includes/ProductStatusRollback.php
🧰 Additional context used
🪛 GitHub Check: Run PHPCS inspection
includes/DependencyManagement/Providers/CommonServiceProvider.php
[warning] 70-70:
Found precision alignment of 1 spaces.
[warning] 71-71:
Found precision alignment of 1 spaces.
mrabbani
added
👍 Dev Review Done
and removed
Needs: Dev Review
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
New Features
Improvements