-
Notifications
You must be signed in to change notification settings - Fork 151
fix: show subscription plan in field options #1717
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
fix: show subscription plan in field options #1717
Conversation
WalkthroughTwo template files are updated to check for the namespaced Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes The changes follow a consistent, repetitive pattern across two files—straightforward class reference replacements with no logic modifications or error-handling alterations. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
admin/form-builder/assets/js/components/field-visibility/template.php(1 hunks)assets/js-templates/form-components.php(1 hunks)
🔇 Additional comments (2)
admin/form-builder/assets/js/components/field-visibility/template.php (2)
742-742: Good consistency with the parallel template.This change mirrors the update in
admin/form-builder/assets/js/components/field-visibility/template.phpat line 95. Both files now consistently check for the namespacedWeDevs\Wpuf\Admin\Subscriptionclass instead of the legacyWPUF_Subscription.Note: The same verification concerns mentioned in the other file apply here as well.
95-95: Migration verified: class check and instantiation are correct.The new namespaced
WeDevs\Wpuf\Admin\Subscriptionclass exists and is properly registered in the service container atwpuf.php:173. The template'sclass_exists()check will pass, andwpuf()->subscriptionon line 96 correctly resolves to the instantiated service container object. The methodget_subscription()is available in the new class, so the change will function correctly without breaking existing installations.
| <?php | ||
|
|
||
| if ( class_exists( 'WPUF_Subscription' ) ) { | ||
| if ( class_exists( 'WeDevs\Wpuf\Admin\Subscription' ) ) { |
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
Verify the namespaced class is available.
The class name has been correctly updated to use the namespaced WeDevs\Wpuf\Admin\Subscription instead of the legacy WPUF_Subscription. This aligns with modernizing the codebase architecture.
Run the following script to verify the new class exists and is properly autoloaded:
🏁 Script executed:
#!/bin/bash
# Verify the namespaced Subscription class exists in the codebase
# Search for the class definition
rg -n "class\s+Subscription" --type=php -C3
# Search for namespace declaration
rg -n "namespace\s+WeDevs\\\\Wpuf\\\\Admin" --type=php -C2
# Verify there are no remaining references to the old class name in visibility contexts
rg -n "WPUF_Subscription" --type=php -C2Length of output: 13571
Critical inconsistency detected: namespace migration is incomplete.
The template change at line 742 references the new WeDevs\Wpuf\Admin\Subscription class, but the codebase still heavily uses the legacy WPUF_Subscription class:
- Frontend rendering code (
includes/class-field-manager.php:311,includes/class-frontend-render-form.php:917,class/render-form.php:611) continues to callWPUF_Subscription::init()->get_user_pack() - The template attempts to call
wpuf()->subscription->get_subscriptions()which must be properly registered in the service container for the new class
If the service container registration is missing or the new class isn't properly hooked into the plugin's initialization, this change will silently fail and subscriptions won't display in field visibility options.
Verify:
- That
wpuf()->subscriptionis properly initialized withWeDevs\Wpuf\Admin\Subscriptionin the service container - That the new class provides the same interface as the legacy
WPUF_Subscriptionclass (specifically theget_subscriptions()method) - Whether the full namespace migration should be completed across all frontend code
🤖 Prompt for AI Agents
In assets/js-templates/form-components.php around line 742, the template now
references the namespaced WeDevs\Wpuf\Admin\Subscription but the codebase still
uses the legacy WPUF_Subscription and the service container may not register the
new class; ensure wpuf()->subscription is initialized to an instance of
WeDevs\Wpuf\Admin\Subscription (or register a service alias) in the plugin
bootstrap/service container, confirm the new class implements the same public
API used elsewhere (at least get_subscriptions()), and if necessary add an
adapter that exposes the legacy static methods or update all legacy calls (e.g.
WPUF_Subscription::init()->get_user_pack() and other frontend usages) to resolve
the subscription service from wpuf() so the template and frontend code
consistently use the same implementation.
fixes #1178
In Post Forms > Field Options > Visibility, selecting the
Subscription users onlyoption will now let the user select the subscription pack. Only the selected subscription pack holder will see this field in the frontend.Summary by CodeRabbit