-
Notifications
You must be signed in to change notification settings - Fork 121
Add IPP plugin status tags to ZenDesk #8008
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
Conversation
You can test the changes from this Pull Request by:
|
WooCommerce/Classes/ViewRelated/Dashboard/Settings/Settings/SettingsViewModel.swift
Outdated
Show resolved
Hide resolved
We’re moving the logic to retrieve IPP plugin statuses directly to ZenDesk, as if a merchant attempts to open a ticket without passing through the SettingsView, we wouldn’t have these available.
WooCommerce/Classes/ViewRelated/Dashboard/Settings/Settings/SettingsViewModel.swift
Outdated
Show resolved
Hide resolved
itsmeichigo
left a 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.
I've tested the PR following the first method (adding a breakpoint to tags). I tried switching between different stores with different statuses for Stripe and WCPay. It looks like the tags are not correctly reflecting the plugin statuses when switching stores.
Maybe we should synchronize plugins at some point to have the correct data in storage?
As ZendeskManager is initialized only once, moving the ResultsController outside of the init allows for the siteID to be recalculated when we switch stores.
Generated by 🚫 dangerJS |
Thanks for your review and help with this, this was definitely tricky. Fixed on e88a7a3 . As we thought, we need to also call Ready for review! 🙇 |
itsmeichigo
left a 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.
This works perfectly now 💯 I left a few more nits, please feel free to merge after updating them.
| /// | ||
| private lazy var pluginResultsController: ResultsController<StorageSitePlugin> = updatePluginResultsController() | ||
|
|
||
| private func updatePluginResultsController() -> ResultsController<StorageSitePlugin> { |
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.
Super nit:
| private func updatePluginResultsController() -> ResultsController<StorageSitePlugin> { | |
| /// Returns a `pluginResultsController` using the latest selected site ID for predicate. | |
| private func createPluginResultsController() -> ResultsController<StorageSitePlugin> { |
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.
I see! create makes more sense than update as we're not really updating it but creating a new one on each call. Changed on 9984996
Closes: #7994
Description
With this PR, we'll be sending extra tags to ZenDesk that reflect the plugin status of
woocommerce-gateway-stripeandwoocommerce-paymentsplugins. There are 3 tags for each plugin:_installed_and_activated_installed_and_not_activated_not_installedChanges
In order to add these IPP statuses to ZenDesk tickets, we need to pass these tags through the SupportSDK's
RequestUiConfigurationclass when we create a new request. As we need to do this before ZenDesk tabs loads, we'll be using aStorageSitePluginresults controller directly through theZendeskManagerclass, in order to fetch plugin statuses and pass these as tags.Testing instructions
The easiest way is through the debugger:
It can also be tested directly in ZenDesk, which requires extra steps:
RELEASE-NOTES.txtif necessary.