-
Notifications
You must be signed in to change notification settings - Fork 206
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
Added commission settings sub-categories reset toggle. #2543
base: develop
Are you sure you want to change the base?
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
<label class="!p-0 m-0 !mb-[6px] block" for="_subscription_product_admin_commission_type"> | ||
{{__( 'Apply Parent Category Commission to All Subcategories ', 'dokan-lite' )}} | ||
|
||
<span class="dokan-tooltips-help tips" v-tooltip :title="__( 'When enabled, changing a parent category\'s commission rate will automatically update all its subscription. Disable this option to maintain independent commission rates for subcategories', 'dokan-lite' )"> |
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.
subscription might be typo?
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
🔭 Outside diff range comments (1)
src/admin/components/VendorCommissionFields.vue (1)
88-88
: 🛠️ Refactor suggestionAdd error handling for external dependencies.
The code assumes window.dokanAdmin and SweetAlert are always available. Consider adding error handling:
created() { - this.commissionTypes = window.dokanAdmin.commission_types ?? {}; + try { + if (!window.dokanAdmin) { + throw new Error('dokanAdmin not initialized'); + } + this.commissionTypes = window.dokanAdmin.commission_types ?? {}; + } catch (error) { + console.error('Failed to initialize commission types:', error); + this.commissionTypes = {}; + } } handleResetToggle(value) { + if (typeof Swal === 'undefined') { + console.error('SweetAlert not loaded'); + this.resetSubCategory = value; + this.vendorInfo.reset_sub_category = value; + return; + } // ... rest of the method }Also applies to: 126-139
♻️ Duplicate comments (1)
src/admin/components/VendorCommissionFields.vue (1)
19-20
:⚠️ Potential issueFix incorrect label attributes and text.
The label's
for
attribute references_subscription_product_admin_commission_type
which seems incorrect for this context. This appears to be copied from another section.Apply this diff to fix the label:
-<label class="!p-0 m-0 !mb-[6px] block" for="_subscription_product_admin_commission_type"> +<label class="!p-0 m-0 !mb-[6px] block" for="reset_sub_category_toggle">
🧹 Nitpick comments (3)
src/admin/components/VendorCommissionFields.vue (3)
39-39
: Add prop validation for resetSubCategory.Consider adding prop validation to ensure type safety.
<category-based-commission :value="categoryCommission" @change="onCategoryUpdate" :resetSubCategory="resetSubCategory" />In the CategoryBasedCommission component, add:
props: { resetSubCategory: { type: Boolean, required: true } }
119-139
: Improve code clarity and maintainability.Consider the following improvements:
- Extract SweetAlert strings to constants
- Rename
updatableValue
tocurrentValue
for clarity- Remove unnecessary
self
variable since arrow functions preservethis
handleResetToggle(value) { - const confirmTitle = value ? this.__("Enable Commission Inheritance Setting?", "dokan-lite") : this.__("Disable Commission Inheritance Setting?", "dokan-lite"); - const htmlText = value ? this.__("Parent category commission changes will automatically update all subcategories. Existing rates will remain unchanged until parent category is modified.", "dokan-lite") : this.__("Subcategories will maintain their independent commission rates when parent category rates are changed.", "dokan-lite"); - const confirmBtnText = value ? this.__("Enable", "dokan-lite") : this.__("Disable", "dokan-lite"); - const updatableValue = !value; - const self = this; + const ALERT_TEXTS = { + enable: { + title: this.__("Enable Commission Inheritance Setting?", "dokan-lite"), + html: this.__("Parent category commission changes will automatically update all subcategories. Existing rates will remain unchanged until parent category is modified.", "dokan-lite"), + confirm: this.__("Enable", "dokan-lite") + }, + disable: { + title: this.__("Disable Commission Inheritance Setting?", "dokan-lite"), + html: this.__("Subcategories will maintain their independent commission rates when parent category rates are changed.", "dokan-lite"), + confirm: this.__("Disable", "dokan-lite") + } + }; + const currentValue = !value; + const texts = value ? ALERT_TEXTS.enable : ALERT_TEXTS.disable; Swal.fire({ icon: "warning", - html: htmlText, - title: confirmTitle, + html: texts.html, + title: texts.title, showCancelButton: true, cancelButtonText: this.__("Cancel", "dokan-lite"), - confirmButtonText: confirmBtnText - }).then((response) => { - const status = response.isConfirmed ? value : updatableValue; - self.resetSubCategory = status; - - self.vendorInfo.reset_sub_category = status; + confirmButtonText: texts.confirm + }).then(({ isConfirmed }) => { + const status = isConfirmed ? value : currentValue; + this.resetSubCategory = status; + this.vendorInfo.reset_sub_category = status; }); }
111-113
: Simplify initialization logic.The current initialization uses a double negative and verbose property check.
-if ( this.vendorInfo.hasOwnProperty( 'reset_sub_category' ) ) { - this.resetSubCategory = this.vendorInfo.reset_sub_category !== false; -} +this.resetSubCategory = this.vendorInfo?.reset_sub_category ?? true;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/admin/components/VendorCommissionFields.vue
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (1, 3)
- GitHub Check: api tests (1, 1)
🔇 Additional comments (2)
src/admin/components/VendorCommissionFields.vue (2)
22-24
: LGTM! Clear and informative tooltip.The tooltip effectively explains the feature's behavior and implications.
66-165
: Add unit tests for the new feature.Consider adding tests to verify:
- Toggle initialization with different vendor info states
- Toggle interaction and state updates
- SweetAlert dialog interactions
- Error handling for missing dependencies
Would you like me to generate a test suite for this component?
All Submissions:
Changes proposed in this Pull Request:
Enhanced category commission experience.
Related Pull Request(s)
Closes
Changelog entry
Summary by CodeRabbit
New Features
UI Improvements