enhance: unify product brand integration with dedicated template and manager methods #2773
Conversation
…duct add, update, and edit actions
mralaminahamed
added
Needs: Testing
|
""" WalkthroughThis change refactors the handling of product brands in the product management workflow. Brand selection is moved from a static form field to a dynamic, hook-based approach, with new methods for saving and retrieving brand data. A dedicated template for brand selection is introduced, and legacy brand-handling code is removed. Changes
Sequence Diagram(s)sequenceDiagram
participant Vendor as Vendor
participant Dashboard as Vendor Dashboard
participant Hooks as Product Hooks
participant Manager as Product Manager
Vendor->>Dashboard: Edit Product
Dashboard->>Hooks: do_action('dokan_product_edit_after_pricing_fields')
Hooks->>Manager: get_brands(product_id)
Manager-->>Hooks: brand terms
Hooks->>Dashboard: include product-brand.php template with brand data
Vendor->>Dashboard: Submit Product Form
Dashboard->>Hooks: do_action('dokan_new_product_added' or 'dokan_product_updated')
Hooks->>Manager: save_brands(product_id, brand_ids)
Manager-->>Hooks: brands saved
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 phpcs (3.7.2)includes/Product/Manager.php📜 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 (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (1)
includes/Product/Manager.php (1)
805-821: Consider simplifying the error handling logic.The method has redundant error checking since
get_brands()already handles WP_Error cases by returning an empty array.public function get_brand_ids( int $product_id ): array { $brands = $this->get_brands( $product_id, 'ids' ); - if ( is_wp_error( $brands ) || count( $brands ) === 0 ) { + if ( count( $brands ) === 0 ) { return []; } return $brands; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
includes/Dashboard/Templates/Products.php(0 hunks)includes/Product/Hooks.php(2 hunks)includes/Product/Manager.php(1 hunks)templates/products/edit-product-single.php(1 hunks)templates/products/product-brand.php(1 hunks)
💤 Files with no reviewable changes (1)
- includes/Dashboard/Templates/Products.php
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (7)
includes/Product/Manager.php (2)
771-784: LGTM! Brand saving method follows WordPress standards.The
save_brands()method correctly sanitizes input and uses the appropriate WordPress functionwp_set_object_terms()for taxonomy management. Theabsint()sanitization ensures brand IDs are valid integers.
786-803: Excellent error handling and flexibility in brand retrieval.The
get_brands()method provides good flexibility with the$fieldsparameter and properly handles WP_Error cases by returning an empty array. The use of type hints enhances code reliability.templates/products/product-brand.php (1)
1-24: Well-structured template with proper security measures.The template correctly implements:
- Proper escaping functions (
esc_html_e,esc_attr_e,esc_attr,esc_html)- Internationalization with appropriate text domain
- Semantic HTML structure with proper labels and form controls
- Accessibility considerations with
data-placeholderattributetemplates/products/edit-product-single.php (1)
392-392: Excellent modularization with hook-based architecture.The replacement of static brand selection UI with the
dokan_product_edit_after_pricing_fieldshook promotes extensibility and separation of concerns. The hook placement after pricing fields is logical and maintains good UX flow.includes/Product/Hooks.php (3)
36-39: Well-integrated hook registration for brand management.The hooks are properly registered for the complete product lifecycle:
- Brand saving on product creation and updates
- Template rendering via dedicated hook
This ensures consistent brand handling across all product operations.
577-594: Robust template rendering with proper security checks.The
add_product_brand_template()method correctly:
- Validates user capabilities with
dokan_edit_product- Retrieves brand data using the Manager class
- Uses proper template loading with
dokan_get_template_part()- Follows WordPress naming conventions
596-613: Secure brand updating with proper data handling.The
update_product_brands_by_id()method implements:
- Capability verification for product editing
- Safe array access with null coalescing operator
- Integration with Manager's brand saving method
- Proper parameter typing
mrabbani
added
Needs: Author Reply
and removed
Needs: Dev Review
mralaminahamed
added
Needs: Dev Review
mrabbani
added
Dev Review Done
and removed
Needs: Dev Review
dev-shahed
added
QA In Progress
and removed
Needs: Testing
|
@mralaminahamed All YITH brand moved into the WooCommerce brand field - OK |
dev-shahed
added
🎉 QA Approved
All Submissions:
Changes proposed in this Pull Request:
This PR enhances the product brand integration in Dokan by refactoring the brand management system with a more structured approach. The changes include:
product-brand.phptemplate for better code organization and reusabilitysave_brands(),get_brands(),get_brand_ids())Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title: Enhanced Product Brand Integration with Dedicated Template and Manager Methods
Detailed Description:
Previous Behavior: Brand management logic was directly embedded in the Products template class with inline HTML and basic functionality.
New Behavior: Brand management now uses a dedicated template system with proper Manager methods for data handling. The code follows WordPress coding standards with improved security, better organization, and enhanced maintainability. Brand operations are now handled through dedicated methods in the Product Manager class with proper hooks integration.
Before Changes
After Changes
Feature Video (optional)
N/A - This is a code refactoring enhancement
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Refactor
Bug Fixes