-
Notifications
You must be signed in to change notification settings - Fork 215
feat: add dedicated vendor onboarding page and separate registration … #3047
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
base: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a Vendor Onboarding feature: settings to select an onboarding page, installer and upgrade logic to create the page, a shortcode and template to render onboarding (login + registration), an admin notice when the onboarding page is missing/unpublished, and a registration form adjustment. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant WP_Shortcode as Shortcode
participant Template
participant WP_Core as WP
participant Dokan_Options as Options
Note over User,Browser: User requests Vendor Onboarding page
User->>Browser: GET /vendor-onboarding
Browser->>WP: Request page (WP)
WP->>Options: get_option('dokan_pages')
Options-->>WP: pages map (maybe vendor_onboarding id)
WP->>Shortcode: render 'dokan-vendor-onboarding-registration'
Shortcode->>Options: (validate page / assets)
Shortcode->>WP_Core: enqueue scripts/styles
Shortcode->>Template: capture templates/account/vendor-onboarding.php with data
Template-->>Shortcode: rendered HTML
Shortcode-->>WP: return HTML
WP-->>Browser: deliver page
Note over WP,Options: Admin notice flow (background)
WP->>Options: admin init -> Manager::show_vendor_onboarding_page_notice()
Manager->>Options: check page existence/state
Options-->>Manager: page id / post status
Manager-->>WP: append admin notice if missing/unpublished
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (6)
templates/account/vendor-onboarding.php (3)
25-319: Consider moving inline CSS to a separate stylesheet.The extensive inline CSS (295 lines) has several drawbacks:
- Cannot be cached by browsers
- Increases initial page load size
- Harder to maintain and reuse
- No benefit from CSS minification
Consider creating a dedicated CSS file (e.g.,
assets/css/vendor-onboarding.css) and enqueuing it via the shortcode handler'srender_shortcodemethod.🔎 Example implementation
In
includes/Shortcodes/VendorOnboardingRegistration.php, add:public function render_shortcode( $atts ) { if ( is_user_logged_in() ) { return esc_html__( 'You are already logged in', 'dokan-lite' ); } dokan()->scripts->load_form_validate_script(); wp_enqueue_script( 'dokan-form-validate' ); wp_enqueue_script( 'dokan-vendor-registration' ); wp_enqueue_script( 'dokan-vendor-address' ); + wp_enqueue_style( 'dokan-vendor-onboarding', DOKAN_PLUGIN_ASSEST . '/css/vendor-onboarding.css', [], DOKAN_PLUGIN_VERSION ); $data = dokan_get_seller_registration_form_data();
321-342: Consider moving inline JavaScript to a separate file.The inline JavaScript for subscription pack switching would be better maintained as a separate file that can be:
- Minified in production
- Cached by browsers
- Tested independently
- Reused if needed
Consider creating
assets/js/vendor-onboarding.jsand enqueuing it in the shortcode handler.
206-224: Remove commented-out code.The commented CSS block (lines 206-224) should be removed to keep the codebase clean. If this styling might be needed later, consider using version control history instead.
🔎 Proposed fix
- /* .dokan-vendor-onboarding .button, - .dokan-vendor-onboarding button[type="submit"], - .dokan-vendor-onboarding .dokan-btn-theme { - background: #000 !important; - color: #fff !important; - border: none !important; - padding: 14px 28px !important; - font-weight: 600 !important; - text-transform: uppercase; - letter-spacing: 1px; - border-radius: 4px !important; - cursor: pointer; - width: auto; - transition: opacity 0.3s; - } - - .dokan-vendor-onboarding button:hover { - opacity: 0.8; - } */ - .dokan-registration-section .help-block,includes/Shortcodes/VendorOnboardingRegistration.php (1)
16-19: Consider documenting the unused$attsparameter.The
$attsparameter is required by the WordPress shortcode API but is unused here. While this is acceptable, you can suppress the PHPMD warning by documenting it in the PHPDoc.🔎 Suggested PHPDoc update
/** * Vendor onboarding form shortcode callback * + * @param array $atts Shortcode attributes (unused). + * * @return string */ public function render_shortcode( $atts ) {includes/Admin/Notices/Manager.php (2)
239-305: Consider notice scope and logic simplification.The notice logic is sound, but there are a couple of optional improvements:
The
scope => 'global'will show this notice on all admin pages. Consider if this is the intended behavior or if it should be limited to Dokan-related pages only.Line 254's condition
$page_id && (int) $page_id > 0can be simplified since the helper methodis_vendor_onboarding_page_configured()already performs empty/zero checks.🔎 Suggested simplification
$dokan_pages = get_option( 'dokan_pages', [] ); $page_id = isset( $dokan_pages['vendor_onboarding'] ) ? $dokan_pages['vendor_onboarding'] : 0; // Check if page was configured but is now deleted or unpublished - if ( $page_id && (int) $page_id > 0 ) { + if ( $page_id ) { $page = get_post( $page_id );
314-337: Minor redundancy in validation checks.The validation logic is thorough and correct, but there's slight redundancy between the
empty()check on line 318 and the! $page_idcheck on line 325. Sinceempty()already catches falsy values, the subsequent check is defensive but not strictly necessary.This is a minor point and the current defensive approach is acceptable for clarity.
🔎 Optional simplification
$page_id = $dokan_pages['vendor_onboarding']; - // Verify the page ID is not zero or empty - if ( ! $page_id || 0 === (int) $page_id ) { - return false; - } - // Verify the page actually exists and is published $page = get_post( $page_id );The
empty()check on line 318 already ensures$page_idis truthy, andget_post()handles invalid IDs gracefully.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
includes/Admin/Notices/Manager.phpincludes/Admin/Settings.phpincludes/Install/Installer.phpincludes/Shortcodes/Shortcodes.phpincludes/Shortcodes/VendorOnboardingRegistration.phpincludes/Upgrade/Upgrades.phpincludes/Upgrade/Upgrades/V_4_2_2.phptemplates/account/vendor-onboarding.phptemplates/account/vendor-registration.phptemplates/global/seller-registration-form.php
🧰 Additional context used
🧬 Code graph analysis (6)
templates/account/vendor-onboarding.php (1)
includes/functions.php (1)
dokan_get_template_part(843-882)
includes/Shortcodes/VendorOnboardingRegistration.php (5)
includes/Shortcodes/Shortcodes.php (1)
Shortcodes(13-50)includes/Abstracts/DokanShortcode.php (1)
DokanShortcode(5-22)includes/Assets.php (1)
load_form_validate_script(967-989)includes/template-tags.php (1)
dokan_get_seller_registration_form_data(611-649)includes/functions.php (1)
dokan_get_template_part(843-882)
includes/Upgrade/Upgrades/V_4_2_2.php (1)
includes/Upgrade/Upgrades.php (1)
Upgrades(5-121)
includes/Admin/Notices/Manager.php (1)
includes/REST/AdminMiscController.php (1)
get_option(87-93)
includes/Shortcodes/Shortcodes.php (1)
includes/Shortcodes/VendorOnboardingRegistration.php (1)
VendorOnboardingRegistration(7-35)
includes/Upgrade/Upgrades.php (1)
includes/Upgrade/Upgrades/V_4_2_2.php (1)
V_4_2_2(14-55)
🪛 GitHub Check: Run PHPCS inspection
templates/account/vendor-onboarding.php
[warning] 206-206:
Found precision alignment of 3 spaces.
includes/Shortcodes/VendorOnboardingRegistration.php
[failure] 35-35:
Expected 1 newline at end of file; 0 found
includes/Admin/Notices/Manager.php
[failure] 256-256:
Whitespace found at end of line
🪛 PHPMD (2.15.0)
includes/Shortcodes/VendorOnboardingRegistration.php
16-16: Avoid unused parameters such as '$atts'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (12)
templates/account/vendor-registration.php (1)
116-116: LGTM! Improved consistency with WooCommerce styling.The button class change from Dokan-specific classes to WooCommerce standard classes improves styling consistency across the platform.
includes/Install/Installer.php (1)
250-255: LGTM! Vendor Onboarding page properly integrated into installer.The new page is correctly configured with appropriate title, slug, page_id, and shortcode reference. This ensures fresh installations will have the onboarding page available.
includes/Upgrade/Upgrades.php (1)
48-48: LGTM! Upgrade path correctly registered.The new upgrade class V_4_2_2 is properly registered and will ensure existing installations get the Vendor Onboarding page created.
includes/Admin/Settings.php (1)
814-821: LGTM! Admin settings field properly configured.The vendor_onboarding settings field is correctly structured and follows the same pattern as other page selector fields in the dokan_pages section.
includes/Shortcodes/Shortcodes.php (2)
11-11: LGTM! Import correctly added.The VendorOnboardingRegistration class is properly imported.
35-35: LGTM! Shortcode properly registered.The dokan-vendor-onboarding-registration shortcode is correctly instantiated and registered, matching the shortcode reference in the page content.
templates/account/vendor-onboarding.php (2)
3-5: LGTM! Proper notice handling.The conditional check for
wc_print_noticesbefore calling it is good defensive programming.
9-23: LGTM! Clean and semantic HTML structure.The two-column layout with separate login and registration sections provides a good user experience. The use of WooCommerce and Dokan template functions ensures consistency.
includes/Upgrade/Upgrades/V_4_2_2.php (1)
23-54: No action needed - the method is automatically invoked.The
create_vendor_onboarding_page()method will be automatically called. TheDokanUpgrader::run()method uses reflection to dynamically invoke all public static methods in the child class (exceptrunandupdate_db_version), so your method will be executed as part of the upgrade flow without requiring explicit invocation.Likely an incorrect or invalid review comment.
includes/Shortcodes/VendorOnboardingRegistration.php (2)
1-9: LGTM! Clean class structure.The class properly extends
DokanShortcodeand follows the established pattern for Dokan shortcodes. The shortcode name is descriptive and matches the registration inShortcodes.php.
21-31: LGTM! Proper script and template handling.The script enqueuing follows the established pattern, and the template rendering correctly uses output buffering with the standard Dokan template loading function.
includes/Admin/Notices/Manager.php (1)
54-54: LGTM! Proper hook registration.The filter is correctly registered following the same pattern as other admin notice hooks in this class.
| <p class="form-row form-group user-role vendor-customer-registration"> | ||
|
|
||
| <label class="radio"> | ||
| <input type="radio" name="role" value="customer"<?php checked( $role, 'customer' ); ?> class="dokan-role-customer"> | ||
| <?php esc_html_e( 'I am a customer', 'dokan-lite' ); ?> | ||
| </label> | ||
| <br/> | ||
| <label class="radio"> | ||
| <input type="radio" name="role" value="seller"<?php checked( $role, 'seller' ); ?> class="dokan-role-seller"> | ||
| <?php esc_html_e( 'I am a vendor', 'dokan-lite' ); ?> | ||
| </label> | ||
| <input type="hidden" name="role" value="customer"> | ||
| <?php do_action( 'dokan_registration_form_role', $role ); ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify usage of this template and check if vendor registration still works via existing flows
echo "=== Searching for usages of seller-registration-form template ==="
rg -n "seller-registration-form" --type=php -C3
echo -e "\n=== Checking for role field references in registration processing ==="
rg -n "role.*seller" --type=php -C3 -g '!**/node_modules/**' -g '!**/vendor/**'
echo -e "\n=== Looking for dokan_registration_form_role hook usage ==="
rg -n "dokan_registration_form_role" --type=php -C3Repository: getdokan/dokan
Length of output: 21703
🏁 Script executed:
#!/bin/bash
# Check where each registration form is called and entry points
echo "=== Finding where seller-registration-form template is called ==="
rg -n "dokan_get_seller_registration_form_data\|seller-registration-form" --type=php -A2 -B2 | head -100
echo -e "\n=== Finding where vendor-registration template is called ==="
rg -n "vendor-registration\|dokan_vendor_registration" --type=php -A2 -B2 | head -80
echo -e "\n=== Checking dokan_get_seller_registration_default_role usage ==="
rg -n "dokan_seller_registration_default_role" --type=php -A3 -B3
echo -e "\n=== Checking registration form role handling in recent context ==="
rg -n "dokan_get_seller_registration_form_data|dokan_get_seller_registration_default_role" --type=phpRepository: getdokan/dokan
Length of output: 1557
🏁 Script executed:
#!/bin/bash
# Understanding the shortcode implementations
echo "=== VendorRegistration shortcode implementation ==="
cat includes/Shortcodes/VendorRegistration.php
echo -e "\n=== VendorOnboardingRegistration shortcode implementation ==="
cat includes/Shortcodes/VendorOnboardingRegistration.php
echo -e "\n=== Checking template-tags.php function that calls the template ==="
sed -n '440,470p' includes/template-tags.phpRepository: getdokan/dokan
Length of output: 3073
🏁 Script executed:
#!/bin/bash
# Check how the default role is used in the dokan_get_seller_registration_form_data function
echo "=== Full context of dokan_get_seller_registration_form_data function ==="
sed -n '611,680p' includes/template-tags.php
echo -e "\n=== Verify what the $role parameter receives in the template ==="
sed -n '442,456p' includes/template-tags.php
echo -e "\n=== Check if there's any documentation on the change ==="
rg -n "hardcoded\|role.*customer\|role.*fixed" --type=php -C2 | head -50
echo -e "\n=== Verify vendor registration form has seller role ==="
sed -n '40,50p' templates/account/vendor-registration.phpRepository: getdokan/dokan
Length of output: 3601
Confirm that the generic WooCommerce registration form is now customer-only by design.
The form hardcoding to "customer" is intentional: vendor registration has been separated into a dedicated [dokan-vendor-registration] shortcode (templates/account/vendor-registration.php) which hardcodes role="seller". The generic WooCommerce form via woocommerce_register_form hook is now exclusively for customer registration.
However, there is a code quality concern: the $role parameter is still passed to the template (line 454) from dokan_get_seller_registration_form_data(), but the template ignores it with a hardcoded hidden input. If the default role were ever filtered to 'seller', this mismatch would cause confusion. Either remove the unused parameter or use it consistently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
includes/Admin/Notices/Manager.php (1)
260-293: Reduce code duplication in notice descriptions.The notice description building logic (lines 260-268, 272-280, and 285-293) is duplicated three times with only the message text varying. Consider extracting this into a helper method.
🔎 Proposed refactor to reduce duplication
Add a private helper method:
private function build_vendor_onboarding_settings_link() { $settings_url = add_query_arg( [ 'page' => 'dokan#/tools' ], admin_url( 'admin.php' ) ); return sprintf( '<a href="%s">%s</a>', esc_url( $settings_url ), esc_html__( 'Dokan → Tools → Page Installation', 'dokan-lite' ) ); }Then simplify the notice descriptions:
- $notice_description = sprintf( - /* translators: %s: Settings link */ - esc_html__( 'The configured Vendor Onboarding page has been deleted. Please set a new page in %s', 'dokan-lite' ), - sprintf( - '<a href="%s">%s</a>', - esc_url( $settings_url ), - esc_html__( 'Dokan → Tools → Page Installation', 'dokan-lite' ) - ) - ); + $notice_description = sprintf( + /* translators: %s: Settings link */ + esc_html__( 'The configured Vendor Onboarding page has been deleted. Please set a new page in %s', 'dokan-lite' ), + $this->build_vendor_onboarding_settings_link() + );Apply the same pattern to the other two notice descriptions.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
includes/Admin/Notices/Manager.phpincludes/Shortcodes/VendorOnboardingRegistration.phptemplates/account/vendor-onboarding.php
🚧 Files skipped from review as they are similar to previous changes (1)
- templates/account/vendor-onboarding.php
🧰 Additional context used
🧬 Code graph analysis (2)
includes/Admin/Notices/Manager.php (1)
includes/REST/AdminMiscController.php (1)
get_option(87-93)
includes/Shortcodes/VendorOnboardingRegistration.php (5)
includes/Shortcodes/Shortcodes.php (1)
Shortcodes(13-50)includes/Abstracts/DokanShortcode.php (1)
DokanShortcode(5-22)includes/Assets.php (1)
load_form_validate_script(967-989)includes/template-tags.php (1)
dokan_get_seller_registration_form_data(611-649)includes/functions.php (1)
dokan_get_template_part(843-882)
🪛 PHPMD (2.15.0)
includes/Shortcodes/VendorOnboardingRegistration.php
16-16: Avoid unused parameters such as '$atts'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (3, 3)
🔇 Additional comments (5)
includes/Shortcodes/VendorOnboardingRegistration.php (2)
17-19: LGTM!The early return for logged-in users is appropriate and prevents unnecessary processing.
21-33: LGTM!The implementation follows WordPress and Dokan conventions correctly:
- Scripts are properly enqueued with dependencies
- Template rendering uses standard output buffering
- Filter hook provides extensibility
includes/Admin/Notices/Manager.php (3)
54-54: LGTM!The filter hook registration is consistent with other notice methods in the class.
233-233: Verify the @SInCE version number.The
@since 3.7.11tag should be verified against the actual version where this feature will be released. Version numbers are often placeholders that need updating before release.Please confirm the correct version number for this feature release, or update the @SInCE tags (also on line 310) to match the target release version.
314-337: LGTM!The validation logic is thorough and handles all edge cases correctly:
- Validates option exists and has a value
- Checks page ID is valid (not 0 or empty)
- Verifies the page exists in the database
- Confirms the page status is 'publish'
…flows
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
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
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.